Hashing passwords for a website

Question

I have spent about 2 weeks reading about hashing passwords and website security. As there seems to be many different ways to achieve this, I'm a bit confused as to whether my code is secure.

Can anyone please have a look through and let me know what they think and what needs to be changed?

I have not added any error trapping yet, as to allow me to concentrate on just the hashing.

The steps I have took are as follows:

1. Create random salt
2. Add random salt and email together to create salt for password (email will be encrypted in database)
3. Hash password and salt and store in database

public const int HashBytes = 128;
public const int DefaultIterations = 10000;

    //Create random bytes for salt
    public static string SaltSHA256()
    {
        const int minSaltSize = 8;
        const int maxSaltSize = 16;
        var random = new Random();
        int saltSize = random.Next(minSaltSize, maxSaltSize);
        byte[] saltBytes = new byte[saltSize];
        var rng = new RNGCryptoServiceProvider();
        rng.GetNonZeroBytes(saltBytes);
        HashAlgorithm hash = new SHA256Managed();
        byte[] bytes = hash.ComputeHash(saltBytes);
        return Convert.ToBase64String(bytes);
    }

    //Create salt using email and public static string SaltSHA256()
    //Store email and public static string SaltSHA256() in database
    public static string SaltRfc2898(string email,string hashedSalt)
    {
        var salt = new Rfc2898DeriveBytes(email, Encoding.UTF8.GetBytes(hashedSalt), DefaultIterations);
        return Convert.ToBase64String(salt.GetBytes(HashBytes));
    }

    //Hash password and salt
    public static string PasswordHashRfc2898(string password,string salt)
    {
        var hashedPassword = new Rfc2898DeriveBytes(password, Encoding.UTF8.GetBytes(salt), DefaultIterations);
        return Convert.ToBase64String(hashedPassword.GetBytes(HashBytes));
    }
    //Get salt and password from database based on username
    //Salt in from data created by public static string SaltSHA256()

    public static bool DoPasswordsMatch(string password,string salt)
    {
        //Password would be pulled from db based on username
        byte[] testPassword = Convert.FromBase64String("basestring");

        //Salt would be pulled from database based on username
        var saltFromDatabase = salt;

        //Hash password and salt
        var hashUserInputAndSalt = PasswordHashRfc2898(password, saltFromDatabase);

        //Convert to byte[] ready for comparison
        byte[] convertUserInputFromBase64 = Convert.FromBase64String(hashUserInputAndSalt);

        //Compare and return true or false
        return convertUserInputFromBase64.SequenceEqual(testPassword);

    }

Answer 1

1. Random length salt is silly

2. Hashing the salt with SHA-256 is silly. Makes it longer without increasing entropy.

   A salt just needs to be unique. Best practice is using RNGCryptoServiceProvider to get 16 random bytes. Using more than 16 bytes doesn't improve security.

3. It's a bad idea to derive more than 20 bytes with Rfc2898DeriveBytes since that only decreases performance on your system, but not for the attacker. Deriving 20 bytes should speed your code up 7x. More that 20 bytes certainly doesn't improve security, I'd probably even go down a bit. 15 and 18 bytes encode nicely without padding using Base64.

4. If you can afford the performance hit, increase the number of iterations a bit. If you could afford your original code, you should now be able to afford 70000 iterations thanks to the speedup from the previous suggestion.

5. Don't use UTF-8 encoding on the salt. You can pass it to Rfc2898DeriveBytes directly as a byte[]. When you really need to convert it to/from text, use Base64.

6. It's best practice to use a constant time comparison instead of SequenceEquals to avoid timing side-channels.

Answer 2

As CodesInChaos pointed out in the comments, a far more secure way to hash passwords is to use a deliberately expensive hash. This will all but eliminate any hope of a successful brute-force attack.

The "best" algorithm I know of for this purpose would be BCrypt. For .NET, you can use BCrypt.Net. Disclaimer: I don't know who, if anyone, has verified the integrity of this particular port.

See this StackOverflow question for a full discussion regarding the use of BCrypt as a password hashing algorithm.

---

Original answer

So long as the salt is sufficiently unpredictable, the hashing function sufficiently secure, and the user's password is never stored in plain-text, emailed, or logged, then your implementation should be adequate.

While your salt is unpredictable, 8 bytes may be short for extremely sensitive information. A salt is stored in plain-text along with the hashed password, so we're not worried about keeping it a secret. What we're worried about is that someone may have already pre-computed SHA-256 hashes for common passwords and salts up to 8 bytes salt (though unlikely). Having a random length salt really doesn't add much except computational complexity and potentially wasted storage space. I would settle for a consistent-width salt of 16 bytes.

SHA-256 is a suitable hashing algorithm for most purposes at this time.
Edit: But BCrypt is better for passwords (see above).