[creativecoding]

Here's the situation: You've created a successful service with over 1000 loyal members. The problem is, you decided to use md5, a common but bad hashing algorithm, for passwords. Somehow you've made it this far without getting some users account hacked so you still have time to convert to something more secure. The only problem is, you don't want to tell your users about this. It may freak them out and make them worried about their information in the wrong hands.

The solution: It's simple, really. Everything will look all fine and dandy on the outside where the user logs in, but secretly, on the inside, we will be converting their passwords to a new format (sha512 for this tutorial).


Here is exactly how we can achieve this. What we are looking at is the page that handles the login form.

<?php
// Get variables
$username = $_POST['username'];
$password = $_POST['password'];

// Setup our connection to database
$mysqli = new mysqli("localhost", "username", "password", "database");

// Never trust anything from the user!
$username = $mysqli->real_escape_string($username);

// Setup out query
$sql = "SELECT * FROM users WHERE username='$username'";
$result = $mysqli->query($sql) or die("There was an error.");

if(!$result){
echo "That user does not exists";
} else {
$result = $result->fetch_object();
}

// Check for our password
$md5pass = md5($password);
$shapass = hash("SHA512", $password, false);

if($result->password == $shapass){
// User's pass has been updated and is correct with pass in database.
// Run whatever they needed to login for
echo "Thank you for logging in";
} else if($result->password == $md5pass){
// User is correct, but his password has not been updated
// Update his password
// No fancy spanshy result checking, because they can still login even if the update fails.
$mysqli->query("UPDATE users SET password='$shapass' WHERE username='$username'");
// Run your login stuff.
echo "Thank you for logging in";
} else {
// User's pass is incorrect.
echo "Wrong username/password";
}
?>

Boom, now when people login, the script will first of all make a sha512 version of the pass and an md5 version of the pass. If the sha version matches the one in the DB, the user must be updated and correct. If it matches the md5, than update the password to the sha version and login. If it doesn't match at all, the password must be incorrect. A simple solution for a big problem!

[Ace26]

Wow! Nice thinking, creativecoding. That's a creative solution to such a problem. Welldone.

[Atli]

Yea, this is a very good method. I've used it myself. Highly recommend it for anybody still using MD5, or even SHA1!

One thing I would change in that code though. You always create the MD5 hash, even when the user has already been updated to use the SHA version. (And creating hashes is a relatively expensive operation.) - I would only create the MD5 hash after the SHA test has failed.

Also: prepared statements > manual escaping!

[armon]

Can you tell me why not to do it that way?

UPDATE users SET pw = sha2(pw,512)

and just change the script that is checking PW from

$pw = md5($pw_send_from_input);
SELECT * FROM users WHERE pw = '{$pw}'

to

$pw = hash('sha512',md5($pw_send_from_input));
SELECT * FROM users WHERE pw = '{$pw}'

and also remember about increasing the TYPE of pw from CHAR(32) to CHAR(128)

???

This post has been edited by armon: 30 September 2011 - 02:31 AM

[Atli]

armon, on 30 September 2011 - 09:31 AM, said:

Can you tell me why not to do it that way?

There are 2^128 possible values for a MD5 hash, and 2^512 for a SHA512 hash. However if the input of the SHA512 hash is a MD5 hash, then you are limiting the range of the SHA512 hashes to those 2^128 keys, making the resulting SHA512 hashes about as easy to brute-force as their MD5 equivalents would be.

[armon]

Atli, on 30 September 2011 - 03:32 AM, said:

armon, on 30 September 2011 - 09:31 AM, said:

Can you tell me why not to do it that way?

There are 2^128 possible values for a MD5 hash, and 2^512 for a SHA512 hash. However if the input of the SHA512 hash is a MD5 hash, then you are limiting the range of the SHA512 hashes to those 2^128 keys, making the resulting SHA512 hashes about as easy to brute-force as their MD5 equivalents would be.

Thanks that's right but what if I added salt like that:

$salt = "ASDF@42135rsadjio235`23651!@$^!&^!&&";
$query = "UPDATE users SET pw = sha2(CONCAT('$salt',pw),512)";

$salt = "ASDF@42135rsadjio235`23651!@$^!&^!&&";
$pw = hash('sha512',$salt.md5($pw_send_from_input));
SELECT * FROM users WHERE pw = '{$pw}'

salt now belongs to SHA512 so it means that if hacker got into our database without the salt he would need to break through SHA512(salt.md5(pw)) through brute force anyway? So we increase by that operation the security of everyone, not only people who are going to log in and update their password from md5 to sha512.

If hacker got access to the files also then he could break password easy. But we could use both solutions at once while updating database:
- the update i mentioned with a salt
- and then the script that was presented by creativecoding a little edited to work with the thing i presented

am i right?

[Atli]

Sure, you could do that. Adding the salt doesn't really increase the range, but like you say, if the salt remains unknown then the key may be outside the known MD5 values and brute-forcing based on them alone would not be an option. A an attacker would have to either know the salt or go ahead and try to crack the SHA512 hash itself.

