Skip to content

Commit

Permalink
Pass potentially Sensitive params as Sensitive
Browse files Browse the repository at this point in the history
In 699f944 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: 699f944
  • Loading branch information
ekohl committed Jan 20, 2022
1 parent 699f944 commit 447bf08
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 12 deletions.
9 changes: 9 additions & 0 deletions data/common.yaml
Original file line number Diff line number Diff line change
@@ -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"
10 changes: 10 additions & 0 deletions hiera.yaml
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 2 additions & 2 deletions manifests/cli.pp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 },
}
),
}
Expand Down
4 changes: 2 additions & 2 deletions manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions manifests/params.pp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions spec/classes/foreman_cli_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion templates/database.yml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
2 changes: 1 addition & 1 deletion templates/hammer_root.yml.epp
Original file line number Diff line number Diff line change
@@ -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
Expand Down
6 changes: 4 additions & 2 deletions templates/settings.yaml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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") %>
Expand Down

0 comments on commit 447bf08

Please sign in to comment.