From 3693b18840d868f11e273d24a63eb3e6b7b9d2de Mon Sep 17 00:00:00 2001 From: Conrad Meyer Date: Tue, 26 Sep 2017 16:18:10 +0000 Subject: [PATCH] opencrypto: Loosen restriction on HMAC key sizes Theoretically, HMACs do not actually have any limit on key sizes. Transforms should compact input keys larger than the HMAC block size by using the transform (hash) on the input key. (Short input keys are padded out with zeros to the HMAC block size.) Still, not all FreeBSD crypto drivers that provide HMAC functionality handle longer-than-blocksize keys appropriately, so enforce a "maximum" key length in the crypto API for auth_hashes that previously expressed a requirement. (The "maximum" is the size of a single HMAC block for the given transform.) Unconstrained auth_hashes are left as-is. I believe the previous hardcoded sizes were committed in the original import of opencrypto from OpenBSD and are due to specific protocol details of IPSec. Note that none of the previous sizes actually matched the appropriate HMAC block size. The previous hardcoded sizes made the SHA tests in cryptotest.py useless for testing FreeBSD crypto drivers; none of the NIST-KAT example inputs had keys sized to the previous expectations. The following drivers were audited to check that they handled keys up to the block size of the HMAC safely: Software HMAC: * padlock(4) * cesa * glxsb * safe(4) * ubsec(4) Hardware accelerated HMAC: * ccr(4) * hifn(4) * sec(4) (Only supports up to 64 byte keys despite claiming to support SHA2 HMACs, but validates input key sizes) * cryptocteon (MIPS) * nlmsec (MIPS) * rmisec (MIPS) (Amusingly, does not appear to use key material at all -- presumed broken) Reviewed by: jhb (previous version), rlibby (previous version) Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D12437 --- sys/crypto/via/padlock_hash.c | 6 ++++-- sys/netipsec/xform_ah.c | 4 ++-- sys/opencrypto/cryptodev.c | 3 ++- sys/opencrypto/cryptodev.h | 7 ------- sys/opencrypto/xform_md5.c | 4 ++-- sys/opencrypto/xform_null.c | 2 +- sys/opencrypto/xform_rmd160.c | 2 +- sys/opencrypto/xform_sha1.c | 4 ++-- sys/opencrypto/xform_sha2.c | 6 +++--- 9 files changed, 17 insertions(+), 21 deletions(-) diff --git a/sys/crypto/via/padlock_hash.c b/sys/crypto/via/padlock_hash.c index 658043b4946a..33ef89bb3358 100644 --- a/sys/crypto/via/padlock_hash.c +++ b/sys/crypto/via/padlock_hash.c @@ -82,7 +82,8 @@ static void padlock_sha256_final(uint8_t *hash, struct padlock_sha_ctx *ctx); static struct auth_hash padlock_hmac_sha1 = { CRYPTO_SHA1_HMAC, "HMAC-SHA1", - 20, SHA1_HASH_LEN, SHA1_HMAC_BLOCK_LEN, sizeof(struct padlock_sha_ctx), + SHA1_HMAC_BLOCK_LEN, SHA1_HASH_LEN, sizeof(struct padlock_sha_ctx), + SHA1_HMAC_BLOCK_LEN, (void (*)(void *))padlock_sha_init, NULL, NULL, (int (*)(void *, const uint8_t *, uint16_t))padlock_sha_update, (void (*)(uint8_t *, void *))padlock_sha1_final @@ -90,7 +91,8 @@ static struct auth_hash padlock_hmac_sha1 = { static struct auth_hash padlock_hmac_sha256 = { CRYPTO_SHA2_256_HMAC, "HMAC-SHA2-256", - 32, SHA2_256_HASH_LEN, SHA2_256_HMAC_BLOCK_LEN, sizeof(struct padlock_sha_ctx), + SHA2_256_HMAC_BLOCK_LEN, SHA2_256_HASH_LEN, + sizeof(struct padlock_sha_ctx), SHA2_256_HMAC_BLOCK_LEN, (void (*)(void *))padlock_sha_init, NULL, NULL, (int (*)(void *, const uint8_t *, uint16_t))padlock_sha_update, (void (*)(uint8_t *, void *))padlock_sha256_final diff --git a/sys/netipsec/xform_ah.c b/sys/netipsec/xform_ah.c index fada7b7e005f..9ef87ebaf236 100644 --- a/sys/netipsec/xform_ah.c +++ b/sys/netipsec/xform_ah.c @@ -193,9 +193,9 @@ ah_init0(struct secasvar *sav, struct xformsw *xsp, struct cryptoini *cria) return EINVAL; } keylen = _KEYLEN(sav->key_auth); - if (keylen != thash->keysize && thash->keysize != 0) { + if (keylen > thash->keysize && thash->keysize != 0) { DPRINTF(("%s: invalid keylength %d, algorithm %s requires " - "keysize %d\n", __func__, + "keysize less than %d\n", __func__, keylen, thash->name, thash->keysize)); return EINVAL; } diff --git a/sys/opencrypto/cryptodev.c b/sys/opencrypto/cryptodev.c index 6e8bf192b85d..d73e09fcb6a5 100644 --- a/sys/opencrypto/cryptodev.c +++ b/sys/opencrypto/cryptodev.c @@ -520,7 +520,8 @@ cryptof_ioctl( if (thash) { cria.cri_alg = thash->type; cria.cri_klen = sop->mackeylen * 8; - if (sop->mackeylen != thash->keysize) { + if (thash->keysize != 0 && + sop->mackeylen > thash->keysize) { CRYPTDEB("invalid mac key length"); error = EINVAL; goto bail; diff --git a/sys/opencrypto/cryptodev.h b/sys/opencrypto/cryptodev.h index b9a7634298d5..c42515a66356 100644 --- a/sys/opencrypto/cryptodev.h +++ b/sys/opencrypto/cryptodev.h @@ -95,13 +95,6 @@ #define HMAC_IPAD_VAL 0x36 #define HMAC_OPAD_VAL 0x5C /* HMAC Key Length */ -#define NULL_HMAC_KEY_LEN 0 -#define MD5_HMAC_KEY_LEN 16 -#define SHA1_HMAC_KEY_LEN 20 -#define RIPEMD160_HMAC_KEY_LEN 20 -#define SHA2_256_HMAC_KEY_LEN 32 -#define SHA2_384_HMAC_KEY_LEN 48 -#define SHA2_512_HMAC_KEY_LEN 64 #define AES_128_GMAC_KEY_LEN 16 #define AES_192_GMAC_KEY_LEN 24 #define AES_256_GMAC_KEY_LEN 32 diff --git a/sys/opencrypto/xform_md5.c b/sys/opencrypto/xform_md5.c index 3a8751d9c020..130335786fe4 100644 --- a/sys/opencrypto/xform_md5.c +++ b/sys/opencrypto/xform_md5.c @@ -58,14 +58,14 @@ static int MD5Update_int(void *, const u_int8_t *, u_int16_t); /* Authentication instances */ struct auth_hash auth_hash_hmac_md5 = { CRYPTO_MD5_HMAC, "HMAC-MD5", - MD5_HMAC_KEY_LEN, MD5_HASH_LEN, sizeof(MD5_CTX), MD5_HMAC_BLOCK_LEN, + MD5_HMAC_BLOCK_LEN, MD5_HASH_LEN, sizeof(MD5_CTX), MD5_HMAC_BLOCK_LEN, (void (*) (void *)) MD5Init, NULL, NULL, MD5Update_int, (void (*) (u_int8_t *, void *)) MD5Final }; struct auth_hash auth_hash_key_md5 = { CRYPTO_MD5_KPDK, "Keyed MD5", - NULL_HMAC_KEY_LEN, MD5_KPDK_HASH_LEN, sizeof(MD5_CTX), 0, + 0, MD5_KPDK_HASH_LEN, sizeof(MD5_CTX), 0, (void (*)(void *)) MD5Init, NULL, NULL, MD5Update_int, (void (*)(u_int8_t *, void *)) MD5Final }; diff --git a/sys/opencrypto/xform_null.c b/sys/opencrypto/xform_null.c index 74f410cba945..42b6f6b11d57 100644 --- a/sys/opencrypto/xform_null.c +++ b/sys/opencrypto/xform_null.c @@ -78,7 +78,7 @@ struct enc_xform enc_xform_null = { /* Authentication instances */ struct auth_hash auth_hash_null = { /* NB: context isn't used */ CRYPTO_NULL_HMAC, "NULL-HMAC", - NULL_HMAC_KEY_LEN, NULL_HASH_LEN, sizeof(int), NULL_HMAC_BLOCK_LEN, + 0, NULL_HASH_LEN, sizeof(int), NULL_HMAC_BLOCK_LEN, null_init, null_reinit, null_reinit, null_update, null_final }; diff --git a/sys/opencrypto/xform_rmd160.c b/sys/opencrypto/xform_rmd160.c index 4bce0729fa0b..334b3ce5adcb 100644 --- a/sys/opencrypto/xform_rmd160.c +++ b/sys/opencrypto/xform_rmd160.c @@ -58,7 +58,7 @@ static int RMD160Update_int(void *, const u_int8_t *, u_int16_t); /* Authentication instances */ struct auth_hash auth_hash_hmac_ripemd_160 = { CRYPTO_RIPEMD160_HMAC, "HMAC-RIPEMD-160", - RIPEMD160_HMAC_KEY_LEN, RIPEMD160_HASH_LEN, sizeof(RMD160_CTX), + RIPEMD160_HMAC_BLOCK_LEN, RIPEMD160_HASH_LEN, sizeof(RMD160_CTX), RIPEMD160_HMAC_BLOCK_LEN, (void (*)(void *)) RMD160Init, NULL, NULL, RMD160Update_int, (void (*)(u_int8_t *, void *)) RMD160Final diff --git a/sys/opencrypto/xform_sha1.c b/sys/opencrypto/xform_sha1.c index 29a5916559c2..640d93b9e317 100644 --- a/sys/opencrypto/xform_sha1.c +++ b/sys/opencrypto/xform_sha1.c @@ -60,13 +60,13 @@ static void SHA1Final_int(u_int8_t *, void *); /* Authentication instances */ struct auth_hash auth_hash_hmac_sha1 = { CRYPTO_SHA1_HMAC, "HMAC-SHA1", - SHA1_HMAC_KEY_LEN, SHA1_HASH_LEN, sizeof(SHA1_CTX), SHA1_HMAC_BLOCK_LEN, + SHA1_HMAC_BLOCK_LEN, SHA1_HASH_LEN, sizeof(SHA1_CTX), SHA1_HMAC_BLOCK_LEN, SHA1Init_int, NULL, NULL, SHA1Update_int, SHA1Final_int }; struct auth_hash auth_hash_key_sha1 = { CRYPTO_SHA1_KPDK, "Keyed SHA1", - NULL_HMAC_KEY_LEN, SHA1_KPDK_HASH_LEN, sizeof(SHA1_CTX), 0, + 0, SHA1_KPDK_HASH_LEN, sizeof(SHA1_CTX), 0, SHA1Init_int, NULL, NULL, SHA1Update_int, SHA1Final_int }; diff --git a/sys/opencrypto/xform_sha2.c b/sys/opencrypto/xform_sha2.c index 389cb8dab981..866b55490ee0 100644 --- a/sys/opencrypto/xform_sha2.c +++ b/sys/opencrypto/xform_sha2.c @@ -62,7 +62,7 @@ static int SHA512Update_int(void *, const u_int8_t *, u_int16_t); /* Authentication instances */ struct auth_hash auth_hash_hmac_sha2_256 = { CRYPTO_SHA2_256_HMAC, "HMAC-SHA2-256", - SHA2_256_HMAC_KEY_LEN, SHA2_256_HASH_LEN, sizeof(SHA256_CTX), + SHA2_256_HMAC_BLOCK_LEN, SHA2_256_HASH_LEN, sizeof(SHA256_CTX), SHA2_256_HMAC_BLOCK_LEN, (void (*)(void *)) SHA256_Init, NULL, NULL, SHA256Update_int, (void (*)(u_int8_t *, void *)) SHA256_Final @@ -70,7 +70,7 @@ struct auth_hash auth_hash_hmac_sha2_256 = { struct auth_hash auth_hash_hmac_sha2_384 = { CRYPTO_SHA2_384_HMAC, "HMAC-SHA2-384", - SHA2_384_HMAC_KEY_LEN, SHA2_384_HASH_LEN, sizeof(SHA384_CTX), + SHA2_384_HMAC_BLOCK_LEN, SHA2_384_HASH_LEN, sizeof(SHA384_CTX), SHA2_384_HMAC_BLOCK_LEN, (void (*)(void *)) SHA384_Init, NULL, NULL, SHA384Update_int, (void (*)(u_int8_t *, void *)) SHA384_Final @@ -78,7 +78,7 @@ struct auth_hash auth_hash_hmac_sha2_384 = { struct auth_hash auth_hash_hmac_sha2_512 = { CRYPTO_SHA2_512_HMAC, "HMAC-SHA2-512", - SHA2_512_HMAC_KEY_LEN, SHA2_512_HASH_LEN, sizeof(SHA512_CTX), + SHA2_512_HMAC_BLOCK_LEN, SHA2_512_HASH_LEN, sizeof(SHA512_CTX), SHA2_512_HMAC_BLOCK_LEN, (void (*)(void *)) SHA512_Init, NULL, NULL, SHA512Update_int, (void (*)(u_int8_t *, void *)) SHA512_Final