linux-kernel - Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <17310.1438036987@warthog.procyon.org.uk>
Date:	Mon, 27 Jul 2015 23:43:07 +0100
From:	David Howells <dhowells@...hat.com>
To:	Andy Lutomirski <luto@...nel.org>
Cc:	dhowells@...hat.com, jmorris@...ei.org, dwmw2@...radead.org,
	mcgrof@...il.com, keyrings@...ux-nfs.org,
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [GIT PULL] MODSIGN: Use PKCS#7 for module signatures

Andy Lutomirski <luto@...nel.org> 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 <dhowells@...hat.com>
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 <luto@...nel.org>
    Signed-off-by: David Howells <dhowells@...hat.com>

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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/