Tcrypt hmac(crc32) test can work only on Blackfin

DISCUSSION Milan Broz 3 participants 7 messages Nov 20, 2012 – Dec 6, 2012 Crypto API

lore

Share & tools

lore

Milan Broz

author

2012-11-20 14:04 UTC #0 lore raw

Hi,

commit a482b081a2d4d74d16bc9ea8779f9f6055f95852

Author: Sonic Zhang <[email protected]>
Date:   Fri May 25 17:54:13 2012 +0800

crypto: testmgr - Add new test cases for Blackfin CRC crypto driver

added tcrypt mode=110 test for hmac(crc32)

It seems, that this mode is only directly implemented by Blackfin driver and must fail on all other architectures because:

- nobody implements "crc32" but "crc32c"

- the block size is 1 and digest size is 4 for crc32[c], so hmac(crc32c) must fail because ds > block_size (test in hmac_create)

And it doesn't fail here because you implement it directly and not using hmac.c...

Just try modprobe tcrypt mode=110 on so other arch
alg: hash: Failed to load transform for hmac(crc32): -2

Why was such one-device dependent test vector added?

IMHO either it should provide crc32c (as do generic, sparc or x86 hw drivers) or the block size for crc32c should be 4... (and test vector is wrong then)

But now we have test vector which must fail for most of systems.

Milan

Zhang, Sonic

2012-11-21 11:33 UTC #1 lore raw

Quoted 26 lines

-----Original Message-----
From: Milan Broz [mailto:[email protected]]
Sent: Tuesday, November 20, 2012 10:04 PM
To: [email protected]
Cc: Herbert Xu; Zhang, Sonic
Subject: Tcrypt hmac(crc32) test can work only on Blackfin
Hi,
commit a482b081a2d4d74d16bc9ea8779f9f6055f95852
Author: Sonic Zhang <[email protected]>
Date:   Fri May 25 17:54:13 2012 +0800
crypto: testmgr - Add new test cases for Blackfin CRC crypto driver
added tcrypt mode=110 test for hmac(crc32)
It seems, that this mode is only directly implemented by Blackfin driver and must fail on all other architectures because:
- nobody implements "crc32" but "crc32c"
- the block size is 1 and digest size is 4 for crc32[c], so hmac(crc32c) must fail because ds > block_size (test in hmac_create)

Quoted 7 lines

And it doesn't fail here because you implement it directly and not using hmac.c...
Just try modprobe tcrypt mode=110 on so other arch
alg: hash: Failed to load transform for hmac(crc32): -2
Why was such one-device dependent test vector added?

Blackfin CRC engine is flexible to support many CRC32 polynomials (include crc32c) and is able to append 0 to any block size which is not a multiple of 4. This Blackfin CRC engine specific test vector show these capabilities. Is there a policy that the CRC test vector in testmgr.h should support all CRC drivers? If so, I am fine to drop this test vector.

Quoted 3 lines

IMHO either it should provide crc32c (as do generic, sparc or x86 hw drivers) or the block size for crc32c should be 4... (and test vector is wrong then)

I can set the default polynomials to that of crc32c for blackfin crc32 driver. But, it should not be limited to crc32c.

Regards,

Sonic

Quoted 3 lines

But now we have test vector which must fail for most of systems.
Milan

Milan Broz

author

2012-11-21 12:02 UTC #2 lore raw

Quoted 3 lines · Sonic

Is there a policy that the CRC test vector
in testmgr.h should support all CRC drivers?
If so, I am fine to drop this test vector.

Question for Herbert...
But the problem I see is that it confuses people, it simply returns fail everytime (except Blackfin platform).

My mail originates from bug report "tcrypt tests started failing", and it was not obvious why (for people not familiar with tcrypt internals, they just tried all test modes available).

Quoted 6 lines

Quoted 3 lines
IMHO either it should provide crc32c (as do generic, sparc or x86 hw drivers) or the block size for crc32c should be 4... (and test vector is wrong then)
I can set the default polynomials to that of crc32c for blackfin crc32 driver. But, it should not be limited to crc32c.

Sure, my question was just about test vector, hw driver support for something more is of course good thing.

But it would be nice to have "crc32c" directly supported/accelerated as well.

Thanks,
Milan

Herbert Xu

maintainer

2012-11-22 22:40 UTC #3 lore raw

Quoted 6 lines · Milan Broz

Quoted 3 lines · Sonic
Is there a policy that the CRC test vector
in testmgr.h should support all CRC drivers?
If so, I am fine to drop this test vector.

Quoted 7 lines

Question for Herbert...
But the problem I see is that it confuses people, it simply returns fail everytime (except Blackfin platform).
My mail originates from bug report "tcrypt tests started failing", and it was not obvious why (for people not familiar with tcrypt internals, they just tried all test modes available).

We should just make a generic version of crc32 available.

Thanks,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Zhang, Sonic

2012-11-22 18:42 UTC #4 lore raw

