From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932121AbbG0WnR (ORCPT ); Mon, 27 Jul 2015 18:43:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54045 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754702AbbG0WnO convert rfc822-to-8bit (ORCPT ); Mon, 27 Jul 2015 18:43:14 -0400 Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells In-Reply-To: <55B6988D.4060805@kernel.org> References: <55B6988D.4060805@kernel.org> <5299.1438025624@warthog.procyon.org.uk> To: Andy Lutomirski Cc: dhowells@redhat.com, jmorris@namei.org, dwmw2@infradead.org, mcgrof@gmail.com, keyrings@linux-nfs.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org Subject: Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <17309.1438036987.1@warthog.procyon.org.uk> Content-Transfer-Encoding: 8BIT Date: Mon, 27 Jul 2015 23:43:07 +0100 Message-ID: <17310.1438036987@warthog.procyon.org.uk> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Andy Lutomirski 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 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 Signed-off-by: David Howells 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;