| Message ID | 20200702043648.GA21823@gondor.apana.org.au (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: caam - Remove broken arc4 support | expand |
On Thu, 2 Jul 2020 at 06:36, Herbert Xu <herbert@gondor.apana.org.au> 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@gondor.apana.org.au> > Acked-by: Ard Biesheuvel <ardb@kernel.org> 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@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, 2 Jul 2020 at 09:27, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Thu, 2 Jul 2020 at 06:36, Herbert Xu <herbert@gondor.apana.org.au> 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@gondor.apana.org.au> > > > > Acked-by: Ard Biesheuvel <ardb@kernel.org> > > 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. > Actually, it seems the generic ecb(arc4) is broken as well in this regard.
On Thu, Jul 02, 2020 at 09:27:33AM +0200, Ard Biesheuvel wrote: > > 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? bcm at least appears to be doing the right thing, but of course without the hardware or a reference manual I can't be sure. David can probably tell us whether n2 is capable of updating the state at ent->enc_key_addr after each arc4 operation. > 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. Yes we could certainly add an arc4-only test. Alternatively we could kill the only in-kernel user (auth_gss) and then try to remove the arc4 interface completely (and hope that nobody complains :) Thanks,
On Thu, 2 Jul 2020 at 09:32, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Thu, 2 Jul 2020 at 09:27, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Thu, 2 Jul 2020 at 06:36, Herbert Xu <herbert@gondor.apana.org.au> 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@gondor.apana.org.au> > > > > > > > Acked-by: Ard Biesheuvel <ardb@kernel.org> > > > > 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. > > > > Actually, it seems the generic ecb(arc4) is broken as well in this regard. This may be strictly true, but perhaps reusing the key is not such a great idea to begin with, given the lack of an IV, so the fact that skcipher::setkey() operates on the TFM and not the request simply does not match the ARC4 model. I suppose you are looking into this for chaining algif_skipcher requests, right? So in that case, the ARC4 state should really be treated as an IV, which is owned by the caller, and not stored in either the TFM or the skcipher request object.
On Thu, Jul 02, 2020 at 09:32:32AM +0200, Ard Biesheuvel wrote: > > Actually, it seems the generic ecb(arc4) is broken as well in this regard. Not in the same way. The generic arc4 stores the state in the tfm so it works until you have two requests that share the same tfm. Since we only have two users right now we can fix this by moving the state into the request and I'm working on it. Cheers,
On Thu, Jul 02, 2020 at 09:40:42AM +0200, Ard Biesheuvel wrote: > > I suppose you are looking into this for chaining algif_skipcher > requests, right? So in that case, the ARC4 state should really be > treated as an IV, which is owned by the caller, and not stored in > either the TFM or the skcipher request object. Yes I have considered this approach previously but it's just too messy. What I'm trying to do now is to allow the state to be stored in the request object. When combined with the proposed REQ_MORE flag, this should be sufficient. It evens works on XTS. Cheers,
On Thu, 2 Jul 2020 at 09:45, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Thu, Jul 02, 2020 at 09:40:42AM +0200, Ard Biesheuvel wrote: > > > > I suppose you are looking into this for chaining algif_skipcher > > requests, right? So in that case, the ARC4 state should really be > > treated as an IV, which is owned by the caller, and not stored in > > either the TFM or the skcipher request object. > > Yes I have considered this approach previously but it's just too > messy. What I'm trying to do now is to allow the state to be stored > in the request object. When combined with the proposed REQ_MORE > flag, this should be sufficient. It evens works on XTS. > But that requires the caller to reuse the same skcipher request object when doing chaining, right? Currently, we have no such requirement, and it would mean that the request object's context struct should be aligned between different implementations, e.g., when a driver ends up invoking a fallback for the last N bytes of a chained request. I'll wait for the code to be posted (please put me on cc), but my suspicion is that carrying opaque state like that is going to bite us down the road.
On Thu, Jul 02, 2020 at 09:51:29AM +0200, Ard Biesheuvel wrote: > > I'll wait for the code to be posted (please put me on cc), but my Sure I will. > suspicion is that carrying opaque state like that is going to bite us > down the road. Well it's only going to be arc4 at first, where it's definitely an improvement over modifying the tfm context in encrypt/decrypt. For XTS I haven't decided whether to go this way or not. If it does work out though we could even extend it to AEAD. Cheers,
On Thu, Jul 02, 2020 at 05:56:16PM +1000, Herbert Xu wrote: > > For XTS I haven't decided whether to go this way or not. If it > does work out though we could even extend it to AEAD. But there is clearly a need for this functionality, and it's not just af_alg. Have a look at net/sunrpc/auth_gss/gss_krb5_crypto.c, it has three versions of the same crypto code (arc4, cts, and everything else), in order to deal with continuing requests just like algif_skcipher. Perhaps at the end of this we could distill it down to just one. Cheers,
On Thu, 2 Jul 2020 at 09:56, Herbert Xu <herbert@gondor.apana.org.au> wrote: > > On Thu, Jul 02, 2020 at 09:51:29AM +0200, Ard Biesheuvel wrote: > > > > I'll wait for the code to be posted (please put me on cc), but my > > Sure I will. > > > suspicion is that carrying opaque state like that is going to bite us > > down the road. > > Well it's only going to be arc4 at first, where it's definitely > an improvement over modifying the tfm context in encrypt/decrypt. > I agree that the current approach is flawed, but starting multiple requests with the same state essentially comes down to IV reuse in a stream cipher, which will cause it to fail catastrophically. (ciphertext1 ^ ciphertext2 == plaintext1 ^ plaintext2) I wonder if we should simply try to get rid of arc4 in the crypto API, as you suggested. There are a couple of WEP implementations that could be switched over to the library interface, and the KerberosV implementation of RC4-HMAC(md5) was added for Windows 2000 compatibility based on RFC 4757 [0], which was deprecated by RFC 8429 [1], since Windows Domain Controllers running Windows Server 2008r2 or later can use newer algorithms. [0] https://tools.ietf.org/html/rfc4757 [1] https://tools.ietf.org/html/rfc8429 > > On Thu, Jul 02, 2020 at 05:56:16PM +1000, Herbert Xu wrote: > > > > For XTS I haven't decided whether to go this way or not. If it > > does work out though we could even extend it to AEAD. > > But there is clearly a need for this functionality, and it's > not just af_alg. Have a look at net/sunrpc/auth_gss/gss_krb5_crypto.c, > it has three versions of the same crypto code (arc4, cts, and > everything else), in order to deal with continuing requests just > like algif_skcipher. > > Perhaps at the end of this we could distill it down to just one. > I agree that there is a gap here. Perhaps we can decouple ARC4 from the other cases? ARC4 is too quirky and irrelevant to model this on top of, I think.
On 7/2/2020 7:36 AM, Herbert Xu 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. > The fact that smth. is broken doesn't necessarily means it has to be removed. Looking at the HW capabilities, I am sure the implementation could be modified to save/restore the internal state to/from the request context. Anyhow I would like to know if only the correctness is being debated, or this patch should be dealt with in the larger context of removing crypto API based ecb(arc4) altogether: [RFC PATCH 0/7] crypto: get rid of ecb(arc4) https://lore.kernel.org/linux-crypto/20200702101947.682-1-ardb@kernel.org/ Thanks, Horia
On Sun, 5 Jul 2020 at 22:11, Horia Geantă <horia.geanta@nxp.com> wrote: > > On 7/2/2020 7:36 AM, Herbert Xu 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. > > > The fact that smth. is broken doesn't necessarily means it has to be removed. > > Looking at the HW capabilities, I am sure the implementation could be > modified to save/restore the internal state to/from the request context. > > Anyhow I would like to know if only the correctness is being debated, > or this patch should be dealt with in the larger context of > removing crypto API based ecb(arc4) altogether: > [RFC PATCH 0/7] crypto: get rid of ecb(arc4) > https://lore.kernel.org/linux-crypto/20200702101947.682-1-ardb@kernel.org/ > The problem with 'fixing' ecb(arc4) is that it will make it less secure than it already is. For instance, imagine two peers using the generic ecb(arc4) implementation, and using a pair of skcipher TFMs to en/decrypt the communication between them, similar to how the WEP code works today. if we 'fix' the implementation, every request will be served from the same initial state (including the key), and therefore reuse the same keystream, resulting in catastrophic failure. (Of course, the code should set a different key for each request anyway, but failure to do so does not result in the same security fail with the current implementation) So the problem is really that the lack of a key vs IV distinction in ARC4 means that it does not fit the skcipher model cleanly, and issuing more than a single request without an intermediate setkey() operation should be forbidden in any case. The reason I suggested removing it is that we really have no use for it in the kernel, and the only known af_alg users are dubious as well, and so we'd be much better off simply getting rid of ecb(arc4) entirely, not because any of the implementations are flawed, but simply because I don't think we should waste more time on it in general.
On 7/6/2020 4:43 PM, Ard Biesheuvel wrote: > On Sun, 5 Jul 2020 at 22:11, Horia Geantă <horia.geanta@nxp.com> wrote: >> >> On 7/2/2020 7:36 AM, Herbert Xu 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. >>> >> The fact that smth. is broken doesn't necessarily means it has to be removed. >> >> Looking at the HW capabilities, I am sure the implementation could be >> modified to save/restore the internal state to/from the request context. >> >> Anyhow I would like to know if only the correctness is being debated, >> or this patch should be dealt with in the larger context of >> removing crypto API based ecb(arc4) altogether: >> [RFC PATCH 0/7] crypto: get rid of ecb(arc4) >> https://lore.kernel.org/linux-crypto/20200702101947.682-1-ardb@kernel.org/ >> > > The problem with 'fixing' ecb(arc4) is that it will make it less > secure than it already is. For instance, imagine two peers using the > generic ecb(arc4) implementation, and using a pair of skcipher TFMs to > en/decrypt the communication between them, similar to how the WEP code > works today. if we 'fix' the implementation, every request will be > served from the same initial state (including the key), and therefore > reuse the same keystream, resulting in catastrophic failure. (Of > course, the code should set a different key for each request anyway, > but failure to do so does not result in the same security fail with > the current implementation) > > So the problem is really that the lack of a key vs IV distinction in > ARC4 means that it does not fit the skcipher model cleanly, and > issuing more than a single request without an intermediate setkey() > operation should be forbidden in any case. > > The reason I suggested removing it is that we really have no use for > it in the kernel, and the only known af_alg users are dubious as well, > and so we'd be much better off simply getting rid of ecb(arc4) > entirely, not because any of the implementations are flawed, but > simply because I don't think we should waste more time on it in > general. > Thanks Ard. My understanding was along these lines, just wanted to make sure. I think the commit message should be updated to reflect this logic: indeed, caam's implementation of ecb(arc4) is broken, but instead of fixing it, crypto API-based ecb(arc4) is removed completely from the kernel (hence from caam driver) due to skcipher limitations in terms of handling the keystream. Horia
On Wed, Jul 08, 2020 at 07:24:08PM +0300, Horia Geantă wrote: > > I think the commit message should be updated to reflect this logic: > indeed, caam's implementation of ecb(arc4) is broken, > but instead of fixing it, crypto API-based ecb(arc4) > is removed completely from the kernel (hence from caam driver) > due to skcipher limitations in terms of handling the keystream. Actually that's not quite true. The reason I create this patch in the first place is to remove this limitation from skcipher. Cheers,
On 7/9/2020 3:47 AM, Herbert Xu wrote: > On Wed, Jul 08, 2020 at 07:24:08PM +0300, Horia Geantă wrote: >> >> I think the commit message should be updated to reflect this logic: >> indeed, caam's implementation of ecb(arc4) is broken, >> but instead of fixing it, crypto API-based ecb(arc4) >> is removed completely from the kernel (hence from caam driver) >> due to skcipher limitations in terms of handling the keystream. > > Actually that's not quite true. The reason I create this patch > in the first place is to remove this limitation from skcipher. > But the reason / context has changed in the meantime right? If skcipher limitation is eliminated, will it be possible to add ecb(arc4) implementation back in caam, this time with the state stored in the request object? My understanding is: no, if Ard's arc4 RFC series is merged. Thanks, Horia
On Thu, 9 Jul 2020 at 11:53, Horia Geantă <horia.geanta@nxp.com> wrote: > > On 7/9/2020 3:47 AM, Herbert Xu wrote: > > On Wed, Jul 08, 2020 at 07:24:08PM +0300, Horia Geantă wrote: > >> > >> I think the commit message should be updated to reflect this logic: > >> indeed, caam's implementation of ecb(arc4) is broken, > >> but instead of fixing it, crypto API-based ecb(arc4) > >> is removed completely from the kernel (hence from caam driver) > >> due to skcipher limitations in terms of handling the keystream. > > > > Actually that's not quite true. The reason I create this patch > > in the first place is to remove this limitation from skcipher. > > > But the reason / context has changed in the meantime right? > > If skcipher limitation is eliminated, > will it be possible to add ecb(arc4) implementation back in caam, > this time with the state stored in the request object? > > My understanding is: no, if Ard's arc4 RFC series is merged. > I would like the skcipher chaining discussion to focus on relevant and typical skciphers like XTS and CBC-CTS, which is already tricky enough to get right. As 'fixing' ecb(arc4) may open up security holes (given that leaving the TFM version of the key untouched makes it more likely that it gets reused inadvertently), I would prefer to get rid of it entirely. And what is the point of accelerated ecb(arc4) anyway? The internal users all require sync skciphers (and those WEP/WPA fallbacks are rarely used in practice, given that only ancient Wifi hardware relies on them). We did identify a AF_ALG skcipher user that will be better off using a C implementation in userland. And using RC4 for TLS was explicitly forbidden by RFC 7465 5 years ago ...
On 7/2/2020 7:36 AM, Herbert Xu 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@gondor.apana.org.au> Given the circumstances, it's not feasible to fix arc4 in caam. Link: https://lore.kernel.org/linux-crypto/CAMj1kXGvMe_A_iQ43Pmygg9xaAM-RLy=_M=v+eg--8xNmv9P+w@mail.gmail.com Link: https://lore.kernel.org/linux-crypto/20200702101947.682-1-ardb@kernel.org Acked-by: Horia Geantă <horia.geanta@nxp.com> Horia
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>
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@gondor.apana.org.au>