Storing passwords in a database

Question

I just wrote this function and am wondering if it's secure enough for storing passwords in a database:

public static function hash($salt, $str) {
    $salt = md5($salt);
    $salt = '$2y$12$HfuPgoDY94nHJssqQeVLmH' . $salt . '$';

    return crypt($str, $salt);
}

Answer 1

Yes

Blowfish is a great algorithm for storing passwords, it makes generating a hash slow such that brute force attacks are unviable. It's also php's recommended means of storing passwords.

But

The salt for blowfish is supposed to be:

22 digits from the alphabet "./0-9A-Za-z".

This applies to the salt string which in the question is:

'HfuPgoDY94nHJssqQeVLmH' . $salt . '$'

That's 22 chars + 32 chars + '$', which is 65 characters long and contains an illegal '$'. The consequence of that is that the salt actually used is a fixed string, the first 22 characters and is the same for all generated hashes.

The salt should be random, and for example you could use something as simple as this:

$salt = '$2y$12$' . substr(md5(time()), 0, 22);
return crypt($str, $salt);

Note however that this code is simple for brevity and can be significantly improved upon since the salt will only be made up of hexadecimal (0-9a-f) characters. Here's an example implementation that could be used to generate a better salt.

Make cost a parameter

A cost of 12 is fine for passwords, but you may wish to make it a parameter so that e.g. unit tests are not needlessly slow whenever they generate a password.

Answer 2

No

md5 gives you a hexadecimal output, 0-9 & a-f. Salt for bcrypt takes base64 encoded string. This means that if you use (partial) md5 as salt, you are loosing a lot of entropy. See #9: Bonus 2 from this post. It's a good read on the subject.

I have the same advice as that poster: use a library.

Answer 3

No

If you can't pass hash length, salt length, and work units (iterations) into the hash function, you have no way of keeping up with faster CPU speeds.

The salt should be at least 64 characters, and it should be generated using a cryptographically secure random number generator. It needs to be unique for each hash. It should be the same size as the hash. Any less is too little entropy. Any more is wasted. Always pass work units into key stretching algorithms like bcrypt, PBKDF2, etc... That way you can increase the work required as CPUs get faster.

In 2014, a PBKDF2/HMAC+Sha1 algorithm needs 128,000 work units. Double the work every 2 years.

You need to store the work units with the hash so that you know how many to use for comparison, and you'll know to ask the users to select a new password when its time to upgrade their hash.

Addressing comments (there wasn't enough room in the comments field):

The salt should be at least 64 characters because the hash should be at least 64 characters. The salt should be the same size as the hash in order to prevent attackers from creating lookup tables for every possible salt. This is why it's a very bad idea to use a username as a salt -- especially now that we're fighting off attackers with enormous amounts of storage, CPU, and memory space compared to just a few years ago. In other words, having an equal length salt guarantees that generating tables for each salt (combining tables + dictionary attacks of the most common passwords) would take at least as long as a brute force attack.

The minimum length of 64 characters is the current recommendation (for the year 2014, 2013 is almost over) in response to changing CPU speeds. The recommendation of 22 was first made years ago -- CPUs double in speed every year, and current botnets exist with over 90,000 nodes, capable of generating billions of salts per second. Super cheap supercomputing clusters can be built today for less than $2k. The old standards just don't cut it anymore.

Salts need to be generated by a CSRNG in order to prevent collisions. Pseudo-random generators that are not CSRNGs are notoriously bad at that, often generating a collision within a few thousand hashes. For sites with hundreds of thousands or millions of users, that's not an acceptable rate.

It's important to prevent collisions because a collision means that the same tables can be used to attack multiple hashes at the same time.

Work units = cost parameter?

Work units = cost parameter, yes, but the original question is about a function which does not take a cost parameter. The cost is hard-coded in the function.

Sha1+HMAC is inappropriate

No, "HMACs are substantially less affected by collisions than their underlying hashing algorithms alone." Sha1+HMAC is far more secure than Sha1, and it is a currently accepted standard for password hashing.

and PBKDF2 is weaker than bcrypt so overall..

No, it isn't. Once you go over 60 characters or so, the cost of PBKDF2 trumps bcrypt by orders of magnitude.

It's ubiquitous and time-tested with an academic pedigree from RSA Labs, you know, the guys who invented much of the cryptographic ecosystem we use today. Like bcrypt, PBKDF2 has an adjustable work factor. Unlike bcrypt, PBKDF2 has been the subject of intense research and still remains the best conservative choice. There has been considerably less research into the soundness of bcrypt as a key derivation function as compared to PBKDF2, and simply for that reason alone bcrypt is much more of an unknown as to what future attacks may be discovered against it.

In other words, bcrypt is overrated. Especially if you think it's better than PBKDF2.

Source: http://www.unlimitednovelty.com/2012/03/dont-use-bcrypt.html