Skip to contentSkip to conversation

vmac test fails on s390

PATCH In review Jan Glauber 3 participants 10 messages Jan 14, 2010 – Mar 16, 2010 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hi,

Jan 14 14:47:38 h42lp52 kernel: alg: hash: Test 1 failed for vmac(aes-s390)
Jan 14 14:47:38 h42lp52 kernel: 00000000: e7 79 33 b7 fd 8a d7 cb

Looking at the digest from the failing test vector:

.digest = "\xcb\xd7\x8a\xfd\xb7\x33\x79\xe7",

the output looks somehow familiar... Maybe a missing endian conversion? The patch fixes the test for me but it is just a wild guess and maybe it should be done at another place?

Jan

crypto/vmac.c +1 -1
index 0a9468e..0e23dfb 100644
@@ -359,7 +359,7 @@ static u64 l3hash(u64 p1, u64 p2,
 	rl += t;
 	rl += (0 - (rl < t)) & 257;
 	rl += (0 - (rl > p64-1)) & 257;
-	return rl;
+	return le64_to_cpup(&rl);
 }
 
 static void vhash_update(const unsigned char *m,
maintainer
#1 lore raw
Quoted 5 lines · Jan Glauber

Hi,

Jan 14 14:47:38 h42lp52 kernel: alg: hash: Test 1 failed for vmac(aes-s390)
Jan 14 14:47:38 h42lp52 kernel: 00000000: e7 79 33 b7 fd 8a d7 cb
Quoted 7 lines

Looking at the digest from the failing test vector:

.digest = "\xcb\xd7\x8a\xfd\xb7\x33\x79\xe7",

the output looks somehow familiar... Maybe a missing endian conversion? The patch fixes the test for me but it is just a wild guess and maybe it should be done at another place?

Shane, please look into this.

Thanks,

Signature · 4 lines
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
#2 lore raw

PATCH 1/1 to fix vmac test fails on s390

Herbert,

Here is the patch to fix it. I missed an important fact that the digest of vmac is not a string or a char array, but an 64bit integer.

This patch is to fix the vmac algorithm, add more test cases for vmac, and fix the test failure on some big endian system like s390. (the attachment is for applying the patch)

crypto/testmgr.h |   96 +++++++++++++++++++++++++++++++++++++++++++--
crypto/vmac.c    |   74 ++++++++++++++++------------------
2 files changed, 128 insertions(+), 42 deletions(-)
crypto/testmgr.h +92 -4
diff -r 91636f5ce7a2 crypto/testmgr.h
@@ -1669,17 +1669,105 @@ static struct hash_testvec aes_xcbc128_t
 	}
 };
 
