crypto: vmac - remove insecure version with hardcoded nonce
authorEric Biggers <ebiggers@google.com>
Mon, 18 Jun 2018 17:22:40 +0000 (10:22 -0700)
committerHerbert Xu <herbert@gondor.apana.org.au>
Sun, 1 Jul 2018 13:00:44 +0000 (21:00 +0800)
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>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
crypto/tcrypt.c
crypto/testmgr.c
crypto/testmgr.h
crypto/vmac.c

index d5bcdd905007a217767ca86b228fa57eaeea0b12..078ec36007bf71cebca79710161bfb90c32cc30a 100644 (file)
@@ -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:
index 60a557b0f8d39a1d17edb52dbe556afbe763cb7c..63f263fd1daef5af50dc2138060dbecf822bb723 100644 (file)
@@ -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,
index 7b022c47a6231ed46f51920312cdf80e6a384742..b6362169771a293ac347dc0c02b597fca0e0141c 100644 (file)
@@ -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',
index bf1e385bc6847f73c4ad0a4cf43abd6cb32537e6..5f436dfdfc61e90264802e41490a5901c75e2c3e 100644 (file)
@@ -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 @@ out_put_alg:
        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");