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
* 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
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
* 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
Adrian-Ken R??egsegger <[email protected]> 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~} <[email protected]>
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;
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
* 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