Loui Chang wrote: > 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? >
Thanks for your support. Here goes an attempt. I have mixed my suggestions with Denis idea of changing the hash algorithm at the same time plus a few bits I found on the way. Actions performed by this patch: salt to be NULL, in which case it is treated as md5 hashed password. *All entries can be automatically updated by a -to be written- script. *Removes add salt on login code, per above. *Salted passwords now use sha512 instead of md5. *Adds requirement on hash extension (usually bundled as static). *try_login() only performs one query to verify user login instead of 5. *generate_salt now uses mt_rand() *Reject passwords given by GET.
diff --git a/support/schema/aur-schema.sql b/support/schema/aur-schema.sql index 250d405..f12c57d 100644 --- a/support/schema/aur-schema.sql +++ b/support/schema/aur-schema.sql @@ -25,8 +25,8 @@ CREATE TABLE Users ( Suspended TINYINT UNSIGNED NOT NULL DEFAULT 0, Username CHAR(32) NOT NULL, Email CHAR(64) NOT NULL, - Passwd CHAR(32) NOT NULL, - Salt CHAR(32) NOT NULL DEFAULT '', + Passwd CHAR(128) NOT NULL, + Salt CHAR(32), ResetKey CHAR(32) NOT NULL DEFAULT '', RealName CHAR(64) NOT NULL DEFAULT '', LangPreference CHAR(5) NOT NULL DEFAULT 'en', diff --git a/web/README b/web/README index 041b521..f3c1f60 100644 --- a/web/README +++ b/web/README @@ -44,6 +44,8 @@ Setup on Arch Linux: If those php extensions are separate packages on your system, install them. + The hash extension is usually statically compiled into php. If it is + provided as an extension, add it too. AUR requires PEAR and the File_Find module. Installing PEAR will vary depending on the system and may already diff --git a/web/lib/acctfuncs.inc b/web/lib/acctfuncs.inc index 9c172bb..d87d5e6 100644 --- a/web/lib/acctfuncs.inc +++ b/web/lib/acctfuncs.inc @@ -601,25 +601,19 @@ function display_account_info($U="", $T="", $E="", $R="", $I="") { * SID of 0 means login failed. */ function try_login() { - $login_error = ""; + $login_error = "Missing username or password."; $new_sid = ""; $userID = null; - if ( isset($_REQUEST['user']) || isset($_REQUEST['passwd']) ) { + if ( isset($_REQUEST['user']) && isset($_POST['passwd']) ) { + list($userID, $login_error) = valid_user_passwd($_REQUEST['user'], $_REQUEST['passwd']); - $userID = valid_user($_REQUEST['user']); - - if ( user_suspended( $userID ) ) { - $login_error = "Account Suspended."; - } - elseif ( $userID && isset($_REQUEST['passwd']) - && valid_passwd($userID, $_REQUEST['passwd']) ) { + if ( isset($userID) ) { $logged_in = 0; $num_tries = 0; # Account looks good. Generate a SID and store it. - $dbh = db_connect(); while (!$logged_in && $num_tries < 5) { $new_sid = new_sid(); @@ -699,28 +693,6 @@ function valid_username( $user ) return; } -/* - * Checks if the username is valid and if it exists in the database - * Returns the username ID or nothing - */ -function valid_user( $user ) -{ - /* if ( $user = valid_username($user) ) { */ - if ( $user ) { - $dbh = db_connect(); - $q = "SELECT ID FROM Users WHERE Username = '" - . mysql_real_escape_string($user). "'"; - - $result = mysql_fetch_row(db_query($q, $dbh)); - - # Is the username in the database? - if ($result[0]) { - return $result[0]; - } - } - return; -} - function good_passwd( $passwd ) { if ( strlen($passwd) >= PASSWD_MIN_LEN ) { @@ -729,58 +701,33 @@ function good_passwd( $passwd ) return false; } -/* Verifies that the password is correct for the userID specified. - * Returns true or false +/* Verifies that for the user name specified the user exists, the + * password is correct and it is not suspended. + * Returns userid and error message */ -function valid_passwd( $userID, $passwd ) +function valid_user_passwd( $userName, $passwd ) { if ( strlen($passwd) > 0 ) { $dbh = db_connect(); - - # get salt for this user - $salt = get_salt($userID); - if ($salt) { - # use salt - $passwd_q = "SELECT ID FROM Users" . - " WHERE ID = '$userID' AND Passwd = '" . - salted_hash($passwd, $salt) . "'"; - $passwd_result = mysql_fetch_row(db_query($passwd_q, $dbh)); - if ($passwd_result[0]) { - return true; - } - } else { - # check without salt - $nosalt_q = "SELECT ID FROM Users". - " WHERE ID = '$userID'" . - " AND Passwd = '" . md5($passwd) . "'"; - $nosalt_result = mysql_fetch_row(db_query($nosalt_q, $dbh)); - if ($nosalt_result[0]) { - # password correct, but salt it first - if (!save_salt($userID, $passwd)) { - trigger_error("Unable to salt user's password;" . - " ID $userID", E_USER_WARNING); - return false; - } - - return true; - } + + $query = "SELECT ID, Suspended, Passwd, Salt FROM Users" . + " WHERE Username = '" . mysql_real_escape_string($userName, $dbh) . "'"; + + $result = mysql_fetch_row(db_query($query, $dbh)); + if (!mysql_num_rows($dbh)) { + return array(null, "User doesn't exist."); } + + if ($result[1]) + return array(null, "Account Suspended."); + + if ( salted_hash($passwd, $result[3]) == $result[2] ) { + return array($result[0], "Login successful"); + } + return array(null, "Wrong password."); } - return false; -} - -/* - * Is the user account suspended? - */ -function user_suspended( $id ) -{ - $dbh = db_connect(); - $q = "SELECT Suspended FROM Users WHERE ID = '$id'"; - $result = mysql_fetch_row(db_query($q, $dbh)); - if ($result[0] == 1 ) { - return true; - } - return false; + + return array(null, "Wrong password."); } /* diff --git a/web/lib/aur.inc b/web/lib/aur.inc index 8ccce89..8f8d469 100644 --- a/web/lib/aur.inc +++ b/web/lib/aur.inc @@ -456,33 +456,15 @@ function mkurl($append) { return substr($out, 5); } -function get_salt($user_id) -{ - $dbh = db_connect(); - $salt_q = "SELECT Salt FROM Users WHERE ID = '$user_id'"; - $salt_result = mysql_fetch_row(db_query($salt_q, $dbh)); - return $salt_result[0]; -} - -function save_salt($user_id, $passwd) -{ - $dbh = db_connect(); - $salt = generate_salt(); - $hash = salted_hash($passwd, $salt); - $salting_q = "UPDATE Users SET Salt = '$salt'" . - ", Passwd = '$hash' WHERE ID = '$user_id'"; - return db_query($salting_q, $dbh); -} - function generate_salt() { - return md5(uniqid(rand(), true)); + return md5(uniqid(mt_rand(), true)); } 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 . $passwd); + if (!isset($salt)) + return md5($passwd); /* Legacy */ + + return hash('sha512', pack("H*" , $salt) . "##" . pack("H*" , md5($passwd))); }