cryptosoft: Fix support for variable tag lengths in AES-CCM.

The tag length is included as one of the values in the flags byte of
block 0 passed to CBC_MAC, so merely copying the first N bytes is
insufficient.

To avoid adding more sideband data to the CBC MAC software context,
pull the generation of block 0, the AAD length, and AAD padding out of
cbc_mac.c and into cryptosoft.c.  This matches how GCM/GMAC are
handled where the length block is constructed in cryptosoft.c and
passed as an input to the Update callback.  As a result, the CBC MAC
Update() routine is now much simpler and simply performs the
XOR-and-encrypt step on each input block.

While here, avoid a copy to the staging block in the Update routine
when one or more full blocks are passed as input to the Update
callback.

Reviewed by:	sef
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D32120

sys/dev/cxgbe/crypto/t4_crypto.c

@@ -1893,8 +1893,60 @@ ccr_ccm_done(struct ccr_softc *sc, struct ccr_session *s,
 
 /*
  * Handle a CCM request that is not supported by the crypto engine by
- * performing the operation in software.  Derived from swcr_authenc().
+ * performing the operation in software.  Derived from swcr_ccm().
  */
+static void
+build_ccm_b0(const char *nonce, u_int nonce_length, u_int aad_length,
+    u_int data_length, u_int tag_length, uint8_t *b0)
+{
+	uint8_t *bp;
+	uint8_t flags, L;
+
+	KASSERT(nonce_length >= 7 && nonce_length <= 13,
+	    ("nonce_length must be between 7 and 13 bytes"));
+
+	/*
+	 * Need to determine the L field value.  This is the number of
+	 * bytes needed to specify the length of the message; the length
+	 * is whatever is left in the 16 bytes after specifying flags and
+	 * the nonce.
+	 */
+	L = 15 - nonce_length;
+
+	flags = ((aad_length > 0) << 6) +
+	    (((tag_length - 2) / 2) << 3) +
+	    L - 1;
+
+	/*
+	 * Now we need to set up the first block, which has flags, nonce,
+	 * and the message length.
+	 */
+	b0[0] = flags;
+	memcpy(b0 + 1, nonce, nonce_length);
+	bp = b0 + 1 + nonce_length;
+
+	/* Need to copy L' [aka L-1] bytes of data_length */
+	for (uint8_t *dst = b0 + CCM_CBC_BLOCK_LEN - 1; dst >= bp; dst--) {
+		*dst = data_length;
+		data_length >>= 8;
+	}
+}
+
+/* NB: OCF only supports AAD lengths < 2^32. */
+static int
+build_ccm_aad_length(u_int aad_length, uint8_t *blk)
+{
+	if (aad_length < ((1 << 16) - (1 << 8))) {
+		be16enc(blk, aad_length);
+		return (sizeof(uint16_t));
+	} else {
+		blk[0] = 0xff;
+		blk[1] = 0xfe;
+		be32enc(blk + 2, aad_length);
+		return (2 + sizeof(uint32_t));
+	}
+}
+
 static void
 ccr_ccm_soft(struct ccr_session *s, struct cryptop *crp)
 {
@@ -1904,11 +1956,13 @@ ccr_ccm_soft(struct ccr_session *s, struct cryptop *crp)
 	union authctx *auth_ctx;
 	void *kschedule;
 	char block[CCM_CBC_BLOCK_LEN];
-	char digest[AES_CBC_MAC_HASH_LEN];
+	char tag[AES_CBC_MAC_HASH_LEN];
+	u_int taglen;
 	int error, i, len;
 
 	auth_ctx = NULL;
 	kschedule = NULL;
+	taglen = s->ccm_mac.hash_len;
 
 	csp = crypto_get_params(crp->crp_session);
 	if (crp->crp_payload_length > ccm_max_payload_length(csp)) {
@@ -1956,19 +2010,32 @@ ccr_ccm_soft(struct ccr_session *s, struct cryptop *crp)
 		goto out;
 	}
 
-	auth_ctx->aes_cbc_mac_ctx.authDataLength = crp->crp_aad_length;
-	auth_ctx->aes_cbc_mac_ctx.cryptDataLength = crp->crp_payload_length;
 	axf->Reinit(auth_ctx, crp->crp_iv, csp->csp_ivlen);
 
+	/* Supply MAC with b0. */
+	build_ccm_b0(crp->crp_iv, csp->csp_ivlen, crp->crp_aad_length,
+	    crp->crp_payload_length, taglen, block);
+	axf->Update(auth_ctx, block, CCM_CBC_BLOCK_LEN);
+
 	/* MAC the AAD. */
-	if (crp->crp_aad != NULL)
-		error = axf->Update(auth_ctx, crp->crp_aad,
-		    crp->crp_aad_length);
-	else
-		error = crypto_apply(crp, crp->crp_aad_start,
-		    crp->crp_aad_length, axf->Update, auth_ctx);
-	if (error)
-		goto out;
+	if (crp->crp_aad_length != 0) {
+		len = build_ccm_aad_length(crp->crp_aad_length, block);
+		axf->Update(auth_ctx, block, len);
+		if (crp->crp_aad != NULL)
+			axf->Update(auth_ctx, crp->crp_aad,
+			    crp->crp_aad_length);
+		else
+			crypto_apply(crp, crp->crp_aad_start,
+			    crp->crp_aad_length, axf->Update, auth_ctx);
+
+		/* Pad the AAD (including length field) to a full block. */
+		len = (len + crp->crp_aad_length) % CCM_CBC_BLOCK_LEN;
+		if (len != 0) {
+			len = CCM_CBC_BLOCK_LEN - len;
+			memset(block, 0, CCM_CBC_BLOCK_LEN);
+			axf->Update(auth_ctx, block, len);
+		}
+	}
 
 	exf->reinit(kschedule, crp->crp_iv, csp->csp_ivlen);
 
@@ -1989,19 +2056,17 @@ ccr_ccm_soft(struct ccr_session *s, struct cryptop *crp)
 	}
 
 	/* Finalize MAC. */
-	axf->Final(digest, auth_ctx);
+	axf->Final(tag, auth_ctx);
 
 	/* Inject or validate tag. */
 	if (CRYPTO_OP_IS_ENCRYPT(crp->crp_op)) {
-		crypto_copyback(crp, crp->crp_digest_start, sizeof(digest),
-		    digest);
+		crypto_copyback(crp, crp->crp_digest_start, taglen, tag);
 		error = 0;
 	} else {
-		char digest2[AES_CBC_MAC_HASH_LEN];
+		char tag2[AES_CBC_MAC_HASH_LEN];
 
-		crypto_copydata(crp, crp->crp_digest_start, sizeof(digest2),
-		    digest2);
-		if (timingsafe_bcmp(digest, digest2, sizeof(digest)) == 0) {
+		crypto_copydata(crp, crp->crp_digest_start, taglen, tag2);
+		if (timingsafe_bcmp(tag, tag2, taglen) == 0) {
 			error = 0;
 
 			/* Tag matches, decrypt data. */
@@ -2019,14 +2084,14 @@ ccr_ccm_soft(struct ccr_session *s, struct cryptop *crp)
 			}
 		} else
 			error = EBADMSG;
-		explicit_bzero(digest2, sizeof(digest2));
+		explicit_bzero(tag2, sizeof(tag2));
 	}
 
 out:
 	zfree(kschedule, M_CCR);
 	zfree(auth_ctx, M_CCR);
 	explicit_bzero(block, sizeof(block));
-	explicit_bzero(digest, sizeof(digest));
+	explicit_bzero(tag, sizeof(tag));
 	crp->crp_etype = error;
 	crypto_done(crp);
 }

