2

I was looking at our authentication system which hashes passwords differently than I was taught. The salt is a constant byte array and it uses the password + the salt as a key for the password hashing algorithm.

public byte[] GetHash (string password)
{
    var hmac = KeyedHashAlgorithm.Create("HMACSHA512");
    hmac.Key = HashAlgorithm.Create("SHA512").ComputeHash(this.salt + password);
    return hmac.ComputeHash(password);
}

I removed the conversion to byte arrays but simply put,

var hash = keyedHash(hash(salt+password), password);

Is this a secure way of hashing password? I was always told that salt should be generated each time a password is hashed and stored along with the password.

user38044
  • 23
  • 2

2 Answers2

3

This hashing method is a handmade construction, which, like almost all handmade constructions, is weak.

In this case, it commits the two cardinal sins of password hashing:

  • The hashing function allows for parallel attacks.
  • The hashing function is way too fast.

Here, if two users have the same password, then they end with the same hash. This means that if the attacker has a thousand password hashes to work with (e.g. he dumped the corresponding database table through some SQL injection), then he can hash a potential password and look up the hash value in the table: the attacker attacks 1000 passwords simultaneously, for roughly the same cost of attacking one. This is very bad. Salts are meant to avoid that, but this works only if each salt is used for one hashed password, not for all the hashed passwords on a given server.

As for speed: a basic PC will go through several millions of SHA-512 instances per second. Not many user-chosen passwords resist for a long time when the attacker can try millions per second...


@paj28's suggestion is somewhat better, however not as optimal as could be wished for. Using a hash of the username concatenated with a server-global value (but distinct from server to server) ensures some sort of worldwide uniqueness; however, if a user changes his password, then he will use the same "salt" (since this is on the same server, and the user did not switch names, just passwords). The attacker who could get the hashes of the old and the new password will still be able to attack both in parallel. This is better than the previous method, but still not perfect.

Also, the speed issue remains, and it is serious enough to warrant anathema on this proposal.

Read this answer to learn how password should be hashed, and what theoretical concepts are at work here. The short answer is: use bcrypt.

Tom Leek
  • 168,808
  • 28
  • 337
  • 475
1

I take it the salt is constant across all users? While that is better than no salt at all, it still makes a brute force attack easier - they can brute force all the user accounts at once. It also reveals when two users have the same password.

However, consider this minor variation:

hash = keyedHash(hash(system_salt+username), password);

This is basically ok. I wouldn't normally recommend it - it's more conservative to use per-user random salts - but it does gain you the required properties of a salt.

paj28
  • 32,736
  • 8
  • 92
  • 130
  • I agree it would reveal which users have the same password, but I don't see how you could brute-force all users at once. Whether it's the username or the password that is combined with the constant "salt", it's still introducing a non-static element to the equation. – Quick Joe Smith Jan 28 '14 at 10:55
  • @QuickJoeSmith - think about how you would perform a brute force attack. To try one password, you would calculate hash(hash(system_salt+password),password) - this is the same for all users, so you could compare that against all your stolen hashes. Repeat for the next billion passwords. Much more efficient that brute forcing users individually, especially if the user database is large. – paj28 Jan 28 '14 at 11:09
  • This may just me being thick, but I don't see how you'd be reducing your effort at all, _except in the case where two users have an identical hash_. For every other value, you'd still need to brute force the users individually. – Quick Joe Smith Jan 28 '14 at 11:27
  • Never mind, I see where it all falls down now. – Quick Joe Smith Jan 28 '14 at 12:05