CODEWITHSUNDEEP
c#securitytimingstring-comparison
Erik Karulf
Erik Karulf

Reputation: 473

Hash Digest / Array Comparison in C#

I'm writing an application that needs to verify HMAC-SHA256 checksums. The code I currently have looks something like this:

    static bool VerifyIntegrity(string secret, string checksum, string data)
    {
        // Verify HMAC-SHA256 Checksum
        byte[] key = System.Text.Encoding.UTF8.GetBytes(secret);
        byte[] value = System.Text.Encoding.UTF8.GetBytes(data);
        byte[] checksum_bytes = System.Text.Encoding.UTF8.GetBytes(checksum);
        using (var hmac = new HMACSHA256(key))
        {
            byte[] expected_bytes = hmac.ComputeHash(value);
            return checksum_bytes.SequenceEqual(expected_bytes);
        }
    }

I know that this is susceptible to timing attacks.

Is there a message digest comparison function in the standard library? I realize I could write my own time hardened comparison method, but I have to believe that this is already implemented elsewhere.

Upvotes: 1

Views: 1508

Answers (3)

Jon Skeet
Jon Skeet

Reputation: 1499860

EDIT: Original answer is below - still worth reading IMO, but regarding the timing attack...

The page you referenced gives some interesting points about compiler optimizations. Given that you know the two byte arrays will be the same length (assuming the size of the checksum isn't particularly secret, you can immediately return if the lengths are different) you might try something like this:

public static bool CompareArraysExhaustively(byte[] first, byte[] second)
{
    if (first.Length != second.Length)
    {
        return false;
    }
    bool ret = true;
    for (int i = 0; i < first.Length; i++)
    {
        ret = ret & (first[i] == second[i]);
    }
    return ret;
}

Now that still won't take the same amount of time for all inputs - if the two arrays are both in L1 cache for example, it's likely to be faster than if it has to be fetched from main memory. However, I suspect that is unlikely to cause a significant issue from a security standpoint.

Is this okay? Who knows. Different processors and different versions of the CLR may take different amounts of time for an & operation depending on the two operands. Basically this is the same as the conclusion of the page you referenced - that it's probably as good as we'll get in a portable way, but that it would require validation on every platform you try to run on.

At least the above code only uses relatively simple operations. I would personally avoid using LINQ operations here as there could be sneaky optimizations going on in some cases. I don't think there would be in this case - or they'd be easy to defeat - but you'd at least have to think about them. With the above code, there's at least a reasonably close relationship between the source code and IL - leaving "only" the JIT compiler and processor optimizations to worry about :)

Original answer

There's one significant problem with this: in order to provide the checksum, you have to have a string whose UTF-8 encoded form is the same as the checksum. There are plenty of byte sequences which simply don't represent UTF-8-encoded text. Basically, trying to encode arbitrary binary data as text using UTF-8 is a bad idea.

Base64, on the other hand, is basically designed for this:

static bool VerifyIntegrity(string secret, string checksum, string data)
{
    // Verify HMAC-SHA256 Checksum
    byte[] key = Encoding.UTF8.GetBytes(secret);
    byte[] value = Encoding.UTF8.GetBytes(data);
    byte[] checksumBytes = Convert.FromBase64String(checksum);
    using (var hmac = new HMACSHA256(key))
    {
        byte[] expectedBytes = hmac.ComputeHash(value);
        return checksumBytes.SequenceEqual(expectedBytes);
    }
}

On the other hand, instead of using SequenceEqual on the byte array, you could Base64 encode the actual hash, and see whether that matches:

static bool VerifyIntegrity(string secret, string checksum, string data)
{
    // Verify HMAC-SHA256 Checksum
    byte[] key = Encoding.UTF8.GetBytes(secret);
    byte[] value = Encoding.UTF8.GetBytes(data);
    using (var hmac = new HMACSHA256(key))
    {
        return checksum == Convert.ToBase64String(hmac.ComputeHash(value));
    }
}

I don't know of anything better within the framework. It wouldn't be too hard to write a specialized SequenceEqual operator for arrays (or general ICollection<T> implementations) which checked for equal lengths first... but given that the hashes are short, I wouldn't worry about that.

Upvotes: 2

MerickOWA
MerickOWA

Reputation: 7592

If you're worried about the timing of the SequenceEqual, you could always replace it with something like this:

checksum_bytes.Zip( expected_bytes, (a,b) => a == b ).Aggregate( true, (a,r) => a && r );

This returns the same result as SequenceEquals but always check every element before given an answer this less chance of revealing anything through a timing attack.

Upvotes: 2

Nickolay Olshevsky
Nickolay Olshevsky

Reputation: 14160

How it is susceptible to timing attacks? Your code works the same amount of time in the case of valid or invalid digest. And calculate digest/check digest looks like the easiest way to check this.

Upvotes: 0

Related Questions