PBKDF2 C# Implementation

Question

This is my first attempt to do any hashing. I wanted to make a class that handled everything I needed to implement PBKDF2. The only question I have is am I ok to store the iterations in the database record or is this a bad practice? I was planning on creating a column to store salt, another for the hashed password, and possibly one for the amount of iterations. My thought with the iterations was that I read that it should increase every so often so when creating new users or they update their passwords it would pull the value to pass in from the database.

    public static byte[] GenerateSalt()
    {
        using (var randomNumberGenerator = new RNGCryptoServiceProvider())
        {
            var salt = new byte[32];
            randomNumberGenerator.GetBytes(salt);

            return salt;
        }
    }

    public static byte[] HashPassword(byte[] password, byte[] salt, int iterations)
    {
        using (var rfc2898 = new Rfc2898DeriveBytes(password, salt, iterations))
        {
            return rfc2898.GetBytes(32);
        }
    }

    public static bool CompareByteArrays(byte[] array1, byte[] array2)
    {
        if (array1.Length != array2.Length)
        {
            return false;
        }

        for (int i = 0; i < array1.Length; i++)
        {
            if (array1[i] != array2[i])
            {
                return false;
            }
        }

        return true;
    }

Answer 1

The code provided might fulfill the functionality however the API is hard to use.

Passwords are usually strings and not binary (byte[]). It would also be nice that the salt could be generated automatically when you hash the password. That way you would call a single method to hash a password instead of calling two methods, in this case that would be GenerateSalt followed by HashPassword.

According to my argument I would suggest you to implement the following API:

// returns hashed password + salt
Tuple<byte[], byte[]> HashPassword(string password, int iterations) 
bool ComparePassword(string attempt, byte[] hashedPassword, byte[] salt)

You can always replace the Tuple type by another class that would have the properties byte[] HashedPassword and byte[] Salt. Also often, but not always, the salt and the password are usually base64 encoded. That usually allows you to have a slightly nicer view of the data that is stored while also avoiding storing binary data in your persistence mechanism.

Additionally Rfc2898DeriveBytes outputs 20 bytes. So return rfc2898.GetBytes(32); should be return rfc2898.GetBytes(20);

Answering your additional question about the iteration count, there is nothing wrong with that. You can even use that as a pepper, different users can have slightly different iteration counts. Which makes the job of a hacker harder. In some scenarios people also store the actually algorithm that was used to hash the password. That might allow you to switch between algorithms in a easier fashion.

Answer 2

I've finally had a chance to make an answer to this, but I want it noted that Bruno Costa had a great answer, and this is only an improvement on his.

First and foremost:

The only question I have is am I ok to store the iterations in the database record or is this a bad practice? I was planning on creating a column to store salt, another for the hashed password, and possibly one for the amount of iterations.

This is 100% acceptable, and encouraged. It's necessary, especially to change the iteration count (which you should be doing annually), as you'll want to increase it a little regularly. (There's other guidance out there for how often and how much to increase it, but basically do what is necessary to keep the user cost low-ish, and keep attackers having to work for it.)

My thought with the iterations was that I read that it should increase every so often so when creating new users or they update their passwords it would pull the value to pass in from the database.

You should increase the iteration count when necessary, and do the rehashing on user-login. Forget about making them change their password, you can change their iteration and salt each time they login (as at that moment you absolutely must know the password anyway). I even go out-of-the-way to change the salt each time users login to my websites.

---

Now some notes on the other answer:

While Rfc2898DeriveBytes only gives you 20 bytes of entropy (as indicated), I always use one of the constructors that takes a HashAlgorithmName, and I always provide HashAlgorithmName.SHA512. This gives you entropy up to 64 bytes, so you can use your 32, or go all the way up to the 64. (That's up to you.)

Additionally:

Answering your additional question about the iteration count, there is nothing wrong with that. You can even use that as a pepper, different users can have slightly different iteration counts. Which makes the job of a hacker harder. In some scenarios people also store the actually algorithm that was used to hash the password. That might allow you to switch between algorithms in a easier fashion.

There are some good and bad points here (alright, I shouldn't say "bad", just less optimal):

1. DO store the ITERATION_COUNT definitely;
2. DO store the HASH_ALGORITHM_NAME at your discretion (probably worth doing so);
3. DO NOT intentionally vary ITERATION_COUNT per user, this offers no advantage to securing the user, especially if the attacker gets a copy of the database, and only complicates your code and introduces potential vulnerabilities;
4. DO consider adding an efficient random delay to prevent side-channel analysis and timing attacks;