2010-05-19 13:24:46

by Shikhar

[permalink] [raw]
Subject: Possible bug in AUTHENC

Hi,

I am currently working on a driver for our crypto HW and the initial
aim was to offload IPSec (ESP for now) to HW. I did this by
registering the cipher and hash algorithms (both asynchronous)
separately (AES-CBC, HMAC-SHA1 only for now) and let the API handle
the request using the AUTHENC interface.

The problem seems to be after the asynchronous hash request is
completed, the length of the ABLKCIPHER decrypt request is not
calculated correctly in "authenc_verify_ahash_update_done()" and
"authenc_verify_ahash_done()". The length should be subtracted by
"authsize" as done in "crypto_authenc_decrypt()".

The following patch (applied against 2.6.34) should fix it. I hope I
didn't miss anything.

Thanks,
Shikhar

--------
diff -pu linux-2.6.34/crypto/authenc.c linux-2.6.34_mod/crypto/authenc.c

--- linux-2.6.34/crypto/authenc.c 2010-05-16 23:17:36.000000000 +0200
+++ linux-2.6.34_mod/crypto/authenc.c 2010-05-19 14:32:51.000000000 +0200
@@ -181,6 +181,7 @@ static void authenc_verify_ahash_update_
struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
struct authenc_request_ctx *areq_ctx = aead_request_ctx(req);
struct ahash_request *ahreq = (void *)(areq_ctx->tail + ctx->reqoff);
+ unsigned int cryptlen = req->cryptlen;

if (err)
goto out;
@@ -196,6 +197,7 @@ static void authenc_verify_ahash_update_
goto out;

authsize = crypto_aead_authsize(authenc);
+ cryptlen -= authsize;
ihash = ahreq->result + authsize;
scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
authsize, 0);
@@ -209,7 +211,7 @@ static void authenc_verify_ahash_update_
ablkcipher_request_set_callback(abreq, aead_request_flags(req),
req->base.complete, req->base.data);
ablkcipher_request_set_crypt(abreq, req->src, req->dst,
- req->cryptlen, req->iv);
+ cryptlen, req->iv);

err = crypto_ablkcipher_decrypt(abreq);

@@ -228,11 +230,13 @@ static void authenc_verify_ahash_done(st
struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
struct authenc_request_ctx *areq_ctx = aead_request_ctx(req);
struct ahash_request *ahreq = (void *)(areq_ctx->tail + ctx->reqoff);
+ unsigned int cryptlen = req->cryptlen;

if (err)
goto out;

authsize = crypto_aead_authsize(authenc);
+ cryptlen -= authsize;
ihash = ahreq->result + authsize;
scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
authsize, 0);
@@ -246,7 +250,7 @@ static void authenc_verify_ahash_done(st
ablkcipher_request_set_callback(abreq, aead_request_flags(req),
req->base.complete, req->base.data);
ablkcipher_request_set_crypt(abreq, req->src, req->dst,
- req->cryptlen, req->iv);
+ cryptlen, req->iv);

err = crypto_ablkcipher_decrypt(abreq);

--- linux-2.6.34/crypto/authenc.c 2010-05-16 23:17:36.000000000 +0200
+++ linux-2.6.34_mod/crypto/authenc.c 2010-05-19 14:32:51.000000000 +0200
@@ -181,6 +181,7 @@ static void authenc_verify_ahash_update_
struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
struct authenc_request_ctx *areq_ctx = aead_request_ctx(req);
struct ahash_request *ahreq = (void *)(areq_ctx->tail + ctx->reqoff);
+ unsigned int cryptlen = req->cryptlen;

if (err)
goto out;
@@ -196,6 +197,7 @@ static void authenc_verify_ahash_update_
goto out;

authsize = crypto_aead_authsize(authenc);
+ cryptlen -= authsize;
ihash = ahreq->result + authsize;
scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
authsize, 0);
@@ -209,7 +211,7 @@ static void authenc_verify_ahash_update_
ablkcipher_request_set_callback(abreq, aead_request_flags(req),
req->base.complete, req->base.data);
ablkcipher_request_set_crypt(abreq, req->src, req->dst,
- req->cryptlen, req->iv);
+ cryptlen, req->iv);

err = crypto_ablkcipher_decrypt(abreq);

@@ -228,11 +230,13 @@ static void authenc_verify_ahash_done(st
struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
struct authenc_request_ctx *areq_ctx = aead_request_ctx(req);
struct ahash_request *ahreq = (void *)(areq_ctx->tail + ctx->reqoff);
+ unsigned int cryptlen = req->cryptlen;

if (err)
goto out;

authsize = crypto_aead_authsize(authenc);
+ cryptlen -= authsize;
ihash = ahreq->result + authsize;
scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
authsize, 0);
@@ -246,7 +250,7 @@ static void authenc_verify_ahash_done(st
ablkcipher_request_set_callback(abreq, aead_request_flags(req),
req->base.complete, req->base.data);
ablkcipher_request_set_crypt(abreq, req->src, req->dst,
- req->cryptlen, req->iv);
+ cryptlen, req->iv);

err = crypto_ablkcipher_decrypt(abreq);

--------


2010-05-20 05:17:52

by Herbert Xu

[permalink] [raw]
Subject: Re: Possible bug in AUTHENC

