FS#52297 - Passwords should be stored more securely

Attached to Project: AUR web interface
Opened by Alex Muller (alexmuller) - Thursday, 29 December 2016, 07:25 GMT
Last edited by Lukas Fleischer (lfleischer) - Tuesday, 11 July 2017, 05:09 GMT
Task Type Bug Report
Category Backend
Status Closed
Assigned To No-one
Architecture All
Severity Medium
Priority Normal
Reported Version 4.3.0
Due in Version 4.5.0
Due Date Undecided
Percent Complete 100%
Votes 2
Private No

Details

I'm new to the aurweb codebase but I've had quick look around and
it looks like the `salted_hash` function that creates a password
returns `md5($salt . $passwd)`.

Also, `passwd_min_len` is set to 4 characters.

If the aur database was compromised, an attacker could enumerate
passwords easily [1].

I'd recommend using a different hash to MD5 to store passwords,
and increasing the minimum length of a password.

[1]: https://security.stackexchange.com/questions/61712/cracking-md5-passwords-with-the-same-salt
This task depends upon

Closed by  Lukas Fleischer (lfleischer)
Tuesday, 11 July 2017, 05:09 GMT
Reason for closing:  Fixed
Additional comments about closing:  Fixed in 4.5.0.
Comment by Mark Weiman (markzz) - Friday, 20 January 2017, 06:45 GMT
One of the things I'd be concerned with is how would the transition be handled if the hashing algorithm is changed.

Would all users be forced to change their passwords on their next login?
Would there be "backwards compatibility" with old passwords and allow users to change their passwords at their leasure?

Plus, I'm not entirely sure if this change is urgent or not. On a personal note, if someone figured out my password on the AUR, I would not be too bothered by it since I don't use the password anywhere else.

Would be interesting to have a discussion on this.
Comment by Eli Schwartz (eschwartz) - Friday, 20 January 2017, 07:16 GMT
The aurweb maintainers could always crack the password list themselves, and upgrade it without user intervention... it is the exact same threat model aurweb is currently vulnerable to, except carried out by the good guys. ;)
Comment by Alex Muller (alexmuller) - Sunday, 22 January 2017, 10:10 GMT
The kind of transition that I've seen for this kind of thing in the past is to add an additional column for a new password hash and then compute that hash each time a user logs in (because that's when you have the plaintext password available). At some point in time later (based on the data you'll have about what proportion of users have had their password migrated) you can make the decision to drop the old column, and everyone else will have to do a reset.

I think this is probably fairly important - if there is a single user who reuses passwords across multiple services then any leak of the aurweb database is essentially exposing accounts for other services. I don't know anything about the infrastructure that runs aurweb but if this change isn't going to happen soon it might be worth rotating database passwords, verifying which users have access to the production database and things like that.
Comment by Mark Weiman (markzz) - Sunday, 22 January 2017, 15:21 GMT
Since md5 hashes are only 32 characters and say, sha256 is 64 characters (if I'm correct), would it be more effective to test and see if the hash stored is long enough and if not, then ask the user to reset (of course after checking to see if the old hash matches the hash of the password entered)?

I see an issue if you just drop an old column in the database with old password information, anyone can then hijack accounts that weren't reset. That would be a bigger problem in my opinion. This is assuming that the method you suggested is used.
Comment by Mortan (Mortan1961) - Monday, 13 March 2017, 04:21 GMT
I would recommend reading this article on how to store passwords using Argon2i: https://paragonie.com/blog/2016/02/how-safely-store-password-in-2016
Comment by Eli Schwartz (eschwartz) - Monday, 13 March 2017, 04:44 GMT
Thanks for the advice, but given that the auweb maintainers aren't *complete* morons, it turns out they are *already* using bcrypt as of February 26. This is rather common advice, so it is not surprising it was thought of. :)

This bug ticket should be closed.

Loading...