Secure password hashing implementation with salt and pepper

Question

I share below my password hashing implementation with salt and pepper in C#, for your review and feedback. In my implementation methods like ValidatePassword(), ChangePassword() are in the user class.

I recommend random.org's free random bytes generator coded in hexadecimal to get a secure Pepper for each installation. I prefer 128 bytes length. It separates each byte (every 2 digits in hexadecimal) with space. Copy-paste the pepper generated by random.org to the config file with the with the key "Pepper" like so:

<add key="Pepper" value="25 4d 58 42 05 ba 0a f0 8a 72 .. .. e3"/>

The simplified source code:

using System;
using System.Text;
using System.Configuration;
using System.Globalization;
using System.Security.Cryptography;

namespace CryptoPassword
{
    public static class Password
    {
        private const int SaltLength = 32;
        private const int KeyLength = 32;
        private const int IterationCount = 100000;

        private static readonly byte[] Pepper = ConvertSpacedHexToByteArray(ConfigurationManager.AppSettings["Pepper"]);
        //I recommend random.org's free 128 byte random bytes generator coded in hexadecimal
        //to get a secure pepper for each installation.
        //Copy-paste the pepper generated by the random.org to the config file with the key "Pepper"
        //like so: "<add key="Pepper" value="25 4d 58 42 05 ba 0a f0 8a 72 .. .. e3"/>"


        /// <summary>
        /// hexString should have its double hex digits corresponding to each byte separated by ' '
        /// like so: "A1 23 F3 14".
        /// </summary>
        /// <param name="hexString"></param>
        /// <returns>byte[] form</returns>
        public static byte[] ConvertSpacedHexToByteArray(string hexString)
        {   
            string[] hexValuesSplit = hexString.Split(' ');
            byte[] data = new byte[hexValuesSplit.Length];

            for (var index = 0; index < hexValuesSplit.Length; index++)
            {
                data[index] = byte.Parse(hexValuesSplit[index], NumberStyles.HexNumber, CultureInfo.InvariantCulture);
            }

            return data;
        }

        // I dont recommend playing with salt length a lot. Make it a const,
        // and use the non-parametrized GenerateSalt() unless you know what you are doing.
        // Change the const SaltLength at major releases.
        public static byte[] GenerateSalt()
        {
            return GenerateSalt(SaltLength);
        }
        //this parameterized version is for the flexibility at knowledgable hands.
        public static byte[] GenerateSalt(int saltLength)
        {
            using (RNGCryptoServiceProvider saltCellar = new RNGCryptoServiceProvider())
            {
                byte[] salt = new byte[saltLength];
                saltCellar.GetBytes(salt);
                return salt;
            }
        }

        public static string GenerateHash(string password)
        {
            byte[] salt = GenerateSalt();
            byte[] hash = GenerateHash(password, salt);

            return Convert.ToBase64String(salt) + ":" + Convert.ToBase64String(hash);
            //store salt and hash together with ':' as the separator, coded in Base64.
        }

        public static byte[] GenerateHash(string password, byte [] salt)
        {
            //create an hmac hash of the password using the pepper value as the key
            using (var hmac = new HMACSHA512(Pepper))
            {
                byte[] initialHash = hmac.ComputeHash(Encoding.UTF8.GetBytes(password));

                //generate a key value using pbkdf2 that will serve as the password hash
                using (var pbkdf2 = new Rfc2898DeriveBytes(initialHash, salt, IterationCount)) //int.Parse(ConfigurationManager.AppSettings["IterationCount"])))
                {
                    return pbkdf2.GetBytes(KeyLength);
                }
            }
        }

        public static bool ValidatePassword(string password, string testPassword)
        {
            string[] hashParts = password.Split(':');
            byte[] salt = Convert.FromBase64String(hashParts[0]);
            byte[] hash = Convert.FromBase64String(hashParts[1]);
            byte[] testHash = GenerateHash(testPassword, salt);

            //IMPORTANT!!! The following is required to defend against timing attacks.
            //We dont want to exit at the first byte mismatch but test every byte no matter what
            //so that timing is the same for valid and invalid passwords and
            //we dont want to leak information to the attacker about upto which byte his guess is correct.
            uint differences = (uint)hash.Length ^ (uint)testHash.Length;
            for (int position = 0; position < Math.Min(hash.Length, testHash.Length); position++)
                differences |= (uint)(hash[position] ^ testHash[position]);
            return differences == 0;
        }
    }
}

Answer 1

Not bad at all. Some remarks:

• The class should be parameterized and an instance should be used rather than just static methods.
• I'd never make the class itself dependent on a configured Pepper value (it's fine for an application to be configurable, but not for a class instance).
• The salt and pepper can be simply concatenated instead of using HMAC for the password & pepper.
• The salt and pepper are both 32 bytes, which is a bit much; using 16 bytes for both is fine.
• The iteration count on the other hand is on the low side and should really be configurable.
• A lot of static functions are used. It should not be necessary to make all these public (for instance the hex decoder).
• There should be a protocol description, which should also contain things like base 64 encoding and UTF-8 encoding.

Very clear variable and method naming. However you messed up a bit for the final method:

public static bool ValidatePassword(string password, string testPassword)

... the password is the previously hashed password and testPassword a real password? That's completely unclear.