Crypt function does not work when comparing a string with a hash

Question

I use a fairly standard way to log into cookies - I give the user two cookies, one with my username and the other with a randomly generated string plus a user-defined salt.

This is what happens when you log in:

$_SESSION['username']=$row[username]; $_SESSION['user_id']=$row['id']; $loginhash=generateRandomBase64String()."_".$row['salt']; $number_of_days = 14; $date_of_expiry = time() + 60 * 60 * 24 * $number_of_days ; setcookie( "userlogin", $row['username'], $date_of_expiry, "/" ) ; setcookie( "loginhash", $loginhash, $date_of_expiry, "/" ) ; $cryptedhash=crypt($loginhash); $today=date("Ymd"); mysql_query("update members set last_login='$today',loginhash='$cryptedhash' where id='$row[id]' ") or die(mysql_error()); 

Thus, the value of $loginhash is Pe0vFou8qe++CqhcJgFtRmoAldpuIs+d_g5oijF76 , and the encrypted version is in the database. Salt is already in the database, because it is generated for each user during registration.

I use session variables ( $_SESSION[username] ) so that users are not logged in. Then, when the user visits the site, I check two things: if $_SESSION[username] not installed, and $_COOKIE[userlogin] is there, I check that the hash is correct, so I can log in to the user system. The problem is that a hash is never correct.

 if($_COOKIE['userlogin'] && !isset($_SESSION[user_id])){ $username=mysql_real_escape_string($_COOKIE['userlogin']); $loginhash=mysql_real_escape_string($_COOKIE['loginhash']); $salt=substr($loginhash,-8); $result=mysql_query("select * from members where (username='$username' || email='$username') && salt='$salt' limit 1 ") or die (mysql_error()); $row=mysql_fetch_assoc($result); $cryptedhash=$row['loginhash']; if (crypt($loginhash, $cryptedhash) == $cryptedhash){ $_SESSION['username']=$row[username]; $_SESSION['user_id']=$row['id']; } } 

$_COOKIE[userlogin] is the correct value. When I check the username / salt combination in the database, I get the correct result ( echo $row[username] gives the correct value). However, the if condition if lower, which is never fulfilled. I would think that there is something strange in my PHP configuration, but I use the same encryption mechanism to store passwords and it works correctly there.

So can anyone see what is going wrong here?

PS I'm not going to start discussing the security of cookies or the many hash functions available here.

Answer 1

Here's the problem:

The first time you call crypt (), you do not specify a salt. In the second crypt () call, you pass $ cryptedhash as salt.

crypt() documented to create a random salt if you haven't provided one, then add that salt to the returned hash. This has a side effect: if you pass the returned salt + hash as a hash for a subsequent call, crypt () will still pull the correct salt from it.

Unfortunately, the algorithm used and the length / format of the salt + hash are based on a combination of your operating system, version of PHP, and regardless of whether the salt parameter is specified. When you used your code earlier, you had a lucky chance that DES was selected in both crypt () calls. Now your environment uses a different algorithm for 2 crypt () calls, since you only put the hash in one of them.

The solution is to simply pass the consistent salt for both calls to crypt (). You can stop adding salt to the string you want to hash, and actually pass your user salt as a salt parameter, and everything will be fine.

Answer 2

This line is incorrect $loginhash=mysql_real_escape_string($_COOKIE['loginhash']); You do not need to avoid it (you do not use it with the database), you should use it without changes when you pass it to crypt (), so the line can be written as $loginhash=$_COOKIE['loginhash']; (at least for this part of the code)

Answer 3

If you are using PHP Version 5.5.0 or higher, you should take a look at PHPDoc - Password Hashing !
I know that you said that you didn’t want to discuss different hash functions, but I thought that this makes your work easier, since it should get rid of your problem and (in my oppion) is easier to use!

Here is your code with new features: (not tested if comment broke)

 $_SESSION['username']=$row[username]; $_SESSION['user_id']=$row['id']; $loginhash=generateRandomBase64String()."_".$row['salt']; $number_of_days = 14; $date_of_expiry = time() + 60 * 60 * 24 * $number_of_days ; setcookie( "userlogin", $row['username'], $date_of_expiry, "/" ) ; setcookie( "loginhash", $loginhash, $date_of_expiry, "/" ) ; $cryptedhash=password_hash($loginhash, PASSWORD_DEFAULT, array("cost" => 10)); // a cost of 10 is standard, you may want to adjust it according to your hardware (lower/higher cost means faster/slower) $today=date("Ymd"); mysql_query("update members set last_login='$today',loginhash='$cryptedhash' where id='$row[id]' ") or die(mysql_error()); 

Using PASSWORD_DEFAULT , you will see that even in future versions the strongest algorithm will always be used.

and

 if($_COOKIE['userlogin'] && !isset($_SESSION[user_id])){ $username=mysql_real_escape_string($_COOKIE['userlogin']); $loginhash=$_COOKIE['loginhash']; // i guess you should not use mysql_real_escape_string $salt=mysql_real_escape_string(substr($loginhash,-8)); // here would be the place to use it $result=mysql_query("select * from members where (username='$username' || email='$username') && salt='$salt' limit 1 ") or die (mysql_error()); $row=mysql_fetch_assoc($result); $cryptedhash=$row['loginhash']; if (password_verify($loginhash, $cryptedhash)){ $_SESSION['username']=$row[username]; $_SESSION['user_id']=$row['id']; } }