diff --git a/.fixtures.yml b/.fixtures.yml index 6df0f59..2296adb 100644 --- a/.fixtures.yml +++ b/.fixtures.yml @@ -3,11 +3,4 @@ --- fixtures: forge_modules: - repositories: - provision: 'https://github.com/puppetlabs/provision.git' - puppet_agent: 'https://github.com/puppetlabs/puppetlabs-puppet_agent.git' - facts: 'https://github.com/puppetlabs/puppetlabs-facts.git' - # Should go away when Attempting to work around PDK-1137(https://tickets.puppetlabs.com/browse/PDK-1137) - # is resolved. - symlinks: - azure_key_vault: '#{source_dir}' +# stdlib: "puppetlabs/stdlib" diff --git a/.puppet-lint.rc b/.puppet-lint.rc index 81f5bff..9e15c6e 100644 --- a/.puppet-lint.rc +++ b/.puppet-lint.rc @@ -6,4 +6,4 @@ --no-autoloader_layout-check --no-documentation-check --no-single_quote_string_with_variables-check ---ignore-paths=.vendor/**/*.pp,bundle/**/*.pp,pkg/**/*.pp,spec/**/*.pp,tests/**/*.pp,types/**/*.pp,vendor/**/*.pp +--ignore-paths=.vendor/**/*.pp,.bundle/**/*.pp,pkg/**/*.pp,spec/**/*.pp,tests/**/*.pp,types/**/*.pp,vendor/**/*.pp diff --git a/Gemfile b/Gemfile index 7b60ba8..7e77909 100644 --- a/Gemfile +++ b/Gemfile @@ -22,9 +22,12 @@ group :development do gem "racc", '~> 1.4.0', require: false if Gem::Requirement.create(['>= 2.7.0', '< 3.0.0']).satisfied_by?(Gem::Version.new(RUBY_VERSION.dup)) gem "deep_merge", '~> 1.2.2', require: false gem "voxpupuli-puppet-lint-plugins", '~> 5.0', require: false - gem "facterdb", '~> 2.1', require: false + gem "facterdb", '~> 2.1', require: false if Gem::Requirement.create(['< 3.0.0']).satisfied_by?(Gem::Version.new(RUBY_VERSION.dup)) + gem "facterdb", '~> 3.0', require: false if Gem::Requirement.create(['>= 3.0.0']).satisfied_by?(Gem::Version.new(RUBY_VERSION.dup)) gem "metadata-json-lint", '~> 4.0', require: false - gem "rspec-puppet-facts", '~> 4.0', require: false + gem "json-schema", '< 5.1.1', require: false + gem "rspec-puppet-facts", '~> 4.0', require: false if Gem::Requirement.create(['< 3.0.0']).satisfied_by?(Gem::Version.new(RUBY_VERSION.dup)) + gem "rspec-puppet-facts", '~> 5.0', require: false if Gem::Requirement.create(['>= 3.0.0']).satisfied_by?(Gem::Version.new(RUBY_VERSION.dup)) gem "dependency_checker", '~> 1.0.0', require: false gem "parallel_tests", '= 3.12.1', require: false gem "pry", '~> 0.10', require: false @@ -34,7 +37,6 @@ group :development do gem "rubocop-performance", '= 1.16.0', require: false gem "rubocop-rspec", '= 2.19.0', require: false gem "rb-readline", '= 0.5.5', require: false, platforms: [:mswin, :mingw, :x64_mingw] - gem "rexml", '>= 3.0.0', '< 3.2.7', require: false gem "github_changelog_generator", require: false if Gem::Version.new(RUBY_VERSION.dup) >= Gem::Version.new('2.2.2') gem "webmock", require: false end diff --git a/README.md b/README.md index 5295848..d674117 100644 --- a/README.md +++ b/README.md @@ -5,8 +5,6 @@ [![Puppet Forge Downloads](https://img.shields.io/puppetforge/dt/tragiccode/azure_key_vault.svg)](https://forge.puppetlabs.com/tragiccode/azure_key_vault) [![Puppet Forge Endorsement](https://img.shields.io/puppetforge/e/tragiccode/azure_key_vault.svg)](https://forge.puppetlabs.com/tragiccode/azure_key_vault) - - #### Table of Contents 1. [Description](#description) @@ -166,22 +164,7 @@ client_id: '00000000-0000-1234-1234-000000000000' client_secret: some-secret ``` -To retrieve a secret in puppet code you can use the `lookup` function: - -```puppet -notify { 'lookup': - message => lookup('important-secret'), -} -``` - -The alias function can also be used in hiera files, for example to set class parameters: - -```yaml -some_class::password: "%{alias('important-secret')}" -``` - -**NOTE: The alias function must be used in the above example. Attempting to use the lookup function inside of your hiera files will not work. This is because, when using lookup, the result is interpolated as a string. Since this module is safe by default, it always returns secrets as Sensitive[String]. The reason we have to use alias is because it will preserve the datatype of the value. More information can be found [here](https://www.puppet.com/docs/puppet/7/hiera_merging.html#interpolation_functions)** - +#### Using facts for the vault name You can use a fact to specify different vaults for different groups of nodes. It is recommended to use a trusted fact such as trusted.extensions.pp_environment as these facts @@ -202,6 +185,24 @@ Alternatively a custom trusted fact can be included [in the certificate request] - '^password.*' ``` +#### Manual lookups + +To retrieve a secret in puppet code you can use the `lookup` function: + +```puppet +notify { 'lookup': + message => lookup('important-secret'), +} +``` + +The alias function can also be used in hiera files, for example to set class parameters: + +```yaml +some_class::password: "%{alias('important-secret')}" +``` + +**NOTE: The alias function must be used in the above example. Attempting to use the lookup function inside of your hiera files will not work. This is because, when using lookup, the result is interpolated as a string. Since this module is safe by default, it always returns secrets as Sensitive[String]. The reason we have to use alias is because it will preserve the datatype of the value. More information can be found [here](https://www.puppet.com/docs/puppet/7/hiera_merging.html#interpolation_functions)** + **NOTE: While the above examples show manual lookups happening, it's recommended and considered a best practice to utilize Hiera's automatic parameter lookup (APL) within your puppet code** ### What is confine_to_keys? @@ -230,7 +231,7 @@ As an example, if you defined your confine_to_keys as shown below, hiera will on KeyVault secret names can only contain the characters `0-9`, `a-z`, `A-Z`, and `-`. -When relying on automatic parameter lookup, this is almost always going to contain the module delimiter (`::`) or underscores. +When relying on automatic parameter lookup (APL), this is almost always going to contain the module delimiter (`::`) or underscores. This module will automatically convert the variable name to a valid value by replacing every invalid character with the `key_replacement_token` value, which defaults to `-`. @@ -240,6 +241,60 @@ When troubleshooting, you can run hiera from the commandline with the `--explain Using normalized KeyVault secret key for lookup: puppetdb--master--config--puppetdb-server +### What is strip_from_keys? + +The `strip_from_keys` option allows you to specify one or more patterns to be stripped from the secret name just before looking up the secret in Azure Key Vault. To understand how this is useful, let's walk through an example. + +```yaml +- name: 'Azure Key Vault Secrets' + lookup_key: azure_key_vault::lookup + options: + vault_name: "prod-key-vault" + vault_api_version: '2016-10-01' + metadata_api_version: '2018-04-02' + key_replacement_token: '-' + confine_to_keys: + - '^azure_.*' +``` + +In the example above, `confine_to_keys` is used to scope certain secrets for lookup in Azure Key Vault, ensuring they are retrieved only when they truly exist there. However, as a side effect, `confine_to_keys` influences the secret name. In this case, the Azure Key Vault named "prod-key-vault" would need to have a secret named "profile--windows--sqlserver--azure-sql-user-password". + +To prevent this naming requirement, the `strip_from_keys` option was introduced. It allows you to remove specific patterns from the key just before lookup in Azure Key Vault. Below is an updated example demonstrating how `strip_from_keys` can be applied. + +```yaml +- name: 'Azure Key Vault Secrets' + lookup_key: azure_key_vault::lookup + options: + vault_name: "prod-key-vault" + vault_api_version: '2016-10-01' + metadata_api_version: '2018-04-02' + key_replacement_token: '-' + strip_from_keys: + - 'azure_' + confine_to_keys: + - '^azure_.*' +``` + +Now, with `strip_from_keys`, the "azure_" string pattern is removed from the secret name just before the lookup occurs. This ensures that the system searches for a secret named "profile--windows--sqlserver--sql-user-password" instead of "profile--windows--sqlserver--azure-sql-user-password" in the Azure Key Vault named "prod-key-vault". + +How flexible can this get? Below shows an example of how you "could" remove profile::* from all your keys! + +```yaml +- name: 'Azure Key Vault Secrets' + lookup_key: azure_key_vault::lookup + options: + vault_name: "prod-key-vault" + vault_api_version: '2016-10-01' + metadata_api_version: '2018-04-02' + key_replacement_token: '-' + strip_from_keys: + - '^profile::.*::' + confine_to_keys: + - '^azure_.*' +``` + +A lookup to 'profile::windows::sqlserver::azure_sql_user_password' or 'profile::linux::blah::blah_again::some_secret' would end up searching for secrets named azure_sql_user_password and some_secret in your Azure Key Vault named "prod-key-vault". + ## How it's secure by default In order to prevent accidental leakage of your secrets throughout all of the locations puppet stores information the returned value of the `azure_key_vault::secret` function & Hiera backend return a string wrapped in a Sensitive data type. Lets look at an example of what this means and why it's important. Below is an example of pulling a secret and trying to output the value in a notice function. diff --git a/Rakefile b/Rakefile index 7a140cb..857cfd0 100644 --- a/Rakefile +++ b/Rakefile @@ -15,5 +15,5 @@ PuppetLint.configuration.send('disable_autoloader_layout') PuppetLint.configuration.send('disable_documentation') PuppetLint.configuration.send('disable_single_quote_string_with_variables') PuppetLint.configuration.fail_on_warnings = true -PuppetLint.configuration.ignore_paths = [".vendor/**/*.pp", "bundle/**/*.pp", "pkg/**/*.pp", "spec/**/*.pp", "tests/**/*.pp", "types/**/*.pp", "vendor/**/*.pp"] +PuppetLint.configuration.ignore_paths = [".vendor/**/*.pp", ".bundle/**/*.pp", "pkg/**/*.pp", "spec/**/*.pp", "tests/**/*.pp", "types/**/*.pp", "vendor/**/*.pp"] diff --git a/lib/puppet/functions/azure_key_vault/lookup.rb b/lib/puppet/functions/azure_key_vault/lookup.rb index fa02cb2..7dae637 100644 --- a/lib/puppet/functions/azure_key_vault/lookup.rb +++ b/lib/puppet/functions/azure_key_vault/lookup.rb @@ -8,6 +8,7 @@ vault_api_version => String, Optional[metadata_api_version] => String, confine_to_keys => Array[String], + Optional[strip_from_keys] => Array[String], Optional[key_replacement_token] => String, Optional[service_principal_credentials] => String, Optional[use_azure_arc_authentication] => Boolean @@ -38,6 +39,20 @@ def lookup_key(secret_name, options, context) end end + strip_from_keys = options['strip_from_keys'] + if strip_from_keys + raise ArgumentError, 'strip_from_keys must be an array' unless strip_from_keys.is_a?(Array) + + strip_from_keys.each do |prefix| + secret_name_before_strippers = secret_name + regex = Regexp.new(prefix) + if secret_name.match?(regex) + secret_name = secret_name.gsub(regex, '') + context.explain { "Stripping the following pattern of #{prefix} from secret_name. The stripped secret_name has now changed from #{secret_name_before_strippers} to #{secret_name}" } + end + end + end + normalized_secret_name = TragicCode::Azure.normalize_object_name(secret_name, options['key_replacement_token'] || '-') context.explain { "Using normalized KeyVault secret key for lookup: #{normalized_secret_name}" } return Puppet::Pops::Types::PSensitiveType::Sensitive.new(context.cached_value(normalized_secret_name)) if context.cache_has_key(normalized_secret_name) diff --git a/metadata.json b/metadata.json index 8df8c53..8ec572e 100644 --- a/metadata.json +++ b/metadata.json @@ -83,7 +83,7 @@ "azure key vault", "azure vault" ], - "pdk-version": "3.2.0", - "template-url": "https://github.com/puppetlabs/pdk-templates.git#main", - "template-ref": "heads/main-0-g8842cf7" + "pdk-version": "3.4.0", + "template-url": "https://github.com/puppetlabs/pdk-templates#main", + "template-ref": "tags/3.4.0.3-0-g8fb22fc" } diff --git a/spec/functions/azure_key_vault_lookup_spec.rb b/spec/functions/azure_key_vault_lookup_spec.rb index 3a96aa8..5fc7d66 100644 --- a/spec/functions/azure_key_vault_lookup_spec.rb +++ b/spec/functions/azure_key_vault_lookup_spec.rb @@ -86,6 +86,12 @@ ).and_raise_error(ArgumentError, %r{'confine_to_keys' expects an Array value}i) end + it 'errors when strip_from_keys is no array' do + is_expected.to run.with_params( + 'profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'strip_from_keys' => '^vault.*$' }), lookup_context + ).and_raise_error(ArgumentError, %r{'strip_from_keys' expects an Array value}i) + end + it "errors when using both 'metadata_api_version' and 'service_principal_credentials'" do is_expected.to run.with_params( 'profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'service_principal_credentials' => 'path' }), lookup_context @@ -131,6 +137,50 @@ ) end + describe 'strip_from_keys' do + [ + { + input_secret_name: 'profile::windows::sqlserver::azure_sql_user_password', + expected_secret_name: 'profile--windows--sqlserver--sql-user-password', + secret_value: 'secret_value', + strip_from_keys: ['azure_'], + confine_to_keys: ['^.*azure_.*'] + }, + { + input_secret_name: 'profile::windows::sqlserver::azure_sql_user_password', + expected_secret_name: 'azure-sql-user-password', + secret_value: 'secret_value', + strip_from_keys: ['^profile::.*::'], + confine_to_keys: ['^.*azure_.*'] + }, + ].each do |test_case| + it "strips the patterns #{test_case[:strip_from_keys]} from the secret_name changing it from #{test_case[:input_secret_name]} to #{test_case[:expected_secret_name]}" do + access_token_value = 'access_value' + + expect(TragicCode::Azure).to receive(:get_access_token).and_return(access_token_value) + + expect(TragicCode::Azure).to receive(:get_secret).with( + options['vault_name'], + test_case[:expected_secret_name], + options['vault_api_version'], + access_token_value, + '', + ).and_return(test_case[:secret_value]) + + # rubocop:disable RSpec/NamedSubject + expect(subject.execute( + test_case[:input_secret_name], + options.merge({ + 'confine_to_keys' => test_case[:confine_to_keys], + 'strip_from_keys' => test_case[:strip_from_keys] + }), + lookup_context, + ).unwrap).to eq test_case[:secret_value] + # rubocop:enable RSpec/NamedSubject + end + end + end + it 'calls context.not_found when secret is not found in vault' do access_token_value = 'access_value'