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

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

[Herbert Xu]

Hi:

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

commit 9d20b571f5bda7273656e1b86ef91eddc94adacc
Author: Herbert Xu <herbert@gondor.apana.org.au>
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 <herbert@gondor.apana.org.au>

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 <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: sha-s390 - Reset index after processing partial block

[Jan Glauber]

Ouch.

Acked-by: Jan Glauber <jang@linux.vnet.ibm.com>

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 <herbert@gondor.apana.org.au>
> 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 <herbert@gondor.apana.org.au>
> 
> 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,

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

[Herbert Xu]

Hi:

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

commit bd1f2996b44a1c8bde76a6fecd10f36b6eb948d7
Author: Herbert Xu <herbert@gondor.apana.org.au>
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 <herbert@gondor.apana.org.au>

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 <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: sha1 - Add test vector to test partial block processing

[Jan Glauber]

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 <herbert@gondor.apana.org.au>
> 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 <herbert@gondor.apana.org.au>
> 
> 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,

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

[Herbert Xu]

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 <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: sha1 - Add test vector to test partial block processing

[Jan Glauber]

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!

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

[Herbert Xu]

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt