Re: [PATCH] crypto: caam - Remove broken arc4 support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2 Jul 2020 at 06:36, Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
>
> The arc4 algorithm requires storing state in the request context
> in order to allow more than one encrypt/decrypt operation.  As this
> driver does not seem to do that, it means that using it for more
> than one operation is broken.
>
> Fixes: eaed71a44ad9 ("crypto: caam - add ecb(*) support")
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
>

Acked-by: Ard Biesheuvel <ardb@xxxxxxxxxx>

All internal users of ecb(arc4) use sync skciphers, so this should
only affect user space.

I do wonder if the others are doing any better - n2 and bcm iproc also
appear to keep the state in the TFM object, while I'd expect the
setkey() to be a simple memcpy(), and the initial state derivation to
be part of the encrypt flow, right?

Maybe we should add a test for this to tcrypt, i.e., do setkey() once
and do two encryptions of the same input, and check whether we get
back the original data.


> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index b2f9882bc010f..797bff9b93182 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
> @@ -810,12 +810,6 @@ static int ctr_skcipher_setkey(struct crypto_skcipher *skcipher,
>         return skcipher_setkey(skcipher, key, keylen, ctx1_iv_off);
>  }
>
> -static int arc4_skcipher_setkey(struct crypto_skcipher *skcipher,
> -                               const u8 *key, unsigned int keylen)
> -{
> -       return skcipher_setkey(skcipher, key, keylen, 0);
> -}
> -
>  static int des_skcipher_setkey(struct crypto_skcipher *skcipher,
>                                const u8 *key, unsigned int keylen)
>  {
> @@ -1967,21 +1961,6 @@ static struct caam_skcipher_alg driver_algs[] = {
>                 },
>                 .caam.class1_alg_type = OP_ALG_ALGSEL_3DES | OP_ALG_AAI_ECB,
>         },
> -       {
> -               .skcipher = {
> -                       .base = {
> -                               .cra_name = "ecb(arc4)",
> -                               .cra_driver_name = "ecb-arc4-caam",
> -                               .cra_blocksize = ARC4_BLOCK_SIZE,
> -                       },
> -                       .setkey = arc4_skcipher_setkey,
> -                       .encrypt = skcipher_encrypt,
> -                       .decrypt = skcipher_decrypt,
> -                       .min_keysize = ARC4_MIN_KEY_SIZE,
> -                       .max_keysize = ARC4_MAX_KEY_SIZE,
> -               },
> -               .caam.class1_alg_type = OP_ALG_ALGSEL_ARC4 | OP_ALG_AAI_ECB,
> -       },
>  };
>
>  static struct caam_aead_alg driver_aeads[] = {
> @@ -3457,7 +3436,6 @@ int caam_algapi_init(struct device *ctrldev)
>         struct caam_drv_private *priv = dev_get_drvdata(ctrldev);
>         int i = 0, err = 0;
>         u32 aes_vid, aes_inst, des_inst, md_vid, md_inst, ccha_inst, ptha_inst;
> -       u32 arc4_inst;
>         unsigned int md_limit = SHA512_DIGEST_SIZE;
>         bool registered = false, gcm_support;
>
> @@ -3477,8 +3455,6 @@ int caam_algapi_init(struct device *ctrldev)
>                            CHA_ID_LS_DES_SHIFT;
>                 aes_inst = cha_inst & CHA_ID_LS_AES_MASK;
>                 md_inst = (cha_inst & CHA_ID_LS_MD_MASK) >> CHA_ID_LS_MD_SHIFT;
> -               arc4_inst = (cha_inst & CHA_ID_LS_ARC4_MASK) >>
> -                           CHA_ID_LS_ARC4_SHIFT;
>                 ccha_inst = 0;
>                 ptha_inst = 0;
>
> @@ -3499,7 +3475,6 @@ int caam_algapi_init(struct device *ctrldev)
>                 md_inst = mdha & CHA_VER_NUM_MASK;
>                 ccha_inst = rd_reg32(&priv->ctrl->vreg.ccha) & CHA_VER_NUM_MASK;
>                 ptha_inst = rd_reg32(&priv->ctrl->vreg.ptha) & CHA_VER_NUM_MASK;
> -               arc4_inst = rd_reg32(&priv->ctrl->vreg.afha) & CHA_VER_NUM_MASK;
>
>                 gcm_support = aesa & CHA_VER_MISC_AES_GCM;
>         }
> @@ -3522,10 +3497,6 @@ int caam_algapi_init(struct device *ctrldev)
>                 if (!aes_inst && (alg_sel == OP_ALG_ALGSEL_AES))
>                                 continue;
>
> -               /* Skip ARC4 algorithms if not supported by device */
> -               if (!arc4_inst && alg_sel == OP_ALG_ALGSEL_ARC4)
> -                       continue;
> -
>                 /*
>                  * Check support for AES modes not available
>                  * on LP devices.
> diff --git a/drivers/crypto/caam/compat.h b/drivers/crypto/caam/compat.h
> index 60e2a54c19f11..c3c22a8de4c00 100644
> --- a/drivers/crypto/caam/compat.h
> +++ b/drivers/crypto/caam/compat.h
> @@ -43,7 +43,6 @@
>  #include <crypto/akcipher.h>
>  #include <crypto/scatterwalk.h>
>  #include <crypto/skcipher.h>
> -#include <crypto/arc4.h>
>  #include <crypto/internal/skcipher.h>
>  #include <crypto/internal/hash.h>
>  #include <crypto/internal/rsa.h>
> --
> Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux