diff mbox

[4/4] crypto: vmac - remove insecure version with hardcoded nonce

Message ID 20180618172240.46651-5-ebiggers3@gmail.com (mailing list archive)
State Accepted
Delegated to: Herbert Xu
Headers show

Commit Message

Eric Biggers June 18, 2018, 5:22 p.m. UTC 
From: Eric Biggers <ebiggers@google.com>

Remove the original version of the VMAC template that had the nonce
hardcoded to 0 and produced a digest with the wrong endianness.  I'm
unsure whether this had users or not (there are no explicit in-kernel
references to it), but given that the hardcoded nonce made it wildly
insecure unless a unique key was used for each message, let's try
removing it and see if anyone complains.

Leave the new "vmac64" template that requires the nonce to be explicitly
specified as the first 16 bytes of data and uses the correct endianness
for the digest.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/tcrypt.c  |   2 +-
 crypto/testmgr.c |   6 ---
 crypto/testmgr.h | 102 -----------------------------------------------
 crypto/vmac.c    |  84 ++++----------------------------------
 4 files changed, 8 insertions(+), 186 deletions(-)
diff mbox

Patch

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index d5bcdd905007..078ec36007bf 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1939,7 +1939,7 @@  static int do_test(const char *alg, u32 type, u32 mask, int m, u32 num_mb)
 		break;
 
 	case 109:
-		ret += tcrypt_test("vmac(aes)");
+		ret += tcrypt_test("vmac64(aes)");
 		break;
 
 	case 111:
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 60a557b0f8d3..63f263fd1dae 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -3477,12 +3477,6 @@  static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.hash = __VECS(tgr192_tv_template)
 		}
-	}, {
-		.alg = "vmac(aes)",
-		.test = alg_test_hash,
-		.suite = {
-			.hash = __VECS(aes_vmac128_tv_template)
-		}
 	}, {
 		.alg = "vmac64(aes)",
 		.test = alg_test_hash,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 7b022c47a623..b6362169771a 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -4603,108 +4603,6 @@  static const struct hash_testvec aes_xcbc128_tv_template[] = {
 	}
 };
 
-static const char vmac_string1[128] = {'\x01', '\x01', '\x01', '\x01',
-				       '\x02', '\x03', '\x02', '\x02',
-				       '\x02', '\x04', '\x01', '\x07',
-				       '\x04', '\x01', '\x04', '\x03',};
-static const char vmac_string2[128] = {'a', 'b', 'c',};
-static const 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 const char vmac_string4[17] = {'b', 'c', 'e', 'f',
-				      'i', 'j', 'l', 'm',
-				      'o', 'p', 'r', 's',
-				      't', 'u', 'w', 'x', 'z'};
-
-static const char vmac_string5[127] = {'r', 'm', 'b', 't', 'c',
-				       'o', 'l', 'k', ']', '%',
-				       '9', '2', '7', '!', 'A'};
-
-static const char vmac_string6[129] = {'p', 't', '*', '7', 'l',
-				       'i', '!', '#', 'w', '0',
-				       'z', '/', '4', 'A', 'n'};
-
-static const 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_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,
-	}, {
-		.key = "a09b5cd!f#07K\x00\x00\x00",
-		.plaintext = vmac_string4,
-		.digest = "\xab\xa5\x0f\xea\x42\x4e\xa1\x5f",
-		.psize = sizeof(vmac_string4),
-		.ksize = 16,
-	}, {
-		.key = "a09b5cd!f#07K\x00\x00\x00",
-		.plaintext = vmac_string5,
-		.digest = "\x25\x31\x98\xbc\x1d\xe8\x67\x60",
-		.psize = sizeof(vmac_string5),
-		.ksize = 16,
-	}, {
-		.key = "a09b5cd!f#07K\x00\x00\x00",
-		.plaintext = vmac_string6,
-		.digest = "\xc4\xae\x9b\x47\x95\x65\xeb\x41",
-		.psize = sizeof(vmac_string6),
-		.ksize = 16,
-	},
-};
-
 static const char vmac64_string1[144] = {
 	'\0',     '\0',   '\0',   '\0',   '\0',   '\0',   '\0',   '\0',
 	'\0',     '\0',   '\0',   '\0',   '\0',   '\0',   '\0',   '\0',
diff --git a/crypto/vmac.c b/crypto/vmac.c
index bf1e385bc684..5f436dfdfc61 100644
--- a/crypto/vmac.c
+++ b/crypto/vmac.c
@@ -490,16 +490,6 @@  static int vmac_init(struct shash_desc *desc)
 	return 0;
 }
 
-static int vmac_init_with_hardcoded_nonce(struct shash_desc *desc)
-{
-	struct vmac_desc_ctx *dctx = shash_desc_ctx(desc);
-
-	vmac_init(desc);
-	memset(&dctx->nonce, 0, VMAC_NONCEBYTES);
-	dctx->nonce_size = VMAC_NONCEBYTES;
-	return 0;
-}
-
 static int vmac_update(struct shash_desc *desc, const u8 *p, unsigned int len)
 {
 	const struct vmac_tfm_ctx *tctx = crypto_shash_ctx(desc->tfm);
@@ -570,7 +560,7 @@  static u64 vhash_final(const struct vmac_tfm_ctx *tctx,
 	return l3hash(ch, cl, tctx->l3key[0], tctx->l3key[1], partial * 8);
 }
 
-static int __vmac_final(struct shash_desc *desc, u64 *mac)
+static int vmac_final(struct shash_desc *desc, u8 *out)
 {
 	const struct vmac_tfm_ctx *tctx = crypto_shash_ctx(desc->tfm);
 	struct vmac_desc_ctx *dctx = shash_desc_ctx(desc);
@@ -601,31 +591,7 @@  static int __vmac_final(struct shash_desc *desc, u64 *mac)
 	pad = be64_to_cpu(dctx->nonce.pads[index]);
 
 	/* The VMAC is the sum of VHASH and the pseudorandom pad */
-	*mac = hash + pad;
-	return 0;
-}
-
-static int vmac_final_le(struct shash_desc *desc, u8 *out)
-{
-	u64 mac;
-	int err;
-
-	err = __vmac_final(desc, &mac);
-	if (err)
-		return err;
-	put_unaligned_le64(mac, out);
-	return 0;
-}
-
-static int vmac_final_be(struct shash_desc *desc, u8 *out)
-{
-	u64 mac;
-	int err;
-
-	err = __vmac_final(desc, &mac);
-	if (err)
-		return err;
-	put_unaligned_be64(mac, out);
+	put_unaligned_be64(hash + pad, out);
 	return 0;
 }
 
@@ -651,8 +617,7 @@  static void vmac_exit_tfm(struct crypto_tfm *tfm)
 	crypto_free_cipher(tctx->cipher);
 }
 
-static int vmac_create_common(struct crypto_template *tmpl, struct rtattr **tb,
-			      bool vmac64)
+static int vmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 {
 	struct shash_instance *inst;
 	struct crypto_alg *alg;
@@ -692,15 +657,9 @@  static int vmac_create_common(struct crypto_template *tmpl, struct rtattr **tb,
 
 	inst->alg.descsize = sizeof(struct vmac_desc_ctx);
 	inst->alg.digestsize = VMAC_TAG_LEN / 8;
-	if (vmac64) {
-		inst->alg.init = vmac_init;
-		inst->alg.final = vmac_final_be;
-	} else {
-		pr_warn("vmac: using insecure hardcoded nonce\n");
-		inst->alg.init = vmac_init_with_hardcoded_nonce;
-		inst->alg.final = vmac_final_le;
-	}
+	inst->alg.init = vmac_init;
 	inst->alg.update = vmac_update;
+	inst->alg.final = vmac_final;
 	inst->alg.setkey = vmac_setkey;
 
 	err = shash_register_instance(tmpl, inst);
@@ -714,48 +673,20 @@  static int vmac_create_common(struct crypto_template *tmpl, struct rtattr **tb,
 	return err;
 }
 
-static int vmac_create(struct crypto_template *tmpl, struct rtattr **tb)
-{
-	return vmac_create_common(tmpl, tb, false);
-}
-
-static int vmac64_create(struct crypto_template *tmpl, struct rtattr **tb)
-{
-	return vmac_create_common(tmpl, tb, true);
-}
-
-static struct crypto_template vmac_tmpl = {
-	.name = "vmac",
-	.create = vmac_create,
-	.free = shash_free_instance,
-	.module = THIS_MODULE,
-};
-
 static struct crypto_template vmac64_tmpl = {
 	.name = "vmac64",
-	.create = vmac64_create,
+	.create = vmac_create,
 	.free = shash_free_instance,
 	.module = THIS_MODULE,
 };
 
 static int __init vmac_module_init(void)
 {
-	int err;
-
-	err = crypto_register_template(&vmac_tmpl);
-	if (err)
-		return err;
-
-	err = crypto_register_template(&vmac64_tmpl);
-	if (err)
-		crypto_unregister_template(&vmac_tmpl);
-
-	return err;
+	return crypto_register_template(&vmac64_tmpl);
 }
 
 static void __exit vmac_module_exit(void)
 {
-	crypto_unregister_template(&vmac_tmpl);
 	crypto_unregister_template(&vmac64_tmpl);
 }
 
@@ -764,5 +695,4 @@  module_exit(vmac_module_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("VMAC hash algorithm");
-MODULE_ALIAS_CRYPTO("vmac");
 MODULE_ALIAS_CRYPTO("vmac64");