* [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure
@ 2023-09-21 12:42 Gaurav Jain
2023-09-22 2:41 ` Eric Biggers
2023-10-01 8:34 ` Herbert Xu
0 siblings, 2 replies; 6+ messages in thread
From: Gaurav Jain @ 2023-09-21 12:42 UTC (permalink / raw)
To: Horia Geanta, Pankaj Gupta, Varun Sethi, Meenakshi Aggarwal,
Herbert Xu, David S . Miller, Aisheng Dong
Cc: Silvano Di Ninno, linux-crypto, linux-kernel, linux-imx,
Gaurav Jain
key buffer is not copied in chachapoly_setkey function,
results in wrong output for encryption/decryption operation.
fix this by memcpy the key in caam_ctx key arrary
Fixes: d6bbd4eea243 ("crypto: caam/jr - add support for Chacha20 + Poly1305")
Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
---
drivers/crypto/caam/caamalg.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index eba2d750c3b0..066f08a3a040 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -575,7 +575,8 @@ static int chachapoly_setkey(struct crypto_aead *aead, const u8 *key,
if (keylen != CHACHA_KEY_SIZE + saltlen)
return -EINVAL;
- ctx->cdata.key_virt = key;
+ memcpy(ctx->key, key, keylen);
+ ctx->cdata.key_virt = ctx->key;
ctx->cdata.keylen = keylen - saltlen;
return chachapoly_set_sh_desc(aead);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure 2023-09-21 12:42 [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure Gaurav Jain @ 2023-09-22 2:41 ` Eric Biggers 2023-09-22 6:34 ` [EXT] " Gaurav Jain 2023-10-01 8:34 ` Herbert Xu 1 sibling, 1 reply; 6+ messages in thread From: Eric Biggers @ 2023-09-22 2:41 UTC (permalink / raw) To: Gaurav Jain Cc: Horia Geanta, Pankaj Gupta, Varun Sethi, Meenakshi Aggarwal, Herbert Xu, David S . Miller, Aisheng Dong, Silvano Di Ninno, linux-crypto, linux-kernel, linux-imx On Thu, Sep 21, 2023 at 06:12:37PM +0530, Gaurav Jain wrote: > key buffer is not copied in chachapoly_setkey function, > results in wrong output for encryption/decryption operation. > > fix this by memcpy the key in caam_ctx key arrary > > Fixes: d6bbd4eea243 ("crypto: caam/jr - add support for Chacha20 + Poly1305") > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> > --- > drivers/crypto/caam/caamalg.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c > index eba2d750c3b0..066f08a3a040 100644 > --- a/drivers/crypto/caam/caamalg.c > +++ b/drivers/crypto/caam/caamalg.c > @@ -575,7 +575,8 @@ static int chachapoly_setkey(struct crypto_aead *aead, const u8 *key, > if (keylen != CHACHA_KEY_SIZE + saltlen) > return -EINVAL; > > - ctx->cdata.key_virt = key; > + memcpy(ctx->key, key, keylen); > + ctx->cdata.key_virt = ctx->key; > ctx->cdata.keylen = keylen - saltlen; > Huh, so this driver just ignored the key? Is anyone using the ChaCha20Poly1305 support in this driver? Based on this bug existing, that seems unlikely. If that's the case, wouldn't it be better just to remove the ChaCha20Poly1305 support from this driver so that the code doesn't need to be maintained? - Eric ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXT] Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure 2023-09-22 2:41 ` Eric Biggers @ 2023-09-22 6:34 ` Gaurav Jain 2023-09-22 7:17 ` [EXTERNAL] " Kamlesh Gurudasani 0 siblings, 1 reply; 6+ messages in thread From: Gaurav Jain @ 2023-09-22 6:34 UTC (permalink / raw) To: Eric Biggers Cc: Horia Geanta, Pankaj Gupta, Varun Sethi, Meenakshi Aggarwal, Herbert Xu, David S . Miller, Aisheng Dong, Silvano Di Ninno, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, dl-linux-imx Hi Eric > -----Original Message----- > From: Eric Biggers <ebiggers@kernel.org> > Sent: Friday, September 22, 2023 8:11 AM > To: Gaurav Jain <gaurav.jain@nxp.com> > Cc: Horia Geanta <horia.geanta@nxp.com>; Pankaj Gupta > <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Meenakshi > Aggarwal <meenakshi.aggarwal@nxp.com>; Herbert Xu > <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; > Aisheng Dong <aisheng.dong@nxp.com>; Silvano Di Ninno > <silvano.dininno@nxp.com>; linux-crypto@vger.kernel.org; linux- > kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > Subject: [EXT] Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test > failure > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report this > email' button > > > On Thu, Sep 21, 2023 at 06:12:37PM +0530, Gaurav Jain wrote: > > key buffer is not copied in chachapoly_setkey function, results in > > wrong output for encryption/decryption operation. > > > > fix this by memcpy the key in caam_ctx key arrary > > > > Fixes: d6bbd4eea243 ("crypto: caam/jr - add support for Chacha20 + > > Poly1305") > > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> > > --- > > drivers/crypto/caam/caamalg.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/crypto/caam/caamalg.c > > b/drivers/crypto/caam/caamalg.c index eba2d750c3b0..066f08a3a040 > > 100644 > > --- a/drivers/crypto/caam/caamalg.c > > +++ b/drivers/crypto/caam/caamalg.c > > @@ -575,7 +575,8 @@ static int chachapoly_setkey(struct crypto_aead *aead, > const u8 *key, > > if (keylen != CHACHA_KEY_SIZE + saltlen) > > return -EINVAL; > > > > - ctx->cdata.key_virt = key; > > + memcpy(ctx->key, key, keylen); > > + ctx->cdata.key_virt = ctx->key; > > ctx->cdata.keylen = keylen - saltlen; > > > > Huh, so this driver just ignored the key? Is anyone using the ChaCha20Poly1305 > support in this driver? Based on this bug existing, that seems unlikely. If that's > the case, wouldn't it be better just to remove the ChaCha20Poly1305 support > from this driver so that the code doesn't need to be maintained? This algorithm is used in IPSEC and we are also going to use ChaCha20Poly1305 support for Kernel TLS. Gaurav > > - Eric ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [EXTERNAL] RE: [EXT] Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure 2023-09-22 6:34 ` [EXT] " Gaurav Jain @ 2023-09-22 7:17 ` Kamlesh Gurudasani 2023-09-25 9:03 ` Gaurav Jain 0 siblings, 1 reply; 6+ messages in thread From: Kamlesh Gurudasani @ 2023-09-22 7:17 UTC (permalink / raw) To: Gaurav Jain, Eric Biggers Cc: Horia Geanta, Pankaj Gupta, Varun Sethi, Meenakshi Aggarwal, Herbert Xu, David S . Miller, Aisheng Dong, Silvano Di Ninno, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, dl-linux-imx Gaurav Jain <gaurav.jain@nxp.com> writes: > Hi Eric > >> -----Original Message----- >> From: Eric Biggers <ebiggers@kernel.org> >> Sent: Friday, September 22, 2023 8:11 AM >> To: Gaurav Jain <gaurav.jain@nxp.com> >> Cc: Horia Geanta <horia.geanta@nxp.com>; Pankaj Gupta >> <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Meenakshi >> Aggarwal <meenakshi.aggarwal@nxp.com>; Herbert Xu >> <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; >> Aisheng Dong <aisheng.dong@nxp.com>; Silvano Di Ninno >> <silvano.dininno@nxp.com>; linux-crypto@vger.kernel.org; linux- >> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> >> Subject: [EXT] Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test >> failure >> >> Caution: This is an external email. Please take care when clicking links or >> opening attachments. When in doubt, report the message using the 'Report this >> email' button >> >> >> On Thu, Sep 21, 2023 at 06:12:37PM +0530, Gaurav Jain wrote: >> > key buffer is not copied in chachapoly_setkey function, results in >> > wrong output for encryption/decryption operation. >> > >> > fix this by memcpy the key in caam_ctx key arrary Not sure, but do you mean array*? >> > >> > Fixes: d6bbd4eea243 ("crypto: caam/jr - add support for Chacha20 + >> > Poly1305") >> > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> >> > --- >> > drivers/crypto/caam/caamalg.c | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/crypto/caam/caamalg.c >> > b/drivers/crypto/caam/caamalg.c index eba2d750c3b0..066f08a3a040 >> > 100644 >> > --- a/drivers/crypto/caam/caamalg.c >> > +++ b/drivers/crypto/caam/caamalg.c >> > @@ -575,7 +575,8 @@ static int chachapoly_setkey(struct crypto_aead *aead, >> const u8 *key, >> > if (keylen != CHACHA_KEY_SIZE + saltlen) >> > return -EINVAL; >> > >> > - ctx->cdata.key_virt = key; >> > + memcpy(ctx->key, key, keylen); >> > + ctx->cdata.key_virt = ctx->key; >> > ctx->cdata.keylen = keylen - saltlen; >> > >> >> Huh, so this driver just ignored the key? Is anyone using the ChaCha20Poly1305 >> support in this driver? Based on this bug existing, that seems unlikely. If that's >> the case, wouldn't it be better just to remove the ChaCha20Poly1305 support >> from this driver so that the code doesn't need to be maintained? > > This algorithm is used in IPSEC and we are also going to use ChaCha20Poly1305 support for Kernel TLS. > Gaurav Does this mean IPSEC doesn't call setkey() or the key value was such that it didn't affect even after failing to actually set the key? -Kamlesh ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] RE: [EXT] Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure 2023-09-22 7:17 ` [EXTERNAL] " Kamlesh Gurudasani @ 2023-09-25 9:03 ` Gaurav Jain 0 siblings, 0 replies; 6+ messages in thread From: Gaurav Jain @ 2023-09-25 9:03 UTC (permalink / raw) To: Kamlesh Gurudasani, Eric Biggers Cc: Horia Geanta, Pankaj Gupta, Varun Sethi, Meenakshi Aggarwal, Herbert Xu, David S . Miller, Aisheng Dong, Silvano Di Ninno, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, dl-linux-imx Hi Kamlesh > -----Original Message----- > From: Kamlesh Gurudasani <kamlesh@ti.com> > Sent: Friday, September 22, 2023 12:47 PM > To: Gaurav Jain <gaurav.jain@nxp.com>; Eric Biggers <ebiggers@kernel.org> > Cc: Horia Geanta <horia.geanta@nxp.com>; Pankaj Gupta > <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Meenakshi > Aggarwal <meenakshi.aggarwal@nxp.com>; Herbert Xu > <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; > Aisheng Dong <aisheng.dong@nxp.com>; Silvano Di Ninno > <silvano.dininno@nxp.com>; linux-crypto@vger.kernel.org; linux- > kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > Subject: Re: [EXTERNAL] RE: [EXT] Re: [PATCH] crypto: caam/jr - fix Chacha20 + > Poly1305 self test failure > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report this > email' button > > > Gaurav Jain <gaurav.jain@nxp.com> writes: > > > Hi Eric > > > >> -----Original Message----- > >> From: Eric Biggers <ebiggers@kernel.org> > >> Sent: Friday, September 22, 2023 8:11 AM > >> To: Gaurav Jain <gaurav.jain@nxp.com> > >> Cc: Horia Geanta <horia.geanta@nxp.com>; Pankaj Gupta > >> <pankaj.gupta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Meenakshi > >> Aggarwal <meenakshi.aggarwal@nxp.com>; Herbert Xu > >> <herbert@gondor.apana.org.au>; David S . Miller > >> <davem@davemloft.net>; Aisheng Dong <aisheng.dong@nxp.com>; Silvano > >> Di Ninno <silvano.dininno@nxp.com>; linux-crypto@vger.kernel.org; > >> linux- kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > >> Subject: [EXT] Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 > >> self test failure > >> > >> Caution: This is an external email. Please take care when clicking > >> links or opening attachments. When in doubt, report the message using > >> the 'Report this email' button > >> > >> > >> On Thu, Sep 21, 2023 at 06:12:37PM +0530, Gaurav Jain wrote: > >> > key buffer is not copied in chachapoly_setkey function, results in > >> > wrong output for encryption/decryption operation. > >> > > >> > fix this by memcpy the key in caam_ctx key arrary > Not sure, but do you mean array*? > > >> > > >> > Fixes: d6bbd4eea243 ("crypto: caam/jr - add support for Chacha20 + > >> > Poly1305") > >> > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> > >> > --- > >> > drivers/crypto/caam/caamalg.c | 3 ++- > >> > 1 file changed, 2 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/crypto/caam/caamalg.c > >> > b/drivers/crypto/caam/caamalg.c index eba2d750c3b0..066f08a3a040 > >> > 100644 > >> > --- a/drivers/crypto/caam/caamalg.c > >> > +++ b/drivers/crypto/caam/caamalg.c > >> > @@ -575,7 +575,8 @@ static int chachapoly_setkey(struct crypto_aead > >> > *aead, > >> const u8 *key, > >> > if (keylen != CHACHA_KEY_SIZE + saltlen) > >> > return -EINVAL; > >> > > >> > - ctx->cdata.key_virt = key; > >> > + memcpy(ctx->key, key, keylen); > >> > + ctx->cdata.key_virt = ctx->key; > >> > ctx->cdata.keylen = keylen - saltlen; > >> > > >> > >> Huh, so this driver just ignored the key? Is anyone using the > >> ChaCha20Poly1305 support in this driver? Based on this bug existing, > >> that seems unlikely. If that's the case, wouldn't it be better just > >> to remove the ChaCha20Poly1305 support from this driver so that the code > doesn't need to be maintained? > > > > This algorithm is used in IPSEC and we are also going to use > ChaCha20Poly1305 support for Kernel TLS. > > Gaurav > Does this mean IPSEC doesn't call setkey() or the key value was such that it didn't > affect even after failing to actually set the key? Our test passed with older kernel versions, This issue was caught in testing with the latest kernel version only. Regards Gaurav > > -Kamlesh ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure 2023-09-21 12:42 [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure Gaurav Jain 2023-09-22 2:41 ` Eric Biggers @ 2023-10-01 8:34 ` Herbert Xu 1 sibling, 0 replies; 6+ messages in thread From: Herbert Xu @ 2023-10-01 8:34 UTC (permalink / raw) To: Gaurav Jain Cc: Horia Geanta, Pankaj Gupta, Varun Sethi, Meenakshi Aggarwal, David S . Miller, Aisheng Dong, Silvano Di Ninno, linux-crypto, linux-kernel, linux-imx On Thu, Sep 21, 2023 at 06:12:37PM +0530, Gaurav Jain wrote: > key buffer is not copied in chachapoly_setkey function, > results in wrong output for encryption/decryption operation. > > fix this by memcpy the key in caam_ctx key arrary > > Fixes: d6bbd4eea243 ("crypto: caam/jr - add support for Chacha20 + Poly1305") > Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com> > --- > drivers/crypto/caam/caamalg.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Patch applied. Thanks. -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-01 8:34 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-21 12:42 [PATCH] crypto: caam/jr - fix Chacha20 + Poly1305 self test failure Gaurav Jain 2023-09-22 2:41 ` Eric Biggers 2023-09-22 6:34 ` [EXT] " Gaurav Jain 2023-09-22 7:17 ` [EXTERNAL] " Kamlesh Gurudasani 2023-09-25 9:03 ` Gaurav Jain 2023-10-01 8:34 ` Herbert Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).