Skip to content

Commit b490661

Browse files
committed
(PUP-11507) Fix macOS agent failing to retrieve password hash bug
Added a third parameter, the user's name, to get_salted_sha512_pbkdf2() so the user's name can be included in the error messages for easier debugging. Additionally, checks for nil values for the salt and entropy fields are added to prevent unpack() being called with nil, which initially caused the bug.
1 parent c0f6920 commit b490661

File tree

2 files changed

+11
-9
lines changed

2 files changed

+11
-9
lines changed

lib/puppet/provider/user/directoryservice.rb

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,9 @@ def self.generate_attribute_hash(input_hash)
147147
else
148148
embedded_binary_plist = get_embedded_binary_plist(attribute_hash[:shadowhashdata])
149149
if embedded_binary_plist['SALTED-SHA512-PBKDF2']
150-
attribute_hash[:password] = get_salted_sha512_pbkdf2('entropy', embedded_binary_plist)
151-
attribute_hash[:salt] = get_salted_sha512_pbkdf2('salt', embedded_binary_plist)
152-
attribute_hash[:iterations] = get_salted_sha512_pbkdf2('iterations', embedded_binary_plist)
150+
attribute_hash[:password] = get_salted_sha512_pbkdf2('entropy', embedded_binary_plist, attribute_hash[:name])
151+
attribute_hash[:salt] = get_salted_sha512_pbkdf2('salt', embedded_binary_plist, attribute_hash[:name])
152+
attribute_hash[:iterations] = get_salted_sha512_pbkdf2('iterations', embedded_binary_plist, attribute_hash[:name])
153153
elsif embedded_binary_plist['SALTED-SHA512']
154154
attribute_hash[:password] = get_salted_sha512(embedded_binary_plist)
155155
end
@@ -205,16 +205,18 @@ def self.get_salted_sha512(embedded_binary_plist)
205205
# according to which field is passed. Arguments passed are the hash
206206
# containing the value read from the 'ShadowHashData' key in the User's
207207
# plist, and the field to be read (one of 'entropy', 'salt', or 'iterations')
208-
def self.get_salted_sha512_pbkdf2(field, embedded_binary_plist)
208+
def self.get_salted_sha512_pbkdf2(field, embedded_binary_plist, user_name = "")
209209
case field
210210
when 'salt', 'entropy'
211-
embedded_binary_plist['SALTED-SHA512-PBKDF2'][field].unpack('H*').first
211+
value = embedded_binary_plist['SALTED-SHA512-PBKDF2'][field]
212+
if value == nil
213+
raise Puppet::Error, "Invalid #{field} given for user #{user_name}"
214+
end
215+
value.unpack('H*').first
212216
when 'iterations'
213217
Integer(embedded_binary_plist['SALTED-SHA512-PBKDF2'][field])
214218
else
215-
raise Puppet::Error, 'Puppet has tried to read an incorrect value from the ' +
216-
"SALTED-SHA512-PBKDF2 hash. Acceptable fields are 'salt', " +
217-
"'entropy', or 'iterations'."
219+
raise Puppet::Error, "Puppet has tried to read an incorrect value from the user #{user_name} in the SALTED-SHA512-PBKDF2 hash. Acceptable fields are 'salt', 'entropy', or 'iterations'."
218220
end
219221
end
220222

spec/unit/provider/user/directoryservice_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,7 @@ module Puppet::Util::Plist
840840
expect(provider.class.get_salted_sha512_pbkdf2('iterations', pbkdf2_embedded_bplist_hash)).to be_a(Integer)
841841
end
842842
it "should raise an error if a field other than 'entropy', 'salt', or 'iterations' is passed" do
843-
expect { provider.class.get_salted_sha512_pbkdf2('othervalue', pbkdf2_embedded_bplist_hash) }.to raise_error(Puppet::Error, /Puppet has tried to read an incorrect value from the SALTED-SHA512-PBKDF2 hash. Acceptable fields are 'salt', 'entropy', or 'iterations'/)
843+
expect { provider.class.get_salted_sha512_pbkdf2('othervalue', pbkdf2_embedded_bplist_hash, 'test_user') }.to raise_error(Puppet::Error, /Puppet has tried to read an incorrect value from the user test_user in the SALTED-SHA512-PBKDF2 hash. Acceptable fields are 'salt', 'entropy', or 'iterations'/)
844844
end
845845
end
846846

0 commit comments

Comments
 (0)