Date: Mon, 8 Sep 2014 04:50:10 GMT From: John-Mark Gurney <jmg@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 1199106 for review Message-ID: <201409080450.s884oAxU023836@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@1199106?ac=10 Change 1199106 by jmg@jmg_carbon2 on 2014/08/23 00:36:11 Fix various findings durning the review by Watson Ladd... Force that the IV (nonce) is provided for GCM and ICM.. using a random IV is bad as it can leak data... Make sure that we cannot go over the limits for GCM... All the limits for GCM are larger than the lengths provided, so just provide a CTASSERT that int never gets too large... Make sure we return an error when the tag does not match durning GCM decrypt... When the tag does not match we don't do the decrypt to prevent an attacker from passing bogus data to try to figure out what the cipher key is... Make sure that the user doesn't use different key sizes between the encryption and auth part for GCM... Sponsored by: FreeBSD Foundation Affected files ... .. //depot/projects/opencrypto/sys/opencrypto/cryptodev.c#7 edit .. //depot/projects/opencrypto/sys/opencrypto/cryptosoft.c#6 edit Differences ... ==== //depot/projects/opencrypto/sys/opencrypto/cryptodev.c#7 (text+ko) ==== @@ -904,7 +904,7 @@ struct cryptodesc *crde = NULL, *crda = NULL; int error; - if (caead->len > 256*1024-4) + if (caead->len > 256*1024-4 || caead->aadlen > 256*1024-4) return (E2BIG); if (cse->txform == NULL || cse->thash == NULL || caead->tag == NULL || ==== //depot/projects/opencrypto/sys/opencrypto/cryptosoft.c#6 (text+ko) ==== @@ -43,6 +43,7 @@ #include <sys/lock.h> #include <sys/rwlock.h> #include <sys/endian.h> +#include <sys/limits.h> #include <crypto/blowfish/blowfish.h> #include <crypto/sha1.h> @@ -94,6 +95,10 @@ if (crd->crd_len % blks) return EINVAL; + if (crd->crd_alg == CRYPTO_AES_ICM && + (crd->crd_flags & CRD_F_IV_EXPLICIT) == 0) + return (EINVAL); + /* Initialize the IV */ if (crd->crd_flags & CRD_F_ENCRYPT) { /* IV explicitly provided ? */ @@ -590,6 +595,9 @@ return 0; } +CTASSERT(INT_MAX <= (1ll<<39) - 256); /* GCM: plain text < 2^39-256 */ +CTASSERT(INT_MAX <= (uint64_t)-1); /* GCM: associated data <= 2^64-1 */ + /* * Apply a combined encryption-authentication transformation */ @@ -646,6 +654,13 @@ if (crde == NULL || crda == NULL) return (EINVAL); + if (crde->crd_alg == CRYPTO_AES_NIST_GCM_16 && + (crde->crd_flags & CRD_F_IV_EXPLICIT) == 0) + return (EINVAL); + + if (crde->crd_klen != crda->crd_klen) + return (EINVAL); + /* Initialize the IV */ if (crde->crd_flags & CRD_F_ENCRYPT) { /* IV explicitly provided ? */ @@ -725,32 +740,36 @@ axf->Final(aalg, &ctx); /* Validate tag */ - crypto_copydata(crp->crp_flags, buf, crda->crd_inject, axf->hashsize, - uaalg); + if (!(crde->crd_flags & CRD_F_ENCRYPT)) { + crypto_copydata(crp->crp_flags, buf, crda->crd_inject, + axf->hashsize, uaalg); - r = 0; - for (i = 0; i < axf->hashsize; i++) - r |= aalg[i] ^ uaalg[i]; + r = 0; + for (i = 0; i < axf->hashsize; i++) + r |= aalg[i] ^ uaalg[i]; - if (r == 0) { - for (i = 0; i < crde->crd_len; i += blksz) { - len = MIN(crde->crd_len - i, blksz); - if (len < blksz) - bzero(blk, blksz); - crypto_copydata(crp->crp_flags, buf, - crde->crd_skip + i, len, blk); - if (!(crde->crd_flags & CRD_F_ENCRYPT)) { - exf->decrypt(swe->sw_kschedule, blk); + if (r == 0) { + /* tag matches, decrypt data */ + for (i = 0; i < crde->crd_len; i += blksz) { + len = MIN(crde->crd_len - i, blksz); + if (len < blksz) + bzero(blk, blksz); + crypto_copydata(crp->crp_flags, buf, + crde->crd_skip + i, len, blk); + if (!(crde->crd_flags & CRD_F_ENCRYPT)) { + exf->decrypt(swe->sw_kschedule, blk); + } + crypto_copyback(crp->crp_flags, buf, + crde->crd_skip + i, len, blk); } - crypto_copyback(crp->crp_flags, buf, - crde->crd_skip + i, len, blk); - } + } else + return (EBADMSG); + } else { + /* Inject the authentication data */ + crypto_copyback(crp->crp_flags, buf, crda->crd_inject, + axf->hashsize, aalg); } - /* Inject the authentication data */ - crypto_copyback(crp->crp_flags, buf, crda->crd_inject, axf->hashsize, - aalg); - return (0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201409080450.s884oAxU023836>