From ccab14590b836089aa29afe0559ef2c867c99c28 Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" Date: Sun, 17 Dec 2023 11:35:29 -0800 Subject: [PATCH 1/4] fix: upgrade to aruba-2/cucumber-8 Update tests and code to support aruba-2/cucumber-8 as needed by Ruby 3.2. Notably, HOME is treated specially, and environment variables are handled by the test suite. HOME is nominally a relative path, so take special precautions to work with an absolute path. Signed-off-by: Robin H. Johnson --- Gemfile | 4 +- Rakefile | 2 +- features/parser.feature | 6 +++ .../step_definitions/environment_overrides.rb | 12 ++--- features/step_definitions/parser_steps.rb | 44 ++++++++++++++----- features/step_definitions/recrypt_steps.rb | 4 +- features/support/env.rb | 21 +++++---- features/support/puppet.rb | 2 +- features/support/setup_sandbox.rb | 3 +- lib/hiera/backend/eyaml/encryptors/pkcs7.rb | 16 ++++--- lib/hiera/backend/eyaml/subcommand.rb | 9 +++- 11 files changed, 83 insertions(+), 40 deletions(-) diff --git a/Gemfile b/Gemfile index 6f8952a8..b1be99a4 100644 --- a/Gemfile +++ b/Gemfile @@ -18,8 +18,8 @@ gemspec group :development do gem 'activesupport' - gem 'aruba', '~> 0.6.2' - gem 'cucumber', '~> 1.1' + gem 'aruba', '~> 2.1' + gem 'cucumber', '~> 8' gem 'hiera-eyaml-plaintext' gem 'puppet', *location_for(ENV['PUPPET_VERSION']) if ENV['PUPPET_VERSION'] end diff --git a/Rakefile b/Rakefile index be8d5045..7cf2a2b1 100644 --- a/Rakefile +++ b/Rakefile @@ -27,7 +27,7 @@ require 'bundler/gem_tasks' # https://stackoverflow.com/questions/6473419/using-simplecov-to-display-cucumber-code-coverage require 'cucumber/rake/task' Cucumber::Rake::Task.new(:features) do |t| - t.cucumber_opts = '--format progress' # Any valid command line option can go here. + t.cucumber_opts = %w(--format progress) # Any valid command line option can go here. end begin diff --git a/features/parser.feature b/features/parser.feature index 6cb42cfd..3a042512 100644 --- a/features/parser.feature +++ b/features/parser.feature @@ -37,6 +37,8 @@ Feature: Parser Scenario: Parse decrypted yaml Given I make a parser instance with the DEC regexs + And I configure the keypair using envvars + And I load the keypair into envvars And I load a file called test_plain.yaml When I parse the content Then I should have 2 tokens @@ -45,6 +47,8 @@ Feature: Parser Scenario: Parse decrypted yaml with index Given I make a parser instance with the DEC regexs + And I configure the keypair using envvars + And I load the keypair into envvars And I load a file called test_plain_with_index.yaml When I parse the content Then I should have 5 tokens @@ -57,6 +61,8 @@ Feature: Parser Scenario: Output indexed decryption tokens Given I make a parser instance with the ENC regexs + And I configure the keypair using envvars + And I load the keypair into envvars And I load a file called test_input.yaml When I parse the content And map it to index decrypted values diff --git a/features/step_definitions/environment_overrides.rb b/features/step_definitions/environment_overrides.rb index dd32fa5a..08f72a30 100644 --- a/features/step_definitions/environment_overrides.rb +++ b/features/step_definitions/environment_overrides.rb @@ -1,18 +1,20 @@ Given(/^my EDITOR is set to "(.*?)"$/) do |editor_command| - ENV['EDITOR'] = editor_command + set_environment_variable 'EDITOR', editor_command end Given(/^my HOME is set to "(.*?)"$/) do |home_dir| - ENV['SANDBOX_HOME'] = home_dir + # HOME must be absolute + set_environment_variable 'HOME', expand_path(home_dir) end Given(/^my EYAML_CONFIG is set to "(.*?)"$/) do |config_file| - ENV['EYAML_CONFIG'] = config_file + set_environment_variable 'EYAML_CONFIG', config_file end Given(/^my PATH contains "(.*?)"$/) do |path_value| - return if ENV['PATH'].start_with? path_value - + abspath = expand_path(path_value) + return if ENV['PATH'].start_with? abspath paths = [path_value] + ENV['PATH'].split(File::PATH_SEPARATOR) ENV['PATH'] = paths.join(File::PATH_SEPARATOR) + prepend_environment_variable 'PATH', abspath + File::PATH_SEPARATOR end diff --git a/features/step_definitions/parser_steps.rb b/features/step_definitions/parser_steps.rb index dcfd7e05..471e20ce 100644 --- a/features/step_definitions/parser_steps.rb +++ b/features/step_definitions/parser_steps.rb @@ -19,8 +19,12 @@ Hiera::Backend::Eyaml::Options[:pkcs7_private_key] = 'features/sandbox/keys/private_key.pkcs7.pem' Hiera::Backend::Eyaml::Options[:pkcs7_public_key_env_var] = nil Hiera::Backend::Eyaml::Options[:pkcs7_private_key_env_var] = nil - ENV.delete('EYAML_PUBLIC_KEY') - ENV.delete('EYAML_PRIVATE_KEY') + # This needs to carry over to the later steps, so must modify modify both the + # fake ENV state and the real ENV state. + delete_environment_variable 'EYAML_PUBLIC_KEY' + delete_environment_variable 'EYAML_PRIVATE_KEY' + ENV['EYAML_PUBLIC_KEY']='' + ENV['EYAML_PRIVATE_KEY']='' end And(/^I configure the keypair using envvars$/) do @@ -31,8 +35,26 @@ end And(/^I load the keypair into envvars$/) do - ENV['EYAML_PUBLIC_KEY'] = File.read 'features/sandbox/keys/public_key.pkcs7.pem' - ENV['EYAML_PRIVATE_KEY'] = File.read 'features/sandbox/keys/private_key.pkcs7.pem' + d = aruba.config.root_directory + # Validate that the files exist + pubkeyfile = File.join(d, 'features', 'sandbox', 'keys', 'public_key.pkcs7.pem') + privkeyfile = File.join(d, 'features', 'sandbox', 'keys', 'private_key.pkcs7.pem') + expect(File.exist?(pubkeyfile)).to be_truthy + expect(File.exist?(privkeyfile)).to be_truthy + + # Load the files and validate + pubkey = File.read(pubkeyfile) + privkey = File.read(privkeyfile) + expect(pubkey).not_to be_empty + expect(privkey).not_to be_empty + + # Use keys + # This needs to carry over to the later steps, so must modify modify both the + # fake ENV state and the real ENV state. + set_environment_variable 'EYAML_PUBLIC_KEY', pubkey + set_environment_variable 'EYAML_PRIVATE_KEY', privkey + ENV['EYAML_PUBLIC_KEY']=pubkey + ENV['EYAML_PRIVATE_KEY']=privkey end When(/^I parse the content$/) do @@ -40,27 +62,27 @@ end Then(/^I should have (\d+) tokens?$/) do |number_of_tokens| - @tokens.size.should == number_of_tokens.to_i + expect(@tokens.size).to eq (number_of_tokens.to_i) end Then(/^token (\d+) should be a (.*)$/) do |index, class_name| actual_class_name = @tokens[index.to_i - 1].class.name - actual_class_name.split('::').last.should == class_name + expect(actual_class_name.split('::').last).to eq class_name end Then(/^token (\d+) should start with "(.*)"$/) do |index, content| token = @tokens[index.to_i - 1] - token.match.should =~ /^#{Regexp.escape(content)}/ + expect(token.match).to match(/^#{Regexp.escape(content)}/) end Then(/^token (\d+) should decrypt to start with "(.*)"$/) do |index, plain| token = @tokens[index.to_i - 1] - token.plain_text.should =~ /^#{Regexp.escape(plain)}/ + expect(token.plain_text).to match(/^#{Regexp.escape(plain)}/) end Then(/^token (\d+) should decrypt to a string with UTF-8 encodings$/) do |index| token = @tokens[index.to_i - 1] - token.plain_text.encoding.to_s.should == 'UTF-8' + expect(token.plain_text.encoding.to_s).to eq 'UTF-8' end And(/^map it to index decrypted values$/) do @@ -71,10 +93,10 @@ Then(/^decryption (\d+) should be "(.*)"$/) do |index, content| decrypted = @decrypted[index.to_i] - decrypted.should == content + expect(decrypted).to eq content end Then(/^token (\d+) id should be (\d+)$/) do |index, token_id| token = @tokens[index.to_i - 1] - token.id.should == token_id.to_i + expect(token.id).to eq (token_id.to_i) end diff --git a/features/step_definitions/recrypt_steps.rb b/features/step_definitions/recrypt_steps.rb index cb98b463..dfcc68e4 100644 --- a/features/step_definitions/recrypt_steps.rb +++ b/features/step_definitions/recrypt_steps.rb @@ -15,7 +15,7 @@ end Then(/the recrypted tokens should match/) do - @tokens.size.to_i.should == @tokens_check.size.to_i + expect(@tokens.size).to eq (@tokens_check.size.to_i) end Then(/the recrypted decrypted content should match/) do @@ -29,5 +29,5 @@ Then(/^the tokens at (\d+) should match/) do |index| decrypted1 = @tokens[index.to_i] decrypted2 = @tokens_check[index.to_i] - decrypted1.to_decrypted.should == decrypted2.to_decrypted + expect(decrypted1.to_decrypted).to eq (decrypted2.to_decrypted) end diff --git a/features/support/env.rb b/features/support/env.rb index aa23afb6..55748850 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -2,7 +2,7 @@ rubylib.unshift %(#{File.dirname(__FILE__) + '/../../lib'}) ENV['RUBYLIB'] = rubylib.uniq.join(File::PATH_SEPARATOR) require 'rubygems' -require 'aruba/config' +require 'aruba' require 'aruba/cucumber' require 'fileutils' require 'pathname' @@ -24,19 +24,22 @@ test_files[file_name] = file_contents end -# ENV['EDITOR']="/bin/cat" - Aruba.configure do |config| - config.before_cmd do |_cmd| - SetupSandbox.create_files test_files - # when executing, resolve the SANDBOX_HOME into a real HOME - ENV['HOME'] = Pathname.new(ENV.fetch('SANDBOX_HOME', nil)).realpath.to_s + # A number of checks require absolute paths. + config.allow_absolute_paths = true + # Setup the test environment. + config.before :command do |cmd| + SetupSandbox.create_files aruba.config.working_directory, test_files end end Before do + home_dir = 'clean_home' # set to a non-existant home in order so rogue configs don't confuse - ENV['SANDBOX_HOME'] = 'clean_home' - ENV['EYAML_CONFIG'] = nil + #set_environment_variable 'HOME', home_dir + ## But it must be an absolute path for other code + # e.g. puppet will throw: "Error: Could not initialize global default settings: non-absolute home" + set_environment_variable 'HOME', expand_path(home_dir) + set_environment_variable 'EYAML_CONFIG', '' @aruba_timeout_seconds = 30 end diff --git a/features/support/puppet.rb b/features/support/puppet.rb index 272f0c35..d7ef3f23 100644 --- a/features/support/puppet.rb +++ b/features/support/puppet.rb @@ -1,3 +1,3 @@ Given(/^I set FACTER_(.*?) to "(.*?)"$/) do |facter, value| - ENV["FACTER_#{facter}"] = value + set_environment_variable "FACTER_#{facter}", value end diff --git a/features/support/setup_sandbox.rb b/features/support/setup_sandbox.rb index 13341702..26d94918 100644 --- a/features/support/setup_sandbox.rb +++ b/features/support/setup_sandbox.rb @@ -1,8 +1,9 @@ require 'fileutils' class SetupSandbox - def self.create_files(test_files) + def self.create_files(destdir, test_files) test_files.each do |test_file, contents| + test_file = File.join(destdir, test_file) extension = test_file.split('.').last target_dir = File.dirname(test_file) FileUtils.mkdir_p(target_dir) unless File.directory?(target_dir) diff --git a/lib/hiera/backend/eyaml/encryptors/pkcs7.rb b/lib/hiera/backend/eyaml/encryptors/pkcs7.rb index 776f572d..84d71757 100644 --- a/lib/hiera/backend/eyaml/encryptors/pkcs7.rb +++ b/lib/hiera/backend/eyaml/encryptors/pkcs7.rb @@ -38,17 +38,21 @@ def self.encrypt(plaintext) public_key = option :public_key public_key_env_var = option :public_key_env_var - raise StandardError, 'pkcs7_public_key is not defined' unless public_key or public_key_env_var if public_key and public_key_env_var warn 'both public_key and public_key_env_var specified, using public_key' end - public_key_pem = if public_key_env_var and ENV[public_key_env_var] - ENV[public_key_env_var] - else - File.read public_key - end + if public_key_env_var + raise StandardError, "env #{public_key_env_var} is not set" unless ENV[public_key_env_var] + public_key_pem = ENV[public_key_env_var] + elsif public_key + raise StandardError, "file #{public_key} does not exist" unless File.exist? public_key + public_key_pem = File.read public_key + else + raise StandardError, 'pkcs7_public_key is not defined' unless public_key or public_key_env_var + end + public_key_x509 = OpenSSL::X509::Certificate.new(public_key_pem) cipher = OpenSSL::Cipher.new('aes-256-cbc') diff --git a/lib/hiera/backend/eyaml/subcommand.rb b/lib/hiera/backend/eyaml/subcommand.rb index 2a8de262..9f1815c1 100644 --- a/lib/hiera/backend/eyaml/subcommand.rb +++ b/lib/hiera/backend/eyaml/subcommand.rb @@ -33,8 +33,13 @@ class << self def self.load_config_file config = { options: {}, sources: [] } - ['/etc/eyaml/config.yaml', "#{ENV.fetch('HOME', nil)}/.eyaml/config.yaml", '.eyaml/config.yaml', - "#{ENV.fetch('EYAML_CONFIG', nil)}",].each do |config_file| + config_paths = [ + '/etc/eyaml/config.yaml', + "#{ENV.fetch('HOME', nil)}/.eyaml/config.yaml", + '.eyaml/config.yaml', + "#{ENV.fetch('EYAML_CONFIG', nil)}", + ] + config_paths.each do |config_file| next unless config_file and File.file? config_file begin From a05c8396c159b0bc123f0d98c644517c573a1710 Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" Date: Sun, 24 Dec 2023 22:39:37 -0800 Subject: [PATCH 2/4] fix(pkcs7): fix warning of both public_key/public_key_env_var to reflect code If both public_key and public_key_env_var were set, this message triggred: `both public_key and public_key_env_var specified, using public_key` However, the code actually preferred the `public_key_env_var` path. Change the warning to reflect that. Also change a similar line for private_key/private_key_env_var. Signed-off-by: Robin H. Johnson Resolves: https://github.com/voxpupuli/hiera-eyaml/pull/360#discussion_r1435966701 --- lib/hiera/backend/eyaml/encryptors/pkcs7.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/hiera/backend/eyaml/encryptors/pkcs7.rb b/lib/hiera/backend/eyaml/encryptors/pkcs7.rb index 84d71757..b747c942 100644 --- a/lib/hiera/backend/eyaml/encryptors/pkcs7.rb +++ b/lib/hiera/backend/eyaml/encryptors/pkcs7.rb @@ -40,7 +40,7 @@ def self.encrypt(plaintext) public_key_env_var = option :public_key_env_var if public_key and public_key_env_var - warn 'both public_key and public_key_env_var specified, using public_key' + warn 'both public_key and public_key_env_var specified, using public_key_env_var' end if public_key_env_var @@ -70,10 +70,10 @@ def self.decrypt(ciphertext) raise StandardError, 'pkcs7_private_key is not defined' unless private_key or private_key_env_var if public_key and public_key_env_var - warn 'both public_key and public_key_env_var specified, using public_key' + warn 'both public_key and public_key_env_var specified, using public_key_env_var' end if private_key and private_key_env_var - warn 'both private_key and private_key_env_var specified, using private_key' + warn 'both private_key and private_key_env_var specified, using private_key_env_var' end private_key_pem = if private_key_env_var and ENV[private_key_env_var] From f77a5806b912d3da7a3925136f03a5d03f5de7a4 Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" Date: Sun, 24 Dec 2023 22:55:04 -0800 Subject: [PATCH 3/4] fix(pkcs7): refactor key loading PKCS encrypt & decrypt had duplicate key loading logic with the same validations. Refactor to a single function to load PEM from variable or disk. Signed-off-by: Robin H. Johnson --- lib/hiera/backend/eyaml/encryptors/pkcs7.rb | 77 ++++++++++----------- 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/lib/hiera/backend/eyaml/encryptors/pkcs7.rb b/lib/hiera/backend/eyaml/encryptors/pkcs7.rb index b747c942..9e4f65d3 100644 --- a/lib/hiera/backend/eyaml/encryptors/pkcs7.rb +++ b/lib/hiera/backend/eyaml/encryptors/pkcs7.rb @@ -33,26 +33,11 @@ class Pkcs7 < Encryptor self.tag = 'PKCS7' + def self.encrypt(plaintext) LoggingHelper.trace 'PKCS7 encrypt' - public_key = option :public_key - public_key_env_var = option :public_key_env_var - - if public_key and public_key_env_var - warn 'both public_key and public_key_env_var specified, using public_key_env_var' - end - - if public_key_env_var - raise StandardError, "env #{public_key_env_var} is not set" unless ENV[public_key_env_var] - public_key_pem = ENV[public_key_env_var] - elsif public_key - raise StandardError, "file #{public_key} does not exist" unless File.exist? public_key - public_key_pem = File.read public_key - else - raise StandardError, 'pkcs7_public_key is not defined' unless public_key or public_key_env_var - end - + public_key_pem = self.load_public_key_pem() public_key_x509 = OpenSSL::X509::Certificate.new(public_key_pem) cipher = OpenSSL::Cipher.new('aes-256-cbc') @@ -62,32 +47,10 @@ def self.encrypt(plaintext) def self.decrypt(ciphertext) LoggingHelper.trace 'PKCS7 decrypt' - public_key = option :public_key - private_key = option :private_key - public_key_env_var = option :public_key_env_var - private_key_env_var = option :private_key_env_var - raise StandardError, 'pkcs7_public_key is not defined' unless public_key or public_key_env_var - raise StandardError, 'pkcs7_private_key is not defined' unless private_key or private_key_env_var - - if public_key and public_key_env_var - warn 'both public_key and public_key_env_var specified, using public_key_env_var' - end - if private_key and private_key_env_var - warn 'both private_key and private_key_env_var specified, using private_key_env_var' - end - - private_key_pem = if private_key_env_var and ENV[private_key_env_var] - ENV[private_key_env_var] - else - File.read private_key - end + private_key_pem = self.load_private_key_pem() private_key_rsa = OpenSSL::PKey::RSA.new(private_key_pem) - public_key_pem = if public_key_env_var and ENV[public_key_env_var] - ENV[public_key_env_var] - else - File.read public_key - end + public_key_pem = self.load_public_key_pem() public_key_x509 = OpenSSL::X509::Certificate.new(public_key_pem) pkcs7 = OpenSSL::PKCS7.new(ciphertext) @@ -136,6 +99,38 @@ def self.create_keys EncryptHelper.write_important_file filename: public_key, content: cert.to_pem LoggingHelper.info 'Keys created OK' end + + protected + + def self.load_ANY_key_pem(optname_key, optname_env_var) + opt_key = option (optname_key.to_sym) + opt_key_env_var = option (optname_env_var.to_sym) + + if opt_key and opt_key_env_var + warn "both #{optname_key} and #{optname_env_var} specified, using #{optname_env_var}" + end + + if opt_key_env_var + raise StandardError, "env #{opt_key_env_var} is not set" unless ENV[opt_key_env_var] + opt_key_pem = ENV[opt_key_env_var] + elsif opt_key + raise StandardError, "file #{opt_key} does not exist" unless File.exist? opt_key + opt_key_pem = File.read opt_key + else + raise StandardError, "pkcs7_#{optname_key} is not defined" unless opt_key or opt_key_env_var + end + + return opt_key_pem + end + + def self.load_public_key_pem + return self.load_ANY_key_pem('public_key', 'public_key_env_var') + end + + def self.load_private_key_pem + return self.load_ANY_key_pem('private_key', 'private_key_env_var') + end + end end end From 46e7c7156b0819c26682d886054d446a6841818e Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" Date: Sun, 24 Dec 2023 23:01:32 -0800 Subject: [PATCH 4/4] fix(subcommand): clarify config load behavior (no logic change) Signed-off-by: Robin H. Johnson Resolves: https://github.com/voxpupuli/hiera-eyaml/pull/360#discussion_r1435966277 --- lib/hiera/backend/eyaml/subcommand.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/hiera/backend/eyaml/subcommand.rb b/lib/hiera/backend/eyaml/subcommand.rb index 9f1815c1..bbda6b22 100644 --- a/lib/hiera/backend/eyaml/subcommand.rb +++ b/lib/hiera/backend/eyaml/subcommand.rb @@ -33,12 +33,20 @@ class << self def self.load_config_file config = { options: {}, sources: [] } - config_paths = [ - '/etc/eyaml/config.yaml', - "#{ENV.fetch('HOME', nil)}/.eyaml/config.yaml", - '.eyaml/config.yaml', - "#{ENV.fetch('EYAML_CONFIG', nil)}", - ] + + config_paths = [] + # Global + config_paths += ['/etc/eyaml/config.yaml'] + # Home directory + env_home = ENV.fetch('HOME', nil) + config_paths += [ "#{env_home}/.eyaml/config.yaml" ] if env_home + # Relative to current directory + config_paths += [ ".eyaml/config.yaml" ] + # Explicit ENV variable. + env_eyaml_config = ENV.fetch('EYAML_CONFIG', nil) + config_paths += [env_eyaml_config] if env_eyaml_config + + # Load each path and stack configs. config_paths.each do |config_file| next unless config_file and File.file? config_file