tcrypt: hmac test with keys > blocksize fail

tcrypt: hmac test with keys > blocksize fail

[Adrian-Ken Rüegsegger]

Hi,

using the cryptodev-2.6 tree I noticed that the hmac tests that have
keys larger than blocksize for md5 and the various sha algorithms all
fail (tcrypt mode=10[0-5]). The other tests seem to pass just fine.

The issue seems to have come from commit
de224c309b5631bdaae3fcd6880cfb93b52f5a53.


I have tested 48c8949ea8460216783dd33640121187b9531b60 which does not
contain this bug.  It's just before the various tcrypt-changes from
Sebastian Siewior.

-Adrian

Re: tcrypt: hmac test with keys > blocksize fail

[Sebastian Siewior]

* Adrian-Ken R?egsegger | 2008-05-03 02:10:34 [+0200]:

>Hi,
Hi Adrian-Ken,

>using the cryptodev-2.6 tree I noticed that the hmac tests that have
>keys larger than blocksize for md5 and the various sha algorithms all
>fail (tcrypt mode=10[0-5]). The other tests seem to pass just fine.
>
>The issue seems to have come from commit
>de224c309b5631bdaae3fcd6880cfb93b52f5a53.
Is this a bisect result?

>
>I have tested 48c8949ea8460216783dd33640121187b9531b60 which does not
>contain this bug.  It's just before the various tcrypt-changes from
>Sebastian Siewior.
I tested it with tcrypt mode=0 what covers 100-105 but it may slip
through. I will look into this later.

>
>-Adrian

Sebastian

Re: tcrypt: hmac test with keys > blocksize fail

[Adrian-Ken Rüegsegger]

Sebastian Siewior wrote:
> * Adrian-Ken R?egsegger | 2008-05-03 02:10:34 [+0200]:
> 
>> Hi,
> Hi Adrian-Ken,

Hello Sebastian,

>> using the cryptodev-2.6 tree I noticed that the hmac tests that have
>> keys larger than blocksize for md5 and the various sha algorithms all
>> fail (tcrypt mode=10[0-5]). The other tests seem to pass just fine.
>>
>> The issue seems to have come from commit
>> de224c309b5631bdaae3fcd6880cfb93b52f5a53.
> Is this a bisect result?

No, this was a guess of mine, since I tested the two revisions just
prior and after the three tcrypt-commits from you. I did a full bisect
and the actual guilty commit is:

[562954d5e01d08154cf15c7e12e6e9ec803f50f7] [CRYPTO] tcrypt: Change the
usage of the test vectors

>> I have tested 48c8949ea8460216783dd33640121187b9531b60 which does not
>> contain this bug.  It's just before the various tcrypt-changes from
>> Sebastian Siewior.
> I tested it with tcrypt mode=0 what covers 100-105 but it may slip
> through. I will look into this later.

As I mentioned, it's only the hmac tests with keys larger than blocksize
that fail.
Additionaly I just saw, that ecb(des) encryption test nr 5 passes but it
seems that the setkey operation fails with:

setkey() failed flags=100100

-Adrian

Re: tcrypt: hmac test with keys > blocksize fail

[Sebastian Siewior]

* Adrian-Ken R?egsegger | 2008-05-03 13:44:41 [+0200]:

>Sebastian Siewior wrote:
>Hello Sebastian,
Hello, 

>Additionaly I just saw, that ecb(des) encryption test nr 5 passes but it
>seems that the setkey operation fails with:
>
>setkey() failed flags=100100
That is okey, that one has to fail. The algorithm should not allow
weak keys.

>-Adrian

Sebastian

Re: tcrypt: hmac test with keys > blocksize fail

[Herbert Xu]

Adrian-Ken R??egsegger <rueegsegger@swiss-it.ch> wrote:
> 
> using the cryptodev-2.6 tree I noticed that the hmac tests that have
> keys larger than blocksize for md5 and the various sha algorithms all
> fail (tcrypt mode=10[0-5]). The other tests seem to pass just fine.
> 
> The issue seems to have come from commit
> de224c309b5631bdaae3fcd6880cfb93b52f5a53.
> 
> 
> I have tested 48c8949ea8460216783dd33640121187b9531b60 which does not
> contain this bug.  It's just before the various tcrypt-changes from
> Sebastian Siewior.

Actually this just exposed an ancient bug in hmac.  It relied
on the key to be in identity-mapped memory which has never been
guaranteed.

This patch fixes the problem for me.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/crypto/hmac.c b/crypto/hmac.c
index b60c3c7..14c6351 100644
--- a/crypto/hmac.c
+++ b/crypto/hmac.c
@@ -57,14 +57,35 @@ static int hmac_setkey(struct crypto_hash *parent,
 	if (keylen > bs) {
 		struct hash_desc desc;
 		struct scatterlist tmp;
+		int tmplen;
 		int err;
 
 		desc.tfm = tfm;
 		desc.flags = crypto_hash_get_flags(parent);
 		desc.flags &= CRYPTO_TFM_REQ_MAY_SLEEP;
-		sg_init_one(&tmp, inkey, keylen);
 
-		err = crypto_hash_digest(&desc, &tmp, keylen, digest);
+		err = crypto_hash_init(&desc);
+		if (err)
+			return err;
+
+		tmplen = bs * 2 + ds;
+		sg_init_one(&tmp, ipad, tmplen);
+
+		for (; keylen > tmplen; inkey += tmplen, keylen -= tmplen) {
+			memcpy(ipad, inkey, tmplen);
+			err = crypto_hash_update(&desc, &tmp, tmplen);
+			if (err)
+				return err;
+		}
+
+		if (keylen) {
+			memcpy(ipad, inkey, keylen);
+			err = crypto_hash_update(&desc, &tmp, keylen);
+			if (err)
+				return err;
+		}
+
+		err = crypto_hash_final(&desc, digest);
 		if (err)
 			return err;
 

Re: tcrypt: hmac test with keys > blocksize fail

[Adrian-Ken Rüegsegger]

Herbert Xu wrote:
  > Actually this just exposed an ancient bug in hmac.  It relied
> on the key to be in identity-mapped memory which has never been
> guaranteed.
> 
> This patch fixes the problem for me.

I have tested the patch and it resolves the issue indeed. It also let's 
the same RIPEMD hmac tests (see my submitted patches) run correctly.

Thanks alot!

-Adrian

Re: tcrypt: hmac test with keys > blocksize fail

[Sebastian Siewior]

* Herbert Xu | 2008-05-06 18:28:15 [+0800]:

>Actually this just exposed an ancient bug in hmac.  It relied
>on the key to be in identity-mapped memory which has never been
>guaranteed.

huh. Thanks Herbert. I haven't seen this on 32bit machine.

>Thanks,

Sebastian