On Sun 18 Apr 2010 01:01 +0200, Linas wrote: > Arch User Repository (AUR) wrote: > > The following task has a new comment added: > > > > FS#17109 - AUR passwords are not salted > > User who did this - Loui Chang (louipc) > > > > ---------- > > A patch has been committed to git which will salt the passwords. > > ---------- > > > > More information can be found at the following URL: > > http://bugs.archlinux.org/task/17109#comment60615 > > These are good news, however, I don't think it is as good as "it would be": > > > # get salt for this user > > $salt = get_salt($userID); > > if ($salt) { > > # use salt > > ... > > $passwd_result = mysql_fetch_row(db_query($passwd_q, $dbh)); > > } else { > > ... > > $nosalt_result = mysql_fetch_row(db_query($nosalt_q, $dbh)); > > } > > > This gratuitously changes one sql query for login to three. > > It could be done as: > "SELECT ID,Passwd,Salt FROM Users" .+ " WHERE ID = '$userID'"; > $result = mysql_fetch_row(....) > if ($result[0]) { return false; } //No such user > if ( salted_hash($passwd, $result[2]) == $result[1] ) { return true; } > if ( is_null($result[2]) && (md5($passwd) == $result[1]) { update hash; > return true; } > > > Since it already encapsulates the salting into a function, it would be > trivial to allow the > salting compatible with already stored passwords, so they can be updated > with a script: > > function salted_hash($passwd, $salt) > { > if (strlen($salt) != 32) { > trigger_error('Salt does not look like an md5 hash', E_USER_WARNING); > } > return md5($salt . md5($passwd)); > } > > Finally, generate_salt() would be safer using mt_rand() instead of rand().
Thank you for your suggestions. These are things that are best discussed on the aur-dev mailing list. Seems like you've put some thought into this. Why don't you submit a patch?