On Wed, May 19, 2010 at 03:24:24PM +0200, Shikhar wrote:
> Hi,
>
> I am currently working on a driver for our crypto HW and the initial
> aim was to offload IPSec (ESP for now) to HW. I did this by
> registering the cipher and hash algorithms (both asynchronous)
> separately (AES-CBC, HMAC-SHA1 only for now) and let the API handle
> the request using the AUTHENC interface.
>
> The problem seems to be after the asynchronous hash request is
> completed, the length of the ABLKCIPHER decrypt request is not
> calculated correctly in "authenc_verify_ahash_update_done()" and
> "authenc_verify_ahash_done()". The length should be subtracted by
> "authsize" as done in "crypto_authenc_decrypt()".
>
> The following patch (applied against 2.6.34) should fix it. I hope I
> didn't miss anything.

Your patch looks good to me. Could you please resubmit with
a sign-off (see Documentation/SubmittingPatches)?

Thanks!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2010-05-20 09:26:21

by Shikhar

[permalink] [raw]
Subject: Re: [PATCH] Possible bug in AUTHENC

crypto: authenc - Fix "cryptlen" calculation.

This patch (applied against 2.6.34) fixes the calculation of the
length of the ABLKCIPHER decrypt request ("cryptlen") after an
asynchronous hash request has been completed in the AUTHENC interface.


Signed-off-by: Shikhar Khattar <[email protected]>

diff -pu linux-2.6.34/crypto/authenc.c linux-2.6.34_mod/crypto/authenc.c

--- linux-2.6.34/crypto/authenc.c 2010-05-16 23:17:36.000000000 +0200
+++ linux-2.6.34_mod/crypto/authenc.c 2010-05-19 14:32:51.000000000 +0200
@@ -181,6 +181,7 @@ static void authenc_verify_ahash_update_
struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
struct authenc_request_ctx *areq_ctx = aead_request_ctx(req);
struct ahash_request *ahreq = (void *)(areq_ctx->tail + ctx->reqoff);
+ unsigned int cryptlen = req->cryptlen;

if (err)
goto out;
@@ -196,6 +197,7 @@ static void authenc_verify_ahash_update_
goto out;

authsize = crypto_aead_authsize(authenc);
+ cryptlen -= authsize;
ihash = ahreq->result + authsize;
scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
authsize, 0);
@@ -209,7 +211,7 @@ static void authenc_verify_ahash_update_
ablkcipher_request_set_callback(abreq, aead_request_flags(req),
req->base.complete, req->base.data);
ablkcipher_request_set_crypt(abreq, req->src, req->dst,
- req->cryptlen, req->iv);
+ cryptlen, req->iv);

err = crypto_ablkcipher_decrypt(abreq);

@@ -228,11 +230,13 @@ static void authenc_verify_ahash_done(st
struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
struct authenc_request_ctx *areq_ctx = aead_request_ctx(req);
struct ahash_request *ahreq = (void *)(areq_ctx->tail + ctx->reqoff);
+ unsigned int cryptlen = req->cryptlen;

if (err)
goto out;

authsize = crypto_aead_authsize(authenc);
+ cryptlen -= authsize;
ihash = ahreq->result + authsize;
scatterwalk_map_and_copy(ihash, areq_ctx->sg, areq_ctx->cryptlen,
authsize, 0);
@@ -246,7 +250,7 @@ static void authenc_verify_ahash_done(st
ablkcipher_request_set_callback(abreq, aead_request_flags(req),
req->base.complete, req->base.data);
ablkcipher_request_set_crypt(abreq, req->src, req->dst,
- req->cryptlen, req->iv);
+ cryptlen, req->iv);

err = crypto_ablkcipher_decrypt(abreq);


Thanks,
Shikhar


On Thu, May 20, 2010 at 7:17 AM, Herbert Xu <[email protected]> wrote:
> On Wed, May 19, 2010 at 03:24:24PM +0200, Shikhar wrote:
>> Hi,
>>
>> I am currently working on a driver for our crypto HW and the initial
>> aim was to offload IPSec (ESP for now) to HW. I did this by
>> registering the cipher and hash algorithms (both asynchronous)
>> separately (AES-CBC, HMAC-SHA1 only for now) and let the API handle
>> the request using the AUTHENC interface.
>>
>> The problem seems to be after the asynchronous hash request is
>> completed, the length of the ABLKCIPHER decrypt request is not
>> calculated correctly in "authenc_verify_ahash_update_done()" and
>> "authenc_verify_ahash_done()". The length should be subtracted by
>> "authsize" as done in "crypto_authenc_decrypt()".
>>
>> The following patch (applied against 2.6.34) should fix it. I hope I
>> didn't miss anything.
>
> Your patch looks good to me. ?Could you please resubmit with
> a sign-off (see Documentation/SubmittingPatches)?
>
> Thanks!
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
>



--
There's no place like 127.0.0.1

2010-05-20 09:42:12

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] Possible bug in AUTHENC

On Thu, May 20, 2010 at 11:26:00AM +0200, Shikhar wrote:
> crypto: authenc - Fix "cryptlen" calculation.
>
> This patch (applied against 2.6.34) fixes the calculation of the
> length of the ABLKCIPHER decrypt request ("cryptlen") after an
> asynchronous hash request has been completed in the AUTHENC interface.
>
> Signed-off-by: Shikhar Khattar <[email protected]>

Patch applied. Thanks a lot!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt