Bug #5835
openSHA1 is recommended as a suitable password hashing method
0%
Description
The description of SHA1HashFunction recommends SHA1 as a password hash. Not only SHA1 but none of the SHA* functions should be considered suitable for password hashing, with or without salting.
Producing SHA1 collisions have shown recently and SHA1 joined MD5 as an obsolete hashing function. The compute() method with a salt provides too much of a convenience to lure users to use these hashing methods improperly. I suggest that SHA256 and SHA512 are added without salted compute() methods and that these two get removed in Wt4.
Updated by Wim Dumon over 7 years ago
Hello Kayra,
Thanks for spotting this. Do you happen to have a reference for the claim that SHA* shouldn' be used for password hasing? Is the reason cryptographic in nature or the fact that sha* computation is fast enough to enable brute-force attacks? You're right that SHA1 seems to be showing its age, and we probably shouldn't recommend it for new uses since better alternatives exist.
Can you also elaborate on your suggestion to remove the salt parameter from compute()? I believe Wt::Auth::HashFunction may just as well be named Wt::Auth::PasswordHashFunction, since it's (currently) only intended to be used to compute password hashes... I don't think I get your point.
Thanks,
Wim.
Updated by Kayra Akman over 7 years ago
Hello,
I think OWASP is a credible reference regarding web security: https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet
They recommend either slow to compute functions such as pbkdf2 or bcrypt, or keyed functions (HMAC) which assume a well protected key kept outside of where the web application is deployed.
SHA family of functions are very-fast, facilitating brute-force attacks, have known weaknesses as discussed at length in Cryptography Engineering.
I think Wt should, actually must, provide SHA functions but not for password hashing, hence I recommended to remove the salt parameter for SHA functions. SHA functions should be somewhere else (Utils?) but not in Wt::Auth. Proper password hashing functions should, actually must, keep the salting parameter, there is no question about that. Wt::Auth::PasswordHashFunction would be a much better name, but only after removing SHA* and MD5 functions from there.
OTOH, SHA-HMAC functions would be a nice addition to Wt. These wouldn't have a salt parameter, but a key parameter, or both salt and key as suggested by OWASP.
Updated by Roel Standaert over 7 years ago
I guess it's about time to stop recommending salted SHA-1 as a password hash, indeed. I updated the documentation, but I'll keep this issue open for further consideration. There is already a BCryptHashFunction, of course, which we normally use for password hashing.
Updated by Kayra Akman over 7 years ago
The page from the OWASP website I've linked above recommends bcrypt if PBKDF2 or scrypt is not available. This is not first time I have encountered this recommendation. I don't know why PBKDF2 or scrypt is more preferable than bcrypt but IMHO you should start considering whether it is time to offer "better" alternatives to bcrypt as well.