From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5D640C433F5 for ; Tue, 22 Mar 2022 07:02:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=lW3SthWFHdzB3Lv1m5OIRZW5xCGirZVrdeuP8Gs3vck=; b=B6belwutVx6/dQ vu6ES28wRqbp+zsKn3+ETzg5h8r0yl/ESKY7W4dO31QvM8CcUYnYhaipE9ZkUxY+qAMtj4w05QoUC /yjG/qQ7GLtW1ftAmW1wWwXG1VcAgtjJ41pefy5X0gwWoR6hbKXz189gnoR2YVlmcCXnmjw3GD9vM HHBjAEkSBH+2Vk1n6ev3wm9LHIdmAklu50J5nnD2hb+tISgm5UDC+M/en86W44ERf4ptqNkApddlU mhwh6iE/wAuZlRg2w+IGfVKtMAOjy4LVC5J3al8mLt6bOUvQkX+a6+pDZitAWgsM47qZG2BVqe/BG IFapVLM5WZL0XxUaTfqA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nWYVf-00AEGe-Rn; Tue, 22 Mar 2022 07:00:52 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nWYVc-00AEFv-HV for linux-arm-kernel@lists.infradead.org; Tue, 22 Mar 2022 07:00:50 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id E89BF6148C; Tue, 22 Mar 2022 07:00:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 18A83C340EC; Tue, 22 Mar 2022 07:00:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1647932447; bh=yUcgnO83m6wpw6WaeePnlqcDePF0XZJ7jRcIVZ2+Jbg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=iOwjqVf1C2VN85JgSPne9cK3Q/5CbqGi0QU3tQriKAMeAPh3VllB2ePZjxNK3RdWA mB8K5Sh6OaIoHL2lIgRTaAbvdrxb148iGPKMSH8aAT7bSe1QXdcjM06ZGOTcdxIqNb NjZULbEMnls4SJUquZOswP3vy6tDP6g9hKTFom6tZePkqL/Q3KdclPySL/cjJmrb+b AkpsoZ81JHC2IwoeoFdBHdw1ofb1VoVUl9Kfn54uMD/B+lHxXyFVjHsR7WOGbSO6ZD IBTDW+R4wMnKOXjZSIMFcfuIl2MXKpdICp8E6C54rMsIMzCtSKFlsKFZ3IaCvAuU1o y2qdajpMhe4Fg== Date: Tue, 22 Mar 2022 00:00:45 -0700 From: Eric Biggers To: Nathan Huckleberry Cc: linux-crypto@vger.kernel.org, Herbert Xu , "David S. Miller" , linux-arm-kernel@lists.infradead.org, Paul Crowley , Sami Tolvanen , Ard Biesheuvel Subject: Re: [PATCH v3 3/8] crypto: hctr2 - Add HCTR2 support Message-ID: References: <20220315230035.3792663-1-nhuck@google.com> <20220315230035.3792663-4-nhuck@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220315230035.3792663-4-nhuck@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220322_000048_690062_8A72C8F3 X-CRM114-Status: GOOD ( 20.79 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Mar 15, 2022 at 11:00:30PM +0000, Nathan Huckleberry wrote: > +struct hctr2_tfm_ctx { > + struct crypto_cipher *blockcipher; > + struct crypto_skcipher *xctr; > + struct crypto_shash *polyval; > + u8 L[BLOCKCIPHER_BLOCK_SIZE]; > +}; How about adding a comment at the end of the struct above that says that the struct is followed by the two exported_length_digests? (Or hashed_tweaklen, which is the name suggested in my suggested helper functions below?) > + > +struct hctr2_request_ctx { > + u8 first_block[BLOCKCIPHER_BLOCK_SIZE]; > + u8 xctr_iv[BLOCKCIPHER_BLOCK_SIZE]; > + struct scatterlist *bulk_part_dst; > + struct scatterlist *bulk_part_src; > + struct scatterlist sg_src[2]; > + struct scatterlist sg_dst[2]; > + /* Sub-requests, must be last */ > + union { > + struct shash_desc hash_desc; > + struct skcipher_request xctr_req; > + } u; > +}; Likewise above for the hashed tweak. Also how about adding inline functions or macros that return these new fields, so that the arithmetic to find them doesn't have to be duplicated in the code? The 'exported_length_digests' array local variables are a bit weird. Maybe just use some helper functions directly to get at the fields? How about: static inline u8 *hctr2_hashed_tweaklen(const struct hctr2_tfm_ctx *tctx, bool odd) { u8 *p = (u8 *)tctx + sizeof(*tctx); if (odd) /* For messages not a multiple of block length */ p += crypto_shash_statesize(tctx->polyval); return p; } static inline u8 *hctr2_hashed_tweak(const struct hctr2_tfm_ctx *tctx, struct hctr2_request_ctx *rctx) { return (u8 *)rctx + tctx->hashed_tweak_offset; } > +static int hctr2_setkey(struct crypto_skcipher *tfm, const u8 *key, > + unsigned int keylen) > +{ > + struct hctr2_tfm_ctx *tctx = crypto_skcipher_ctx(tfm); > + u8 hbar[BLOCKCIPHER_BLOCK_SIZE]; > + __le64 tweak_length_block[2]; > + void *exported_length_digests[2]; > + SHASH_DESC_ON_STACK(shash, tfm->polyval); > + int err; > + > + exported_length_digests[0] = (u8 *)tctx + sizeof(*tctx); > + exported_length_digests[1] = (u8 *)tctx + sizeof(*tctx) + > + crypto_shash_descsize(tctx->polyval); The size needed by crypto_shash_export() is crypto_shash_statesize(), not crypto_shash_descsize(). They happen to be the same with all polyval implementations you've proposed, but it's not guaranteed for shash algorithms in general. > + crypto_cipher_clear_flags(tctx->blockcipher, CRYPTO_TFM_REQ_MASK); > + crypto_cipher_set_flags(tctx->blockcipher, > + crypto_skcipher_get_flags(tfm) & > + CRYPTO_TFM_REQ_MASK); > + err = crypto_cipher_setkey(tctx->blockcipher, key, keylen); > + if (err) > + return err; > + > + crypto_skcipher_clear_flags(tctx->xctr, CRYPTO_TFM_REQ_MASK); > + crypto_skcipher_set_flags(tctx->xctr, > + crypto_skcipher_get_flags(tfm) & > + CRYPTO_TFM_REQ_MASK); > + err = crypto_skcipher_setkey(tctx->xctr, key, keylen); > + if (err) > + return err; > + > + memset(tctx->L, 0, sizeof(tctx->L)); > + memset(hbar, 0, sizeof(hbar)); > + tctx->L[0] = 0x01; > + crypto_cipher_encrypt_one(tctx->blockcipher, tctx->L, tctx->L); > + crypto_cipher_encrypt_one(tctx->blockcipher, hbar, hbar); > + > + crypto_shash_clear_flags(tctx->polyval, CRYPTO_TFM_REQ_MASK); > + crypto_shash_set_flags(tctx->polyval, crypto_skcipher_get_flags(tfm) & > + CRYPTO_TFM_REQ_MASK); > + err = crypto_shash_setkey(tctx->polyval, hbar, BLOCKCIPHER_BLOCK_SIZE); > + if (err) > + return err; > + memzero_explicit(hbar, sizeof(hbar)); > + > + shash->tfm = tctx->polyval; > + memset(tweak_length_block, 0, sizeof(tweak_length_block)); > + > + tweak_length_block[0] = cpu_to_le64(TWEAK_SIZE * 8 * 2 + 2); > + err = crypto_shash_init(shash); > + if (err) > + return err; > + err = crypto_shash_update(shash, (u8 *)tweak_length_block, > + POLYVAL_BLOCK_SIZE); > + if (err) > + return err; > + err = crypto_shash_export(shash, exported_length_digests[0]); > + if (err) > + return err; > + > + tweak_length_block[0] = cpu_to_le64(TWEAK_SIZE * 8 * 2 + 3); > + err = crypto_shash_init(shash); > + if (err) > + return err; > + err = crypto_shash_update(shash, (u8 *)tweak_length_block, > + POLYVAL_BLOCK_SIZE); > + if (err) > + return err; > + return crypto_shash_export(shash, exported_length_digests[1]); > +} hctr2_setkey() is getting pretty long. How about splitting the tweak length pre-hashing into a helper function? Also, a comment that explains why the tweak length is being pre-hashed, and why it *can* be pre-hashed, would be helpful. Note that it is only possible because this implementation only supports one tweak length. - Eric _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel