linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] crypto: ccm - preserve the IV buffer
@ 2017-10-31 14:42 Romain Izard
  2017-11-02 14:03 ` Tudor Ambarus
  2017-11-03 13:43 ` Herbert Xu
  0 siblings, 2 replies; 3+ messages in thread
From: Romain Izard @ 2017-10-31 14:42 UTC (permalink / raw)
  To: Herbert Xu, David S . Miller, Tudor Ambarus
  Cc: linux-crypto, linux-kernel, Romain Izard

The IV buffer used during CCM operations is used twice, during both the
hashing step and the ciphering step.

When using a hardware accelerator that updates the contents of the IV
buffer at the end of ciphering operations, the value will be modified.
In the decryption case, the subsequent setup of the hashing algorithm
will interpret the updated IV instead of the original value, which can
lead to out-of-bounds writes.

Reuse the idata buffer, only used in the hashing step, to preserve the
IV's value during the ciphering step in the decryption case.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
 crypto/ccm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/crypto/ccm.c b/crypto/ccm.c
index 1ce37ae0ce56..0a083342ec8c 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -363,7 +363,7 @@ static int crypto_ccm_decrypt(struct aead_request *req)
 	unsigned int cryptlen = req->cryptlen;
 	u8 *authtag = pctx->auth_tag;
 	u8 *odata = pctx->odata;
-	u8 *iv = req->iv;
+	u8 *iv = pctx->idata;
 	int err;
 
 	cryptlen -= authsize;
@@ -379,6 +379,8 @@ static int crypto_ccm_decrypt(struct aead_request *req)
 	if (req->src != req->dst)
 		dst = pctx->dst;
 
+	memcpy(iv, req->iv, 16);
+
 	skcipher_request_set_tfm(skreq, ctx->ctr);
 	skcipher_request_set_callback(skreq, pctx->flags,
 				      crypto_ccm_decrypt_done, req);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] crypto: ccm - preserve the IV buffer
  2017-10-31 14:42 [PATCH] crypto: ccm - preserve the IV buffer Romain Izard
@ 2017-11-02 14:03 ` Tudor Ambarus
  2017-11-03 13:43 ` Herbert Xu
  1 sibling, 0 replies; 3+ messages in thread
From: Tudor Ambarus @ 2017-11-02 14:03 UTC (permalink / raw)
  To: Romain Izard, Herbert Xu, David S . Miller; +Cc: linux-crypto, linux-kernel


On 10/31/2017 04:42 PM, Romain Izard wrote:
> The IV buffer used during CCM operations is used twice, during both the
> hashing step and the ciphering step.
> 
> When using a hardware accelerator that updates the contents of the IV
> buffer at the end of ciphering operations, the value will be modified.
> In the decryption case, the subsequent setup of the hashing algorithm
> will interpret the updated IV instead of the original value, which can
> lead to out-of-bounds writes.
> 
> Reuse the idata buffer, only used in the hashing step, to preserve the
> IV's value during the ciphering step in the decryption case.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
>   crypto/ccm.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/ccm.c b/crypto/ccm.c
> index 1ce37ae0ce56..0a083342ec8c 100644
> --- a/crypto/ccm.c
> +++ b/crypto/ccm.c
> @@ -363,7 +363,7 @@ static int crypto_ccm_decrypt(struct aead_request *req)
>   	unsigned int cryptlen = req->cryptlen;
>   	u8 *authtag = pctx->auth_tag;
>   	u8 *odata = pctx->odata;
> -	u8 *iv = req->iv;
> +	u8 *iv = pctx->idata;
>   	int err;
>   
>   	cryptlen -= authsize;
> @@ -379,6 +379,8 @@ static int crypto_ccm_decrypt(struct aead_request *req)
>   	if (req->src != req->dst)
>   		dst = pctx->dst;
>   
> +	memcpy(iv, req->iv, 16);
> +
>   	skcipher_request_set_tfm(skreq, ctx->ctr);
>   	skcipher_request_set_callback(skreq, pctx->flags,
>   				      crypto_ccm_decrypt_done, req);
> 

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] crypto: ccm - preserve the IV buffer
  2017-10-31 14:42 [PATCH] crypto: ccm - preserve the IV buffer Romain Izard
  2017-11-02 14:03 ` Tudor Ambarus
@ 2017-11-03 13:43 ` Herbert Xu
  1 sibling, 0 replies; 3+ messages in thread
From: Herbert Xu @ 2017-11-03 13:43 UTC (permalink / raw)
  To: Romain Izard; +Cc: David S . Miller, Tudor Ambarus, linux-crypto, linux-kernel

On Tue, Oct 31, 2017 at 03:42:35PM +0100, Romain Izard wrote:
> The IV buffer used during CCM operations is used twice, during both the
> hashing step and the ciphering step.
> 
> When using a hardware accelerator that updates the contents of the IV
> buffer at the end of ciphering operations, the value will be modified.
> In the decryption case, the subsequent setup of the hashing algorithm
> will interpret the updated IV instead of the original value, which can
> lead to out-of-bounds writes.
> 
> Reuse the idata buffer, only used in the hashing step, to preserve the
> IV's value during the ciphering step in the decryption case.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

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] 3+ messages in thread

end of thread, other threads:[~2017-11-03 13:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-31 14:42 [PATCH] crypto: ccm - preserve the IV buffer Romain Izard
2017-11-02 14:03 ` Tudor Ambarus
2017-11-03 13:43 ` 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).