sys/opencrypto/cbc_mac.c

@@ -75,85 +75,23 @@ AES_CBC_MAC_Setkey(void *vctx, const uint8_t *key, u_int klen)
 
 /*
  * This is called to set the nonce, aka IV.
- * Before this call, the authDataLength and cryptDataLength fields
- * MUST have been set.  Sadly, there's no way to return an error.
  *
- * The CBC-MAC algorithm requires that the first block contain the
- * nonce, as well as information about the sizes and lengths involved.
+ * Note that the caller is responsible for constructing b0 as well
+ * as the length and padding around the AAD and passing that data
+ * to _Update.
  */
 void
 AES_CBC_MAC_Reinit(void *vctx, const uint8_t *nonce, u_int nonceLen)
 {
 	struct aes_cbc_mac_ctx *ctx = vctx;
-	uint8_t b0[CCM_CBC_BLOCK_LEN];
-	uint8_t *bp = b0, flags = 0;
-	uint8_t L = 0;
-	uint64_t dataLength = ctx->cryptDataLength;
-
-	KASSERT(nonceLen >= 7 && nonceLen <= 13,
-	    ("nonceLen must be between 7 and 13 bytes"));
 
 	ctx->nonce = nonce;
 	ctx->nonceLength = nonceLen;
-	
-	ctx->authDataCount = 0;
-	ctx->blockIndex = 0;
-	explicit_bzero(ctx->staging_block, sizeof(ctx->staging_block));
-	
-	/*
-	 * Need to determine the L field value.  This is the number of
-	 * bytes needed to specify the length of the message; the length
-	 * is whatever is left in the 16 bytes after specifying flags and
-	 * the nonce.
-	 */
-	L = 15 - nonceLen;
-	
-	flags = ((ctx->authDataLength > 0) << 6) +
-	    (((AES_CBC_MAC_HASH_LEN - 2) / 2) << 3) +
-	    L - 1;
-	/*
-	 * Now we need to set up the first block, which has flags, nonce,
-	 * and the message length.
-	 */
-	b0[0] = flags;
-	bcopy(nonce, b0 + 1, nonceLen);
-	bp = b0 + 1 + nonceLen;
 
-	/* Need to copy L' [aka L-1] bytes of cryptDataLength */
-	for (uint8_t *dst = b0 + sizeof(b0) - 1; dst >= bp; dst--) {
-		*dst = dataLength;
-		dataLength >>= 8;
-	}
-	/* Now need to encrypt b0 */
-	rijndaelEncrypt(ctx->keysched, ctx->rounds, b0, ctx->block);
-	/* If there is auth data, we need to set up the staging block */
-	if (ctx->authDataLength) {
-		size_t addLength;
-		if (ctx->authDataLength < ((1<<16) - (1<<8))) {
-			uint16_t sizeVal = htobe16(ctx->authDataLength);
-			bcopy(&sizeVal, ctx->staging_block, sizeof(sizeVal));
-			addLength = sizeof(sizeVal);
-		} else if (ctx->authDataLength < (1ULL<<32)) {
-			uint32_t sizeVal = htobe32(ctx->authDataLength);
-			ctx->staging_block[0] = 0xff;
-			ctx->staging_block[1] = 0xfe;
-			bcopy(&sizeVal, ctx->staging_block+2, sizeof(sizeVal));
-			addLength = 2 + sizeof(sizeVal);
-		} else {
-			uint64_t sizeVal = htobe64(ctx->authDataLength);
-			ctx->staging_block[0] = 0xff;
-			ctx->staging_block[1] = 0xff;
-			bcopy(&sizeVal, ctx->staging_block+2, sizeof(sizeVal));
-			addLength = 2 + sizeof(sizeVal);
-		}
-		ctx->blockIndex = addLength;
-		/*
-		 * The length descriptor goes into the AAD buffer, so we
-		 * need to account for it.
-		 */
-		ctx->authDataLength += addLength;
-		ctx->authDataCount = addLength;
-	}
+	ctx->blockIndex = 0;
+
+	/* XOR b0 with all 0's on first call to _Update. */
+	memset(ctx->block, 0, CCM_CBC_BLOCK_LEN);
 }
 
 int
