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)));
 }

Reply via email to