2011-02-07 09:28:25

by Herbert Xu

[permalink] [raw]
Subject: crypto: sha-s390 - Reset index after processing partial block

Hi:

This patch fixes an old but nasty bug in the sha-s390 code.

commit 9d20b571f5bda7273656e1b86ef91eddc94adacc
Author: Herbert Xu <[email protected]>
Date: Mon Feb 7 20:26:06 2011 +1100

crypto: sha-s390 - Reset index after processing partial block

The partial block handling in sha-s390 is broken when we get a
partial block that is followed by an update which fills it with
bytes left-over. Instead of storing the newly left-over bytes
at the start of the buffer, it will be stored immediately after
the previous partial block.

This patch fixes this by resetting the index pointer.

Signed-off-by: Herbert Xu <[email protected]>

diff --git a/arch/s390/crypto/sha_common.c b/arch/s390/crypto/sha_common.c
index f42dbab..48884f8 100644
--- a/arch/s390/crypto/sha_common.c
+++ b/arch/s390/crypto/sha_common.c
@@ -38,6 +38,7 @@ int s390_sha_update(struct shash_desc *desc, const u8 *data, unsigned int len)
BUG_ON(ret != bsize);
data += bsize - index;
len -= bsize - index;
+ index = 0;
}

/* process as many blocks as possible */

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


2011-02-07 13:47:53

by Jan Glauber

[permalink] [raw]
Subject: Re: crypto: sha-s390 - Reset index after processing partial block

Ouch.

Acked-by: Jan Glauber <[email protected]>

On Mon, 2011-02-07 at 20:28 +1100, Herbert Xu wrote:
> Hi:
>
> This patch fixes an old but nasty bug in the sha-s390 code.
>
> commit 9d20b571f5bda7273656e1b86ef91eddc94adacc
> Author: Herbert Xu <[email protected]>
> Date: Mon Feb 7 20:26:06 2011 +1100
>
> crypto: sha-s390 - Reset index after processing partial block
>
> The partial block handling in sha-s390 is broken when we get a
> partial block that is followed by an update which fills it with
> bytes left-over. Instead of storing the newly left-over bytes
> at the start of the buffer, it will be stored immediately after
> the previous partial block.
>
> This patch fixes this by resetting the index pointer.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/arch/s390/crypto/sha_common.c b/arch/s390/crypto/sha_common.c
> index f42dbab..48884f8 100644
> --- a/arch/s390/crypto/sha_common.c
> +++ b/arch/s390/crypto/sha_common.c
> @@ -38,6 +38,7 @@ int s390_sha_update(struct shash_desc *desc, const u8 *data, unsigned int len)
> BUG_ON(ret != bsize);
> data += bsize - index;
> len -= bsize - index;
> + index = 0;
> }
>
> /* process as many blocks as possible */
>
> Cheers,

2011-02-17 03:26:04

by Herbert Xu

[permalink] [raw]
Subject: crypto: sha1 - Add test vector to test partial block processing

Hi:

I'm going to add this patch to prevent such issues in future.

commit bd1f2996b44a1c8bde76a6fecd10f36b6eb948d7
Author: Herbert Xu <[email protected]>
Date: Thu Feb 17 14:24:45 2011 +1100

crypto: sha1 - Add test vector to test partial block processing

In light of the recent discovery of the bug with partial block
processing on s390, we need best test coverage for that. This
patch adds a test vector for SHA1 that should catch such problems.

