1

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.

Hurricane Development
  • 2,449
  • 1
  • 19
  • 40
  • 4
    Better post this at http://codereview.stackexchange.com/ – gbestard Aug 14 '15 at 06:11
  • is this for swiss bank, lol. This is fine enough. – Mubin Aug 14 '15 at 06:14
  • @MubinKhalid Maybe...No but a lot of information is stored in users accounts, and there are so many easy ways to hash passwords now that I figured I might as well spend an hour or two to ensure that I am doing it the best way possible. – Hurricane Development Aug 14 '15 at 06:17
  • https://crackstation.net/hashing-security.htm http://stackoverflow.com/questions/20813037/what-way-is-the-best-way-to-hash-a-password – Mubin Aug 14 '15 at 06:23
  • @MubinKhalid Sweet thanks, let the research begin :D – Hurricane Development Aug 14 '15 at 06:26
  • My pleasure mate, happy helping :) – Mubin Aug 14 '15 at 06:27
  • 2
    In PHP you should simply use the [password_hash()](http://www.php.net/manual/en/function.password-hash.php) and [password_verify()](http://www.php.net/manual/en/function.password-verify.php) functions, because they implement all the best practices. BTW where does the `cryptorand()` function comes from, is it ok to pass 0..count as argument or should it be 0..count-1? – martinstoeckli Aug 14 '15 at 07:37
  • 1
    @martinstoeckli The advice to use the build in functionality is OK, but the `cryptorand()` seems to have been included in the source of the question. – Maarten Bodewes Aug 14 '15 at 10:20
  • @MaartenBodewes - Indeed, i should really read a bit more careful. – martinstoeckli Aug 14 '15 at 18:57

1 Answers1

3

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.

Maarten Bodewes
  • 90,524
  • 13
  • 150
  • 263
  • "I'm not sure that those are completely secure given the state of security in PHP, but at least you can blame somebody else" They're as secure as `crypt(3)` is, actually. Ask @ircmaxell for details if you're curious :) – Scott Arciszewski Aug 14 '15 at 13:31
  • Hi @HurricaneDevelopment, is this enough to answer the question? – Maarten Bodewes Aug 23 '15 at 14:29
  • @HurricaneDevelopment I would accept Maarten's answer. It's on-point and contains no factual inaccuracies. (I was merely responding to his parenthetical statement about being unsure if they are completely secure.) – Scott Arciszewski Aug 28 '15 at 21:29