Static memory allocation in HMAC is thread-unsafe.

[Lukasa]

The HMAC function in crypto/hmac/hmac.c has a subtle dangerous behaviour in some use-cases.

The expected usage of the HMAC function is to provide an EVP_MD, a key buffer and length (as void *), a data buffer and length (as unsigned char *), and an output buffer and key (as unsigned char *). This is reasonable.

Helpfully, HMAC allows for the output buffer to be omitted (that is, set as NULL). In that case, HMAC will helpfully return a pointer to a buffer it has allocated for the user. Unhelpfully, that pointer is declared inside the HMAC function as:

static unsigned char m[EVP_MAX_MD_SIZE];

This buffer, as it's declared static, is a single global buffer used by all invocations of HMAC. That means that HMAC is subtly not thread-safe when used in this mode: concurrent invocations of the apparently one-shot HMAC functions can clobber each other's data in surprising (and potentially dangerous) ways.

At the very least the man page should more emphatically note that HMAC is not thread-safe in the absence of a user-allocated buffer. A stronger approach would be to simply forbid the passing of a NULL pointer for md by returning an error. Note that the change must not be to allocate a buffer for the user, as this subtly changes the semantics of the returned pointer, which would be very bad.

[mattcaswell]

Yes - we are stuck with the API the way it is for compatibility reasons (unless maybe we want to think about using some kind of thread local storage solution...but perhaps that is over complicating things). The man page does explicitly note that it goes into a static array if the buffer is NULL - but, you're right, it should probably be more explicit about the thread safety implications of that. I actually thought it did, but apparently not.