10
\$\begingroup\$

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? Are there more secure methods in PHP for hashing tokens and matching with tokens later on?

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;
}
\$\endgroup\$
2
  • \$\begingroup\$ Why don't you use the bcrypt functions? Also, you should read this: security.stackexchange.com/questions/211/…. It's focused on storing passwords, but you can extract good information. \$\endgroup\$ Aug 14, 2015 at 8:06
  • 3
    \$\begingroup\$ The usual comment regarding rolling your own crypto: don't do it. Use the widely-accepted standard cryptographic utilities such as bcrypt or PBKDF2. I mean, sure, play around with cryptography and write some code to learn more about it, but never use it in production with real people's data. Period. \$\endgroup\$ Aug 14, 2015 at 14:30

2 Answers 2

17
\$\begingroup\$

However, there is a lot of talk about what implementations are secure and not secure. How does my method measure up? Is it secure? Are there more secure methods in PHP for hashing tokens and matching with tokens later on?

Since you're specifically asking about security, I think reviewing your security instead of your code is a valid answer in this case.

The rule in Security-land is "don't do it yourself if others have done it better". It's basically a re-wording of "Use the right tool for the job" and "It isn't encryption until you've taken the time to fully understand the concepts behind the encryption code".

If you want to do hashing right, make sure you read this answer on Security.SE.

Basically, if you want to do it secure, use bcrypt. Since PHP 5.5 can be called with password_hash.

PHP itself also recommends bcrypt. Their example usage states something like that in the comments:

<?php
/**
* We just want to hash our password using the current DEFAULT algorithm.
* This is presently BCRYPT, and will produce a 60 character result.
 *
 * Beware that DEFAULT may change over time, so you would want to prepare
 * By allowing your storage to expand past 60 characters (255 would be good)
 */
echo password_hash("rasmuslerdorf", PASSWORD_DEFAULT)."\n";
?>

If you don't want to be surprised by a changed default value, change PASSWORD_DEFAULT to PASSWORD_BCRYPT.

Note they have more examples with varying complexity on that site, but this should convey the basics. It's perfectly possible to hand a whole sleeve of options to the function.

As a last note, you want to handle tokens. Do not store tokens in your database, only store hashed tokens. See also Part II of this answer.

Update:

In the above I assume you're using PHP 5.5 or higher. See this answer for implementations when using a lower version.

\$\endgroup\$
9
\$\begingroup\$

I'm going to assume that this is for educational purposes (otherwise,don't roll your own, use bcrypt).

  • use a timing safe string compare instead of ==.
  • write PHPDoc comments for your functions. It is eg not obvious that cryptorands range is [min, max). If it is reused in a different context, this can easily lead to bugs as max can never be returned.
  • reviewing cryptorand is a bit off-topic here (as it's not your code), you could try it at crypto.SE.

  • you use too many parentheses, which makes your code harder to read. Eg: (generateToken(32)) : ($salt) or return (crypt($token,$hash) == $hash);.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.