From 1bcc8320ca5b92f8d236614c4236a3e1e5bed2b5 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 18 Jun 2018 10:22:39 -0700 Subject: crypto: vmac - add nonced version with big endian digest Currently the VMAC template uses a "nonce" hardcoded to 0, which makes it insecure unless a unique key is set for every message. Also, the endianness of the final digest is wrong: the implementation uses little endian, but the VMAC specification has it as big endian, as do other VMAC implementations such as the one in Crypto++. Add a new VMAC template where the nonce is passed as the first 16 bytes of data (similar to what is done for Poly1305's nonce), and the digest is big endian. Call it "vmac64", since the old name of simply "vmac" didn't clarify whether the implementation is of VMAC-64 or of VMAC-128 (which produce 64-bit and 128-bit digests respectively); so we fix the naming ambiguity too. Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/testmgr.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'crypto/testmgr.c') diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 11e45352..60a557b0 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -3483,6 +3483,12 @@ static const struct alg_test_desc alg_test_descs[] = { .suite = { .hash = __VECS(aes_vmac128_tv_template) } + }, { + .alg = "vmac64(aes)", + .test = alg_test_hash, + .suite = { + .hash = __VECS(vmac64_aes_tv_template) + } }, { .alg = "wp256", .test = alg_test_hash, -- cgit v1.2.3 From df5f4059ced2064a468e3805d885d74332fda0da Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 18 Jun 2018 10:22:40 -0700 Subject: crypto: vmac - remove insecure version with hardcoded nonce 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 Signed-off-by: Herbert Xu --- crypto/tcrypt.c | 2 +- crypto/testmgr.c | 6 ---- crypto/testmgr.h | 102 ------------------------------------------------------- crypto/vmac.c | 84 ++++----------------------------------------- 4 files changed, 8 insertions(+), 186 deletions(-) (limited to 'crypto/testmgr.c') diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c index d5bcdd90..078ec360 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 60a557b0..63f263fd 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 7b022c47..b6362169 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 bf1e385b..5f436dfd 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 @@ 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"); -- cgit v1.2.3 From 140c0080601ecbf4b1f1298799936f22a62d1da8 Mon Sep 17 00:00:00 2001 From: Gilad Ben-Yossef Date: Sun, 1 Jul 2018 08:02:35 +0100 Subject: crypto: testmgr - add hash finup tests The testmgr hash tests were testing init, digest, update and final methods but not the finup method. Add a test for this one too. While doing this, make sure we only run the partial tests once with the digest tests and skip them with the final and finup tests since they are the same. Signed-off-by: Gilad Ben-Yossef Signed-off-by: Herbert Xu --- crypto/testmgr.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 9 deletions(-) (limited to 'crypto/testmgr.c') diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 63f263fd..a1d42245 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -259,9 +259,15 @@ out_nostate: return ret; } +enum hash_test { + HASH_TEST_DIGEST, + HASH_TEST_FINAL, + HASH_TEST_FINUP +}; + static int __test_hash(struct crypto_ahash *tfm, const struct hash_testvec *template, unsigned int tcount, - bool use_digest, const int align_offset) + enum hash_test test_type, const int align_offset) { const char *algo = crypto_tfm_alg_driver_name(crypto_ahash_tfm(tfm)); size_t digest_size = crypto_ahash_digestsize(tfm); @@ -332,14 +338,17 @@ static int __test_hash(struct crypto_ahash *tfm, } ahash_request_set_crypt(req, sg, result, template[i].psize); - if (use_digest) { + switch (test_type) { + case HASH_TEST_DIGEST: ret = crypto_wait_req(crypto_ahash_digest(req), &wait); if (ret) { pr_err("alg: hash: digest failed on test %d " "for %s: ret=%d\n", j, algo, -ret); goto out; } - } else { + break; + + case HASH_TEST_FINAL: memset(result, 1, digest_size); ret = crypto_wait_req(crypto_ahash_init(req), &wait); if (ret) { @@ -371,6 +380,29 @@ static int __test_hash(struct crypto_ahash *tfm, "for %s: ret=%d\n", j, algo, -ret); goto out; } + break; + + case HASH_TEST_FINUP: + memset(result, 1, digest_size); + ret = crypto_wait_req(crypto_ahash_init(req), &wait); + if (ret) { + pr_err("alg: hash: init failed on test %d " + "for %s: ret=%d\n", j, algo, -ret); + goto out; + } + ret = ahash_guard_result(result, 1, digest_size); + if (ret) { + pr_err("alg: hash: init failed on test %d " + "for %s: used req->result\n", j, algo); + goto out; + } + ret = crypto_wait_req(crypto_ahash_finup(req), &wait); + if (ret) { + pr_err("alg: hash: final failed on test %d " + "for %s: ret=%d\n", j, algo, -ret); + goto out; + } + break; } if (memcmp(result, template[i].digest, @@ -383,6 +415,9 @@ static int __test_hash(struct crypto_ahash *tfm, } } + if (test_type) + goto out; + j = 0; for (i = 0; i < tcount; i++) { /* alignment tests are only done with continuous buffers */ @@ -540,24 +575,24 @@ out_nobuf: static int test_hash(struct crypto_ahash *tfm, const struct hash_testvec *template, - unsigned int tcount, bool use_digest) + unsigned int tcount, enum hash_test test_type) { unsigned int alignmask; int ret; - ret = __test_hash(tfm, template, tcount, use_digest, 0); + ret = __test_hash(tfm, template, tcount, test_type, 0); if (ret) return ret; /* test unaligned buffers, check with one byte offset */ - ret = __test_hash(tfm, template, tcount, use_digest, 1); + ret = __test_hash(tfm, template, tcount, test_type, 1); if (ret) return ret; alignmask = crypto_tfm_alg_alignmask(&tfm->base); if (alignmask) { /* Check if alignment mask for tfm is correctly set. */ - ret = __test_hash(tfm, template, tcount, use_digest, + ret = __test_hash(tfm, template, tcount, test_type, alignmask + 1); if (ret) return ret; @@ -1803,9 +1838,11 @@ static int __alg_test_hash(const struct hash_testvec *template, return PTR_ERR(tfm); } - err = test_hash(tfm, template, tcount, true); + err = test_hash(tfm, template, tcount, HASH_TEST_DIGEST); + if (!err) + err = test_hash(tfm, template, tcount, HASH_TEST_FINAL); if (!err) - err = test_hash(tfm, template, tcount, false); + err = test_hash(tfm, template, tcount, HASH_TEST_FINUP); crypto_free_ahash(tfm); return err; } -- cgit v1.2.3