From 41b1b45e355aa43ff662c4ae4ce0477a78580b1a Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 20 Jan 2022 12:51:53 +0100 Subject: [PATCH] Pass potentially Sensitive params as Sensitive In 699f944c93b8cf3718b16e6b9de7143c1e55bbe8 the parameters started to accept Sensitive but it didn't default to Sensitive. They also weren't converting data coming from Hiera. This adds a data-in-modules setup and sets lookup_options for those. This means Kafo (which heavily relies on Hiera) will pass sensitive values. The commit also missed settings.yml.erb and database.yml.erb which now also unwraps if needed. Ideally this would be converted to EPP instead but this fixes it in the short term. However, hammer_root.yml.epp somehow doesn't properly render the sensitive data. That's why it's unwrapper for now, just to make it work. A test case is added to prevent future regressions. It also changes the data type to accept Sensitive[Undef] which is needed if Hiera unconditionally converts the value to Sensitive. Fixes: 699f944c93b8cf3718b16e6b9de7143c1e55bbe8 --- data/common.yaml | 9 +++++++++ hiera.yaml | 10 ++++++++++ manifests/cli.pp | 4 ++-- manifests/init.pp | 4 ++-- manifests/params.pp | 8 ++++---- spec/classes/foreman_cli_spec.rb | 9 +++++++++ templates/database.yml.erb | 2 +- templates/hammer_root.yml.epp | 2 +- templates/settings.yaml.erb | 6 ++++-- 9 files changed, 42 insertions(+), 12 deletions(-) create mode 100644 data/common.yaml create mode 100644 hiera.yaml diff --git a/data/common.yaml b/data/common.yaml new file mode 100644 index 000000000..b3eebb91e --- /dev/null +++ b/data/common.yaml @@ -0,0 +1,9 @@ +lookup_options: + '^foreman::(db|email_smtp|initial_admin)_password$': + convert_to: "Sensitive" + '^foreman::oauth_consumer_(key|secret)$': + convert_to: "Sensitive" + foreman::cli::password: + convert_to: "Sensitive" + foreman::plugin::supervisory_authority::secret_token: + convert_to: "Sensitive" diff --git a/hiera.yaml b/hiera.yaml new file mode 100644 index 000000000..1815cee84 --- /dev/null +++ b/hiera.yaml @@ -0,0 +1,10 @@ +--- +version: 5 + +defaults: # Used for any hierarchy level that omits these keys. + datadir: data # This path is relative to hiera.yaml's directory. + data_hash: yaml_data # Use the built-in YAML backend. + +hierarchy: + - name: "common" + path: "common.yaml" diff --git a/manifests/cli.pp b/manifests/cli.pp index 48e3ee8cd..e4a229cc2 100644 --- a/manifests/cli.pp +++ b/manifests/cli.pp @@ -30,7 +30,7 @@ String $version = $foreman::cli::params::version, Boolean $manage_root_config = $foreman::cli::params::manage_root_config, Optional[String] $username = $foreman::cli::params::username, - Optional[Variant[String, Sensitive[String]]] $password = $foreman::cli::params::password, + Variant[Optional[String], Sensitive[Optional[String]]] $password = $foreman::cli::params::password, Boolean $use_sessions = $foreman::cli::params::use_sessions, Boolean $refresh_cache = $foreman::cli::params::refresh_cache, Integer[-1] $request_timeout = $foreman::cli::params::request_timeout, @@ -93,7 +93,7 @@ 'foreman/hammer_root.yml.epp', { username => $username_real, - password => $password_real, + password => if $password_real =~ Sensitive { $password_real.unwrap } else { $password_real }, } ), } diff --git a/manifests/init.pp b/manifests/init.pp index 3f173726c..6fcf46020 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -215,7 +215,7 @@ Variant[Undef, Enum['UNSET'], Stdlib::Port] $db_port = 'UNSET', Optional[String] $db_database = 'UNSET', Optional[String] $db_username = $foreman::params::db_username, - Optional[Variant[String, Sensitive[String]]] $db_password = $foreman::params::db_password, + Variant[Optional[String], Sensitive[Optional[String]]] $db_password = $foreman::params::db_password, Optional[String] $db_sslmode = 'UNSET', Optional[String] $db_root_cert = undef, Integer[0] $db_pool = $foreman::params::db_pool, @@ -265,7 +265,7 @@ Optional[Stdlib::Fqdn] $email_smtp_domain = $foreman::params::email_smtp_domain, Enum['none', 'plain', 'login', 'cram-md5'] $email_smtp_authentication = $foreman::params::email_smtp_authentication, Optional[String] $email_smtp_user_name = $foreman::params::email_smtp_user_name, - Optional[Variant[String, Sensitive[String]]] $email_smtp_password = $foreman::params::email_smtp_password, + Variant[Optional[String], Sensitive[Optional[String]]] $email_smtp_password = $foreman::params::email_smtp_password, Optional[String] $email_reply_address = $foreman::params::email_reply_address, Optional[String] $email_subject_prefix = $foreman::params::email_subject_prefix, String $telemetry_prefix = $foreman::params::telemetry_prefix, diff --git a/manifests/params.pp b/manifests/params.pp index d661e9392..d532bb01b 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -32,7 +32,7 @@ $db_username = 'foreman' # Generate and cache the password on the master once # In multi-puppetmaster setups, the user should specify their own - $db_password = extlib::cache_data('foreman_cache_data', 'db_password', extlib::random_password(32)) + $db_password = Sensitive(extlib::cache_data('foreman_cache_data', 'db_password', extlib::random_password(32))) # Default database connection pool $db_pool = 5 # if enabled, will run rake jobs, which depend on the database @@ -147,13 +147,13 @@ # We need the REST API interface with OAuth for some REST Puppet providers $oauth_active = true $oauth_map_users = false - $oauth_consumer_key = extlib::cache_data('foreman_cache_data', 'oauth_consumer_key', extlib::random_password(32)) - $oauth_consumer_secret = extlib::cache_data('foreman_cache_data', 'oauth_consumer_secret', extlib::random_password(32)) + $oauth_consumer_key = Sensitive(extlib::cache_data('foreman_cache_data', 'oauth_consumer_key', extlib::random_password(32))) + $oauth_consumer_secret = Sensitive(extlib::cache_data('foreman_cache_data', 'oauth_consumer_secret', extlib::random_password(32))) $oauth_effective_user = 'admin' # Initial admin account details $initial_admin_username = 'admin' - $initial_admin_password = extlib::cache_data('foreman_cache_data', 'admin_password', extlib::random_password(16)) + $initial_admin_password = Sensitive(extlib::cache_data('foreman_cache_data', 'admin_password', extlib::random_password(16))) $initial_admin_first_name = undef $initial_admin_last_name = undef $initial_admin_email = undef diff --git a/spec/classes/foreman_cli_spec.rb b/spec/classes/foreman_cli_spec.rb index 618e7f461..c1cadd64e 100644 --- a/spec/classes/foreman_cli_spec.rb +++ b/spec/classes/foreman_cli_spec.rb @@ -58,6 +58,15 @@ CONFIG ) end + + describe 'using Sensitive' do + let(:params) { super().merge(password: sensitive('secret')) } + + it 'should contain settings' do + is_expected.to contain_file('/root/.hammer/cli.modules.d/foreman.yml') + .with_content(/:password: 'secret'/) + end + end end describe 'with manage_root_config=false' do diff --git a/templates/database.yml.erb b/templates/database.yml.erb index c02600001..cbad0efe2 100644 --- a/templates/database.yml.erb +++ b/templates/database.yml.erb @@ -28,6 +28,6 @@ username: <%= username %> <% end -%> <% unless (password = scope.lookupvar("::foreman::db_password")) == 'UNSET' -%> - password: "<%= password %>" + password: "<%= password.respond_to?(:unwrap) ? password.unwrap : password %>" <% end -%> pool: <%= @db_pool %> diff --git a/templates/hammer_root.yml.epp b/templates/hammer_root.yml.epp index 17ff9f899..2c2b3f48b 100644 --- a/templates/hammer_root.yml.epp +++ b/templates/hammer_root.yml.epp @@ -1,6 +1,6 @@ <%- | Optional[String] $username, - Optional[Variant[String, Sensitive[String]]] $password, + Variant[Optional[String], Sensitive[Optional[String]]] $password, | -%> :foreman: # Credentials. You'll be asked for the interactively if you leave them blank here diff --git a/templates/settings.yaml.erb b/templates/settings.yaml.erb index db56261a2..6e1cd4446 100644 --- a/templates/settings.yaml.erb +++ b/templates/settings.yaml.erb @@ -12,8 +12,10 @@ # The following values are used for providing default settings during db migrate :oauth_active: <%= scope.lookupvar("foreman::oauth_active") %> :oauth_map_users: <%= scope.lookupvar("foreman::oauth_map_users") %> -:oauth_consumer_key: <%= scope.lookupvar("foreman::oauth_consumer_key") %> -:oauth_consumer_secret: <%= scope.lookupvar("foreman::oauth_consumer_secret") %> +<% oauth_key = scope.lookupvar("foreman::oauth_consumer_key") -%> +:oauth_consumer_key: <%= oauth_key.respond_to?(:unwrap) ? oauth_key.unwrap : oauth_key %> +<% oauth_secret = scope.lookupvar("foreman::oauth_consumer_secret") -%> +:oauth_consumer_secret: <%= oauth_secret.respond_to?(:unwrap) ? oauth_secret.unwrap : oauth_secret %> # Websockets :websockets_encrypt: <%= scope.lookupvar("foreman::websockets_encrypt") %>