Is my password management MVC3 compatible?
I read tons of articles on how to store and compare entered passwords, some old and some new, but all different. So I took what I thought was the best and implemented the following.
Please note that I created my own authentication provider and I am not using or inheriting from the standard MS MembershipProvider (too bloated). This is for a lighter site that doesn't have to be "uber-secure", but I'd like to follow best practices. I just wish I didn't do anything bad or open security holes. Also, I looked at the popup for each user, but it seemed too complicated for my needs. Is this a valid assumption?
First, I put the following appSettings in my web.config file , which is returned by my configurationProvider class .
<add key="PasswordEncryptionKey" value="qTnY9lf...40 more random characters here" />
Here is my code that publicly Authenticates the user and privately Checks the stored password against what was entered and also Encodes the password when saved. I have not shown the methods to add or update a password as they use the same private methods.
public bool Authenticate(string emailAddress, string password, bool setAuthCookie = false)
{
bool isAuthenticated = false;
var member = _memberRepository.Find(m => m.Email == emailAddress).SingleOrDefault();
if (member != null)
{
if (CheckPassword(password, member.Password))
{
isAuthenticated = true;
FormsAuthentication.SetAuthCookie(emailAddress, setAuthCookie);
}
}
return isAuthenticated;
}
private bool CheckPassword(string providedPassword, string storedPassword)
{
return EncodePassword(providedPassword) == storedPassword;
}
private string EncodePassword(string password)
{
var hash = new HMACSHA1
{
Key = Encoding.ASCII.GetBytes(_configurationProvider.PasswordEncryptionKey)
};
string encodedPassword = Convert.ToBase64String(hash.ComputeHash(Encoding.Unicode.GetBytes(password)));
return encodedPassword;
}
My only rule is:
- The password must be encrypted asynchronously in the database and cannot be decrypted.
- The method must be reasonably safe for dictionary or other attacks (I am not developing an FDIC insured site, just a basic intranet)
With this, is there something vibrant that I'm missing?
source to share
I believe your circuit is adequate, but could be improved a little.
It looks to me like you have the beginning of the salting scheme here, with your EncryptionKey that you have in your app.config file. However, for best security practice, it is common for people to use different salts for each password and store the salt along with the hash in the database.
class MyAuthClass {
private const int SaltSize = 40;
private ThreadLocal<HashAlgorithm> Hasher;
public MyAuthClass ()
{
// This is 'ThreadLocal' so your methods which use this are thread-safe.
Hasher = new ThreadLocal<HashAlgorithm>(
() => new HMACSHA256(Encoding.ASCII.GetBytes(_configurationProvider.PasswordEncryptionKey)
);
}
public User CreateUser(string email, string password) {
var rng = new RNGCryptoServiceProvider();
var pwBytes = Encoding.Unicode.GetBytes(password);
var salt = new byte[SaltSize];
rng.GetBytes(salt);
var hasher = Hasher.Value;
hasher.TransformBlock(salt, 0, SaltSize, salt, 0);
hasher.TransformFinalBlock(pwBytes, 0, pwBytes.Length);
var finalHash = hasher.Hash;
return new User { UserName = email, PasswordHash = finalHash, Salt = salt };
}
With this scheme, your passwords become more complex because if the hacker managed to get the hashes, he also has to guess the salt during a brute force attack.
This is the same philosophy as your EncodingKey in your config file, but more secure since each hash has its own salt. Checking the entered passwords is similar:
public bool IsPasswordCorrect(User u, string attempt)
{
var hasher = Hasher.Value;
var pwBytes = Encoding.Unicode.GetBytes(attempt);
hasher.TransformBlock(u.Salt, 0, u.Salt.Length, Salt, 0);
hasher.TransformFinalBlock(pwBytes, 0, pwBytes.Length);
// LINQ method that checks element equality.
return hasher.Hash.SequenceEqual(u.PasswordHash);
}
} // end MyAuthClass
Of course, if you prefer storing hashes as strings rather than byte arrays, you can do that.
Just my 2 cents!
source to share
A few thoughts ...
- Instead of HMACSHA1, you should use HMACSHA256 or HMACSHA512. NET4 default hash modification is SHA256 now instead of SHA1.
- When you get the encryption key bytes, you are using ASCII encoding, but you are using a hash using Unicode encoding. Keep them consistent - use Unicode.
- Consider salting the hash.
- You say encryption, but you really are hashing.
source to share