From 90168d93834f329ca037f83bedc5b5e580955fb6 Mon Sep 17 00:00:00 2001 From: Craig Gumbley Date: Wed, 17 Aug 2022 15:23:01 +0000 Subject: [PATCH 1/3] Harden root_password class Prior to this commit there was a possibility that malformed strings could be passed in to the resource. This could lead to unsafe executions on a remote system. The parameters that are susceptible are `install_secret_file`. This commit fixes the above by adding validation to ensure the given values confirm to expectation. `secret_file` is validated with a regular expression that ensures the given value is a valid path. --- manifests/params.pp | 1 - manifests/server.pp | 3 --- manifests/server/root_password.pp | 9 ++------- pdk.yaml | 2 ++ 4 files changed, 4 insertions(+), 11 deletions(-) create mode 100644 pdk.yaml diff --git a/manifests/params.pp b/manifests/params.pp index 551314676..ad497651c 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -9,7 +9,6 @@ $purge_conf_dir = false $restart = false $root_password = 'UNSET' - $install_secret_file = '/.mysql_secret' $server_package_ensure = 'present' $server_package_manage = true $server_service_manage = true diff --git a/manifests/server.pp b/manifests/server.pp index cea7d6250..be98a5914 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -17,8 +17,6 @@ # The location, as a path, of !includedir for custom configuration overrides. # @param install_options # Passes [install_options](https://docs.puppetlabs.com/references/latest/type.html#package-attribute-install_options) array to managed package resources. You must pass the appropriate options for the specified package manager -# @param install_secret_file -# Path to secret file containing temporary root password. # @param manage_config_file # Whether the MySQL configuration file should be managed. Valid values are `true`, `false`. Defaults to `true`. # @param options @@ -81,7 +79,6 @@ $config_file_mode = $mysql::params::config_file_mode, $includedir = $mysql::params::includedir, $install_options = undef, - $install_secret_file = $mysql::params::install_secret_file, $manage_config_file = $mysql::params::manage_config_file, Mysql::Options $options = {}, $override_options = {}, diff --git a/manifests/server/root_password.pp b/manifests/server/root_password.pp index 470fd782e..d5facf88f 100644 --- a/manifests/server/root_password.pp +++ b/manifests/server/root_password.pp @@ -16,20 +16,15 @@ } $options = $mysql::server::_options - $secret_file = $mysql::server::install_secret_file $login_file = $mysql::server::login_file # New installations of MySQL will configure a default random password for the root user # with an expiration. No actions can be performed until this password is changed. The # below exec will remove this default password. If the user has supplied a root # password it will be set further down with the mysql_user resource. - $rm_pass_cmd = join([ - "mysqladmin -u root --password=\$(grep -o '[^ ]\\+\$' ${secret_file}) password ''", - "rm -f ${secret_file}", - ], ' && ') exec { 'remove install pass': - command => $rm_pass_cmd, - onlyif => "test -f ${secret_file}", + command => "mysqladmin -u root --password=\$(grep -o '[^ ]\\+\$' /.mysql_secret) password && (rm -f /.mysql_secret; exit 0) || (rm -f /.mysql_secret; exit 1)", + onlyif => [['test', '-f' ,'/.mysql_secret']], path => '/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin', } diff --git a/pdk.yaml b/pdk.yaml new file mode 100644 index 000000000..4bef4bd0f --- /dev/null +++ b/pdk.yaml @@ -0,0 +1,2 @@ +--- +ignore: [] From 90b8c2632bf7fc6ad60cc51d61a02204a898d783 Mon Sep 17 00:00:00 2001 From: Craig Gumbley Date: Wed, 17 Aug 2022 15:33:04 +0000 Subject: [PATCH 2/3] Add spec tests This commit adds spec tests for the changes made in the previous commit. --- spec/classes/mysql_server_spec.rb | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/spec/classes/mysql_server_spec.rb b/spec/classes/mysql_server_spec.rb index e354f0c9e..7c8e54efb 100644 --- a/spec/classes/mysql_server_spec.rb +++ b/spec/classes/mysql_server_spec.rb @@ -158,8 +158,8 @@ describe 'when defaults' do it { is_expected.to contain_exec('remove install pass').with( - command: 'mysqladmin -u root --password=$(grep -o \'[^ ]\\+$\' /.mysql_secret) password \'\' && rm -f /.mysql_secret', - onlyif: 'test -f /.mysql_secret', + command: "mysqladmin -u root --password=\$(grep -o '[^ ]\\+\$' /.mysql_secret) password && (rm -f /.mysql_secret; exit 0) || (rm -f /.mysql_secret; exit 1)", + onlyif: [['test', '-f', '/.mysql_secret']], path: '/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin', ) } @@ -198,16 +198,6 @@ it { is_expected.not_to contain_mysql_user('root@localhost') } it { is_expected.not_to contain_file('/root/.my.cnf') } end - describe 'when install_secret_file set to /root/.mysql_secret' do - let(:params) { { install_secret_file: '/root/.mysql_secret' } } - - it { - is_expected.to contain_exec('remove install pass').with( - command: 'mysqladmin -u root --password=$(grep -o \'[^ ]\\+$\' /root/.mysql_secret) password \'\' && rm -f /root/.mysql_secret', - onlyif: 'test -f /root/.mysql_secret', - ) - } - end end context 'mysql::server::providers' do From f2b3bde7a74455ed1f08e477f762c004dfda0a92 Mon Sep 17 00:00:00 2001 From: Craig Gumbley Date: Tue, 23 Aug 2022 08:37:06 +0000 Subject: [PATCH 3/3] Ensure tests are idempotent --- spec/acceptance/01_mysql_db_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/acceptance/01_mysql_db_spec.rb b/spec/acceptance/01_mysql_db_spec.rb index c16dd61d6..5f6146656 100644 --- a/spec/acceptance/01_mysql_db_spec.rb +++ b/spec/acceptance/01_mysql_db_spec.rb @@ -37,7 +37,7 @@ class { 'mysql::server': class { 'mysql::server': override_options => { 'root_password' => 'password' } } file { '/tmp/spec.sql': ensure => file, - content => 'CREATE TABLE table1 (id int);', + content => 'CREATE TABLE IF NOT EXISTS table1 (id int);', before => Mysql::Db['spec2'], } mysql::db { 'spec2':