Support PBKDF2 using SHA512 #21
Conversation
|
@carlosantoniodasilva Is this something you're open to accepting? Happy to help move it forward if it's agreeable. |
|
I tested these changes in our own project and provided feedback on what we experienced. Thanks for your contribution @jefftrudeau. I hope the maintainers will accept this. |
| end | ||
|
|
||
| def self.digest(password, stretches, salt, pepper) | ||
| hash = OpenSSL::Digest::SHA512.new |
There was a problem hiding this comment.
| hash = OpenSSL::Digest::SHA512.new | |
| hash = OpenSSL::Digest.new('SHA512').new |
| class Pbkdf2 < Base | ||
| def self.compare(encrypted_password, password, stretches, salt, pepper) | ||
| value_to_test = self.digest(password, stretches, salt, pepper) | ||
| ActiveSupport::SecurityUtils.fixed_length_secure_compare(encrypted_password, value_to_test) |
There was a problem hiding this comment.
If comparing unmatching values of differing length, this fails with mismatch length error. This should use Devise.secure_compare instead, which should also do a fixed length comparison, but also does an initial byte size/length check.
| ActiveSupport::SecurityUtils.fixed_length_secure_compare(encrypted_password, value_to_test) | |
| Devise.secure_compare(encrypted_password, value_to_test) |
| iterations: stretches, | ||
| hash: hash, | ||
| length: hash.digest_length, | ||
| ).unpack('H*')[0] |
There was a problem hiding this comment.
| ).unpack('H*')[0] | |
| ).unpack1('H*') |
| def self.digest(password, stretches, salt, pepper) | ||
| hash = OpenSSL::Digest::SHA512.new | ||
| OpenSSL::KDF.pbkdf2_hmac( | ||
| password, |
There was a problem hiding this comment.
If a password is entirely numeric this function will fail trying due to implicit conversion of integer to string.
| password, | |
| password.to_s, |
|
@dblessing @carlosantoniodasilva I rebased our branch against main and created a new PR: #25 |
No description provided.