Only question is: if the servers have been compromised in such a way that an attacker could steal your databases, will the PHP code, and the salt string, be safe? I would guess not.

[armon]

Atli, on 01 October 2011 - 06:31 AM, said:

Sure, you could do that. Adding the salt doesn't really increase the range, but like you say, if the salt remains unknown then the key may be outside the known MD5 values and brute-forcing based on them alone would not be an option. A an attacker would have to either know the salt or go ahead and try to crack the SHA512 hash itself.

Only question is: if the servers have been compromised in such a way that an attacker could steal your databases, will the PHP code, and the salt string, be safe? I would guess not.

If the salt is very very long and with all the possible characters it's almost impossible to crack it right? As far as I know we have 10^78 of atoms in the viewable universe, 2^512 is like 10^154 sooo yea it's almost impossible to find collision in sha512...

Okay... if hacker get the access to file, he will still have to remake his enormous rainbow table in order to match hashes with user passwords right? As far as I know creating hashes is very long.

He would need to do sha512($salt_that_he_now_knows.(md5($pw_from_rainbow_table)) for each $pw_from_rainbow_table so I at least increased the time he needs to crack the pw? As far as i know creating hashes is very long comparing to just comparison between two hashes from rainbow table?

This post has been edited by armon: 01 October 2011 - 07:37 AM

[carlosm7]

armon, on 30 September 2011 - 02:31 AM, said:

Can you tell me why not to do it that way?

Would not doing it this way add more complexity and/or require more processing power?

[carlosm7]

I would like to propose some changes:

Add a new boolean field to the user record, may be call it "PasswordUpdated", and set it to false for everybody.

Then, starting at the line #22:

// Check for our password
$shapass = hash("SHA512", $password, false);
if($result->password != $shapass){
    if($result->PasswordUpdated){
        // User's pass is incorrect.
        echo "Wrong username/password";
        exit();
        }
    else {
        $md5pass = md5($password);
        if($result->password != $md5pass){
            // User's pass is incorrect.
            echo "Wrong username/password";
            exit();
            }
        else {
            // User is correct, but his password has not been updated
            // Update his password
            // No fancy spanshy result checking, because they can still login even if the update fails.
            $mysqli->query("UPDATE users SET password='$shapass', PasswordUpdated=true WHERE username='$username'");
            }
        }
    }

// If we get here, user's pass has been updated and is correct with pass in database.
// Run whatever they needed to login for
echo "Thank you for logging in";

This way:

1- Except when doing the transition (or wrong passwords before the transition), no need to do more than one hash calculation.
2- No need to keep testing for the old password encoding schema if the user enters the wrong password (after the transition)
3- Only one "successfully logged in" code block

[Atli]

carlosm7, on 17 October 2011 - 07:04 AM, said:

This way:

1- Except when doing the transition (or wrong passwords before the transition), no need to do more than one hash calculation.
2- No need to keep testing for the old password encoding schema if the user enters the wrong password (after the transition)
3- Only one "successfully logged in" code block

The idea to find out which hashing algorithm is being used before doing any hashing is good. However...

For the first two points: you could also just use the length of the stored hash instead of the boolean field. It would only be 32 chars for MD5 but 128 chars for SHA512.

// So, basically, this check
if($result->PasswordUpdated)

// Is pretty much equal to this
if(strlen($result->password) == 128)

For the third point: while that's true, you've still got 3 result blocks. You've just shuffled which message is being printed when. - In fact, one could argue that creativecoding's flow is more natural, seeing as the order of his conditionals make failure the default option. He is checking whether the user's password is valid while you are checking whether it's invalid. (Perhaps not an important distinction, but in case of unforeseen validation errors this might make it more secure.)


Also, if you move the "success" message into the final else block you can remove both exit calls and let the execution end naturally. Better not to explicitly exit the script if you can avoid it.

This post has been edited by Atli: 17 October 2011 - 02:04 AM

[carlosm7]

Atli, on 17 October 2011 - 02:02 AM, said:

For the first two points: you could also just use the length of the stored hash instead of the boolean field. It would only be 32 chars for MD5 but 128 chars for SHA512.

Thank you for calling these into my attention, and to tell you the truth, it's been a while since I did any *real* PHP programming.

Last thing I did was a forums website, in the interest of learning PHP, and I uses MD5. I intend to rewrite it, as I need to get my PHP skills up to date, and certainly will use SHA512!

Atli, on 17 October 2011 - 02:02 AM, said:

For the third point: while that's true, you've still got 3 result blocks. You've just shuffled which message is being printed when. - In fact, one could argue that creativecoding's flow is more natural, seeing as the order of his conditionals make failure the default option. He is checking whether the user's password is valid while you are checking whether it's invalid. (Perhaps not an important distinction, but in case of unforeseen validation errors this might make it more secure.)


Also, if you move the "success" message into the final else block you can remove both exit calls and let the execution end naturally. Better not to explicitly exit the script if you can avoid it.

I should have known better, I remember reading something about it in the book "Code complete," a long time ago. I *really* need to re-read this book!