[PATCH] crypto: gcm - Fix rfc4543 decryption crash

[PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Herbert Xu]

This bug has already bee fixed upstream since 4.2.  However, it
was fixed during the AEAD conversion so no fix was backported to
the older kernels.

When we do an RFC 4543 decryption, we will end up writing the
ICV beyond the end of the dst buffer.  This should lead to a
crash but for some reason it was never noticed.

This patch fixes it by only writing back the ICV for encryption.

Fixes: d733ac90f9fe ("crypto: gcm - fix rfc4543 to handle async...")
Reported-by: Patrick Meyer <patrick.meyer@vasgard.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/gcm.c b/crypto/gcm.c
index b4c2520..cd97cdd 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -1173,6 +1173,9 @@ static struct aead_request *crypto_rfc4543_crypt(struct aead_request *req,
 	aead_request_set_tfm(subreq, ctx->child);
 	aead_request_set_callback(subreq, req->base.flags, crypto_rfc4543_done,
 				  req);
+	if (!enc)
+		aead_request_set_callback(subreq, req->base.flags,
+					  req->base.complete, req->base.data);
 	aead_request_set_crypt(subreq, cipher, cipher, enc ? 0 : authsize, iv);
 	aead_request_set_assoc(subreq, assoc, assoclen);
 
-- 
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

Re: [PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Greg KH]

On Fri, Mar 18, 2016 at 10:42:40PM +0800, Herbert Xu wrote:
> This bug has already bee fixed upstream since 4.2.  However, it
> was fixed during the AEAD conversion so no fix was backported to
> the older kernels.

What was the commit id of that fix?

> 
> When we do an RFC 4543 decryption, we will end up writing the
> ICV beyond the end of the dst buffer.  This should lead to a
> crash but for some reason it was never noticed.
> 
> This patch fixes it by only writing back the ICV for encryption.
> 
> Fixes: d733ac90f9fe ("crypto: gcm - fix rfc4543 to handle async...")
> Reported-by: Patrick Meyer <patrick.meyer@vasgard.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

What stable kernel(s) do you want this in?

thanks,

greg k-h

Re: [PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Herbert Xu]

On Fri, Mar 18, 2016 at 10:34:23AM -0700, Greg KH wrote:
> On Fri, Mar 18, 2016 at 10:42:40PM +0800, Herbert Xu wrote:
> > This bug has already bee fixed upstream since 4.2.  However, it
> > was fixed during the AEAD conversion so no fix was backported to
> > the older kernels.
> 
> What was the commit id of that fix?

commit adcbc688fe2f8107b7f564187593293aa9ea3932
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Tue Jun 16 13:54:18 2015 +0800

    crypto: gcm - Convert to new AEAD interface
 
> What stable kernel(s) do you want this in?

Since the fix was merged in 4.2 this is needed on all kernels
prior to that, i.e., 4.1 and any prior version.

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

Re: [PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]

On Sat, 2016-03-19 at 10:23 +0800, Herbert Xu wrote:
> On Fri, Mar 18, 2016 at 10:34:23AM -0700, Greg KH wrote:
> > 
> > On Fri, Mar 18, 2016 at 10:42:40PM +0800, Herbert Xu wrote:
> > > 
> > > This bug has already bee fixed upstream since 4.2.  However, it
> > > was fixed during the AEAD conversion so no fix was backported to
> > > the older kernels.
> > What was the commit id of that fix?
> commit adcbc688fe2f8107b7f564187593293aa9ea3932
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date:   Tue Jun 16 13:54:18 2015 +0800
> 
>     crypto: gcm - Convert to new AEAD interface
>  
> > 
> > What stable kernel(s) do you want this in?
> Since the fix was merged in 4.2 this is needed on all kernels
> prior to that, i.e., 4.1 and any prior version.

It looks like the bug was introduced in 3.10 by:

d733ac90f9fe8ac284e523f9920b507555b12f6d
Author: Jussi Kivilinna <jussi.kivilinna@iki.fi>
Date:   Sun Apr 7 16:43:46 2013 +0300

    crypto: gcm - fix rfc4543 to handle async crypto correctly
    
So 3.2.y and 3.4.y don't need this fix - or should they get both fixes?

Ben.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                            - Robert Coveyou

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

On Fri, 2016-03-18 at 22:42 +0800, Herbert Xu wrote:
> This bug has already bee fixed upstream since 4.2.  However, it
> was fixed during the AEAD conversion so no fix was backported to
> the older kernels.
> 
> When we do an RFC 4543 decryption, we will end up writing the
> ICV beyond the end of the dst buffer.  This should lead to a
> crash but for some reason it was never noticed.
> 
> This patch fixes it by only writing back the ICV for encryption.
> 
> Fixes: d733ac90f9fe ("crypto: gcm - fix rfc4543 to handle async...")
> Reported-by: Patrick Meyer <patrick.meyer@vasgard.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Queued up for 3.16, thanks.

Ben.

> 
> diff --git a/crypto/gcm.c b/crypto/gcm.c
> index b4c2520..cd97cdd 100644
> --- a/crypto/gcm.c
> +++ b/crypto/gcm.c
> @@ -1173,6 +1173,9 @@ static struct aead_request *crypto_rfc4543_crypt(struct aead_request *req,
>  	aead_request_set_tfm(subreq, ctx->child);
>  	aead_request_set_callback(subreq, req->base.flags, crypto_rfc4543_done,
>  				  req);
> +	if (!enc)
> +		aead_request_set_callback(subreq, req->base.flags,
> +					  req->base.complete, req->base.data);
>  	aead_request_set_crypt(subreq, cipher, cipher, enc ? 0 : authsize, iv);
>  	aead_request_set_assoc(subreq, assoc, assoclen);
>  
-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                            - Robert Coveyou

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Herbert Xu]

On Tue, Apr 26, 2016 at 01:42:56PM +0200, Ben Hutchings wrote:
>
> It looks like the bug was introduced in 3.10 by:
> 
> d733ac90f9fe8ac284e523f9920b507555b12f6d
> Author: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> Date:   Sun Apr 7 16:43:46 2013 +0300
> 
>     crypto: gcm - fix rfc4543 to handle async crypto correctly
>     
> So 3.2.y and 3.4.y don't need this fix - or should they get both fixes?

If that patch is not present then my fix can't be applied.  However,
I think this change itself is probably needed in 3.2/3.4 as otherwise
GCM would be broken if the underlying cipher is async.  It's not a
big deal on x86 because the main async AES provider also provides
GCM directly, but on other architectures it may be an issue.

Cheers,
-- 
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

Re: [PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Herbert Xu]

On Tue, Apr 26, 2016 at 01:42:56PM +0200, Ben Hutchings wrote:
>
> It looks like the bug was introduced in 3.10 by:
> 
> d733ac90f9fe8ac284e523f9920b507555b12f6d
> Author: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> Date:���Sun Apr 7 16:43:46 2013 +0300
> 
> ����crypto: gcm - fix rfc4543 to handle async crypto correctly
> � ��
> So 3.2.y and 3.4.y don't need this fix - or should they get both fixes?

If that patch is not present then my fix can't be applied.  However,
I think this change itself is probably needed in 3.2/3.4 as otherwise
GCM would be broken if the underlying cipher is async.  It's not a
big deal on x86 because the main async AES provider also provides
GCM directly, but on other architectures it may be an issue.

Cheers,
-- 
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

Patch "crypto: gcm - Fix rfc4543 decryption crash" has been added to the 3.14-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    crypto: gcm - Fix rfc4543 decryption crash

to the 3.14-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     crypto-gcm-fix-rfc4543-decryption-crash.patch
and it can be found in the queue-3.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From herbert@gondor.apana.org.au  Sun May  1 15:39:20 2016
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 18 Mar 2016 22:42:40 +0800
Subject: crypto: gcm - Fix rfc4543 decryption crash
To: stable@vger.kernel.org, Linux Crypto Mailing List <linux-crypto@vger.kernel.org>, Jussi Kivilinna <jussi.kivilinna@iki.fi>, patrick.meyer@vasgard.com
Message-ID: <20160318144240.GA20816@gondor.apana.org.au>
Content-Disposition: inline

From: Herbert Xu <herbert@gondor.apana.org.au>

This bug has already bee fixed upstream since 4.2.  However, it
was fixed during the AEAD conversion so no fix was backported to
the older kernels.

When we do an RFC 4543 decryption, we will end up writing the
ICV beyond the end of the dst buffer.  This should lead to a
crash but for some reason it was never noticed.

This patch fixes it by only writing back the ICV for encryption.

Fixes: d733ac90f9fe ("crypto: gcm - fix rfc4543 to handle async...")
Reported-by: Patrick Meyer <patrick.meyer@vasgard.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 crypto/gcm.c |    3 +++
 1 file changed, 3 insertions(+)

--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -1173,6 +1173,9 @@ static struct aead_request *crypto_rfc45
 	aead_request_set_tfm(subreq, ctx->child);
 	aead_request_set_callback(subreq, req->base.flags, crypto_rfc4543_done,
 				  req);
+	if (!enc)
+		aead_request_set_callback(subreq, req->base.flags,
+					  req->base.complete, req->base.data);
 	aead_request_set_crypt(subreq, cipher, cipher, enc ? 0 : authsize, iv);
 	aead_request_set_assoc(subreq, assoc, assoclen);
 


Patches currently in stable-queue which might be from herbert@gondor.apana.org.au are

queue-3.14/crypto-gcm-fix-rfc4543-decryption-crash.patch
queue-3.14/crypto-ccp-prevent-information-leakage-on-export.patch

Re: [PATCH] crypto: gcm - Fix rfc4543 decryption crash

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

On Wed, 2016-04-27 at 12:17 +0800, Herbert Xu wrote:
> On Tue, Apr 26, 2016 at 01:42:56PM +0200, Ben Hutchings wrote:
> > 
> > 
> > It looks like the bug was introduced in 3.10 by:
> > 
> > d733ac90f9fe8ac284e523f9920b507555b12f6d
> > Author: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> > Date:   Sun Apr 7 16:43:46 2013 +0300
> > 
> >     crypto: gcm - fix rfc4543 to handle async crypto correctly
> >     
> > So 3.2.y and 3.4.y don't need this fix - or should they get both
> > fixes?
> If that patch is not present then my fix can't be applied.  However,
> I think this change itself is probably needed in 3.2/3.4 as otherwise
> GCM would be broken if the underlying cipher is async.  It's not a
> big deal on x86 because the main async AES provider also provides
> GCM directly, but on other architectures it may be an issue.

I've queued up both of these for 3.2.

Ben.

-- 
Ben Hutchings
For every action, there is an equal and opposite criticism. - Harrison

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]