From 0630278da01b08b20d58754d6bdf9d148cf113cc Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 30 Dec 2019 21:19:36 -0600 Subject: crypto: remove CRYPTO_TFM_RES_BAD_KEY_LEN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CRYPTO_TFM_RES_BAD_KEY_LEN flag was apparently meant as a way to make the ->setkey() functions provide more information about errors. However, no one actually checks for this flag, which makes it pointless. Also, many algorithms fail to set this flag when given a bad length key. Reviewing just the generic implementations, this is the case for aes-fixed-time, cbcmac, echainiv, nhpoly1305, pcrypt, rfc3686, rfc4309, rfc7539, rfc7539esp, salsa20, seqiv, and xcbc. But there are probably many more in arch/*/crypto/ and drivers/crypto/. Some algorithms can even set this flag when the key is the correct length. For example, authenc and authencesn set it when the key payload is malformed in any way (not just a bad length), the atmel-sha and ccree drivers can set it if a memory allocation fails, and the chelsio driver sets it for bad auth tag lengths, not just bad key lengths. So even if someone actually wanted to start checking this flag (which seems unlikely, since it's been unused for a long time), there would be a lot of work needed to get it working correctly. But it would probably be much better to go back to the drawing board and just define different return values, like -EINVAL if the key is invalid for the algorithm vs. -EKEYREJECTED if the key was rejected by a policy like "no weak keys". That would be much simpler, less error-prone, and easier to test. So just remove this flag. Signed-off-by: Eric Biggers Reviewed-by: Horia Geantă Signed-off-by: Herbert Xu --- crypto/vmac.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'crypto/vmac.c') diff --git a/crypto/vmac.c b/crypto/vmac.c index f50a8506..0bbb34dc 100644 --- a/crypto/vmac.c +++ b/crypto/vmac.c @@ -435,10 +435,8 @@ static int vmac_setkey(struct crypto_shash *tfm, unsigned int i; int err; - if (keylen != VMAC_KEY_LEN) { - crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN); + if (keylen != VMAC_KEY_LEN) return -EINVAL; - } err = crypto_cipher_setkey(tctx->cipher, key, keylen); if (err) -- cgit v1.2.3 From 7682d720c350d7aad4543379da6d90d164f6c0c7 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 2 Jan 2020 19:59:03 -0800 Subject: crypto: vmac - use crypto_grab_cipher() and simplify error paths Make the vmac64 template use the new function crypto_grab_cipher() to initialize its cipher spawn. This is needed to make all spawns be initialized in a consistent way. This required making vmac_create() allocate the instance directly rather than use shash_alloc_instance(). Also simplify the error handling by taking advantage of crypto_drop_*() now accepting (as a no-op) spawns that haven't been initialized yet, and by taking advantage of crypto_grab_*() now handling ERR_PTR() names. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/vmac.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) (limited to 'crypto/vmac.c') diff --git a/crypto/vmac.c b/crypto/vmac.c index 0bbb34dc..9b000aaa 100644 --- a/crypto/vmac.c +++ b/crypto/vmac.c @@ -618,6 +618,7 @@ static void vmac_exit_tfm(struct crypto_tfm *tfm) static int vmac_create(struct crypto_template *tmpl, struct rtattr **tb) { struct shash_instance *inst; + struct crypto_cipher_spawn *spawn; struct crypto_alg *alg; int err; @@ -625,25 +626,24 @@ static int vmac_create(struct crypto_template *tmpl, struct rtattr **tb) if (err) return err; - alg = crypto_get_attr_alg(tb, CRYPTO_ALG_TYPE_CIPHER, - CRYPTO_ALG_TYPE_MASK); - if (IS_ERR(alg)) - return PTR_ERR(alg); + inst = kzalloc(sizeof(*inst) + sizeof(*spawn), GFP_KERNEL); + if (!inst) + return -ENOMEM; + spawn = shash_instance_ctx(inst); + + err = crypto_grab_cipher(spawn, shash_crypto_instance(inst), + crypto_attr_alg_name(tb[1]), 0, 0); + if (err) + goto err_free_inst; + alg = crypto_spawn_cipher_alg(spawn); err = -EINVAL; if (alg->cra_blocksize != VMAC_NONCEBYTES) - goto out_put_alg; + goto err_free_inst; - inst = shash_alloc_instance(tmpl->name, alg); - err = PTR_ERR(inst); - if (IS_ERR(inst)) - goto out_put_alg; - - err = crypto_init_spawn(shash_instance_ctx(inst), alg, - shash_crypto_instance(inst), - CRYPTO_ALG_TYPE_MASK); + err = crypto_inst_setname(shash_crypto_instance(inst), tmpl->name, alg); if (err) - goto out_free_inst; + goto err_free_inst; inst->alg.base.cra_priority = alg->cra_priority; inst->alg.base.cra_blocksize = alg->cra_blocksize; @@ -662,12 +662,9 @@ static int vmac_create(struct crypto_template *tmpl, struct rtattr **tb) err = shash_register_instance(tmpl, inst); if (err) { -out_free_inst: +err_free_inst: shash_free_instance(shash_crypto_instance(inst)); } - -out_put_alg: - crypto_mod_put(alg); return err; } -- cgit v1.2.3 From fa41653e64a18f116a0e62c38709f2e953c3b76a Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 2 Jan 2020 19:59:05 -0800 Subject: crypto: cipher - make crypto_spawn_cipher() take a crypto_cipher_spawn Now that all users of single-block cipher spawns have been converted to use 'struct crypto_cipher_spawn' rather than the less specifically typed 'struct crypto_spawn', make crypto_spawn_cipher() take a pointer to a 'struct crypto_cipher_spawn' rather than a 'struct crypto_spawn'. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/adiantum.c | 2 +- crypto/ccm.c | 2 +- crypto/cmac.c | 2 +- crypto/skcipher.c | 2 +- crypto/vmac.c | 2 +- crypto/xcbc.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) (limited to 'crypto/vmac.c') diff --git a/crypto/adiantum.c b/crypto/adiantum.c index 5b8aa14c..4d7a6cac 100644 --- a/crypto/adiantum.c +++ b/crypto/adiantum.c @@ -408,7 +408,7 @@ static int adiantum_init_tfm(struct crypto_skcipher *tfm) if (IS_ERR(streamcipher)) return PTR_ERR(streamcipher); - blockcipher = crypto_spawn_cipher(&ictx->blockcipher_spawn.base); + blockcipher = crypto_spawn_cipher(&ictx->blockcipher_spawn); if (IS_ERR(blockcipher)) { err = PTR_ERR(blockcipher); goto err_free_streamcipher; diff --git a/crypto/ccm.c b/crypto/ccm.c index 411c3973..f4abaefd 100644 --- a/crypto/ccm.c +++ b/crypto/ccm.c @@ -866,7 +866,7 @@ static int cbcmac_init_tfm(struct crypto_tfm *tfm) { struct crypto_cipher *cipher; struct crypto_instance *inst = (void *)tfm->__crt_alg; - struct crypto_spawn *spawn = crypto_instance_ctx(inst); + struct crypto_cipher_spawn *spawn = crypto_instance_ctx(inst); struct cbcmac_tfm_ctx *ctx = crypto_tfm_ctx(tfm); cipher = crypto_spawn_cipher(spawn); diff --git a/crypto/cmac.c b/crypto/cmac.c index c6bf78b5..58dc6444 100644 --- a/crypto/cmac.c +++ b/crypto/cmac.c @@ -201,7 +201,7 @@ static int cmac_init_tfm(struct crypto_tfm *tfm) { struct crypto_cipher *cipher; struct crypto_instance *inst = (void *)tfm->__crt_alg; - struct crypto_spawn *spawn = crypto_instance_ctx(inst); + struct crypto_cipher_spawn *spawn = crypto_instance_ctx(inst); struct cmac_tfm_ctx *ctx = crypto_tfm_ctx(tfm); cipher = crypto_spawn_cipher(spawn); diff --git a/crypto/skcipher.c b/crypto/skcipher.c index 950ff143..42add1e0 100644 --- a/crypto/skcipher.c +++ b/crypto/skcipher.c @@ -887,7 +887,7 @@ static int skcipher_setkey_simple(struct crypto_skcipher *tfm, const u8 *key, static int skcipher_init_tfm_simple(struct crypto_skcipher *tfm) { struct skcipher_instance *inst = skcipher_alg_instance(tfm); - struct crypto_spawn *spawn = skcipher_instance_ctx(inst); + struct crypto_cipher_spawn *spawn = skcipher_instance_ctx(inst); struct skcipher_ctx_simple *ctx = crypto_skcipher_ctx(tfm); struct crypto_cipher *cipher; diff --git a/crypto/vmac.c b/crypto/vmac.c index 9b000aaa..28358a6a 100644 --- a/crypto/vmac.c +++ b/crypto/vmac.c @@ -596,7 +596,7 @@ static int vmac_final(struct shash_desc *desc, u8 *out) static int vmac_init_tfm(struct crypto_tfm *tfm) { struct crypto_instance *inst = crypto_tfm_alg_instance(tfm); - struct crypto_spawn *spawn = crypto_instance_ctx(inst); + struct crypto_cipher_spawn *spawn = crypto_instance_ctx(inst); struct vmac_tfm_ctx *tctx = crypto_tfm_ctx(tfm); struct crypto_cipher *cipher; diff --git a/crypto/xcbc.c b/crypto/xcbc.c index 9b97fa51..9265e00e 100644 --- a/crypto/xcbc.c +++ b/crypto/xcbc.c @@ -167,7 +167,7 @@ static int xcbc_init_tfm(struct crypto_tfm *tfm) { struct crypto_cipher *cipher; struct crypto_instance *inst = (void *)tfm->__crt_alg; - struct crypto_spawn *spawn = crypto_instance_ctx(inst); + struct crypto_cipher_spawn *spawn = crypto_instance_ctx(inst); struct xcbc_tfm_ctx *ctx = crypto_tfm_ctx(tfm); cipher = crypto_spawn_cipher(spawn); -- cgit v1.2.3 From 5356607e443a5cb3e129fb1f0947bb400f95a64e Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 2 Jan 2020 20:04:38 -0800 Subject: crypto: shash - convert shash_free_instance() to new style Convert shash_free_instance() and its users to the new way of freeing instances, where a ->free() method is installed to the instance struct itself. This replaces the weakly-typed method crypto_template::free(). This will allow removing support for the old way of freeing instances. Also give shash_free_instance() a more descriptive name to reflect that it's only for instances with a single spawn, not for any instance. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/ccm.c | 5 +++-- crypto/cmac.c | 5 +++-- crypto/hmac.c | 5 +++-- crypto/shash.c | 8 ++++---- crypto/vmac.c | 5 +++-- crypto/xcbc.c | 5 +++-- 6 files changed, 19 insertions(+), 14 deletions(-) (limited to 'crypto/vmac.c') diff --git a/crypto/ccm.c b/crypto/ccm.c index f4abaefd..241ecdc5 100644 --- a/crypto/ccm.c +++ b/crypto/ccm.c @@ -927,10 +927,12 @@ static int cbcmac_create(struct crypto_template *tmpl, struct rtattr **tb) inst->alg.final = crypto_cbcmac_digest_final; inst->alg.setkey = crypto_cbcmac_digest_setkey; + inst->free = shash_free_singlespawn_instance; + err = shash_register_instance(tmpl, inst); if (err) { err_free_inst: - shash_free_instance(shash_crypto_instance(inst)); + shash_free_singlespawn_instance(inst); } return err; } @@ -939,7 +941,6 @@ static struct crypto_template crypto_ccm_tmpls[] = { { .name = "cbcmac", .create = cbcmac_create, - .free = shash_free_instance, .module = THIS_MODULE, }, { .name = "ccm_base", diff --git a/crypto/cmac.c b/crypto/cmac.c index 58dc6444..143a6544 100644 --- a/crypto/cmac.c +++ b/crypto/cmac.c @@ -280,10 +280,12 @@ static int cmac_create(struct crypto_template *tmpl, struct rtattr **tb) inst->alg.final = crypto_cmac_digest_final; inst->alg.setkey = crypto_cmac_digest_setkey; + inst->free = shash_free_singlespawn_instance; + err = shash_register_instance(tmpl, inst); if (err) { err_free_inst: - shash_free_instance(shash_crypto_instance(inst)); + shash_free_singlespawn_instance(inst); } return err; } @@ -291,7 +293,6 @@ err_free_inst: static struct crypto_template crypto_cmac_tmpl = { .name = "cmac", .create = cmac_create, - .free = shash_free_instance, .module = THIS_MODULE, }; diff --git a/crypto/hmac.c b/crypto/hmac.c index 0a42b707..e38bfb94 100644 --- a/crypto/hmac.c +++ b/crypto/hmac.c @@ -224,10 +224,12 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb) inst->alg.init_tfm = hmac_init_tfm; inst->alg.exit_tfm = hmac_exit_tfm; + inst->free = shash_free_singlespawn_instance; + err = shash_register_instance(tmpl, inst); if (err) { err_free_inst: - shash_free_instance(shash_crypto_instance(inst)); + shash_free_singlespawn_instance(inst); } return err; } @@ -235,7 +237,6 @@ err_free_inst: static struct crypto_template hmac_tmpl = { .name = "hmac", .create = hmac_create, - .free = shash_free_instance, .module = THIS_MODULE, }; diff --git a/crypto/shash.c b/crypto/shash.c index 2f6adb49..e05e75b0 100644 --- a/crypto/shash.c +++ b/crypto/shash.c @@ -590,12 +590,12 @@ int shash_register_instance(struct crypto_template *tmpl, } EXPORT_SYMBOL_GPL(shash_register_instance); -void shash_free_instance(struct crypto_instance *inst) +void shash_free_singlespawn_instance(struct shash_instance *inst) { - crypto_drop_spawn(crypto_instance_ctx(inst)); - kfree(shash_instance(inst)); + crypto_drop_spawn(shash_instance_ctx(inst)); + kfree(inst); } -EXPORT_SYMBOL_GPL(shash_free_instance); +EXPORT_SYMBOL_GPL(shash_free_singlespawn_instance); MODULE_LICENSE("GPL"); MODULE_DESCRIPTION("Synchronous cryptographic hash type"); diff --git a/crypto/vmac.c b/crypto/vmac.c index 28358a6a..2d906830 100644 --- a/crypto/vmac.c +++ b/crypto/vmac.c @@ -660,10 +660,12 @@ static int vmac_create(struct crypto_template *tmpl, struct rtattr **tb) inst->alg.final = vmac_final; inst->alg.setkey = vmac_setkey; + inst->free = shash_free_singlespawn_instance; + err = shash_register_instance(tmpl, inst); if (err) { err_free_inst: - shash_free_instance(shash_crypto_instance(inst)); + shash_free_singlespawn_instance(inst); } return err; } @@ -671,7 +673,6 @@ err_free_inst: static struct crypto_template vmac64_tmpl = { .name = "vmac64", .create = vmac_create, - .free = shash_free_instance, .module = THIS_MODULE, }; diff --git a/crypto/xcbc.c b/crypto/xcbc.c index 9265e00e..598ec88a 100644 --- a/crypto/xcbc.c +++ b/crypto/xcbc.c @@ -239,10 +239,12 @@ static int xcbc_create(struct crypto_template *tmpl, struct rtattr **tb) inst->alg.final = crypto_xcbc_digest_final; inst->alg.setkey = crypto_xcbc_digest_setkey; + inst->free = shash_free_singlespawn_instance; + err = shash_register_instance(tmpl, inst); if (err) { err_free_inst: - shash_free_instance(shash_crypto_instance(inst)); + shash_free_singlespawn_instance(inst); } return err; } @@ -250,7 +252,6 @@ err_free_inst: static struct crypto_template crypto_xcbc_tmpl = { .name = "xcbc", .create = xcbc_create, - .free = shash_free_instance, .module = THIS_MODULE, }; -- cgit v1.2.3