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?

Anders
  • 64,406
  • 24
  • 178
  • 215
Džuris
  • 269
  • 1
  • 7
  • 1
    Did you check out [the question](https://security.stackexchange.com/questions/211/how-to-securely-hash-passwords) 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 '19 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 '19 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 '19 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 '19 at 06: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 '19 at 06:34

3 Answers3

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.

Glorfindel
  • 2,235
  • 6
  • 18
  • 30
forest
  • 64,616
  • 20
  • 206
  • 257
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.

LTPCGO
  • 965
  • 1
  • 5
  • 22
  • 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 '19 at 19:36
  • I am aware of best practices. I am asking if the actual solution is bad enough to warrant changing it and requiring the existing users to reset their passwords? Or can it stay as it is until the whole app is remade? – Džuris Aug 30 '19 at 20:24
  • @dandavis The purpose of the salt is prevent pre-computing the possible password hashes (rainbow table). While simply using a unique salt per user achieves most of the benefits of a salt, an attacker who knows when a user account was registered and therefore knows what salt they'd have (assuming a time-seeded PRNG) could pre-compute the hashes for that userbefore gaining access to the database at all, and therefore compromise specific user(s) immediately upon gaining access to the auth DB. It's a small difference but it's still a good idea to use a secure [P]RNG. – CBHacking Aug 30 '19 at 20:53
  • 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 '19 at 20:54
  • @CBHacking: wouldn't the attacker also need to know the OP's `$_salt` before building a rainbow table? that comes from a different process, so I don't see the mechanical advantage of pre-guessing the user's salt in this setup. Am I missing something? – dandavis Aug 30 '19 at 22:05
  • @dandavis having a large salt means it's expensive for an attacker to generate a rainbow table. If the salt can be pre-determined, it's as good as having no salt as there is only one table that needs to be generated. – LTPCGO Aug 30 '19 at 22:12
  • 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 '19 at 22:35
  • I would not call cracking a password in 3 minutes "quite slow". Note that the scale on the diagram in your link i logarithmic. – Anders Aug 31 '19 at 06:30
  • @anders it isn't the slowest but minutes compared to seconds is 'quite' slow – LTPCGO Aug 31 '19 at 09:41
  • @LTPCGO Yes, that is correct. Maybe I read your answer wrong, but from the "quite slow" part I sort of inferred that the algorithm could be seen as good enough for password hashing. Which it isn't. But maybe I am reading in to much there. – Anders Aug 31 '19 at 10:02
  • Actually, Whirlpool, when implemented in silicon, is _significantly_ faster than SHA-512, so from an ASIC perspective, it's slow for honest parties but fast for attackers, which is the opposite of what you want. – forest Sep 02 '19 at 08:04
  • @forest do you have a source or the paper on that? Not doubting it, but it sounds interesting! – LTPCGO Sep 02 '19 at 08:06
  • 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](https://chat.stackexchange.com/transcript/message/51289541#51289541) 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 02 '19 at 08:07
  • 2
    Thanks. Removed any comment about the speed and clarified the answer, which hopefully improves it somewhat. – LTPCGO Sep 02 '19 at 08:13
  • 1
    It did. I upvoted your answer! – forest Sep 02 '19 at 08:14
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.

mentallurg
  • 8,536
  • 4
  • 26
  • 41