IncrementalHash.CreateHMAC key hashing mistake

[samuel-lucas6]

This page says the following:

a key longer than the output size of the specified hash algorithm will be hashed to derive a correctly-sized key. Therefore, the recommended size of the secret key is the output size of the specified hash algorithm.

However, the HMAC RFC specifies:

Applications that use keys longer than B bytes will first hash the key using H and then use the resultant L byte string as the actual key to HMAC.

B bytes refers to the block size of the hash function, which is different from the output size.

The non-incremental HMAC pages seem to be correct:

The secret key for HMACSHA256 encryption. The key can be any length. However, the recommended size is 64 bytes. If the key is more than 64 bytes long, it is hashed (using SHA-256) to derive a 64-byte key.

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This page says the following:

a key longer than the output size of the specified hash algorithm will be hashed to derive a correctly-sized key. Therefore, the recommended size of the secret key is the output size of the specified hash algorithm.

However, the HMAC RFC specifies:

Applications that use keys longer than B bytes will first hash the key using H and then use the resultant L byte string as the actual key to HMAC.

B bytes refers to the block size of the hash function, which is different from the output size.

The non-incremental HMAC pages seem to be correct:

The secret key for HMACSHA256 encryption. The key can be any length. However, the recommended size is 64 bytes. If the key is more than 64 bytes long, it is hashed (using SHA-256) to derive a 64-byte key.

Author:	samuel-lucas6
Assignees:	-
Labels:	area-System.Security, untriaged, Pri3
Milestone:	-

[jozkee]

cc @bartonjs

[bartonjs]

Yep. A bad generalization goof.

Since the block size isn't really part of our API (there's a protected property for it on HMAC, but nothing public) it's hard to talk about. https://learn.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptcreatehash solves the problem by just never talking about the length.

The best I'm coming up with right now is something like

The secret key for the HMAC. The key can be of any length, but it should generally not be shorter than the output size of the specified hash algorithm.

And then maybe trying to explain the effective key calculation in the remarks? Or just leave it at that and we've said "don't use short keys."

FWIW, the documentation on HMACSHA256, recommending a 64-byte (512-bit) key was a topic of discussion on Cryptography Stack Exchange (https://crypto.stackexchange.com/questions/34864/key-size-for-hmac-sha256). The answer (which I agree with) boils down to "once you're already at 256 bits of input, if your key is stochastic the difference between 256 and 512 is not really significant, because 256 is itself already impossibly hard to break". For HMAC-SHA1 and HMAC-MD5 we could do better than "hash output size" and recommend a minimum length of 32 bytes... or generalize that to recommending 32 (or even 64) bytes, or the hash output length, whichever is larger...

Or, maybe we should just follow CNG's precedent and not document opinions at all.

[bartonjs]

Looking at other platforms (CNG, OpenSSL, Ruby, Go), no one tries to offer guidance for the length of an HMAC key (probably because it's hard). So the best edit is probably just to reduce the paramdoc all the way down to "The secret key for the HMAC."

[samuel-lucas6]

I think all three are good suggestions. My preference would be recommending 256 bits because I also agree with Cryptography Stack Exchange. However, fair enough not recommending anything given what you've said.