crypto_ahash_setkey

crypto_ahash_setkey

[Kasatkin, Dmitry]

Hi,

I have noticed very odd behavior with hmac calculation on my dual
core, 4 HTs PC.
I am using async hash API to to calculate hmac over the page.
I am using "hmac(sha1)" and the same key to calculate different pages.

I have a work queue, which calculates the hmac like...

int()
{
    tfm = crypto_alloc_ahash(...);
}

work_task()
{
     crypto_ahash_setkey(tfm, key, keylen);
     crypto_ahash_digest(req);
}

HMAC result "sometimes" is incorrect.

But when I move crypto_ahash_setkey() do the initialization code then
HMAC result is always correct...
(key is the same, so I can initialize it only once)

int()
{
     tfm = crypto_alloc_ahash(...);
     crypto_ahash_setkey(tfm, key, keylen);
}

work_task()
{
     crypto_ahash_digest(req);
}

It seems that crypto_ahash_setkey() somehow sometimes does wrong things...

I hope my explanation is clear.

Any ideas why it might happen?

Thanks,

- Dmitry

Re: crypto_ahash_setkey

[Steffen Klassert]

Hi.

On Wed, Nov 23, 2011 at 12:00:29AM +0200, Kasatkin, Dmitry wrote:
> Hi,
> 
> I have noticed very odd behavior with hmac calculation on my dual
> core, 4 HTs PC.
> I am using async hash API to to calculate hmac over the page.
> I am using "hmac(sha1)" and the same key to calculate different pages.
> 
> I have a work queue, which calculates the hmac like...
> 
> int()
> {
>     tfm = crypto_alloc_ahash(...);
> }
> 
> work_task()
> {
>      crypto_ahash_setkey(tfm, key, keylen);
>      crypto_ahash_digest(req);
> }
> 
> HMAC result "sometimes" is incorrect.

Looks like a race. HMAC precalculates the hash of the ipaded/opaded key
and saves this hash on the transform. So the setkey method should be used
just once in the initialization path.

> 
> But when I move crypto_ahash_setkey() do the initialization code then
> HMAC result is always correct...
> (key is the same, so I can initialize it only once)
> 
> int()
> {
>      tfm = crypto_alloc_ahash(...);
>      crypto_ahash_setkey(tfm, key, keylen);
> }

That's how it should be. And in this case it works, as you
already noticed :)

Re: crypto_ahash_setkey

[Herbert Xu]

Kasatkin, Dmitry <dmitry.kasatkin@intel.com> wrote:
> Hi,
> 
> I have noticed very odd behavior with hmac calculation on my dual
> core, 4 HTs PC.
> I am using async hash API to to calculate hmac over the page.
> I am using "hmac(sha1)" and the same key to calculate different pages.
> 
> I have a work queue, which calculates the hmac like...
> 
> int()
> {
>    tfm = crypto_alloc_ahash(...);
> }
> 
> work_task()
> {
>     crypto_ahash_setkey(tfm, key, keylen);
>     crypto_ahash_digest(req);
> }
> 
> HMAC result "sometimes" is incorrect.

Is your tfm shared by multiple instances of work_task? The key
is a property of the tfm, so you cannot have multiple users of a
tfm all changing keys at the same time.

The correct usage model allowing parallel use is to dedicate a
tfm to each key.

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: crypto_ahash_setkey

[Kasatkin, Dmitry]

Hi,

On Wed, Nov 23, 2011 at 9:44 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Kasatkin, Dmitry <dmitry.kasatkin@intel.com> wrote:
>> Hi,
>>
>> I have noticed very odd behavior with hmac calculation on my dual
>> core, 4 HTs PC.
>> I am using async hash API to to calculate hmac over the page.
>> I am using "hmac(sha1)" and the same key to calculate different pages.
>>
>> I have a work queue, which calculates the hmac like...
>>
>> int()
>> {
>>    tfm = crypto_alloc_ahash(...);
>> }
>>
>> work_task()
>> {
>>     crypto_ahash_setkey(tfm, key, keylen);
>>     crypto_ahash_digest(req);
>> }
>>
>> HMAC result "sometimes" is incorrect.
>
> Is your tfm shared by multiple instances of work_task? The key
> is a property of the tfm, so you cannot have multiple users of a
> tfm all changing keys at the same time.

Yes. I know that....
Work queue also uses WQ_NON_REENTRANT, so it should not be any races,
and it should be possible to call crypto_ahash_setkey() without any problems.

This problem happens on 64 bit kernel.
It does not happen on my QEMU 32 bit kernel,

I will do some more tests...

>
> The correct usage model allowing parallel use is to dedicate a
> tfm to each key.
>

Sure.

Thanks...

> 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: crypto_ahash_setkey

[Kasatkin, Dmitry]

On Wed, Nov 23, 2011 at 8:08 AM, Steffen Klassert
<steffen.klassert@secunet.com> wrote:
> Hi.
>
> On Wed, Nov 23, 2011 at 12:00:29AM +0200, Kasatkin, Dmitry wrote:
>> Hi,
>>
>> I have noticed very odd behavior with hmac calculation on my dual
>> core, 4 HTs PC.
>> I am using async hash API to to calculate hmac over the page.
>> I am using "hmac(sha1)" and the same key to calculate different pages.
>>
>> I have a work queue, which calculates the hmac like...
>>
>> int()
>> {
>>     tfm = crypto_alloc_ahash(...);
>> }
>>
>> work_task()
>> {
>>      crypto_ahash_setkey(tfm, key, keylen);
>>      crypto_ahash_digest(req);
>> }
>>
>> HMAC result "sometimes" is incorrect.
>
> Looks like a race. HMAC precalculates the hash of the ipaded/opaded key
> and saves this hash on the transform. So the setkey method should be used
> just once in the initialization path.
>
>>
>> But when I move crypto_ahash_setkey() do the initialization code then
>> HMAC result is always correct...
>> (key is the same, so I can initialize it only once)
>>
>> int()
>> {
>>      tfm = crypto_alloc_ahash(...);
>>      crypto_ahash_setkey(tfm, key, keylen);
>> }
>
> That's how it should be. And in this case it works, as you
> already noticed :)
>
>

Thanks...
See my another reply...

Re: crypto_ahash_setkey

[Herbert Xu]

On Wed, Nov 23, 2011 at 11:08:29AM +0200, Kasatkin, Dmitry wrote:
>
> Yes. I know that....
> Work queue also uses WQ_NON_REENTRANT, so it should not be any races,
> and it should be possible to call crypto_ahash_setkey() without any problems.
> 
> This problem happens on 64 bit kernel.
> It does not happen on my QEMU 32 bit kernel,
> 
> I will do some more tests...

Which particular hmac(sha1) implementation are you using?

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: crypto_ahash_setkey

[Kasatkin, Dmitry]

On Wed, Nov 23, 2011 at 12:07 PM, Herbert Xu
<herbert@gondor.apana.org.au> wrote:
> On Wed, Nov 23, 2011 at 11:08:29AM +0200, Kasatkin, Dmitry wrote:
>>
>> Yes. I know that....
>> Work queue also uses WQ_NON_REENTRANT, so it should not be any races,
>> and it should be possible to call crypto_ahash_setkey() without any problems.
>>

Ok... I found it out..
There is no magic here.
For some reason work_task is called on different CPUs simultaneously.
What is the purpose of WQ_NON_REENTRANT flag then?

- Dmitry


>> This problem happens on 64 bit kernel.
>> It does not happen on my QEMU 32 bit kernel,
>>
>> I will do some more tests...
>
> Which particular hmac(sha1) implementation are you using?
>
> 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
>