Quoted 20 lines

-----Original Message-----
From: Herbert Xu [mailto:[email protected]]
Sent: Thursday, November 22, 2012 10:33 AM
To: Milan Broz
Cc: Zhang, Sonic; [email protected]
Subject: Re: Tcrypt hmac(crc32) test can work only on Blackfin
Quoted 10 lines · Milan Broz
Quoted 3 lines · Sonic
Is there a policy that the CRC test vector in testmgr.h should support all CRC drivers?
If so, I am fine to drop this test vector.
Question for Herbert...
But the problem I see is that it confuses people, it simply returns fail everytime (except Blackfin platform).

Quoted 6 lines

Quoted 3 lines
My mail originates from bug report "tcrypt tests started failing", and it was not obvious why (for people not familiar with tcrypt internals, they just tried all test modes available).
We should just make a generic version of crc32 available.

Hi Herbert and Broz,

Unfortunately, after investigating the hardware manual of Blackfin CRC engine and the CRC algorithm, I have to say the Blackfin CRC driver can't use current crc32c test vector in testmgr.h. Blackfin CRC engine only supports big endian CRC algorithm, while current crypto crc32c driver uses little endian CRC algorithm. The big endian CRC digest is different from that of the little endian.

There are 2 solutions. One is to add a big endian crc32c test vector. The other is to keep the Blackfin specific test vector. However, neither meets Broz's expectation of a generic crc32 test vector.

Any idea?

Regard,

Sonic

Herbert Xu

maintainer

2012-12-06 09:07 UTC #5 lore raw

Quoted 19 lines · Sonic

Quoted 12 lines · Milan Broz
Quoted 10 lines · Sonic
Quoted 3 lines
Is there a policy that the CRC test vector in testmgr.h should support all CRC drivers?
If so, I am fine to drop this test vector.
Question for Herbert...
But the problem I see is that it confuses people, it simply returns fail everytime (except Blackfin platform).
Quoted 6 lines
Quoted 3 lines
My mail originates from bug report "tcrypt tests started failing", and it was not obvious why (for people not familiar with tcrypt internals, they just tried all test modes available).
We should just make a generic version of crc32 available.

Quoted 7 lines

Hi Herbert and Broz,
Unfortunately, after investigating the hardware manual of Blackfin CRC engine and the CRC algorithm, I have to say the Blackfin CRC driver can't use current crc32c test vector in testmgr.h. Blackfin CRC engine only supports big endian CRC algorithm, while current crypto crc32c driver uses little endian CRC algorithm. The big endian CRC digest is different from that of the little endian.
There are 2 solutions. One is to add a big endian crc32c test vector. The other is to keep the Blackfin specific test vector. However, neither meets Broz's expectation of a generic crc32 test vector.
Any idea?

I think we should just remove the blackfin crc driver altogether as nothing in the kernel ever uses hmac(crc32).

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Zhang, Sonic

2012-12-06 09:59 UTC #6 lore raw

Hi Herbert,

Quoted 35 lines

-----Original Message-----
From: Herbert Xu [mailto:[email protected]]
Sent: Thursday, December 06, 2012 5:07 PM
To: Zhang, Sonic
Cc: Milan Broz; [email protected]
Subject: Re: Tcrypt hmac(crc32) test can work only on Blackfin
Quoted 16 lines · Sonic
Quoted 16 lines · Milan Broz
Quoted 13 lines · Sonic
Quoted 3 lines
Is there a policy that the CRC test vector in testmgr.h should support all CRC drivers?
If so, I am fine to drop this test vector.
Question for Herbert...
But the problem I see is that it confuses people, it simply returns fail everytime (except Blackfin platform).
My mail originates from bug report "tcrypt tests started failing", and it was not obvious why (for people not familiar with tcrypt internals, they just tried all test modes available).
Quoted 6 lines
Quoted 2 lines
We should just make a generic version of crc32 available.
Hi Herbert and Broz,
Unfortunately, after investigating the hardware manual of Blackfin CRC engine
and the CRC algorithm, I have to say the Blackfin CRC driver can't use current crc32c test vector in testmgr.h. Blackfin CRC engine only supports big endian CRC algorithm, while current crypto crc32c driver uses little endian CRC algorithm. The big endian CRC digest is different from that of the little endian.
Quoted 1 line

Quoted 9 lines

Quoted 1 line
There are 2 solutions. One is to add a big endian crc32c test vector. The other
is to keep the Blackfin specific test vector. However, neither meets Broz's expectation of a generic crc32 test vector.
Quoted 2 lines
Any idea?
I think we should just remove the blackfin crc driver altogether as nothing in the kernel ever uses hmac(crc32).

Blackfin Linux applications may use the crypto bfin_crc driver. I wish you could remove the blackfin crc test patch from testmgr, while keeping the driver as it is.

Thanks,

Sonic

Quoted 5 lines

Cheers,
--
Email: Herbert Xu <[email protected]> Home Page:
http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt