Password hashing

Question

I use the following code to store user passwords.

string password = "...";
string user_salt = System.Web.Security.Membership.GeneratePassword(20, 5);
string common_salt = "...";
byte[] hash_target = Encoding.UTF8.GetBytes(password + user_salt + common_salt);
string password_hash = BitConverter.
    ToString(new SHA512CryptoServiceProvider().ComputeHash(hash_target)).
    Replace("-", string.Empty).
    ToUpper();

The user salt is stored together with the hashed password in the database. Additionally, every time a user's password is changed, the user salt is regenerated.

The reasoning behind a common salt and a user salt is: access to the database alone isn't enough to figure out how the hashing works. And then in case someone manages to disassemble the code, the malicious user cannot make use of a single rainbow table.

Is this a good method?

Answer 1

In SQL Server, we store password salts as an int and password hashes as binary. There really is no reason to have the salt as a string value. Also, there is no need to have the hash as a string. You can use the bytes from the computed hash (which is a fixed, known number of bytes) and store the binary value directly in the database. These two things make it very fast to check check against when attempting to verify the password.

See my answer on Stack Overflow here about hashing passwords using SHA-1. You are using SHA-512, which the code can easily be adopted to support.

Answer 2

Including a secret shared salt, in addition to a per-hash salt, is a decent idea, at the very least it doesn't hurt. I typically put the common salt(which is essentially a key) into a config file instead embedding it in the application code.

On the other hand SHA-512 is completely wrong, since it's fast, and thus cheap to brute-force. You should use a slow key derivation function, such as PBKDF2, bcrypt, or scrypt. The only one of these built into .net is PBKDF2-HMAC-SHA-1, exposed through the Rfc2898DeriveBytes class. Choose the number of iterations as high as possible without hurting the performance of your application.