BouncyCastle HMAC SHA1

Question

I have the following code using BouncyCastle (dotnet version) to get a HMAC-SHA1 from a message.

I have this small library class:

public class HashingTools
{
    static string hmacKey = "81310910a060c5705c1d3cedf370bcf9";
    public static int HashSizeInBytes = 20;
    static KeyParameter keyParameter = null;
    private static HMac hmacInstance;

    static HashingTools()
    {
        hmacInstance = new HMac(new Sha1Digest());
        hmacInstance.Init(newKeyParameter(Encoding.UTF8.GetBytes(hmacKey)));
    }

    public static byte[] HashSha1(byte[] message)
    {
        byte[] result = new byte[hmacInstance.GetMacSize()];

        hmacInstance.BlockUpdate(message, 0, message.Length);
        hmacInstance.DoFinal(result, 0);

        return result;
    }
}

And I have a lot of messages going through this method, all using the same key: hmacKey, and I would like to speed it up as much as I can, and reuse as much as I can, still with respect to security parameters (randomness, freshness...).

If I try to reuse or parallellize the hmac instance, I get an "array out of bounds" exception inside Org.BouncyCastle.Crypto.Macs.Hmac.BlockUpdate.

I've created this unittest for reproduction (1 or 2 parallel hash functions goes fine, 100 goes wrong):

[Test]
public void TestBulkHashing()
{
    var messages = new List<byte[]>();

    foreach (var index in Enumerable.Range(0, 100))
    {
        var buffer = new byte[4096];
        Random r = new Random();
        r.NextBytes(buffer);

        messages.Add(buffer);
    }

    Parallel.ForEach(messages, m =>
    {
        HashingTools.HashSha1(m);
    });
}

Answer 1

As correctly surmised by @dlatikay, this is a synchronization mistake. Bouncycastle's classes are not thread-safe unless they explicitly say it is.

If you modify your HashSha1 method to explicitly synchronize threads you will not get the exception:

public static byte[] HashSha1(byte[] message) {
    byte[] result = new byte[hmacInstance.GetMacSize()];
    lock(hmacInstance) {
        hmacInstance.BlockUpdate(message, 0, message.Length);
        hmacInstance.DoFinal(result, 0);
    }

    return result;
}

As to your question about optimization, Bouncycastle already pre-computes the part of the computation that involves the key. When you call DoFinal(...) the internal state is reset to this pre-computed value so you do not need to call Init() again for the next HMac if you use the same key. Your code already takes advantage of this optimization so I don't think there is more you can do unless you want to write your own hashing code.