Skip to content

Commit c58f327

Browse files
hunnerhunner
committed
Merge branch 'wavesoftware-feature/using-env-to-hide-dbpassword-in-output'
2 parents 0f4c718 + 6b8c3a9 commit c58f327

File tree

9 files changed

+149
-30
lines changed

9 files changed

+149
-30
lines changed

‎lib/puppet/provider/postgresql_psql/ruby.rb‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def run_sql_command(sql)
1414
command = [resource[:psql_path]]
1515
command.push("-d", resource[:db]) if resource[:db]
1616
command.push("-p", resource[:port]) if resource[:port]
17-
command.push("-t", "-c", sql)
17+
command.push("-t", "-c", '"' + sql.gsub('"', '\"') + '"')
1818

1919
if resource[:cwd]
2020
Dir.chdir resource[:cwd] do
@@ -27,17 +27,42 @@ def run_sql_command(sql)
2727

2828
private
2929

30+
def get_environment
31+
environment = {}
32+
if envlist = resource[:environment]
33+
envlist = [envlist] unless envlist.is_a? Array
34+
envlist.each do |setting|
35+
if setting =~ /^(\w+)=((.|\n)+)$/
36+
env_name = $1
37+
value = $2
38+
if environment.include?(env_name) || environment.include?(env_name.to_sym)
39+
warning "Overriding environment setting '#{env_name}' with '#{value}'"
40+
end
41+
environment[env_name] = value
42+
else
43+
warning "Cannot understand environment setting #{setting.inspect}"
44+
end
45+
end
46+
end
47+
return environment
48+
end
49+
3050
def run_command(command, user, group)
51+
command = command.join ' '
52+
environment = get_environment
3153
if Puppet::PUPPETVERSION.to_f < 3.4
32-
Puppet::Util::SUIDManager.run_and_capture(command, user, group)
54+
require 'puppet/util/execution'
55+
Puppet::Util::Execution.withenv environment do
56+
Puppet::Util::SUIDManager.run_and_capture(command, user, group)
57+
end
3358
else
3459
output = Puppet::Util::Execution.execute(command, {
3560
:uid => user,
3661
:gid => group,
3762
:failonfail => false,
3863
:combine => true,
3964
:override_locale => true,
40-
:custom_environment => {}
65+
:custom_environment => environment
4166
})
4267
[output, $CHILD_STATUS.dup]
4368
end

‎lib/puppet/type/postgresql_psql.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,20 @@ def matches(value)
8080
defaultto("/tmp")
8181
end
8282

83+
newparam(:environment) do
84+
desc "Any additional environment variables you want to set for a
85+
SQL command. Multiple environment variables should be
86+
specified as an array."
87+
88+
validate do |values|
89+
Array(values).each do |value|
90+
unless value =~ /\w+=/
91+
raise ArgumentError, "Invalid environment setting '#{value}'"
92+
end
93+
end
94+
end
95+
end
96+
8397
newparam(:refreshonly, :boolean => true) do
8498
desc "If 'true', then the SQL will only be executed via a notify/subscribe event."
8599

‎manifests/server/passwd.pp‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,25 @@
1212
# without specifying a password ('ident' or 'trust' security). This is
1313
# the default for pg_hba.conf.
1414
$escaped = postgresql_escape($postgres_password)
15-
$env = "env PGPASSWORD='${postgres_password}'"
1615
exec { 'set_postgres_postgrespw':
1716
# This command works w/no password because we run it as postgres system
1817
# user
19-
command => "${psql_path} -c 'ALTER ROLE \"${user}\" PASSWORD ${escaped}'",
20-
user => $user,
21-
group => $group,
22-
logoutput => true,
23-
cwd => '/tmp',
18+
command => "${psql_path} -c \"ALTER ROLE \\\"${user}\\\" PASSWORD \${NEWPASSWD_ESCAPED}\"",
19+
user => $user,
20+
group => $group,
21+
logoutput => true,
22+
cwd => '/tmp',
23+
environment => [
24+
"PGPASSWORD='${postgres_password}'",
25+
"NEWPASSWD_ESCAPED='${escaped}'",
26+
],
2427
# With this command we're passing -h to force TCP authentication, which
2528
# does require a password. We specify the password via the PGPASSWORD
2629
# environment variable. If the password is correct (current), this
2730
# command will exit with an exit code of 0, which will prevent the main
2831
# command from running.
29-
unless => "${env} ${psql_path} -h localhost -p ${port} -c 'select 1' > /dev/null",
30-
path => '/usr/bin:/usr/local/bin:/bin',
32+
unless => "${psql_path} -h localhost -p ${port} -c 'select 1' > /dev/null",
33+
path => '/usr/bin:/usr/local/bin:/bin',
3134
}
3235
}
3336
}

‎manifests/server/role.pp‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@
2424
$superuser_sql = $superuser ? { true => 'SUPERUSER', default => 'NOSUPERUSER' }
2525
$replication_sql = $replication ? { true => 'REPLICATION', default => '' }
2626
if ($password_hash != false) {
27-
$password_sql = "ENCRYPTED PASSWORD '${password_hash}'"
27+
$environment = "NEWPGPASSWD=${password_hash}"
28+
$password_sql = "ENCRYPTED PASSWORD '\$NEWPGPASSWD'"
2829
} else {
2930
$password_sql = ''
31+
$environment = []
3032
}
3133

3234
Postgresql_psql {
@@ -35,12 +37,17 @@
3537
psql_user => $psql_user,
3638
psql_group => $psql_group,
3739
psql_path => $psql_path,
38-
require => [ Postgresql_psql["CREATE ROLE \"${username}\" ${password_sql} ${login_sql} ${createrole_sql} ${createdb_sql} ${superuser_sql} ${replication_sql} CONNECTION LIMIT ${connection_limit}"], Class['postgresql::server'] ],
40+
require => [
41+
Postgresql_psql["CREATE ROLE ${username} ENCRYPTED PASSWORD ****"],
42+
Class['postgresql::server'],
43+
],
3944
}
4045

41-
postgresql_psql {"CREATE ROLE \"${username}\" ${password_sql} ${login_sql} ${createrole_sql} ${createdb_sql} ${superuser_sql} ${replication_sql} CONNECTION LIMIT ${connection_limit}":
42-
unless => "SELECT rolname FROM pg_roles WHERE rolname='${username}'",
43-
require => Class['Postgresql::Server'],
46+
postgresql_psql { "CREATE ROLE ${username} ENCRYPTED PASSWORD ****":
47+
command => "CREATE ROLE \"${username}\" ${password_sql} ${login_sql} ${createrole_sql} ${createdb_sql} ${superuser_sql} ${replication_sql} CONNECTION LIMIT ${connection_limit}",
48+
unless => "SELECT rolname FROM pg_roles WHERE rolname='${username}'",
49+
require => Class['Postgresql::Server'],
50+
environment => $environment,
4451
}
4552

4653
postgresql_psql {"ALTER ROLE \"${username}\" ${superuser_sql}":
@@ -86,8 +93,10 @@
8693
$pwd_md5 = md5("${password_hash}${username}")
8794
$pwd_hash_sql = "md5${pwd_md5}"
8895
}
89-
postgresql_psql {"ALTER ROLE \"${username}\" ${password_sql}":
90-
unless => "SELECT usename FROM pg_shadow WHERE usename='${username}' and passwd='${pwd_hash_sql}'",
96+
postgresql_psql { "ALTER ROLE ${username} ENCRYPTED PASSWORD ****":
97+
command => "ALTER ROLE \"${username}\" ${password_sql}",
98+
unless => "SELECT usename FROM pg_shadow WHERE usename='${username}' and passwd='${pwd_hash_sql}'",
99+
environment => $environment,
91100
}
92101
}
93102
}

‎spec/acceptance/postgresql_psql_spec.rb‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,41 @@ class { 'postgresql::server': } ->
113113
apply_manifest(pp, :expect_changes => true)
114114
end
115115
end
116+
117+
context 'with secure password passing by environment' do
118+
it 'should run SQL that contanins password passed by environment' do
119+
select = "select \\'$PASS_TO_EMBED\\'"
120+
pp = <<-EOS.unindent
121+
class { 'postgresql::server': } ->
122+
postgresql_psql { 'password embedded by environment: #{select}':
123+
db => 'postgres',
124+
psql_user => 'postgres',
125+
command => '#{select}',
126+
environment => [
127+
'PASS_TO_EMBED=pa$swD',
128+
],
129+
}
130+
EOS
131+
apply_manifest(pp, :catch_failures => true)
132+
apply_manifest(pp, :expect_changes => false)
133+
end
134+
it 'should run SQL that contanins password passed by environment in check' do
135+
select = "select 1 where \\'$PASS_TO_EMBED\\'=\\'passwD\\'"
136+
pp = <<-EOS.unindent
137+
class { 'postgresql::server': } ->
138+
postgresql_psql { 'password embedded by environment in check: #{select}':
139+
db => 'postgres',
140+
psql_user => 'postgres',
141+
command => 'invalid sql query',
142+
unless => '#{select}',
143+
environment => [
144+
'PASS_TO_EMBED=passwD',
145+
],
146+
}
147+
EOS
148+
149+
apply_manifest(pp, :catch_failures => true)
150+
apply_manifest(pp, :expect_changes => false)
151+
end
152+
end
116153
end

‎spec/acceptance/z_alternative_pgdata_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
# location properly.
55

66
# Allow postgresql to use /tmp/* as a datadir
7-
if fact('osfamily') == 'RedHat'
8-
shell("setenforce 0")
7+
if fact('osfamily') == 'RedHat' and fact('selinux') == true
8+
shell 'setenforce 0'
99
end
1010

1111
describe 'postgres::server', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do

‎spec/unit/classes/server_spec.rb‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,29 @@
2626
end
2727

