3

I copied the title from another question, but it wasn't answered there and my case is a bit different.

I have inherited an old website to maintain. I noticed that the password hashing is not up to modern best practices, but I would like to understand if it is reasonably safe or needs changing.

Here is the code that does the hashing:

public function changePW($password)
{
    //
    $new_salt = $this->random();
    $new_password = $this->hash($password, $new_salt);
    //
}

public function random($limit = 10, $from = 32, $to = 126) {
    $phrase = '';
    for ($i=0; $i<$limit; $i++) {
        $phrase .= chr(rand($from,$to));
    }
    return $phrase;
}

// self::$_salt is a constant 12 character string
// of 3 words with some letters replaced by numbers
public static function hash($password, $salt) {
    return hash('whirlpool', self::$_salt . $password . $salt);
}

Should I consider this procedure flawed/vulnerable for passwords of 8 characters and longer?

  • 1
    Did you check out the question the other question was closed as a duplicate of? The gist of the answer is that password hashing functions should be slow. Whirlpool is fast. So you really need to fix it. – Anders Aug 30 at 19:12
  • 1
    yes it's a problem; it's way too fast and cheap to compute the hash, allowing for quick brute-force guessing. you could at least hash it 100,000 times to spend CPU and repeat the password 10,000 times before first hash so as to spend RAM. Better yet would be to use a purpose-made KDF. – dandavis Aug 30 at 19:37
  • @Anders I am aware of the basic principles but I wasn't able to find an assessment of whirlpool. The linked question is one of the main results that came up when searching "whirlpool password hash" and it doesn't analyze whirlpool at all. "It's not considered the best practice these days" does not sound convincing enough to tell client we should reset every user's password to replace hash with a safer one. – Džuris Aug 30 at 20:36
  • @Džuris You can rehash passwords with a new alogirthm when users log in. That way you don't need to reset any passwords. But on the other hand, users who don't log in in a long time (or ever) will still have their password stored unsecurely. – Anders Aug 31 at 6:33
  • 2
    @Anders You could run all the hashes through a slow KDF even if the users don't log in (e.g. instead of storing H(pass) compute a KDF over that value and store KDF(H(pass)). Then optionally recompute it to just KDF(pass) when the user next logs in to their account if the complexity is a problem. – forest Aug 31 at 6:34
4

Whirlpool itself is a cryptographically-secure hash function like SHA-512 and has no known weaknesses that would be relevant to hashing secrets. However, using it directly for password hashing is a bad idea because it is fast, allowing an attacker to guess many passwords per second. This is not unique to the Whirlpool hash. A memory-hard KDF like Argon2 should be used, or at least a slow KDF like PBKDF2.

See How to securely hash passwords? for information on why this is important.

3

Use password-hash (https://www.php.net/manual/en/function.password-hash.php) which will continually use the most secure method as PHP continues to release new versions. Asking users to update passwords isn't a huge issue. Whirlpool doesn't have any known cryptographic flaws, but there are certainly better algorithms to use. Source: https://www.novatec-gmbh.de/en/blog/choosing-right-hashing-algorithm-slowness/

Additionally, the salt function shouldn't use rand() because it's not cryptographically secure; if an attacker knows what the salt is before getting access to the database, it can speed up crack time once access has been gained. All salt should be globally unique as much as can be reasonable.

  • 1
    why would a salt need to be generated in a cryptographically secure fashion? It's stored right next to the hash in the DB anyway... – dandavis Aug 30 at 19:36
  • 1
    @Džuris it can be made more secure, it's a usability issue. But why not fill the database with the current time, include the new password hashing function, and update the database the next time the user logs in and your hash function returns true? Then all users who have logged in once you've implemented the change use the more secure function. You can advise users to update their password and not make it a requirement if it really is a huge issue. Otherwise it's not a huge inconvenience to update the algorithm used. – LTPCGO Aug 30 at 20:54
  • 1
    @dandavis Well, in theory a system should be secure regardless of its implementation and thus the attacker could be presumed to know any hardcoded secrets, but yes, in practice (unless this thing is open source or the attacker has access to the internal repo or .php files on the server) that would prevent pre-computing the table even for specific users. It's still a bad idea to use a predictable salt in the general case, though. – CBHacking Aug 30 at 22:35
  • 1
    @LTPCGO The original design paper goes to extreme lengths to ensure that it is fast in hardware. I also got an opinion from a low-power silicon researcher in chat who estimates it's about 10x faster. It's like Keccak (SHA-3) in that sense. Slower in software, but much faster in hardware, which makes it a bad KDF. Not to mention, Whirlpool uses 8-bit operations and fast transpositions, whereas SHA-512 uses 64-bit operations, which is a much slower in hardware but decent in software. Though SHA-512 isn't a good KDF either... – forest Sep 2 at 8:07
  • 2
    Thanks. Removed any comment about the speed and clarified the answer, which hopefully improves it somewhat. – LTPCGO Sep 2 at 8:13
1

The algorithm you describe is normal fast hash. Such hashing algorithms should only be use to check integrity of files, but not for hashing password. Whirlpool and similar algorithms are designed to be fast and thus make brute force attacks easier.

Use key stretching / password derivation. For instance, use PBKDF2, scrypt or Argon2. The are designed to make brute force attacks inefficient and thus to make password protection better.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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