FS#17109 - AUR passwords are not salted

Attached to Project: AUR web interface
Opened by Gavin Bisesi (Daenyth) - Thursday, 12 November 2009, 16:04 GMT
Last edited by Loui Chang (louipc) - Monday, 20 September 2010, 01:27 GMT
Task Type General Gripe
Category Backend
Status Closed
Assigned To No-one
Architecture All
Severity High
Priority Normal
Reported Version 1.6.0
Due in Version 1.7.0
Due Date Undecided
Percent Complete 100%
Votes 12
Private No

Details

With the current work by foutrelis on password resets, I noticed that the AUR does not salt user passwords. This means that if the aur password db is comprimised, the user passwords are MUCH easier to crack.

Due to the fact that all existing users have their hashes stored unsalted, we would need two code paths in the login page. I think this would require a new db entry, "salted", as a bool so that you could do

login_user {
if pass_is_salted: check_hash(md5(username + entered_pass [+ get_salt_string_from_config()]))
else: check_hash(md5(entered_pass)
}


See also: http://en.wikipedia.org/wiki/Salt_%28cryptography%29
This task depends upon

Closed by  Loui Chang (louipc)
Monday, 20 September 2010, 01:27 GMT
Reason for closing:  Implemented
Additional comments about closing:  1.7.0
Comment by Gavin Bisesi (Daenyth) - Thursday, 12 November 2009, 16:05 GMT
Related to  FS#3061 
Comment by Gavin Bisesi (Daenyth) - Thursday, 12 November 2009, 16:06 GMT
The unsalted code path should also give the user a small alert linking to an in-depth explanation of why they should change their password (to get the benefits of being salted)
Comment by Gavin Bisesi (Daenyth) - Thursday, 12 November 2009, 16:14 GMT
And at a future date (say, if 90+% of logins within $given_timeframe are salted), disabled the unsalted codepath altogether.
Comment by Sven-Hendrik Haase (Svenstaro) - Wednesday, 02 December 2009, 15:55 GMT
Oh dear this is an important one. Did any work get done for this yet?
Comment by Gavin Bisesi (Daenyth) - Wednesday, 02 December 2009, 18:07 GMT
I think the current state is "patches very welcome". I suck at php, so I'm not really the best one to do this.
Comment by xduugu (xduugu) - Wednesday, 02 December 2009, 18:25 GMT
> And at a future date (say, if 90+% of logins within $given_timeframe are salted), disabled the unsalted codepath altogether.

I think the common approach is:

if (md5sum($_POST['passwd'] . $salt) == $dbhash) {
login();
} elseif (md5sum($_POST['passwd']) == $dbhash) {
$salt = random_salt();
update_database("salt = $salt", "dbhash = md5sum($_POST['passwd'] . $salt)");
login();
} else {
die "Wrong password";
}
Comment by Gavin Bisesi (Daenyth) - Wednesday, 02 December 2009, 19:06 GMT
Hmm, that seems like it might work. Good call!
Comment by Marti (intgr) - Monday, 21 December 2009, 10:09 GMT
While you're at it, you should change your password hashing function from a simple md5sum() to something more involved, like 1000 or 10,000 iterations of MD5. This will cost nearly nothing in terms of CPU power (authenticaiton is hardly the bottleneck in any application), but means that any password cracker needs to do magnitudes more work.

The concept is called "key strengthening": http://en.wikipedia.org/wiki/Key_strengthening
Comment by webnull (webnull) - Friday, 01 January 2010, 18:56 GMT
$sql -> query ( 'SELECT `password`,`uid` FROM `users`');

while ( $row = $sql -> fetch_assoc ( ) )
{
# $sql -> query ()...? # x 100 = 100 queries... lol
}
Comment by Linas (Linas) - Wednesday, 10 March 2010, 15:15 GMT
No need to have two code paths or wait until the user logs in.
Add a salted column and replace the hash(password) with hash(salt + hash(password))
Comment by Gavin Bisesi (Daenyth) - Wednesday, 10 March 2010, 15:17 GMT
The database contains the unsalted hashes, and if you try to look up a salted hash, it will not match their stored password, meaning every single aur user would need a password reset.
Comment by Linas (Linas) - Wednesday, 10 March 2010, 15:59 GMT
You would update the db to a salted format, the users wouldn't need to request resets.
The only thing you need is to switch the files at the same time you update the database.
Comment by Gavin Bisesi (Daenyth) - Wednesday, 10 March 2010, 16:35 GMT
I see what you mean now.

Can anyone comment as to whether this would be more or less secure than any other proposed method?
Comment by Marti (intgr) - Wednesday, 10 March 2010, 16:53 GMT
@Daenyth: Best to use PBKDF2 (Password Based Key Derivation Function) which is a well-known standard in the cryptography world (RFC 2898, PKCS #5 v2.0).
It includes a salt as well as a configurable number of iterations.

Here's a PHP tutorial for PBKDF2:
http://www.itnewb.com/v/Encrypting-Passwords-with-PHP-for-Storage-Using-the-RSA-PBKDF2-Standard
Comment by Loui Chang (louipc) - Saturday, 17 April 2010, 22:14 GMT
A patch has been committed to git which will salt the passwords.
Comment by Marti (intgr) - Sunday, 18 April 2010, 12:03 GMT
If you're going to change the way that passwords work, then please do it PROPERLY once and for all.
Comment by Marti (intgr) - Sunday, 18 April 2010, 12:06 GMT
And really, nobody should be using MD5 in the year 2010. It was already broken back in 2004, and although the consequences for password hashing are small so far, there's no reason to use a broken hash function if secure ones exist (SHA256, SHA512).
Comment by Loui Chang (louipc) - Sunday, 18 April 2010, 16:12 GMT
When it comes to AUR devel, my job is mainly to review and apply patches.
Don't expect anything to be done without them.

So why don't you send in a patch if it worries you so much?

Thanks.
Comment by revel (revel) - Thursday, 15 July 2010, 03:37 GMT
what is available on the server?
do we have 'sha1' function? (PHP 4 >= 4.3.0, PHP 5) - SHA1 is still weak but better than MD5
do we have 'mhash' function? ('mhash' extension, PHP 4, PHP 5) - obsoleted by 'hash' extension listed below
do we have 'hash' function? ('hash' extension, PHP 5 >= 5.1.2, PECL hash >= 1.1)

Loading...