d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix another bug introduced during the review process of r344140:

Browse Source 
the tag wasn't being computed properly due to chaning a >= comparison
to an == comparison.

Specifically:  CBC-MAC encodes the length of the authorization data
into the the stream to be encrypted/hashed.  For short data, this is
two bytes (big-endian 16 bit value); for larger data, it's 6 bytes
(a prefix of 0xff, 0xfe, followed by a 32-bit big-endian length).  And
there's a larger size, which is 10 bytes.  These extra bytes weren't
being accounted for with the post-review code.  The other bit that then came
into play was that OCF only calls the Update code with blksiz=16, which
meant that I had to ignore the length variable.  (It also means that it
can't be called with a single buffer containing the AAD and payload;
however, OCF doesn't do this for the software-only algorithsm.)

I tested with this script:

ALG=aes-ccm
DEV=soft

for aad in 0 1 2 3 4 14 16 24 30 32 34 36 1020
do
        for dln in 16 32 1024 2048 10240
        do
                echo "Testing AAD length ${aad} data length ${dln}"
                /root/cryptocheck -A ${aad} -a ${ALG} -d ${DEV} ${dln}
        done
done

Reviewed by:	cem
Sponsored by:	iXsystems Inc.
This commit is contained in:
Sean Eric Fagan 2019-02-25 19:14:16 +00:00
parent 559af1ec16
commit f42230d856
1 changed files with 22 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
sys/opencrypto/cbc_mac.c
View File

@ -124,23 +124,31 @@ AES_CBC_MAC_Reinit(struct aes_cbc_mac_ctx *ctx, const uint8_t *nonce, uint16_t n
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));
ctx->blockIndex = 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));
ctx->blockIndex = 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));
ctx->blockIndex = 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;
}
}
@ -181,10 +189,9 @@ AES_CBC_MAC_Update(struct aes_cbc_mac_ctx *ctx, const uint8_t *data,
ctx->authDataCount += copy_amt;
ctx->blockIndex += copy_amt;
ctx->blockIndex %= sizeof(ctx->staging_block);
if (ctx->authDataCount == ctx->authDataLength)
length = 0;
if (ctx->blockIndex == 0 ||
ctx->authDataCount >= ctx->authDataLength) {
ctx->authDataCount == ctx->authDataLength) {
/*
* We're done with this block, so we
* xor staging_block with block, and then
@ -193,8 +200,17 @@ AES_CBC_MAC_Update(struct aes_cbc_mac_ctx *ctx, const uint8_t *data,
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);
}
/*