[aur-dev] FS#17109: AUR passwords are not salted

[Loui Chang]

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?