-#define VMAC_AES_TEST_VECTORS	1
-static char vmac_string[128] = {'\x01', '\x01', '\x01', '\x01',
+#define VMAC_AES_TEST_VECTORS	8
+static char vmac_string1[128] = {'\x01', '\x01', '\x01', '\x01',
 				'\x02', '\x03', '\x02', '\x02',
 				'\x02', '\x04', '\x01', '\x07',
 				'\x04', '\x01', '\x04', '\x03',};
+static char vmac_string2[128] = {'a', 'b', 'c',};
+static char vmac_string3[128] = {'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				};
+
 static struct hash_testvec aes_vmac128_tv_template[] = {
 	{
+		.key	= "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = NULL,
+#ifdef __LITTLE_ENDIAN
+		.digest	= "\x07\x58\x80\x35\x77\xa4\x7b\x54",
+#else
+		.digest = "\x54\x7b\xa4\x77\x35\x80\x58\x07",
+#endif
+		.psize	= 0,
+		.ksize	= 16,
+	}, {
 		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
 			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
-		.plaintext = vmac_string,
-		.digest = "\xcb\xd7\x8a\xfd\xb7\x33\x79\xe7",
+		.plaintext = vmac_string1,
+#ifdef __LITTLE_ENDIAN
+		.digest = "\xce\xf5\x3c\xd3\xae\x68\x8c\xa1",
+#else
+		.digest = "\xa1\x8c\x68\xae\xd3\x3c\xf5\xce",
+#endif
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = vmac_string2,
+#ifdef __LITTLE_ENDIAN
+		.digest = "\xc9\x27\xb0\x73\x81\xbd\x14\x2d",
+#else
+		.digest = "\x2d\x14\xbd\x81\x73\xb0\x27\xc9",
+#endif
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = vmac_string3,
+#ifdef __LITTLE_ENDIAN
+		.digest = "\x8d\x1a\x95\x8c\x98\x47\x0b\x19",
+#else
+		.digest = "\x19\x0b\x47\x98\x8c\x95\x1a\x8d",
+#endif
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key	= "abcdefghijklmnop",
+		.plaintext = NULL,
+#ifdef __LITTLE_ENDIAN
+		.digest	= "\x3b\x89\xa1\x26\x9e\x55\x8f\x84",
+#else
+		.digest = "\x84\x8f\x55\x9e\x26\xa1\x89\x3b",
+#endif
+		.psize	= 0,
+		.ksize	= 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string1,
+#ifdef __LITTLE_ENDIAN
+		.digest = "\xab\x5e\xab\xb0\xf6\x8d\x74\xc2",
+#else
+		.digest = "\xc2\x74\x8d\xf6\xb0\xab\x5e\xab",
+#endif
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string2,
+#ifdef __LITTLE_ENDIAN
+		.digest = "\x11\x15\x68\x42\x3d\x7b\x09\xdf",
+#else
+		.digest = "\xdf\x09\x7b\x3d\x42\x68\x15\x11",
+#endif
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string3,
+#ifdef __LITTLE_ENDIAN
+		.digest = "\x8b\x32\x8f\xe1\xed\x8f\xfa\xd4",
+#else
+		.digest = "\xd4\xfa\x8f\xed\xe1\x8f\x32\x8b",
+#endif
 		.psize  = 128,
 		.ksize  = 16,
 	},
crypto/vmac.c +36 -38

139 lines not on this page · raw message

Quoted 11 lines · Herbert Xu
Quoted 10 lines · Jan Glauber

Hi,

Jan 14 14:47:38 h42lp52 kernel: alg: hash: Test 1 failed for

vmac(aes-s390) Jan 14 14:47:38 h42lp52 kernel: 00000000: e7 79 33 b7 fd 8a d7 cb

Looking at the digest from the failing test vector:

.digest = "\xcb\xd7\x8a\xfd\xb7\x33\x79\xe7",

Quoted 8 lines
Quoted 4 lines

the output looks somehow familiar... Maybe a missing endian conversion? The patch fixes the test for me but it is just a wild guess and maybe
it should be done at another place?

Shane, please look into this.

Thanks,

This patch is to fix the vmac algorithm, add more test cases for vmac, and fix the test failure on some big endian system like s390.

crypto/testmgr.h +92 -4

109 lines not on this page · raw message

crypto/vmac.c +36 -38

139 lines not on this page · raw message

maintainer
#3 lore raw

Re: [PATCH] to fix vmac test fails on s390

Quoted 4 lines · Shane

static struct hash_testvec aes_vmac128_tv_template[] = {

crypto/testmgr.h +1 -0
 	{
+		.key	= "\x00\x01\x02\x03\x04\x05\x06\x07"

crypto/testmgr.h static struct hash_testvec aes_xcbc128_t · line 1700

Quoted 10 lines
crypto/testmgr.h +10 -0
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = NULL,
+#ifdef __LITTLE_ENDIAN
+		.digest	= "\x07\x58\x80\x35\x77\xa4\x7b\x54",
+#else
+		.digest = "\x54\x7b\xa4\x77\x35\x80\x58\x07",
+#endif
+		.psize	= 0,
+		.ksize	= 16,
+	}, {

Sorry but you can't fix it like this. Your hash output must be invariant with respect to endianness.

That means, whether I run it on a big-endian machine or a little- endian one it should produce the same output.

Otherwise this hash will be totally useless as soon as you get onto the network.

Cheers,

Signature · 4 lines
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
#4 lore raw

RE: [PATCH] to fix vmac test fails on s390

Quoted 10 lines · Herbert Xu

Sorry but you can't fix it like this. Your hash output must be invariant with respect to endianness.

That means, whether I run it on a big-endian machine or a little- endian one it should produce the same output.

Otherwise this hash will be totally useless as soon as you get onto the network.

Cheers,

OK, I understand.
The problem is the VMAC result is an integer instead of a string. I also tested the original VMAC algorithm (w/ Jan), since I ported from it. It proved the above.

In this case, should I change the original algorithm to produce an invariant string output?

Thanks.
Shane

#5 lore raw

RE: [PATCH] to fix vmac test fails on s390

In reply to Herbert Xu

<Updated & Resend>

This patch is to fix the vmac algorithm, add more test cases for vmac, and fix the test failure on some big endian system like s390.

crypto/testmgr.h |   64 +++++++++++++++++++++++++++++++++++--
crypto/vmac.c    |   77 ++++++++++++++++++++++-----------------------
2 files changed, 99 insertions(+), 42 deletions(-)
crypto/testmgr.h +60 -4
diff -r 91636f5ce7a2 crypto/testmgr.h
@@ -1669,17 +1669,73 @@ static struct hash_testvec aes_xcbc128_t
 	}
 };
 
-#define VMAC_AES_TEST_VECTORS	1
-static char vmac_string[128] = {'\x01', '\x01', '\x01', '\x01',
+#define VMAC_AES_TEST_VECTORS	8
+static char vmac_string1[128] = {'\x01', '\x01', '\x01', '\x01',
 				'\x02', '\x03', '\x02', '\x02',
 				'\x02', '\x04', '\x01', '\x07',
 				'\x04', '\x01', '\x04', '\x03',};
+static char vmac_string2[128] = {'a', 'b', 'c',};
+static char vmac_string3[128] = {'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				};
+
 static struct hash_testvec aes_vmac128_tv_template[] = {
 	{
+		.key	= "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = NULL,
+		.digest	= "\x07\x58\x80\x35\x77\xa4\x7b\x54",
+		.psize	= 0,
+		.ksize	= 16,
+	}, {
 		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
 			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
-		.plaintext = vmac_string,
-		.digest = "\xcb\xd7\x8a\xfd\xb7\x33\x79\xe7",
+		.plaintext = vmac_string1,
+		.digest = "\xce\xf5\x3c\xd3\xae\x68\x8c\xa1",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = vmac_string2,
+		.digest = "\xc9\x27\xb0\x73\x81\xbd\x14\x2d",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = vmac_string3,
+		.digest = "\x8d\x1a\x95\x8c\x98\x47\x0b\x19",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key	= "abcdefghijklmnop",
+		.plaintext = NULL,
+		.digest	= "\x3b\x89\xa1\x26\x9e\x55\x8f\x84",
+		.psize	= 0,
+		.ksize	= 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string1,
+		.digest = "\xab\x5e\xab\xb0\xf6\x8d\x74\xc2",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string2,
+		.digest = "\x11\x15\x68\x42\x3d\x7b\x09\xdf",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string3,
+		.digest = "\x8b\x32\x8f\xe1\xed\x8f\xfa\xd4",
 		.psize  = 128,
 		.ksize  = 16,
 	},
crypto/vmac.c +39 -38

142 lines not on this page · raw message

This patch is to fix the vmac algorithm, add more test cases for vmac, and fix the test failure on some big endian system like s390.

crypto/testmgr.h +60 -4

77 lines not on this page · raw message

crypto/vmac.c +39 -38

142 lines not on this page · raw message

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw

RE: [PATCH] to fix vmac test fails on s390

<Updated again and Resend>

This patch is to fix the vmac algorithm, add more test cases for vmac, and fix the test failure on some big endian system like s390.

 crypto/testmgr.h |   64 +++++++++++++++++++++++++++++++++---
 crypto/vmac.c    |   80 ++++++++++++++++++++++-----------------------
 2 files changed, 100 insertions(+), 44 deletions(-)
crypto/testmgr.h +60 -4
diff -r 91636f5ce7a2 crypto/testmgr.h
@@ -1669,17 +1669,73 @@ static struct hash_testvec aes_xcbc128_t
 	}
 };
 
-#define VMAC_AES_TEST_VECTORS	1
-static char vmac_string[128] = {'\x01', '\x01', '\x01', '\x01',
+#define VMAC_AES_TEST_VECTORS	8
+static char vmac_string1[128] = {'\x01', '\x01', '\x01', '\x01',
 				'\x02', '\x03', '\x02', '\x02',
 				'\x02', '\x04', '\x01', '\x07',
 				'\x04', '\x01', '\x04', '\x03',};
+static char vmac_string2[128] = {'a', 'b', 'c',};
+static char vmac_string3[128] = {'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				'a', 'b', 'c', 'a', 'b', 'c',
+				};
+
 static struct hash_testvec aes_vmac128_tv_template[] = {
 	{
+		.key	= "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = NULL,
+		.digest	= "\x07\x58\x80\x35\x77\xa4\x7b\x54",
+		.psize	= 0,
+		.ksize	= 16,
+	}, {
 		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
 			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
-		.plaintext = vmac_string,
-		.digest = "\xcb\xd7\x8a\xfd\xb7\x33\x79\xe7",
+		.plaintext = vmac_string1,
+		.digest = "\xce\xf5\x3c\xd3\xae\x68\x8c\xa1",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = vmac_string2,
+		.digest = "\xc9\x27\xb0\x73\x81\xbd\x14\x2d",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "\x00\x01\x02\x03\x04\x05\x06\x07"
+			  "\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
+		.plaintext = vmac_string3,
+		.digest = "\x8d\x1a\x95\x8c\x98\x47\x0b\x19",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key	= "abcdefghijklmnop",
+		.plaintext = NULL,
+		.digest	= "\x3b\x89\xa1\x26\x9e\x55\x8f\x84",
+		.psize	= 0,
+		.ksize	= 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string1,
+		.digest = "\xab\x5e\xab\xb0\xf6\x8d\x74\xc2",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string2,
+		.digest = "\x11\x15\x68\x42\x3d\x7b\x09\xdf",
+		.psize  = 128,
+		.ksize  = 16,
+	}, {
+		.key    = "abcdefghijklmnop",
+		.plaintext = vmac_string3,
+		.digest = "\x8b\x32\x8f\xe1\xed\x8f\xfa\xd4",
 		.psize  = 128,
 		.ksize  = 16,
 	},
crypto/vmac.c +40 -40

151 lines not on this page · raw message

Quoted 11 lines · Sebastian Andrzej Siewior
Quoted 7 lines · * Wang, Shane | 2010-02-21 13:32:49 [+0800]:
crypto/vmac.c +0 -0
@@ -42,6 +42,8 @@ const u64 m63   = UINT64_C(0x7ffffffffff

const u64 m63 = UINT64_C(0x7fffffffffffffff); /* 63-bit mask
*/ const u64 m64 = UINT64_C(0xffffffffffffffff); /* 64-bit mask
*/ const u64 mpoly = UINT64_C(0x1fffffff1fffffff); /* Poly key mask
*/ + +#define pe64_to_cpup le64_to_cpup /* Prefer little endian */

Does it mean that I can switch it to be64_to_cpup ?

Yes, per the original vmac algorithm, it is one of the parameters that you can adopt. But the default is le64 and I use it here.

Quoted 4 lines
Quoted 4 lines
crypto/vmac.c +0 -0
@@ -575,6 +572,10 @@ static int vmac_final(struct shash_desc 	u8
 nonce[16] = {}; 
 
Quoted 9 lines

crypto/vmac.c static int vmac_final(struct shash_desc u8 · line 577

Quoted 4 lines
crypto/vmac.c +3 -0
 	mac = vmac(NULL, 0, nonce, NULL, ctx);
+
+	/* set output invariant considering endianness */
+	mac = le64_to_cpup(&mac);

So this is the fix. It would look better if you would include this swap into vmac() itself. I'm not sure but this is probably causing a dereference which could be avoided.
sparse should catch conversion bugs like this one if you were using types likes __be64.

Updated.

Quoted 2 lines

Sebastian

This patch is to fix the vmac algorithm, add more test cases for vmac, and fix the test failure on some big endian system like s390.

crypto/testmgr.h +60 -4

77 lines not on this page · raw message

crypto/vmac.c +40 -40

151 lines not on this page · raw message

maintainer
#8 lore raw

Re: [PATCH] to fix vmac test fails on s390

crypto/vmac.c static u64 vmac(unsigned char m[], unsig · line 481

Quoted 10 lines · Shane
crypto/vmac.c +4 -1
@@ -474,7 +475,10 @@ static u64 vmac(unsigned char m[], unsig
 	}
 	p = be64_to_cpup(out_p + i);
 	h = vhash(m, mbytes, (u64 *)0, &ctx->__vmac_ctx);
-	return p + h;
+	r = p + h;
+	r = le64_to_cpup(&r);
+
+	return r;

Is there any reason why we don't simply do

	return le64_to_cpu(p + h);

Cheers,

Signature · 4 lines
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
#9 lore raw

RE: [PATCH] to fix vmac test fails on s390

That should be simpler and better.

Thanks.
Shane

-----Original Message-----
From: Herbert Xu [mailto:[email protected]]
Sent: 2010年3月2日 21:57
To: Wang, Shane
Cc: Sebastian Andrzej Siewior; Jan Glauber; Cihula, Joseph; linux-crypto
Subject: Re: [PATCH] to fix vmac test fails on s390

crypto/vmac.c static u64 vmac(unsigned char m[], unsig · line 481

Quoted 10 lines · Shane
crypto/vmac.c +4 -1
@@ -474,7 +475,10 @@ static u64 vmac(unsigned char m[], unsig
 	}
 	p = be64_to_cpup(out_p + i);
 	h = vhash(m, mbytes, (u64 *)0, &ctx->__vmac_ctx);
-	return p + h;
+	r = p + h;
+	r = le64_to_cpup(&r);
+
+	return r;

Is there any reason why we don't simply do

	return le64_to_cpu(p + h);

Cheers,

Signature · 4 lines
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
maintainer
#10 lore raw

Re: [PATCH] to fix vmac test fails on s390

Quoted 1 line · Shane

That should be simpler and better.

OK can you please resubmit the patch?

Thanks,

Signature · 4 lines
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