patterncsharpMinor
Secure custom password hashing
Viewed 0 times
hashingcustomsecurepassword
Problem
My team and I have ended up creating this class, which is called directly from ASP.NET Identity as a custom password hasher. I'd like to know whether this would be "overkill"/use a lot of CPU, specially because the site is going to be hosted in Azure.
To take into account:
```
private const int SaltByteLength = 16;
private const int DerivedKeyLength = 20;
private const int minIterationCount = 44000;
private const int maxIterationCount = 50000;
public string HashPassword(string password)
{
return CreateSecurePasswordHash(password);
}
public PasswordVerificationResult VerifyHashedPassword(string hashedPassword, string providedPassword)
{
providedPassword = Convert.ToBase64String(ComputeHash(providedPassword));
bool result = ComparePasswordHashes(providedPassword, hashedPassword);
return result ? PasswordVerificationResult.Success : PasswordVerificationResult.Failed;
}
private static byte[] ComputeHash(string password)
{
using (MemoryStream ms = new MemoryStream())
using (StreamWriter sw = new StreamWriter(ms))
{
sw.Write(password);
sw.Flush();
ms.Position = 0;
using (SHA512CryptoServiceProvider provider = new SHA512CryptoServiceProvider())
return provider.ComputeHash(ms);
}
}
private static string CreateSecurePasswordHash(string password)
{
byte[] hashedPassword = ComputeHash(password);
byte[] salt = GenerateSecureSalt();
Random rand = new Random();
int iterationCount = rand.Next(minIterationCount, maxIterationCount);
byte[] hashValue = GenerateSecureHashValue(hashedPassword, salt, iterationCount);
byte[] iterationCountBtyeArr = BitConverter.GetBytes(iterationCount);
byte[] valueToSave = new byte[SaltByteLength + DerivedKeyLength + iterationCountBtyeArr.Length];
Buffer.BlockCopy(salt, 0, valueToSave, 0, SaltByteLength);
To take into account:
HashPasswordis called when a new user is being created byIdentity
VerifyHashedPasswordis called when a user logs in
```
private const int SaltByteLength = 16;
private const int DerivedKeyLength = 20;
private const int minIterationCount = 44000;
private const int maxIterationCount = 50000;
public string HashPassword(string password)
{
return CreateSecurePasswordHash(password);
}
public PasswordVerificationResult VerifyHashedPassword(string hashedPassword, string providedPassword)
{
providedPassword = Convert.ToBase64String(ComputeHash(providedPassword));
bool result = ComparePasswordHashes(providedPassword, hashedPassword);
return result ? PasswordVerificationResult.Success : PasswordVerificationResult.Failed;
}
private static byte[] ComputeHash(string password)
{
using (MemoryStream ms = new MemoryStream())
using (StreamWriter sw = new StreamWriter(ms))
{
sw.Write(password);
sw.Flush();
ms.Position = 0;
using (SHA512CryptoServiceProvider provider = new SHA512CryptoServiceProvider())
return provider.ComputeHash(ms);
}
}
private static string CreateSecurePasswordHash(string password)
{
byte[] hashedPassword = ComputeHash(password);
byte[] salt = GenerateSecureSalt();
Random rand = new Random();
int iterationCount = rand.Next(minIterationCount, maxIterationCount);
byte[] hashValue = GenerateSecureHashValue(hashedPassword, salt, iterationCount);
byte[] iterationCountBtyeArr = BitConverter.GetBytes(iterationCount);
byte[] valueToSave = new byte[SaltByteLength + DerivedKeyLength + iterationCountBtyeArr.Length];
Buffer.BlockCopy(salt, 0, valueToSave, 0, SaltByteLength);
Solution
At first glance, your code seems to do the right thing security wise and is reasonable performance wise. If performance becomes a problem, you can still reduce the iteration count of pbkdf2. But considering the original recommandation in RFC 2898 (at least 1000 iteration) in 2000 and the increase in processing power since then, 50000 iteration seems pretty conservative to me.
It's also worth considering that your user will enter his password only once per session so the performance cost of checking a password will probably be negligible.
But as always with performance question the best answer is to bench your code and decide wether its performance is acceptable or not.
P.S. I'm not a security expert so don't make any critical decision based on my opinion
It's also worth considering that your user will enter his password only once per session so the performance cost of checking a password will probably be negligible.
But as always with performance question the best answer is to bench your code and decide wether its performance is acceptable or not.
P.S. I'm not a security expert so don't make any critical decision based on my opinion
Context
StackExchange Code Review Q#115953, answer score: 2
Revisions (0)
No revisions yet.