HmacSha512.cs MinKeySize value should be 32 not 64 bytes

[enclave-marc-barry]

Looking at src/Cryptography/HmacSha512.cs in NSec, I note the comment on lines 24-28 referring to minimum keySize for this function being 32 bytes.

//      Key Size - The key for HMAC-SHA-512 can be of any length. A length
//          less than L=64 bytes (the output length of SHA-512) is strongly
//          discouraged. (libsodium recommends a default size of
//          crypto_auth_hmacsha512_KEYBYTES=32 bytes.) Keys longer than L do
//          not significantly increase the function strength.

Checking the constants defined in libSodium, we can see this is indeed the case in crypto_auth_hmacsha512.h#L19 for the crypto_auth_hmacsha512_KEYBYTES constant:

#define crypto_auth_hmacsha512_KEYBYTES 32U

But looking back into src/Cryptography/HmacSha512.cs it seems to me there might be a mistake. Both minimum and maximum key sizes are set by the same libsodium constant.

public static readonly int MinKeySize = crypto_hash_sha512_BYTES;
public static readonly int MaxKeySize = crypto_hash_sha512_BYTES;

It appears that both MinKeySize and MaxKeySize are set to the length of crypto_hash_sha512_BYTES (64U) exported from libsodium, which is not equivalent to the crypto_auth_hmacsha512_KEYBYTES (32U) value referenced in the nsec comment, nor the value used in libsodium, and therefore I believe also not the correct constant to set here for MinKeySize.

It appears that the library correctly uses crypto_hash_sha512_BYTES to set MaxKeySize, but by using that same value to also set MinKeySize the API is incorrectly preventing the use of 32 byte keys with the HmacSha512 function.

I believe it should be:

public static readonly int MinKeySize = crypto_auth_hmacsha512_KEYBYTES;
public static readonly int MaxKeySize = crypto_hash_sha512_BYTES;

Happy to raise a PR if you concur. Also @ektrah, thank you for maintaining this library!

[ektrah]

True, libsodium chooses a value of 32 bytes here. But RFC 2104, Section 3 clearly says that a key length of less than 64 bytes reduces the security strength of HMAC-SHA-512.

Of course, if you want a lower level of security, you can choose lower values. So I think there is no right or wrong with the constants unless you specify your desired security level.

Based on my reading of the RFC, it seems to me that the best strategy for a lower security level is to choose a key length of 64 bytes and then vary the length of the authentication tag. That's what’s currently implemented in HmacSha512.

[jedisct1]

This RFC was published in 1997, and written years before that. MD5 was the holy grail back then, and 5 years before SHA2 was announced.

The recommendation to have a key as large as the block size comes from the fact that the only proof that HMAC could act as a PRF was using that setting. For any reasonable output size (~ >= 192 bit) that all contemporary hash functions support, the output is indistinguishable from random, so theoretical PRF security doesn't do anything. Truncating the output also immediately achieves PRF security.

More importantly, it has been proven that HMAC offers PRF security with any sufficiently large key length (see When Messages are Keys: Is HMAC a dual-PRF?).

There's absolutely no point in using 512 bit keys. Or to use 512-bit authenticators for that matter. A 256 bit authenticator already guarantees that collisions will never occur.

For all these reasons, I'd recommend just using the regular high-level crypto_auth functions, which uses HMAC-SHA512 and truncates the output.

[enclave-marc-barry]

Thanks both for the insightful replies, much appreciated.

So I think there is no right or wrong with the constants unless you specify your desired security level.

I think that's sort of my point @ektrah, in NSec the MinKeySize is initialised to 64U, whereas in libsodium the minimum is 32U. Regardless of the discussion around key length efficacy, for me it is incorrect to force (without option to override, unless I'm mistaken?) a different set of constants on the user vs. those defined in libsodium.

It looks like there's a constructor signalling intent to allow adjustment of keySize, but from my reading both the MinKeySize and MaxKeySize constraints are set to 64, so I'm not seeing any practical use for the constructor?

Apologies in advance if I'm missing something obvious.

[enclave-marc-barry]

Thanks @ektrah!