On Tue, Jun 11, 2019 at 03:47:48PM +0200, Ard Biesheuvel wrote:
> There are no remaining users of the cipher implementation, and there
> are no meaningful ways in which the arc4 cipher can be combined with
> templates other than ECB (and the way we do provide that combination
> is highly dubious to begin with).
>
> So let's drop the arc4 cipher altogether, and only keep the ecb(arc4)
> skcipher, which is used in various places in the kernel.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> ---
> crypto/arc4.c | 46 ++------------------
> 1 file changed, 4 insertions(+), 42 deletions(-)
>
> diff --git a/crypto/arc4.c b/crypto/arc4.c
> index 6974dba1b7b9..79a51e9f90ae 100644
> --- a/crypto/arc4.c
> +++ b/crypto/arc4.c
> @@ -13,23 +13,12 @@
> #include <linux/init.h>
> #include <linux/module.h>
>
> -static int arc4_set_key(struct crypto_tfm *tfm, const u8 *in_key,
> - unsigned int key_len)
> -{
> - struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
> -
> - return arc4_setkey(ctx, in_key, key_len);
> -}
> -
> static int arc4_set_key_skcipher(struct crypto_skcipher *tfm, const u8 *in_key,
> unsigned int key_len)
> {
> - return arc4_set_key(&tfm->base, in_key, key_len);
> -}
> + struct arc4_ctx *ctx = crypto_tfm_ctx(&tfm->base);
>
> -static void arc4_crypt_one(struct crypto_tfm *tfm, u8 *out, const u8 *in)
> -{
> - arc4_crypt(crypto_tfm_ctx(tfm), out, in, 1);
> + return arc4_setkey(ctx, in_key, key_len);
> }
>
> static int ecb_arc4_crypt(struct skcipher_request *req)
Can you clean up the naming here?
arc4_set_key_skcipher() => crypto_arc4_setkey()
ecb_arc4_crypt() => crypto_arc4_crypt()
The current names were intended to distinguish the "skcipher" functions from the
"cipher" functions, but that will no longer be needed.
Also, crypto_arc4_setkey() should use crypto_skcipher_ctx() rather than
crypto_tfm_ctx(), now that it only handles "skcipher".
> @@ -50,23 +39,6 @@ static int ecb_arc4_crypt(struct skcipher_request *req)
> return err;
> }
>
> -static struct crypto_alg arc4_cipher = {
> - .cra_name = "arc4",
> - .cra_flags = CRYPTO_ALG_TYPE_CIPHER,
> - .cra_blocksize = ARC4_BLOCK_SIZE,
> - .cra_ctxsize = sizeof(struct arc4_ctx),
> - .cra_module = THIS_MODULE,
> - .cra_u = {
> - .cipher = {
> - .cia_min_keysize = ARC4_MIN_KEY_SIZE,
> - .cia_max_keysize = ARC4_MAX_KEY_SIZE,
> - .cia_setkey = arc4_set_key,
> - .cia_encrypt = arc4_crypt_one,
> - .cia_decrypt = arc4_crypt_one,
> - },
> - },
> -};
> -
> static struct skcipher_alg arc4_skcipher = {
Similarly this could be renamed from arc4_skcipher to arc4_alg, now that the
skcipher algorithm doesn't need to be distinguished from the cipher algorithm.
> .base.cra_name = "ecb(arc4)",
Given the confusion this name causes, can you leave a comment? Like:
/*
* For legacy reasons, this is named "ecb(arc4)", not "arc4".
* Nevertheless it's actually a stream cipher, not a block cipher.
*/
.base.cra_name = "ecb(arc4)",
Also, due to removing the cipher algorithm, we need the following testmgr change
so that the comparison self-tests consider the generic implementation of this
algorithm to be itself rather than "ecb(arc4-generic)":
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 658a7eeebab28..5d3eb8577605f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4125,6 +4125,7 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}, {
.alg = "ecb(arc4)",
+ .generic_driver = "ecb(arc4)-generic",
.test = alg_test_skcipher,
.suite = {
.cipher = __VECS(arc4_tv_template)
- Eric
