Veggerby : IBlog | Low decoupling, highly incoherent

Jul/08

6

Abuse of FormsAuthentication.HashPasswordForStoringInConfigFile() method?

I can’t help but get a annoyingly creepy feeling whenever I see people using FormsAuthentication.HashPasswordForStoringInConfigFile() to generate a hash cipher for a given value (for example here). On the other hand I can’t really decide wether I think it is the method which has a bad name/should be "located" somewhere else or it is the "abuse" of it as it is today.

If we look at the actual implementation of HashPasswordForStoringInConfigFile using Reflector:

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

The net result of this of course is that you do get the MD5/SHA1 hash of the password or value.

However the method name is HashPasswordForStoringInConfigFile, i.e. the name suggests it is for hashing passwords and further more it is a method of a class called FormsAuthentication… see a pattern!? The name does not suggest it is a generic MD5 method (although the implementation shows it is that).

IMHO using HashPasswordForStoringInConfigFile as a generic MD5 calculation method does nothing but make code less readable and hence harder to maintain. The very, very least a developer can do is to comment the code, but we all know when that’ll happen…

Look at the alternative:

public static string GetMD5(string value)
{
    MD5 algorithm = MD5.Create();
    byte[] data = algorithm.ComputeHash(Encoding.UTF8.GetBytes(value));
    string md5 = "";
    for (int i = 0; i < data.Length; i++)
    {
        md5 += data[i].ToString("x2").ToLowerInvariant();
    }
    return md5;
}

The ONLY problem with not doing this stuff yourself is the conversion from a string and back to a string…

Yes, this might be a little slower due to the string stuff, but you can always "inherit" ;) the MachineKeySection.ByteArrayToHexString method.

The point is that using FormsAuthentication.HashPasswordForStoringInConfigFile to calculate all MD5/SHA-1 hash values just because the implementation happens to support this seems wrong. Writing this I think I actually blame Microsoft the most for not providing this method somewhere else and named more appropriately.

That was just a little rant…

· · ·

3 comments

Leave a Reply

<<

>>

Theme Design by devolux.nh2.me