Hurricane Development
Hurricane Development

Reputation: 2464

How secure is this method of password hashing and matching?

I took information from a series of posts and some prior knowledge to implement the following hashing algorithm. However there is a lot of talk about what implementations are secure and not secure. How does my method measure up? Is it secure?

public static function sha512($token,$cost = 50000,$salt = null) {
        $salt = ($salt == null) ? (generateToken(32)) : ($salt);
        $salt = '$6$rounds=' . $cost . '$' . $salt . ' $';
        return crypt($token, $salt);
}

public static function sha512Equals($token,$hash) {
    return (crypt($token,$hash) == $hash);
}


public static function generateToken($length,$characterPool = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ') {
    $token = '';
    $max = mb_strlen($characterPool);

    for ($i = 0;$i < $length;$i++){
        $token .= $characterPool[cryptorand(0,$max)];
    }

    return $token;
}

public static function cryptorand($min, $max) {
    $range = $max - $min;

    if ($range < 0) 
        return $min;

    $log = log($range, 2);
    $bytes = (int) ($log / 8) + 1; // length in bytes
    $bits = (int) $log + 1; // length in bits
    $filter = (int) (1 << $bits) - 1; // set all lower bits to 1

    do {
        $rnd = hexdec(bin2hex(openssl_random_pseudo_bytes($bytes)));
        $rnd = $rnd & $filter; // discard irrelevant bits
    } while ($rnd >= $range);

    return $min + $rnd;
}

So is this method secure? Are there more secure methods in PHP for hashing tokens and matching with tokens later on? Any criticism is hugely appreciated.

Upvotes: 1

Views: 82

Answers (1)

Maarten Bodewes
Maarten Bodewes

Reputation: 93968

No, because you end up trusting crypt and you are not using a time constant compare in sha512Equals.

There may be platform specific issues too: openssl_random_pseudo_bytes doesn't have to be cryptographically secure. I'm not sure how you know that crypt uses SHA-512 either.

Your calculations in cryptorand are slightly off (e.g. for values of $log that are precisely on a byte boundary) but that's fortunately kept in check by the do/while loop.


Please use the password_hash or password_verify functionality instead.

Upvotes: 3

Related Questions