Hashing passwords for a website 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:


Create random salt
Add random salt and email together to create salt for password (email will be encrypted in database)
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);

    }
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).