Andy Lutomirski <[email protected]> wrote: > With all this stuff applied, will the kernel accept PKCS#7 signatures that > *don't* have authenticated attributes or that are otherwise cryptographically > insecure in that they fail to provide the property that an attacker can't > manipulate a valid signature on one message to look like a valid signature on > a different message?
Hmmm... That's easy enough to fix (see below). However, will that cause kexec problems, I wonder? Does mscode require authattrs? David --- commit 44460686dfb0a4cca06f20e27988965e327e0f93 Author: David Howells <[email protected]> Date: Mon Jul 27 23:32:03 2015 +0100 PKCS#7: Require authenticated attributes Require there to be authenticated attributes in the PKCS#7/CMS message so that an attacker can't drop them to provide greater opportunity for manipulating the message. Suggested-by: Andy Lutomirski <[email protected]> Signed-off-by: David Howells <[email protected]> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c index 404f89a0f852..be0fc3b49b43 100644 --- a/crypto/asymmetric_keys/pkcs7_verify.c +++ b/crypto/asymmetric_keys/pkcs7_verify.c @@ -30,6 +30,7 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7, size_t digest_size, desc_size; void *digest; int ret; + u8 tag; kenter(",%u,%u", sinfo->index, sinfo->sig.pkey_hash_algo); @@ -70,43 +71,45 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7, * message digest attribute amongst them which corresponds to the * digest we just calculated. */ - if (sinfo->msgdigest) { - u8 tag; - - if (sinfo->msgdigest_len != sinfo->sig.digest_size) { - pr_debug("Sig %u: Invalid digest size (%u)\n", - sinfo->index, sinfo->msgdigest_len); - ret = -EBADMSG; - goto error; - } + if (!sinfo->authattrs || !sinfo->msgdigest) { + pr_warn("Sig %u: No authenticatedAttrs\n", sinfo->index); + ret = -EKEYREJECTED; + goto error; + } + + if (sinfo->msgdigest_len != sinfo->sig.digest_size) { + pr_debug("Sig %u: Invalid digest size (%u)\n", + sinfo->index, sinfo->msgdigest_len); + ret = -EBADMSG; + goto error; + } - if (memcmp(digest, sinfo->msgdigest, sinfo->msgdigest_len) != 0) { - pr_debug("Sig %u: Message digest doesn't match\n", - sinfo->index); - ret = -EKEYREJECTED; - goto error; - } + if (memcmp(digest, sinfo->msgdigest, sinfo->msgdigest_len) != 0) { + pr_debug("Sig %u: Message digest doesn't match\n", + sinfo->index); + ret = -EKEYREJECTED; + goto error; + } - /* We then calculate anew, using the authenticated attributes - * as the contents of the digest instead. Note that we need to - * convert the attributes from a CONT.0 into a SET before we - * hash it. - */ - memset(digest, 0, sinfo->sig.digest_size); + /* We then calculate anew, using the authenticated attributes + * as the contents of the digest instead. Note that we need to + * convert the attributes from a CONT.0 into a SET before we + * hash it. + */ + memset(digest, 0, sinfo->sig.digest_size); - ret = crypto_shash_init(desc); - if (ret < 0) - goto error; - tag = ASN1_CONS_BIT | ASN1_SET; - ret = crypto_shash_update(desc, &tag, 1); - if (ret < 0) - goto error; - ret = crypto_shash_finup(desc, sinfo->authattrs, - sinfo->authattrs_len, digest); - if (ret < 0) - goto error; - pr_devel("AADigest = [%*ph]\n", 8, digest); - } + ret = crypto_shash_init(desc); + if (ret < 0) + goto error; + tag = ASN1_CONS_BIT | ASN1_SET; + ret = crypto_shash_update(desc, &tag, 1); + if (ret < 0) + goto error; + ret = crypto_shash_finup(desc, sinfo->authattrs, + sinfo->authattrs_len, digest); + if (ret < 0) + goto error; + pr_devel("AADigest = [%*ph]\n", 8, digest); sinfo->sig.digest = digest; digest = NULL; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