@@ -167,85 +105,35 @@ AES_CBC_MAC_Update(void *vctx, const void *vdata, u_int length)
 	data = vdata;
 
 	/*
-	 * This will be called in one of two phases:
-	 * (1)  Applying authentication data, or
-	 * (2)  Applying the payload data.
-	 *
-	 * Because CBC-MAC puts the authentication data size before the
-	 * data, subsequent calls won't be block-size-aligned.  Which
-	 * complicates things a fair bit.
-	 *
-	 * The payload data doesn't have that problem.
+	 * _Update can be called with non-aligned update lengths.  Use
+	 * the staging block when necessary.
 	 */
-				
-	if (ctx->authDataCount < ctx->authDataLength) {
-		/*
-		 * We need to process data as authentication data.
-		 * Since we may be out of sync, we may also need
-		 * to pad out the staging block.
-		 */
-		const uint8_t *ptr = data;
-		while (length > 0) {
-
-			copy_amt = MIN(length,
-			    sizeof(ctx->staging_block) - ctx->blockIndex);
-
-			bcopy(ptr, ctx->staging_block + ctx->blockIndex,
-			    copy_amt);
-			ptr += copy_amt;
-			length -= copy_amt;
-			ctx->authDataCount += copy_amt;
-			ctx->blockIndex += copy_amt;
-			ctx->blockIndex %= sizeof(ctx->staging_block);
-
-			if (ctx->blockIndex == 0 ||
-			    ctx->authDataCount == ctx->authDataLength) {
-				/*
-				 * We're done with this block, so we
-				 * xor staging_block with block, and then
-				 * encrypt it.
-				 */
-				xor_and_encrypt(ctx, ctx->staging_block, ctx->block);
-				bzero(ctx->staging_block, sizeof(ctx->staging_block));
-				ctx->blockIndex = 0;
-				if (ctx->authDataCount >= ctx->authDataLength)
-					break;
-			}
-		}
-		/*
-		 * We'd like to be able to check length == 0 and return
-		 * here, but the way OCF calls us, length is always
-		 * blksize (16, in this case).  So we have to count on
-		 * the fact that OCF calls us separately for the AAD and
-		 * for the real data.
-		 */
-		return (0);
-	}
-	/*
-	 * If we're here, then we're encoding payload data.
-	 * This is marginally easier, except that _Update can
-	 * be called with non-aligned update lengths. As a result,
-	 * we still need to use the staging block.
-	 */
-	KASSERT((length + ctx->cryptDataCount) <= ctx->cryptDataLength,
-	    ("More encryption data than allowed"));
-
-	while (length) {
+	while (length != 0) {
 		uint8_t *ptr;
-		
+
+		/*
+		 * If there is no partial block and the length is at
+		 * least a full block, encrypt the full block without
+		 * copying to the staging block.
+		 */
+		if (ctx->blockIndex == 0 && length >= CCM_CBC_BLOCK_LEN) {
+			xor_and_encrypt(ctx, data, ctx->block);
+			length -= CCM_CBC_BLOCK_LEN;
+			data += CCM_CBC_BLOCK_LEN;
+			continue;
+		}
+
 		copy_amt = MIN(sizeof(ctx->staging_block) - ctx->blockIndex,
 		    length);
 		ptr = ctx->staging_block + ctx->blockIndex;
 		bcopy(data, ptr, copy_amt);
 		data += copy_amt;
 		ctx->blockIndex += copy_amt;
-		ctx->cryptDataCount += copy_amt;
 		length -= copy_amt;
 		if (ctx->blockIndex == sizeof(ctx->staging_block)) {
 			/* We've got a full block */
 			xor_and_encrypt(ctx, ctx->staging_block, ctx->block);
 			ctx->blockIndex = 0;
-			bzero(ctx->staging_block, sizeof(ctx->staging_block));
 		}
 	}
 	return (0);
@@ -264,11 +152,12 @@ AES_CBC_MAC_Final(uint8_t *buf, void *vctx)
 	 * left over to encrypt.
 	 */
 	if (ctx->blockIndex != 0) {
+		memset(ctx->staging_block + ctx->blockIndex, 0,
+		    CCM_CBC_BLOCK_LEN - ctx->blockIndex);
 		xor_and_encrypt(ctx, ctx->staging_block, ctx->block);
-		ctx->cryptDataCount += ctx->blockIndex;
-		ctx->blockIndex = 0;
-		explicit_bzero(ctx->staging_block, sizeof(ctx->staging_block));
 	}
+	explicit_bzero(ctx->staging_block, sizeof(ctx->staging_block));
+
 	bzero(s0, sizeof(s0));
 	s0[0] = (15 - ctx->nonceLength) - 1;
 	bcopy(ctx->nonce, s0 + 1, ctx->nonceLength);

sys/opencrypto/cbc_mac.h

@@ -46,13 +46,11 @@
  * the encryption one is similar.
  */
 struct aes_cbc_mac_ctx {
-	uint64_t	authDataLength, authDataCount;
-	uint64_t	cryptDataLength, cryptDataCount;
-	int		blockIndex;
 	uint8_t		staging_block[CCM_CBC_BLOCK_LEN];
 	uint8_t		block[CCM_CBC_BLOCK_LEN];
-	const uint8_t	*nonce;
+	int		blockIndex;
 	int		nonceLength;	/* This one is in bytes, not bits! */
+	const uint8_t	*nonce;
 	/* AES state data */
 	int		rounds;
 	uint32_t	keysched[4*(RIJNDAEL_MAXNR+1)];

sys/opencrypto/cryptosoft.c

@@ -636,16 +636,69 @@ out:
 	return (error);
 }
 
+static void
+build_ccm_b0(const char *nonce, u_int nonce_length, u_int aad_length,
+    u_int data_length, u_int tag_length, uint8_t *b0)
+{
+	uint8_t *bp;
+	uint8_t flags, L;
+
+	KASSERT(nonce_length >= 7 && nonce_length <= 13,
+	    ("nonce_length must be between 7 and 13 bytes"));
+
+	/*
+	 * Need to determine the L field value.  This is the number of
+	 * bytes needed to specify the length of the message; the length
+	 * is whatever is left in the 16 bytes after specifying flags and
+	 * the nonce.
+	 */
+	L = 15 - nonce_length;
+
+	flags = ((aad_length > 0) << 6) +
+	    (((tag_length - 2) / 2) << 3) +
+	    L - 1;
+
+	/*
+	 * Now we need to set up the first block, which has flags, nonce,
+	 * and the message length.
+	 */
+	b0[0] = flags;
+	memcpy(b0 + 1, nonce, nonce_length);
+	bp = b0 + 1 + nonce_length;
+
+	/* Need to copy L' [aka L-1] bytes of data_length */
+	for (uint8_t *dst = b0 + CCM_CBC_BLOCK_LEN - 1; dst >= bp; dst--) {
+		*dst = data_length;
+		data_length >>= 8;
+	}
+}
+
+/* NB: OCF only supports AAD lengths < 2^32. */
+static int
+build_ccm_aad_length(u_int aad_length, uint8_t *blk)
+{
+	if (aad_length < ((1 << 16) - (1 << 8))) {
+		be16enc(blk, aad_length);
+		return (sizeof(uint16_t));
+	} else {
+		blk[0] = 0xff;
+		blk[1] = 0xfe;
+		be32enc(blk + 2, aad_length);
+		return (2 + sizeof(uint32_t));
+	}
+}
+
 static int
 swcr_ccm_cbc_mac(struct swcr_session *ses, struct cryptop *crp)
 {
-	u_char tag[AES_CBC_MAC_HASH_LEN];
 	u_char iv[AES_BLOCK_LEN];
+	u_char blk[CCM_CBC_BLOCK_LEN];
+	u_char tag[AES_CBC_MAC_HASH_LEN];
 	union authctx ctx;
 	const struct crypto_session_params *csp;
 	struct swcr_auth *swa;
 	const struct auth_hash *axf;
-	int error, ivlen;
+	int error, ivlen, len;
 
 	csp = crypto_get_params(crp->crp_session);
 	swa = &ses->swcr_auth;
@@ -657,25 +710,24 @@ swcr_ccm_cbc_mac(struct swcr_session *ses, struct cryptop *crp)
 	ivlen = csp->csp_ivlen;
 	crypto_read_iv(crp, iv);
 
-	/*
-	 * AES CCM-CBC-MAC needs to know the length of both the auth
-	 * data and payload data before doing the auth computation.
-	 */
-	ctx.aes_cbc_mac_ctx.authDataLength = crp->crp_payload_length;
-	ctx.aes_cbc_mac_ctx.cryptDataLength = 0;
+	/* Supply MAC with IV */
+	axf->Reinit(&ctx, crp->crp_iv, ivlen);
 
-	axf->Reinit(&ctx, iv, ivlen);
-	if (crp->crp_aad != NULL)
-		error = axf->Update(&ctx, crp->crp_aad, crp->crp_aad_length);
-	else
-		error = crypto_apply(crp, crp->crp_payload_start,
-		    crp->crp_payload_length, axf->Update, &ctx);
-	if (error)
-		return (error);
+	/* Supply MAC with b0. */
+	build_ccm_b0(crp->crp_iv, ivlen, crp->crp_payload_length, 0,
+	    swa->sw_mlen, blk);
+	axf->Update(&ctx, blk, CCM_CBC_BLOCK_LEN);
+
+	len = build_ccm_aad_length(crp->crp_payload_length, blk);
+	axf->Update(&ctx, blk, len);
+
+	crypto_apply(crp, crp->crp_payload_start, crp->crp_payload_length,
+	    axf->Update, &ctx);
 
 	/* Finalize MAC */
 	axf->Final(tag, &ctx);
 
+	error = 0;
 	if (crp->crp_op & CRYPTO_OP_VERIFY_DIGEST) {
 		u_char tag2[AES_CBC_MAC_HASH_LEN];
 
@@ -689,6 +741,7 @@ swcr_ccm_cbc_mac(struct swcr_session *ses, struct cryptop *crp)
 		crypto_copyback(crp, crp->crp_digest_start, swa->sw_mlen, tag);
 	}
 	explicit_bzero(tag, sizeof(tag));
+	explicit_bzero(blk, sizeof(blk));
 	explicit_bzero(iv, sizeof(iv));
 	return (error);
 }
@@ -733,24 +786,35 @@ swcr_ccm(struct swcr_session *ses, struct cryptop *crp)
 
 	ivlen = csp->csp_ivlen;
 
-	/*
-	 * AES CCM-CBC-MAC needs to know the length of both the auth
-	 * data and payload data before doing the auth computation.
-	 */
-	ctx.aes_cbc_mac_ctx.authDataLength = crp->crp_aad_length;
-	ctx.aes_cbc_mac_ctx.cryptDataLength = crp->crp_payload_length;
-
 	/* Supply MAC with IV */
 	axf->Reinit(&ctx, crp->crp_iv, ivlen);
 
+	/* Supply MAC with b0. */
+	_Static_assert(sizeof(blkbuf) >= CCM_CBC_BLOCK_LEN,
+	    "blkbuf too small for b0");
+	build_ccm_b0(crp->crp_iv, ivlen, crp->crp_aad_length,
+	    crp->crp_payload_length, swa->sw_mlen, blk);
+	axf->Update(&ctx, blk, CCM_CBC_BLOCK_LEN);
+
 	/* Supply MAC with AAD */
-	if (crp->crp_aad != NULL)
-		error = axf->Update(&ctx, crp->crp_aad, crp->crp_aad_length);
-	else
-		error = crypto_apply(crp, crp->crp_aad_start,
-		    crp->crp_aad_length, axf->Update, &ctx);
-	if (error)
-		return (error);
+	if (crp->crp_aad_length != 0) {
+		len = build_ccm_aad_length(crp->crp_aad_length, blk);
+		axf->Update(&ctx, blk, len);
+		if (crp->crp_aad != NULL)
+			axf->Update(&ctx, crp->crp_aad,
+			    crp->crp_aad_length);
+		else
+			crypto_apply(crp, crp->crp_aad_start,
+			    crp->crp_aad_length, axf->Update, &ctx);
+
+		/* Pad the AAD (including length field) to a full block. */
+		len = (len + crp->crp_aad_length) % CCM_CBC_BLOCK_LEN;
+		if (len != 0) {
+			len = CCM_CBC_BLOCK_LEN - len;
+			memset(blk, 0, CCM_CBC_BLOCK_LEN);
+			axf->Update(&ctx, blk, len);
+		}
+	}
 
 	if (crp->crp_cipher_key != NULL)
 		exf->setkey(swe->sw_kschedule, crp->crp_cipher_key,