All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: David Howells <dhowells@redhat.com>, keyrings@vger.kernel.org
Cc: linux-crypto@vger.kernel.org,
	Michael Halcrow <mhalcrow@google.com>,
	Eric Biggers <ebiggers@google.com>
Subject: [PATCH 3/9] PKCS#7: fix direct verification of SignerInfo signature
Date: Tue,  6 Feb 2018 17:10:06 -0800	[thread overview]
Message-ID: <20180207011012.5928-4-ebiggers3@gmail.com> (raw)
In-Reply-To: <20180207011012.5928-1-ebiggers3@gmail.com>

From: Eric Biggers <ebiggers@google.com>

If none of the certificates in a SignerInfo's certificate chain match a
trusted key, nor is the last certificate signed by a trusted key, then
pkcs7_validate_trust_one() tries to check whether the SignerInfo's
signature was made directly by a trusted key.  But, it actually fails to
set the 'sig' variable correctly, so it actually verifies the last
signature seen.  That will only be the SignerInfo's signature if the
certificate chain is empty; otherwise it will actually be the last
certificate's signature.

This is not by itself a security problem, since verifying any of the
certificates in the chain should be sufficient to verify the SignerInfo.
Still, it's not working as intended so it should be fixed.

Fix it by setting 'sig' correctly for the direct verification case.

Fixes: 757932e6da6d ("PKCS#7: Handle PKCS#7 messages that contain no X.509 certs")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/asymmetric_keys/pkcs7_trust.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 1f4e25f10049..598906b1e28d 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -106,6 +106,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 		pr_devel("sinfo %u: Direct signer is key %x\n",
 			 sinfo->index, key_serial(key));
 		x509 = NULL;
+		sig = sinfo->sig;
 		goto matched;
 	}
 	if (PTR_ERR(key) != -ENOKEY)
-- 
2.16.0.rc1.238.g530d649a79-goog

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers3@gmail.com>
To: David Howells <dhowells@redhat.com>, keyrings@vger.kernel.org
Cc: linux-crypto@vger.kernel.org,
	Michael Halcrow <mhalcrow@google.com>,
	Eric Biggers <ebiggers@google.com>
Subject: [PATCH 3/9] PKCS#7: fix direct verification of SignerInfo signature
Date: Wed, 07 Feb 2018 01:10:06 +0000	[thread overview]
Message-ID: <20180207011012.5928-4-ebiggers3@gmail.com> (raw)
In-Reply-To: <20180207011012.5928-1-ebiggers3@gmail.com>

From: Eric Biggers <ebiggers@google.com>

If none of the certificates in a SignerInfo's certificate chain match a
trusted key, nor is the last certificate signed by a trusted key, then
pkcs7_validate_trust_one() tries to check whether the SignerInfo's
signature was made directly by a trusted key.  But, it actually fails to
set the 'sig' variable correctly, so it actually verifies the last
signature seen.  That will only be the SignerInfo's signature if the
certificate chain is empty; otherwise it will actually be the last
certificate's signature.

This is not by itself a security problem, since verifying any of the
certificates in the chain should be sufficient to verify the SignerInfo.
Still, it's not working as intended so it should be fixed.

Fix it by setting 'sig' correctly for the direct verification case.

Fixes: 757932e6da6d ("PKCS#7: Handle PKCS#7 messages that contain no X.509 certs")
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/asymmetric_keys/pkcs7_trust.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 1f4e25f10049..598906b1e28d 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -106,6 +106,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
 		pr_devel("sinfo %u: Direct signer is key %x\n",
 			 sinfo->index, key_serial(key));
 		x509 = NULL;
+		sig = sinfo->sig;
 		goto matched;
 	}
 	if (PTR_ERR(key) != -ENOKEY)
-- 
2.16.0.rc1.238.g530d649a79-goog


  parent reply	other threads:[~2018-02-07  1:16 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-07  1:10 [PATCH 0/9] PKCS#7 / X.509 fixes and cleanups Eric Biggers
2018-02-07  1:10 ` Eric Biggers
2018-02-07  1:10 ` [PATCH 1/9] PKCS#7: fix certificate chain verification Eric Biggers
2018-02-07  1:10   ` Eric Biggers
2018-02-07  1:10 ` [PATCH 2/9] PKCS#7: fix certificate blacklisting Eric Biggers
2018-02-07  1:10   ` Eric Biggers
2018-02-07  1:10 ` Eric Biggers [this message]
2018-02-07  1:10   ` [PATCH 3/9] PKCS#7: fix direct verification of SignerInfo signature Eric Biggers
2018-02-07  1:10 ` [PATCH 4/9] X.509: fix BUG_ON() when hash algorithm is unsupported Eric Biggers
2018-02-07  1:10   ` Eric Biggers
2018-02-07  1:10 ` [PATCH 5/9] X.509: fix NULL dereference when restricting key with unsupported_sig Eric Biggers
2018-02-07  1:10   ` Eric Biggers
2018-02-07  1:10 ` [PATCH 6/9] PKCS#7: remove unnecessary check for NULL sinfo->sig->hash_algo Eric Biggers
2018-02-07  1:10   ` Eric Biggers
2018-02-07  1:10 ` [PATCH 7/9] X.509: remove never-set ->unsupported_key flag Eric Biggers
2018-02-07  1:10   ` Eric Biggers
2018-02-07  1:10 ` [PATCH 8/9] X.509: remove dead code that set ->unsupported_sig Eric Biggers
2018-02-07  1:10   ` Eric Biggers
2018-02-07  1:10 ` [PATCH 9/9] X.509: self_signed implies !unsupported_sig Eric Biggers
2018-02-07  1:10   ` Eric Biggers
2018-02-08 14:28 ` [PATCH 0/9] PKCS#7 / X.509 fixes and cleanups David Howells
2018-02-08 15:07 ` [PATCH 4/9] X.509: fix BUG_ON() when hash algorithm is unsupported David Howells
2018-02-20 22:34   ` Eric Biggers
2018-02-20 22:34     ` Eric Biggers
2018-02-08 15:13 ` [PATCH 6/9] PKCS#7: remove unnecessary check for NULL sinfo->sig->hash_algo David Howells
2018-02-08 15:27 ` [PATCH 8/9] X.509: remove dead code that set ->unsupported_sig David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180207011012.5928-4-ebiggers3@gmail.com \
    --to=ebiggers3@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=ebiggers@google.com \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=mhalcrow@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.