From owner-svn-src-head@freebsd.org Mon Feb 25 19:14:17 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id ECFEA151351E; Mon, 25 Feb 2019 19:14:16 +0000 (UTC) (envelope-from sef@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9159A75676; Mon, 25 Feb 2019 19:14:16 +0000 (UTC) (envelope-from sef@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 7DCD4215B4; Mon, 25 Feb 2019 19:14:16 +0000 (UTC) (envelope-from sef@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x1PJEGYD046233; Mon, 25 Feb 2019 19:14:16 GMT (envelope-from sef@FreeBSD.org) Received: (from sef@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x1PJEGme046232; Mon, 25 Feb 2019 19:14:16 GMT (envelope-from sef@FreeBSD.org) Message-Id: <201902251914.x1PJEGme046232@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: sef set sender to sef@FreeBSD.org using -f From: Sean Eric Fagan Date: Mon, 25 Feb 2019 19:14:16 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r344547 - head/sys/opencrypto X-SVN-Group: head X-SVN-Commit-Author: sef X-SVN-Commit-Paths: head/sys/opencrypto X-SVN-Commit-Revision: 344547 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 9159A75676 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.97 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-0.998,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.97)[-0.972,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Feb 2019 19:14:17 -0000 Author: sef Date: Mon Feb 25 19:14:16 2019 New Revision: 344547 URL: https://svnweb.freebsd.org/changeset/base/344547 Log: Fix another bug introduced during the review process of r344140: 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. Modified: head/sys/opencrypto/cbc_mac.c Modified: head/sys/opencrypto/cbc_mac.c ============================================================================== --- head/sys/opencrypto/cbc_mac.c Mon Feb 25 19:07:52 2019 (r344546) +++ head/sys/opencrypto/cbc_mac.c Mon Feb 25 19:14:16 2019 (r344547) @@ -124,23 +124,31 @@ AES_CBC_MAC_Reinit(struct aes_cbc_mac_ctx *ctx, const 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 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 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); } /*