Signed-off-by: Herbert Xu <[email protected]>

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 834af7f..aa6dac0 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -451,8 +451,9 @@ static struct hash_testvec rmd320_tv_template[] = {

/*
* SHA1 test vectors from from FIPS PUB 180-1
+ * Long vector from CAVS 5.0
*/
-#define SHA1_TEST_VECTORS 2
+#define SHA1_TEST_VECTORS 3

static struct hash_testvec sha1_tv_template[] = {
{
@@ -467,6 +468,33 @@ static struct hash_testvec sha1_tv_template[] = {
"\x4a\xa1\xf9\x51\x29\xe5\xe5\x46\x70\xf1",
.np = 2,
.tap = { 28, 28 }
+ }, {
+ .plaintext = "\xec\x29\x56\x12\x44\xed\xe7\x06"
+ "\xb6\xeb\x30\xa1\xc3\x71\xd7\x44"
+ "\x50\xa1\x05\xc3\xf9\x73\x5f\x7f"
+ "\xa9\xfe\x38\xcf\x67\xf3\x04\xa5"
+ "\x73\x6a\x10\x6e\x92\xe1\x71\x39"
+ "\xa6\x81\x3b\x1c\x81\xa4\xf3\xd3"
+ "\xfb\x95\x46\xab\x42\x96\xfa\x9f"
+ "\x72\x28\x26\xc0\x66\x86\x9e\xda"
+ "\xcd\x73\xb2\x54\x80\x35\x18\x58"
+ "\x13\xe2\x26\x34\xa9\xda\x44\x00"
+ "\x0d\x95\xa2\x81\xff\x9f\x26\x4e"
+ "\xcc\xe0\xa9\x31\x22\x21\x62\xd0"
+ "\x21\xcc\xa2\x8d\xb5\xf3\xc2\xaa"
+ "\x24\x94\x5a\xb1\xe3\x1c\xb4\x13"
+ "\xae\x29\x81\x0f\xd7\x94\xca\xd5"
+ "\xdf\xaf\x29\xec\x43\xcb\x38\xd1"
+ "\x98\xfe\x4a\xe1\xda\x23\x59\x78"
+ "\x02\x21\x40\x5b\xd6\x71\x2a\x53"
+ "\x05\xda\x4b\x1b\x73\x7f\xce\x7c"
+ "\xd2\x1c\x0e\xb7\x72\x8d\x08\x23"
+ "\x5a\x90\x11",
+ .psize = 163,
+ .digest = "\x97\x01\x11\xc4\xe7\x7b\xcc\x88\xcc\x20"
+ "\x45\x9c\x02\xb6\x9b\x4a\xa8\xf5\x82\x17",
+ .np = 4,
+ .tap = { 63, 64, 31, 5 }
}
};

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

2011-02-17 15:11:09

by Jan Glauber

[permalink] [raw]
Subject: Re: crypto: sha1 - Add test vector to test partial block processing

Hi Herbert,

good idea. I ended up with using openssl to create a test vector for
that special case but a published test vector is of course better.

With your sha patch applied sha1_s390 survives the tcrypt test.

--Jan

On Thu, 2011-02-17 at 14:25 +1100, Herbert Xu wrote:
> Hi:
>
> I'm going to add this patch to prevent such issues in future.
>
> commit bd1f2996b44a1c8bde76a6fecd10f36b6eb948d7
> Author: Herbert Xu <[email protected]>
> Date: Thu Feb 17 14:24:45 2011 +1100
>
> crypto: sha1 - Add test vector to test partial block processing
>
> In light of the recent discovery of the bug with partial block
> processing on s390, we need best test coverage for that. This
> patch adds a test vector for SHA1 that should catch such problems.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/crypto/testmgr.h b/crypto/testmgr.h
> index 834af7f..aa6dac0 100644
> --- a/crypto/testmgr.h
> +++ b/crypto/testmgr.h
> @@ -451,8 +451,9 @@ static struct hash_testvec rmd320_tv_template[] = {
>
> /*
> * SHA1 test vectors from from FIPS PUB 180-1
> + * Long vector from CAVS 5.0
> */
> -#define SHA1_TEST_VECTORS 2
> +#define SHA1_TEST_VECTORS 3
>
> static struct hash_testvec sha1_tv_template[] = {
> {
> @@ -467,6 +468,33 @@ static struct hash_testvec sha1_tv_template[] = {
> "\x4a\xa1\xf9\x51\x29\xe5\xe5\x46\x70\xf1",
> .np = 2,
> .tap = { 28, 28 }
> + }, {
> + .plaintext = "\xec\x29\x56\x12\x44\xed\xe7\x06"
> + "\xb6\xeb\x30\xa1\xc3\x71\xd7\x44"
> + "\x50\xa1\x05\xc3\xf9\x73\x5f\x7f"
> + "\xa9\xfe\x38\xcf\x67\xf3\x04\xa5"
> + "\x73\x6a\x10\x6e\x92\xe1\x71\x39"
> + "\xa6\x81\x3b\x1c\x81\xa4\xf3\xd3"
> + "\xfb\x95\x46\xab\x42\x96\xfa\x9f"
> + "\x72\x28\x26\xc0\x66\x86\x9e\xda"
> + "\xcd\x73\xb2\x54\x80\x35\x18\x58"
> + "\x13\xe2\x26\x34\xa9\xda\x44\x00"
> + "\x0d\x95\xa2\x81\xff\x9f\x26\x4e"
> + "\xcc\xe0\xa9\x31\x22\x21\x62\xd0"
> + "\x21\xcc\xa2\x8d\xb5\xf3\xc2\xaa"
> + "\x24\x94\x5a\xb1\xe3\x1c\xb4\x13"
> + "\xae\x29\x81\x0f\xd7\x94\xca\xd5"
> + "\xdf\xaf\x29\xec\x43\xcb\x38\xd1"
> + "\x98\xfe\x4a\xe1\xda\x23\x59\x78"
> + "\x02\x21\x40\x5b\xd6\x71\x2a\x53"
> + "\x05\xda\x4b\x1b\x73\x7f\xce\x7c"
> + "\xd2\x1c\x0e\xb7\x72\x8d\x08\x23"
> + "\x5a\x90\x11",
> + .psize = 163,
> + .digest = "\x97\x01\x11\xc4\xe7\x7b\xcc\x88\xcc\x20"
> + "\x45\x9c\x02\xb6\x9b\x4a\xa8\xf5\x82\x17",
> + .np = 4,
> + .tap = { 63, 64, 31, 5 }
> }
> };
>
> Thanks,

2011-02-17 20:52:32

by Herbert Xu

[permalink] [raw]
Subject: Re: crypto: sha1 - Add test vector to test partial block processing

On Thu, Feb 17, 2011 at 04:11:02PM +0100, Jan Glauber wrote:
> Hi Herbert,
>
> good idea. I ended up with using openssl to create a test vector for
> that special case but a published test vector is of course better.
>
> With your sha patch applied sha1_s390 survives the tcrypt test.

Could you check if sha1_s390 fails the test without the patch?

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

2011-02-18 09:38:08

by Jan Glauber

[permalink] [raw]
Subject: Re: crypto: sha1 - Add test vector to test partial block processing

On Fri, 2011-02-18 at 07:52 +1100, Herbert Xu wrote:
> On Thu, Feb 17, 2011 at 04:11:02PM +0100, Jan Glauber wrote:
> > Hi Herbert,
> >
> > good idea. I ended up with using openssl to create a test vector for
> > that special case but a published test vector is of course better.
> >
> > With your sha patch applied sha1_s390 survives the tcrypt test.
>
> Could you check if sha1_s390 fails the test without the patch?

Without 9d20b57 it gives:

[root@h4245005 ~]# modprobe tcrypt mode=2
FATAL: Error inserting tcrypt (/lib/modules/2.6.38-rc4-dirty/kernel/crypto/tcrypt.ko): Resource temporarily unavailable
[root@h4245005 ~]# tail -f /var/log/messages
...
Feb 18 10:22:13 h4245005 kernel: alg: hash: Chunking test 2 failed for sha1-s390
Feb 18 10:22:13 h4245005 kernel: 00000000: 4a d0 d6 bd 53 2b 6b df cd 34 b9 60 d1 90 85 d0
Feb 18 10:22:13 h4245005 kernel: 00000010: 38 70 2b 41

With the patch applied nothing shows up in dmesg.

--Jan


> Thanks!

2011-02-18 09:43:31

by Herbert Xu

[permalink] [raw]
Subject: Re: crypto: sha1 - Add test vector to test partial block processing

On Fri, Feb 18, 2011 at 09:37:17AM +0000, Jan Glauber wrote:
>
> Without 9d20b57 it gives:
>
> [root@h4245005 ~]# modprobe tcrypt mode=2
> FATAL: Error inserting tcrypt (/lib/modules/2.6.38-rc4-dirty/kernel/crypto/tcrypt.ko): Resource temporarily unavailable
> [root@h4245005 ~]# tail -f /var/log/messages
> ...
> Feb 18 10:22:13 h4245005 kernel: alg: hash: Chunking test 2 failed for sha1-s390
> Feb 18 10:22:13 h4245005 kernel: 00000000: 4a d0 d6 bd 53 2b 6b df cd 34 b9 60 d1 90 85 d0
> Feb 18 10:22:13 h4245005 kernel: 00000010: 38 70 2b 41
>
> With the patch applied nothing shows up in dmesg.
>
> --Jan

Excellent. Thanks a lot for testing Jan!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt