Make the should_to_s and is_to_s functions to return a form of 'redacted'.
Rather than send the password hash to system logs in cases of failure or
running in --noop mode, just state whether it's the new or old hash. We're
already doing this with password changes that work, so this just brings it
inline with those, albeit via a slightly different pair of methods.
Signed-off-by: Ben Hughes <b...@puppetlabs.com>
---
...rd-disclosure-when-changing-a-users-password.rb | 23 ++++++++++++++++++++
lib/puppet/type/user.rb | 8 +++++++
spec/unit/type/user_spec.rb | 8 +++++++
3 files changed, 39 insertions(+), 0 deletions(-)
create mode 100644
acceptance/tests/ticket_6857_password-disclosure-when-changing-a-users-password.rb
diff --git
a/acceptance/tests/ticket_6857_password-disclosure-when-changing-a-users-password.rb
b/acceptance/tests/ticket_6857_password-disclosure-when-changing-a-users-password.rb
new file mode 100644
index 0000000..f1e100c
--- /dev/null
+++
b/acceptance/tests/ticket_6857_password-disclosure-when-changing-a-users-password.rb
@@ -0,0 +1,23 @@
+test_name "#6857: redact password hashes when applying in noop mode"
+
+adduser_manifest = <<MANIFEST
+user { 'passwordtestuser':
+ ensure => 'present',
+ password => 'apassword',
+}
+MANIFEST
+
+changepass_manifest = <<MANIFEST
+user { 'passwordtestuser':
+ ensure => 'present',
+ password => 'newpassword',
+ noop => true,
+}
+MANIFEST
+
+apply_manifest_on(agents, adduser_manifest )
+results = apply_manifest_on(agents, changepass_manifest )
+
+results.each do |result|
+ assert_match( /current_value \[old password hash redacted\], should be \[new
password hash redacted\]/ , "#{result.stdout}" )
+end
diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb
index f74e426..8d04fdc 100755
--- a/lib/puppet/type/user.rb
+++ b/lib/puppet/type/user.rb
@@ -165,6 +165,14 @@ module Puppet
return "changed password"
end
end
+
+ def is_to_s( currentvalue )
+ return '[old password hash redacted]'
+ end
+ def should_to_s( newvalue )
+ return '[new password hash redacted]'
+ end
+
end
newproperty(:password_min_age, :required_features =>
:manages_password_age) do
diff --git a/spec/unit/type/user_spec.rb b/spec/unit/type/user_spec.rb
index 5a84af4..594802c 100755
--- a/spec/unit/type/user_spec.rb
+++ b/spec/unit/type/user_spec.rb
@@ -290,6 +290,14 @@ describe user do
@password.change_to_s("other", "mypass").should_not be_include("mypass")
end
+ it "should redact the password when displaying the old value" do
+ @password.is_to_s("currentpassword").should =~ /^\[old password hash
redacted\]$/
+ end
+
+ it "should redact the password when displaying the new value" do
+ @password.should_to_s("newpassword").should =~ /^\[new password hash
redacted\]$/
+ end
+
it "should fail if a ':' is included in the password" do
lambda { @password.should = "some:thing" }.should
raise_error(Puppet::Error)
end
--
1.7.5.1
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
