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>,
stable@vger.kernel.org
Subject: [PATCH 1/9] PKCS#7: fix certificate chain verification
Date: Tue, 6 Feb 2018 17:10:04 -0800 [thread overview]
Message-ID: <20180207011012.5928-2-ebiggers3@gmail.com> (raw)
In-Reply-To: <20180207011012.5928-1-ebiggers3@gmail.com>
From: Eric Biggers <ebiggers@google.com>
When pkcs7_verify_sig_chain() is building the certificate chain for a
SignerInfo using the certificates in the PKCS#7 message, it is passing
the wrong arguments to public_key_verify_signature(). Consequently,
when the next certificate is supposed to be used to verify the previous
certificate, the next certificate is actually used to verify itself.
An attacker can use this bug to create a bogus certificate chain that
has no cryptographic relationship between the beginning and end.
Fortunately I couldn't quite find a way to use this to bypass the
overall signature verification, though it comes very close. Here's the
reasoning: due to the bug, every certificate in the chain beyond the
first actually has to be self-signed (where "self-signed" here refers to
the actual key and signature; an attacker might still manipulate the
certificate fields such that the self_signed flag doesn't actually get
set, and thus the chain doesn't end immediately). But to pass trust
validation (pkcs7_validate_trust()), either the SignerInfo or one of the
certificates has to actually be signed by a trusted key. Since only
self-signed certificates can be added to the chain, the only way for an
attacker to introduce a trusted signature is to include a self-signed
trusted certificate.
But, when pkcs7_validate_trust_one() reaches that certificate, instead
of trying to verify the signature on that certificate, it will actually
look up the corresponding trusted key, which will succeed, and then try
to verify the *previous* certificate, which will fail. Thus, disaster
is narrowly averted (as far as I could tell).
Fixes: 6c2dc5ae4ab7 ("X.509: Extract signature digest and make self-signed cert checks earlier")
Cc: <stable@vger.kernel.org> # v4.7+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/asymmetric_keys/pkcs7_verify.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 39e6de0c2761..2f6a768b91d7 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -270,7 +270,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
sinfo->index);
return 0;
}
- ret = public_key_verify_signature(p->pub, p->sig);
+ ret = public_key_verify_signature(p->pub, x509->sig);
if (ret < 0)
return ret;
x509->signer = p;
--
2.16.0.rc1.238.g530d649a79-goog
next prev parent reply other threads:[~2018-02-07 1:10 UTC|newest]
Thread overview: 14+ 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 [this message]
2018-02-07 1:10 ` [PATCH 2/9] PKCS#7: fix certificate blacklisting Eric Biggers
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 ` [PATCH 5/9] X.509: fix NULL dereference when restricting key with unsupported_sig 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 ` [PATCH 7/9] X.509: remove never-set ->unsupported_key flag 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 ` [PATCH 9/9] X.509: self_signed implies !unsupported_sig 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-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-2-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 \
--cc=stable@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).