2828
describe 'service_ensure => running' do
29-
let(:params) {{ :service_ensure => 'running' }}
29+
let(:params) do
30+
{
31+
:service_ensure => 'running',
32+
:postgres_password => 'new-p@s$word-to-set'
33+
}
34+
end
3035
it { is_expected.to contain_class("postgresql::params") }
3136
it { is_expected.to contain_class("postgresql::server") }
37+
it { is_expected.to contain_class("postgresql::server::passwd") }
3238
it 'should validate connection' do
3339
is_expected.to contain_postgresql__validate_db_connection('validate_service_is_running')
3440
end
41+
it 'should set postgres password' do
42+
is_expected.to contain_exec('set_postgres_postgrespw').with({
43+
'command' => '/usr/bin/psql -c "ALTER ROLE \"postgres\" PASSWORD ${NEWPASSWD_ESCAPED}"',
44+
'user' => 'postgres',
45+
'environment' => [
46+
"PGPASSWORD='new-p@s$word-to-set'",
47+
"NEWPASSWD_ESCAPED='$$new-p@s$word-to-set$$'"
48+
],
49+
'unless' => "/usr/bin/psql -h localhost -c 'select 1' > /dev/null",
50+
})
51+
end
3552
end
3653

3754
describe 'service_ensure => stopped' do

‎spec/unit/defines/server/role_spec.rb‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
let :params do
2121
{
22-
:password_hash => 'test',
22+
:password_hash => 'new-pa$s',
2323
}
2424
end
2525

@@ -28,4 +28,18 @@
2828
end
2929

3030
it { is_expected.to contain_postgresql__server__role('test') }
31+
it 'should have create role for "test" user with password as ****' do
32+
is_expected.to contain_postgresql_psql('CREATE ROLE test ENCRYPTED PASSWORD ****').with({
33+
'command' => "CREATE ROLE \"test\" ENCRYPTED PASSWORD '$NEWPGPASSWD' LOGIN NOCREATEROLE NOCREATEDB NOSUPERUSER CONNECTION LIMIT -1",
34+
'environment' => "NEWPGPASSWD=new-pa$s",
35+
'unless' => "SELECT rolname FROM pg_roles WHERE rolname='test'",
36+
})
37+
end
38+
it 'should have alter role for "test" user with password as ****' do
39+
is_expected.to contain_postgresql_psql('ALTER ROLE test ENCRYPTED PASSWORD ****').with({
40+
'command' => "ALTER ROLE \"test\" ENCRYPTED PASSWORD '$NEWPGPASSWD'",
41+
'environment' => "NEWPGPASSWD=new-pa$s",
42+
'unless' => "SELECT usename FROM pg_shadow WHERE usename='test' and passwd='md5b6f7fcbbabb4befde4588a26c1cfd2fa'",
43+
})
44+
end
3145
end

‎spec/unit/puppet/provider/postgresql_psql/ruby_spec.rb‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414

1515
it "executes with the given psql_path on the given DB" do
1616
expect(provider).to receive(:run_command).with(['psql', '-d',
17-
attributes[:db], '-t', '-c', 'SELECT something'], 'postgres',
17+
attributes[:db], '-t', '-c', '"SELECT \'something\' as \"Custom column\""'], 'postgres',
1818
'postgres')
1919

20-
provider.run_sql_command("SELECT something")
20+
provider.run_sql_command('SELECT \'something\' as "Custom column"')
2121
end
2222
end
2323
describe "with psql_path and db" do
@@ -32,10 +32,10 @@
3232
it "executes with the given psql_path on the given DB" do
3333
expect(Dir).to receive(:chdir).with(attributes[:cwd]).and_yield
3434
expect(provider).to receive(:run_command).with([attributes[:psql_path],
35-
'-d', attributes[:db], '-t', '-c', 'SELECT something'],
35+
'-d', attributes[:db], '-t', '-c', '"SELECT \'something\' as \"Custom column\""'],
3636
attributes[:psql_user], attributes[:psql_group])
3737

38-
provider.run_sql_command("SELECT something")
38+
provider.run_sql_command('SELECT \'something\' as "Custom column"')
3939
end
4040
end
4141
describe "with search_path string" do
@@ -45,10 +45,10 @@
4545

4646
it "executes with the given search_path" do
4747
expect(provider).to receive(:run_command).with(['psql', '-t', '-c',
48-
'set search_path to schema1; SELECT something'],
48+
'"set search_path to schema1; SELECT \'something\' as \"Custom column\""'],
4949
'postgres', 'postgres')
5050

51-
provider.run_sql_command("SELECT something")
51+
provider.run_sql_command('SELECT \'something\' as "Custom column"')
5252
end
5353
end
5454
describe "with search_path array" do
@@ -58,12 +58,12 @@
5858

5959
it "executes with the given search_path" do
6060
expect(provider).to receive(:run_command).with(['psql', '-t', '-c',
61-
'set search_path to schema1,schema2; SELECT something'],
61+
'"set search_path to schema1,schema2; SELECT \'something\' as \"Custom column\""'],
6262
'postgres',
6363
'postgres'
6464
)
6565

66-
provider.run_sql_command("SELECT something")
66+
provider.run_sql_command('SELECT \'something\' as "Custom column"')
6767
end
6868
end
6969
end

0 commit comments

Comments
 (0)