Wednesday, August 11, 2010

Don't use FormsAuthentication.HashPasswordForStoringInConfigFile()

So, a ninja of my acquaintance told me yesterday that MD5 and SHA-1 hashes were not really considered acceptable for hashing passwords nowadays due to attacks having been developed which make it easier than it should be to find a hash collision. "Federal agencies should stop using SHA-1 for digital signatures, digital time stamping and other applications that require collision resistance as soon as practical," says the National Institute of Standards and Technology.

Now, many .NET developers are probably accustomed to using FormsAuthentication.HashPasswordForStoringInConfigFile() to hash passwords. So it's quite an annoyance that this function only supports two options for hash format: MD5 and SHA-1!

Why is this so? I have no idea. The System.Security.Cryptography namespace contains implementations of the newer SHA-2 hash algorithms.

Let's fire up .NET Reflector and disassemble FormsAuthentication.HashPasswordForStoringInConfigFile()

public static string HashPasswordForStoringInConfigFile(string password, string passwordFormat)
{
    HashAlgorithm algorithm;
    if (password == null)
    {
        throw new ArgumentNullException("password");
    }
    if (passwordFormat == null)
    {
        throw new ArgumentNullException("passwordFormat");
    }
    if (StringUtil.EqualsIgnoreCase(passwordFormat, "sha1"))
    {
        algorithm = SHA1.Create();
    }
    else
    {
        if (!StringUtil.EqualsIgnoreCase(passwordFormat, "md5"))
        {
            throw new ArgumentException(SR.GetString("InvalidArgumentValue", new object[] { "passwordFormat" }));
        }
        algorithm = MD5.Create();
    }
    return MachineKeySection.ByteArrayToHexString(algorithm.ComputeHash(Encoding.UTF8.GetBytes(password)), 0);
}

That's some dodgy code. A hard-coded if-else-if construct to lock it down to only supporting MD5 and SHA-1, and they don't seem to be aware that there is actually an overload HashAlgorithm.Create(string hashName) which takes a hash algorithm name parameter! If they'd just used that, they would have had support for all the SHA-2 hash implementations.

Here's my alternative:

public static string HashString(string inputString, string hashName)
{
    HashAlgorithm algorithm = HashAlgorithm.Create(hashName);
    if (algorithm == null)
    {
        throw new ArgumentException("Unrecognized hash name", "hashName");
    }
    byte[] hash = algorithm.ComputeHash(Encoding.UTF8.GetBytes(inputString));
    return Convert.ToBase64String(hash);
}

Handles all of the available hash formats (and indeed any new ones that might be added in later versions of .NET, if HashAlgorithm.Create(string hashName) is updated), and additionally returns the result Base64 encoded rather than as a hexadecimal number, so it's shorter for the same hash length.