Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions .fixtures.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
2 changes: 1 addition & 1 deletion .puppet-lint.rc
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 5 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
93 changes: 74 additions & 19 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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?
Expand Down Expand Up @@ -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 `-`.

Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

15 changes: 15 additions & 0 deletions lib/puppet/functions/azure_key_vault/lookup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
50 changes: 50 additions & 0 deletions spec/functions/azure_key_vault_lookup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'

Expand Down
Loading