Iteration of password hashing in PHP - follow-up

Question

This is a follow-up question to Iteration of password hashing in PHP

I've gone trough a lot of documentations and stuff to get a feel for this hashing jungle, and done some adjustments to my code along the way.

PBKDF2 seemed like a good fit, so that's wath I went for.
Also, it's important to know that I don't have access to PHP 5.5 with my current host. Only 5.2, 5.3 and 5.4 is available. And thy won't let me update it either...

I've created a function - which is just a stripped down modification of the pbkdf2 suggested multiple times on Stack Overflow. (please see links at the bottom).

function pbkdf2($algorithm,$salt,$password,$iterations,$hash_bytes){
    /*  I've removed the checks at the beginning, along with the  if (function_exists("hash_pbkdf2"))  */
    $hash_length = strlen(hash($algorithm, "", true));
    $block_count = ceil($hash_bytes/$hash_length);
    $output = '';
    for($i=1; $i<=$block_count; $i++){
        $last = $salt.pack("N", $i);  //  $i encoded as 4 bytes, big endian.
        $last = $xorsum = hash_hmac($algorithm, $last, $password, true);  //  first iteration
    #   perform the other $iterations - 1 iterations
        for($j=1; $j<$iterations; $j++){
            $xorsum ^= ($last = hash_hmac($algorithm, $last, $password, true));
        }
    //
        $output .= $xorsum;
    }
    /*  I also removed the  $raw_format  variable  */
    return base64_encode(substr($output, 0, $hash_bytes));
}

In addition, I'm taking advantage of this one as well when comparing the hashes on login:
well explained here

function slow_equals($a, $b){
    $diff = strlen($a) ^ strlen($b);
    for($i=0; $i<strlen($a) && $i<strlen($b); $i++){
        $diff |= ord($a[$i]) ^ ord($b[$i]);
    }
    return $diff === 0;
}

The adjustments I've made (original question here) is basically that I've switched

$hash = hash('sha256',$salt.$_POST['secret']);  //  create a 64 character hash based on the salt and the password

out with

$hash = pbkdf2($algorithm, $salt, $_POST['secret'], $iterations, $hash_bytes);  //  create a hash based on the salt and the password

when adding a new user (first script).

Then, inside the second script, where the user is logging in, I'm matching the password/hashes using slow_equals() instead of just is this string equal to the other one? (==) ...

if(slow_equals(pbkdf2($algorithm, $get['salt'], $_POST['secret'], $iterations, $hash_bytes), $get['hash'])){
/*  ...  */
}

My "settings" for the pbkdf2()-function is as follows:

$algorithm = 'sha512';  //  don't really know which algorithm to use
$iterations = 5000;  //  seemed like a good fit
$hash_bytes = 48;  //  produces a 64 character long string

Here are the two scipts with the mentioned adjustments:

/**
 *  This is the script that is executed when I add a new user - after the input has passed the validation.
 */
    $salt = base64_encode(mcrypt_create_iv($hash_bytes,MCRYPT_DEV_URANDOM));  //  generate a random 32 character salt
    $hash = pbkdf2($algorithm, $salt, $_POST['secret'], $iterations, $hash_bytes);  //  create a hash based on the salt and the password

    $ins = $con['site']->prepare('INSERT INTO users (handle, salt, hash) VALUES (:handle, :salt, :hash)');
    $ins->execute(array(':handle'=>$_POST['handle'], ':salt'=>$salt, ':hash'=>$hash));
    if($ins){
        KD::addNotice('success','...message...');
    } else {
        KD::addNotice('error','...message...');
    }

/**
 *  This is the script that is executed when the user is logging in - also after the input has passed the validation.  
 *  Changed to "early returns" as suggested by @tim
 */
    $qry = $con['site']->prepare('SELECT handle, salt, hash FROM _site_users WHERE BINARY handle = :handle');
    $qry->execute(array(':username'=>$_POST['handle']));
    if($qry->rowCount()!=1){
        KD::addNotice('error','...message...');
        return;
    }
    $get = $qry->fetch();
    if(slow_equals(pbkdf2($algorithm, $get['salt'], $_POST['secret'], $iterations, $hash_bytes), $get['hash'])){
        KD::addNotice('success','...message...');
        session_regenerate_id();
        $_SESSION['backend']->login($get['handle']);
    } else {
        KD::addNotice('error','...message...');
    }

Any opinions regarding this updated code? Also, have I in any way made the pbkdf2 vulnerable with the modifications I've done?

Links:

• https://security.stackexchange.com/questions/211/how-to-securely-hash-passwords/31846#31846
• https://crackstation.net/hashing-security.htm

Answer 1

Early returns in this case means that in theory, an attacker could execute a timing attack to enumerate usernames (since, if the name is present, it has to calculate a hash, while it does not have to hash something when the name is not present). In this case, you should use a prepared combination of salt, password and hash to simulate the hashing and comparing of the password. That way, the hashing will always happen and there is less discrepancy in length between authenticating a user and a non-user.

Answer 2

Timing attacks

I noticed you use slow_equals() to prevent timing attacks. This is a relatively complicated function. Why not use the normal == and combine it with:

usleep(rand(0,10000));

This is much easier to understand, and leaves your server free to do other things. You can (almost) ignore the fact that the rand() function is not pefect, it's good enough for this job. You can occasionally seed it, if you're worried a hacker might compensate for the 'random' sequence. I would like to meet that hacker! Nevertheless this is better:

function sleepRandom()
{
  if (rand(0,10) == 1) srand((double)microtime()*1000000); 
  usleep(rand(0,10000));
}

Once every 10 page calls will be quite sufficient.

You should realise that this simple solution might even be better than the solution you use now. slow_equals() only hides one thing, this solution hides all timing issues in your code.

Don't be fooled by people saying you should not think about your code, and argue you should only use theirs. If everybody did that, it would be a field day for the hackers.

Please choose the delay at least 5 times the normal time PHP takes to execute the script. If you make it too small the execution time still largely depends on what the script does. You can use two microtime(TRUE)'s, at the start and the end of the script to measure how long it takes.

$start    = microtime(TRUE);
<.... all your code ....>
$finish   = microtime(TRUE);
$exectime = $finish-$start;

Do not use the browser to measure execution time, since it cannot. A normal script, in a framework, should execute in 10 - 20 milliseconds. This does, of course, depend heavily on the speed of your server, but if your login code takes more than 100 milliseconds to execute it is seriously inefficient.