From af5034e8e4a5838fc77e476c1a91822e449d5869 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 30 Dec 2019 21:19:38 -0600 Subject: crypto: remove propagation of CRYPTO_TFM_RES_* flags The CRYPTO_TFM_RES_* flags were apparently meant as a way to make the ->setkey() functions provide more information about errors. But these flags weren't actually being used or tested, and in many cases they weren't being set correctly anyway. So they've now been removed. Also, if someone ever actually needs to start better distinguishing ->setkey() errors (which is somewhat unlikely, as this has been unneeded for a long time), we'd be much better off just defining 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 CRYPTO_TFM_RES_MASK and all the unneeded logic that propagates these flags around. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/adiantum.c | 8 -------- 1 file changed, 8 deletions(-) (limited to 'crypto/adiantum.c') diff --git a/crypto/adiantum.c b/crypto/adiantum.c index aded26092268..30cffb45b88f 100644 --- a/crypto/adiantum.c +++ b/crypto/adiantum.c @@ -135,9 +135,6 @@ static int adiantum_setkey(struct crypto_skcipher *tfm, const u8 *key, crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_REQ_MASK); err = crypto_skcipher_setkey(tctx->streamcipher, key, keylen); - crypto_skcipher_set_flags(tfm, - crypto_skcipher_get_flags(tctx->streamcipher) & - CRYPTO_TFM_RES_MASK); if (err) return err; @@ -167,9 +164,6 @@ static int adiantum_setkey(struct crypto_skcipher *tfm, const u8 *key, CRYPTO_TFM_REQ_MASK); err = crypto_cipher_setkey(tctx->blockcipher, keyp, BLOCKCIPHER_KEY_SIZE); - crypto_skcipher_set_flags(tfm, - crypto_cipher_get_flags(tctx->blockcipher) & - CRYPTO_TFM_RES_MASK); if (err) goto out; keyp += BLOCKCIPHER_KEY_SIZE; @@ -182,8 +176,6 @@ static int adiantum_setkey(struct crypto_skcipher *tfm, const u8 *key, crypto_shash_set_flags(tctx->hash, crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_REQ_MASK); err = crypto_shash_setkey(tctx->hash, keyp, NHPOLY1305_KEY_SIZE); - crypto_skcipher_set_flags(tfm, crypto_shash_get_flags(tctx->hash) & - CRYPTO_TFM_RES_MASK); keyp += NHPOLY1305_KEY_SIZE; WARN_ON(keyp != &data->derived_keys[ARRAY_SIZE(data->derived_keys)]); out: -- cgit v1.2.3 From b9f76dddb1f9f70e008b982381bbc9a67c9b8c66 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 2 Jan 2020 19:58:45 -0800 Subject: crypto: skcipher - pass instance to crypto_grab_skcipher() Initializing a crypto_skcipher_spawn currently requires: 1. Set spawn->base.inst to point to the instance. 2. Call crypto_grab_skcipher(). But there's no reason for these steps to be separate, and in fact this unneeded complication has caused at least one bug, the one fixed by commit 6db43410179b ("crypto: adiantum - initialize crypto_spawn::inst") So just make crypto_grab_skcipher() take the instance as an argument. To keep the function calls from getting too unwieldy due to this extra argument, also introduce a 'mask' variable into the affected places which weren't already using one. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/adiantum.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'crypto/adiantum.c') diff --git a/crypto/adiantum.c b/crypto/adiantum.c index 30cffb45b88f..567071464fab 100644 --- a/crypto/adiantum.c +++ b/crypto/adiantum.c @@ -493,6 +493,7 @@ static bool adiantum_supported_algorithms(struct skcipher_alg *streamcipher_alg, static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) { struct crypto_attr_type *algt; + u32 mask; const char *streamcipher_name; const char *blockcipher_name; const char *nhpoly1305_name; @@ -511,6 +512,8 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) if ((algt->type ^ CRYPTO_ALG_TYPE_SKCIPHER) & algt->mask) return -EINVAL; + mask = crypto_requires_sync(algt->type, algt->mask); + streamcipher_name = crypto_attr_alg_name(tb[1]); if (IS_ERR(streamcipher_name)) return PTR_ERR(streamcipher_name); @@ -531,11 +534,9 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) ictx = skcipher_instance_ctx(inst); /* Stream cipher, e.g. "xchacha12" */ - crypto_set_skcipher_spawn(&ictx->streamcipher_spawn, - skcipher_crypto_instance(inst)); - err = crypto_grab_skcipher(&ictx->streamcipher_spawn, streamcipher_name, - 0, crypto_requires_sync(algt->type, - algt->mask)); + err = crypto_grab_skcipher(&ictx->streamcipher_spawn, + skcipher_crypto_instance(inst), + streamcipher_name, 0, mask); if (err) goto out_free_inst; streamcipher_alg = crypto_spawn_skcipher_alg(&ictx->streamcipher_spawn); -- cgit v1.2.3 From de95c9574108ec304083ed574304ab3b60b4167c Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 2 Jan 2020 19:58:48 -0800 Subject: crypto: algapi - pass instance to crypto_grab_spawn() Currently, crypto_spawn::inst is first used temporarily to pass the instance to crypto_grab_spawn(). Then crypto_init_spawn() overwrites it with crypto_spawn::next, which shares the same union. Finally, crypto_spawn::inst is set again when the instance is registered. Make this less convoluted by just passing the instance as an argument to crypto_grab_spawn() instead. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/adiantum.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'crypto/adiantum.c') diff --git a/crypto/adiantum.c b/crypto/adiantum.c index 567071464fab..76a41881ee8c 100644 --- a/crypto/adiantum.c +++ b/crypto/adiantum.c @@ -542,9 +542,9 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) streamcipher_alg = crypto_spawn_skcipher_alg(&ictx->streamcipher_spawn); /* Block cipher, e.g. "aes" */ - crypto_set_spawn(&ictx->blockcipher_spawn, - skcipher_crypto_instance(inst)); - err = crypto_grab_spawn(&ictx->blockcipher_spawn, blockcipher_name, + err = crypto_grab_spawn(&ictx->blockcipher_spawn, + skcipher_crypto_instance(inst), + blockcipher_name, CRYPTO_ALG_TYPE_CIPHER, CRYPTO_ALG_TYPE_MASK); if (err) goto out_drop_streamcipher; -- cgit v1.2.3 From ba44840747bdd60095881830af0d75f0e5017c99 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 2 Jan 2020 19:58:52 -0800 Subject: crypto: adiantum - use crypto_grab_{cipher,shash} and simplify error paths Make the adiantum template use the new functions crypto_grab_cipher() and crypto_grab_shash() to initialize its cipher and shash spawns. This is needed to make all spawns be initialized in a consistent way. 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/adiantum.c | 85 ++++++++++++++++--------------------------------------- 1 file changed, 25 insertions(+), 60 deletions(-) (limited to 'crypto/adiantum.c') diff --git a/crypto/adiantum.c b/crypto/adiantum.c index 76a41881ee8c..5b8aa14ccb55 100644 --- a/crypto/adiantum.c +++ b/crypto/adiantum.c @@ -39,8 +39,6 @@ #include #include -#include "internal.h" - /* * Size of right-hand part of input data, in bytes; also the size of the block * cipher's block size and the hash function's output. @@ -64,7 +62,7 @@ struct adiantum_instance_ctx { struct crypto_skcipher_spawn streamcipher_spawn; - struct crypto_spawn blockcipher_spawn; + struct crypto_cipher_spawn blockcipher_spawn; struct crypto_shash_spawn hash_spawn; }; @@ -410,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); + blockcipher = crypto_spawn_cipher(&ictx->blockcipher_spawn.base); if (IS_ERR(blockcipher)) { err = PTR_ERR(blockcipher); goto err_free_streamcipher; @@ -461,7 +459,7 @@ static void adiantum_free_instance(struct skcipher_instance *inst) struct adiantum_instance_ctx *ictx = skcipher_instance_ctx(inst); crypto_drop_skcipher(&ictx->streamcipher_spawn); - crypto_drop_spawn(&ictx->blockcipher_spawn); + crypto_drop_cipher(&ictx->blockcipher_spawn); crypto_drop_shash(&ictx->hash_spawn); kfree(inst); } @@ -494,14 +492,11 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) { struct crypto_attr_type *algt; u32 mask; - const char *streamcipher_name; - const char *blockcipher_name; const char *nhpoly1305_name; struct skcipher_instance *inst; struct adiantum_instance_ctx *ictx; struct skcipher_alg *streamcipher_alg; struct crypto_alg *blockcipher_alg; - struct crypto_alg *_hash_alg; struct shash_alg *hash_alg; int err; @@ -514,20 +509,6 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) mask = crypto_requires_sync(algt->type, algt->mask); - streamcipher_name = crypto_attr_alg_name(tb[1]); - if (IS_ERR(streamcipher_name)) - return PTR_ERR(streamcipher_name); - - blockcipher_name = crypto_attr_alg_name(tb[2]); - if (IS_ERR(blockcipher_name)) - return PTR_ERR(blockcipher_name); - - nhpoly1305_name = crypto_attr_alg_name(tb[3]); - if (nhpoly1305_name == ERR_PTR(-ENOENT)) - nhpoly1305_name = "nhpoly1305"; - if (IS_ERR(nhpoly1305_name)) - return PTR_ERR(nhpoly1305_name); - inst = kzalloc(sizeof(*inst) + sizeof(*ictx), GFP_KERNEL); if (!inst) return -ENOMEM; @@ -536,33 +517,29 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) /* Stream cipher, e.g. "xchacha12" */ err = crypto_grab_skcipher(&ictx->streamcipher_spawn, skcipher_crypto_instance(inst), - streamcipher_name, 0, mask); + crypto_attr_alg_name(tb[1]), 0, mask); if (err) - goto out_free_inst; + goto err_free_inst; streamcipher_alg = crypto_spawn_skcipher_alg(&ictx->streamcipher_spawn); /* Block cipher, e.g. "aes" */ - err = crypto_grab_spawn(&ictx->blockcipher_spawn, - skcipher_crypto_instance(inst), - blockcipher_name, - CRYPTO_ALG_TYPE_CIPHER, CRYPTO_ALG_TYPE_MASK); + err = crypto_grab_cipher(&ictx->blockcipher_spawn, + skcipher_crypto_instance(inst), + crypto_attr_alg_name(tb[2]), 0, mask); if (err) - goto out_drop_streamcipher; - blockcipher_alg = ictx->blockcipher_spawn.alg; + goto err_free_inst; + blockcipher_alg = crypto_spawn_cipher_alg(&ictx->blockcipher_spawn); /* NHPoly1305 ε-∆U hash function */ - _hash_alg = crypto_alg_mod_lookup(nhpoly1305_name, - CRYPTO_ALG_TYPE_SHASH, - CRYPTO_ALG_TYPE_MASK); - if (IS_ERR(_hash_alg)) { - err = PTR_ERR(_hash_alg); - goto out_drop_blockcipher; - } - hash_alg = __crypto_shash_alg(_hash_alg); - err = crypto_init_shash_spawn(&ictx->hash_spawn, hash_alg, - skcipher_crypto_instance(inst)); + nhpoly1305_name = crypto_attr_alg_name(tb[3]); + if (nhpoly1305_name == ERR_PTR(-ENOENT)) + nhpoly1305_name = "nhpoly1305"; + err = crypto_grab_shash(&ictx->hash_spawn, + skcipher_crypto_instance(inst), + nhpoly1305_name, 0, mask); if (err) - goto out_put_hash; + goto err_free_inst; + hash_alg = crypto_spawn_shash_alg(&ictx->hash_spawn); /* Check the set of algorithms */ if (!adiantum_supported_algorithms(streamcipher_alg, blockcipher_alg, @@ -571,7 +548,7 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) streamcipher_alg->base.cra_name, blockcipher_alg->cra_name, hash_alg->base.cra_name); err = -EINVAL; - goto out_drop_hash; + goto err_free_inst; } /* Instance fields */ @@ -580,13 +557,13 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME, "adiantum(%s,%s)", streamcipher_alg->base.cra_name, blockcipher_alg->cra_name) >= CRYPTO_MAX_ALG_NAME) - goto out_drop_hash; + goto err_free_inst; if (snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME, "adiantum(%s,%s,%s)", streamcipher_alg->base.cra_driver_name, blockcipher_alg->cra_driver_name, hash_alg->base.cra_driver_name) >= CRYPTO_MAX_ALG_NAME) - goto out_drop_hash; + goto err_free_inst; inst->alg.base.cra_flags = streamcipher_alg->base.cra_flags & CRYPTO_ALG_ASYNC; @@ -616,22 +593,10 @@ static int adiantum_create(struct crypto_template *tmpl, struct rtattr **tb) inst->free = adiantum_free_instance; err = skcipher_register_instance(tmpl, inst); - if (err) - goto out_drop_hash; - - crypto_mod_put(_hash_alg); - return 0; - -out_drop_hash: - crypto_drop_shash(&ictx->hash_spawn); -out_put_hash: - crypto_mod_put(_hash_alg); -out_drop_blockcipher: - crypto_drop_spawn(&ictx->blockcipher_spawn); -out_drop_streamcipher: - crypto_drop_skcipher(&ictx->streamcipher_spawn); -out_free_inst: - kfree(inst); + if (err) { +err_free_inst: + adiantum_free_instance(inst); + } return err; } -- cgit v1.2.3 From d5ed3b65f7012a6592809f7f928f3e3660df8fd9 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 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crypto/adiantum.c') diff --git a/crypto/adiantum.c b/crypto/adiantum.c index 5b8aa14ccb55..4d7a6cac82ed 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; -- cgit v1.2.3 From 1c08a104360f3e18f4ee6346c21cc3923efb952e Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 5 Jan 2020 22:40:46 -0500 Subject: crypto: poly1305 - add new 32 and 64-bit generic versions These two C implementations from Zinc -- a 32x32 one and a 64x64 one, depending on the platform -- come from Andrew Moon's public domain poly1305-donna portable code, modified for usage in the kernel. The precomputation in the 32-bit version and the use of 64x64 multiplies in the 64-bit version make these perform better than the code it replaces. Moon's code is also very widespread and has received many eyeballs of scrutiny. There's a bit of interference between the x86 implementation, which relies on internal details of the old scalar implementation. In the next commit, the x86 implementation will be replaced with a faster one that doesn't rely on this, so none of this matters much. But for now, to keep this passing the tests, we inline the bits of the old implementation that the x86 implementation relied on. Also, since we now support a slightly larger key space, via the union, some offsets had to be fixed up. Nonce calculation was folded in with the emit function, to take advantage of 64x64 arithmetic. However, Adiantum appeared to rely on no nonce handling in emit, so this path was conditionalized. We also introduced a new struct, poly1305_core_key, to represent the precise amount of space that particular implementation uses. Testing with kbench9000, depending on the CPU, the update function for the 32x32 version has been improved by 4%-7%, and for the 64x64 by 19%-30%. The 32x32 gains are small, but I think there's great value in having a parallel implementation to the 64x64 one so that the two can be compared side-by-side as nice stand-alone units. Signed-off-by: Jason A. Donenfeld Signed-off-by: Herbert Xu --- crypto/adiantum.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crypto/adiantum.c') diff --git a/crypto/adiantum.c b/crypto/adiantum.c index 4d7a6cac82ed..53d5e705a425 100644 --- a/crypto/adiantum.c +++ b/crypto/adiantum.c @@ -70,7 +70,7 @@ struct adiantum_tfm_ctx { struct crypto_skcipher *streamcipher; struct crypto_cipher *blockcipher; struct crypto_shash *hash; - struct poly1305_key header_hash_key; + struct poly1305_core_key header_hash_key; }; struct adiantum_request_ctx { @@ -239,7 +239,7 @@ static void adiantum_hash_header(struct skcipher_request *req) poly1305_core_blocks(&state, &tctx->header_hash_key, req->iv, TWEAK_SIZE / POLY1305_BLOCK_SIZE, 1); - poly1305_core_emit(&state, &rctx->header_hash); + poly1305_core_emit(&state, NULL, &rctx->header_hash); } /* Hash the left-hand part (the "bulk") of the message using NHPoly1305 */ -- cgit v1.2.3