Re: [PATCH 1/4] crypto: add eboiv as a crypto API template
From: Eric Biggers <ebiggers@kernel.org>
Date: 2020-10-26 18:24:56
Also in:
dm-devel, linux-crypto, lkml
On Mon, Oct 26, 2020 at 03:04:44PM +0200, Gilad Ben-Yossef wrote:
quoted hunk ↗ jump to hunk
diff --git a/crypto/eboiv.c b/crypto/eboiv.c new file mode 100644 index 000000000000..467833e89139 --- /dev/null +++ b/crypto/eboiv.c@@ -0,0 +1,295 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * EBOIV skcipher template for block encryption + * + * This template encapsulates the Encrypted byte-offset IV generation algorithm + * used in Bitlocker in CBC mode by dm-crypt, which converts the initial vector + * for the skcipher used for block encryption, by encrypting it using the same + * skcipher key as encryption key. Usually, the input IV is a 64-bit sector + * offset (the byte offset of the start of the sector) in LE representation + * zero-padded to the size of the IV, but this * is not assumed by this driver. + * + * The typical use of this template is to instantiate the skcipher + * 'eboiv(cbc(aes))', which is the only instantiation used by + * dm-crypt for supporting BitLocker AES-CBC mode as specified in + * https://www.jedec.org/sites/default/files/docs/JESD223C.pdf + * + * Copyright (C) 2020 ARM Limited or its affiliates. + * Written by Gilad Ben-Yossef <gilad@benyossef.com> + * + * Heavily based on: + * + * ESSIV skcipher and aead template for block encryption + * Copyright (c) 2019 Linaro, Ltd. <ard.biesheuvel@linaro.org> + * + * and + * + * dm-crypt eboiv code + * by Milan Broz <gmazyland@gmail.com> and Ard Biesheuvel <ardb@kernel.org> + * + */ + +#include <crypto/internal/skcipher.h> +#include <crypto/scatterwalk.h> +#include <linux/module.h> + +#include "internal.h"
internal.h shouldn't be included here.
+
+struct eboiv_instance_ctx {
+ struct crypto_skcipher_spawn skcipher_spawn;
+ char eboiv_cipher_name[CRYPTO_MAX_ALG_NAME];
+};The 'eboiv_cipher_name' field isn't used.
+static void eboiv_skcipher_iv_done(struct crypto_async_request *areq, int err)
+{
+ struct eboiv_req_ctx *req_ctx = areq->data;
+ struct skcipher_request *req = req_ctx->req;
+ int ret;
+
+ if (!err) {
+
+ ret = eboiv_skcipher_do_crypt(req, req_ctx->enc);
+
+ if ((ret == -EINPROGRESS) || (ret == -EBUSY))
+ return;
+ }
+
+ skcipher_request_complete(req, err);
+}This looks wrong, because if eboiv_skcipher_do_crypt() fails, skcipher_request_complete() will be called with err=0.
+static int eboiv_create(struct crypto_template *tmpl, struct rtattr **tb)
+{
+ struct crypto_attr_type *algt;
+ const char *inner_cipher_name;
+ struct skcipher_instance *skcipher_inst = NULL;
+ struct crypto_instance *inst;
+ struct crypto_alg *base, *block_base;
+ struct eboiv_instance_ctx *ictx;
+ struct skcipher_alg *skcipher_alg = NULL;
+ int ivsize;
+ u32 mask;
+ int err;
+
+ algt = crypto_get_attr_type(tb);
+ if (IS_ERR(algt))
+ return PTR_ERR(algt);Need to check that the algorithm is being instantiated as skcipher. crypto_check_attr_type() should be used.
+ + inner_cipher_name = crypto_attr_alg_name(tb[1]); + if (IS_ERR(inner_cipher_name)) + return PTR_ERR(inner_cipher_name);
The result of crypto_attr_alg_name() can be passed directly to crypto_grab_skcipher().
+ mask = crypto_algt_inherited_mask(algt); + + skcipher_inst = kzalloc(sizeof(*skcipher_inst) + sizeof(*ictx), GFP_KERNEL); + if (!skcipher_inst) + return -ENOMEM; + + inst = skcipher_crypto_instance(skcipher_inst); + base = &skcipher_inst->alg.base; + ictx = crypto_instance_ctx(inst); + + /* Symmetric cipher, e.g., "cbc(aes)" */ + err = crypto_grab_skcipher(&ictx->skcipher_spawn, inst, inner_cipher_name, 0, mask); + if (err) + goto out_free_inst; + + skcipher_alg = crypto_spawn_skcipher_alg(&ictx->skcipher_spawn); + block_base = &skcipher_alg->base; + ivsize = crypto_skcipher_alg_ivsize(skcipher_alg); + + if (ivsize != block_base->cra_blocksize) + goto out_drop_skcipher;
Shouldn't it be verified that the underlying algorithm is actually cbc?
+ skcipher_inst->alg.chunksize = crypto_skcipher_alg_chunksize(skcipher_alg); + skcipher_inst->alg.walksize = crypto_skcipher_alg_walksize(skcipher_alg);
Setting these isn't necessary.
+ + skcipher_inst->free = eboiv_skcipher_free_instance; + + err = skcipher_register_instance(tmpl, skcipher_inst); + + if (err) + goto out_drop_skcipher; + + return 0; + +out_drop_skcipher: + crypto_drop_skcipher(&ictx->skcipher_spawn); +out_free_inst: + kfree(skcipher_inst); + return err; +}
eboiv_skcipher_free_instance() can be called on the error path.
+/* eboiv(cipher_name) */
+static struct crypto_template eboiv_tmpl = {
+ .name = "eboiv",
+ .create = eboiv_create,
+ .module = THIS_MODULE,
+};"cipher_name" => "cbc_cipher_name", since it wraps something like "cbc(aes)", not "aes". - Eric