Another example of how to store a salted password hash

I ran across this post titled “Salted Password Hashing” over on dotnetshoutout.com. I’m amazed at all the little problems here, so before we continue with how to do this, let’s look at what you should not do:

• First, Hashed passwords, even when using salt, are possible to crack with a dictionary attack. Computers are fast and so are hash algorithms thus you should add complexity to the algorithm by relying on implementations similar to RFC2898 (see System.Security.Cryptography.Rfc2898DeriveBytes). This allows the consumer to also define an iteration number that controls how many times the hash algorithm is run.
• Do not use MD5. Reasons: It not cryptographically secure, It only has an unmanaged implementation in .Net, and therefore is slower than SHA256.
• The HashAlgorithm implementations are disposable, a using() statement should be used when working with a hash algorithm. Though as we will learn, even Microsoft makes this mistake.
• You should never rely on Encoding.Default as is clearly documented on MSDN. Use a specific encoding, Encoding.UTF8.
• You should use a cryptographically strong random number generator (RNGCryptoServiceProvider) for salt, not a guid or random bytes.
• Because the .Net implementations of the various hash algorithms are not FIPS-approved, you should use the static SHA1.Create() to construct the instance. This allows a system to be configured to use either the managed implementations or the CSP provided by Windows.

 
So if your looking for a light-weight solution all ready to go, use the following class to correctly salt, hash, and store passwords:

    public sealed class PasswordHash
    {
        const int SaltSize = 16, HashSize = 20, HashIter = 10000;
        readonly byte[] _salt, _hash;
        public PasswordHash(string password)
        {
            new RNGCryptoServiceProvider().GetBytes(_salt = new byte[SaltSize]);
            _hash = new Rfc2898DeriveBytes(password, _salt, HashIter).GetBytes(HashSize);
        }
        public PasswordHash(byte[] hashBytes)
        {
            Array.Copy(hashBytes, 0, _salt = new byte[SaltSize], 0, SaltSize);
            Array.Copy(hashBytes, SaltSize, _hash = new byte[HashSize], 0, HashSize);
        }
        public PasswordHash(byte[] salt, byte[] hash)
        {
            Array.Copy(salt, 0, _salt = new byte[SaltSize], 0, SaltSize);
            Array.Copy(hash, 0, _hash = new byte[HashSize], 0, HashSize);
        }
        public byte[] ToArray()
        {
            byte[] hashBytes = new byte[SaltSize + HashSize];
            Array.Copy(_salt, 0, hashBytes, 0, SaltSize);
            Array.Copy(_hash, 0, hashBytes, SaltSize, HashSize);
            return hashBytes;
        }
        public byte[] Salt { get { return (byte[])_salt.Clone(); } }
        public byte[] Hash { get { return (byte[])_hash.Clone(); } }
        public bool Verify(string password)
        {
            byte[] test = new Rfc2898DeriveBytes(password, _salt, HashIter).GetBytes(HashSize);
            for (int i = 0; i < HashSize; i++)
                if (test[i] != _hash[i])
                    return false;
            return true;
        }
    }

Now this class is also not without a down side or two. But it’s probably close enough for most people. To clarify, the shortcomings of the previous example are due to the use of Rfc2898DeriveBytes. Why? well there are few things that concern me here:

• First it limits you to the use of SHA1 (and thus 20 bytes) hash.
• If you try to use more than 20 bytes this can potentially fail to generate the same data
• Your not in control of the HMAC hash algorithm used, and thus rely on .Net to select the SHA1 implementation based on host configuration.
• I dislike the fact that Rfc2898DeriveBytes did not implement Dispose and call the HMAC’s Dispose method. I guess this was just an oversight on Microsoft’s part. It effectively leaves the plain-text password in memory until the GC collects it.

 
I’m sure you could find other ways to improve the above class, but I think you will agree it’s a close match with the desired behavior with the least amount of code. Let’s take one last look at using the above class:

//Store a password hash:
PasswordHash hash = new PasswordHash("password");
byte[] hashBytes = hash.ToArray();

//Check password against a stored hash
byte[] hashBytes = //read from store.
PasswordHash hash = new PasswordHash(hashBytes);
if(!hash.Verify("newly entered password"))
    throw new System.UnauthorizedAccessException();