[openssl] master update
Matt Caswell
matt at openssl.org
Tue Apr 21 13:06:47 UTC 2020
The branch master has been updated
via a87f3fe01a5a894aa27ccd6a239155fd129988e4 (commit)
via 3656c08ab4b1b892730cb5e808b6f4298b08a2e6 (commit)
from b0b0b6a41dd4418bab19cdd28c01ce248fee6af7 (commit)
- Log -----------------------------------------------------------------
commit a87f3fe01a5a894aa27ccd6a239155fd129988e4
Author: Benjamin Kaduk <kaduk at mit.edu>
Date: Fri Apr 10 12:27:28 2020 -0700
Fix NULL dereference in SSL_check_chain() for TLS 1.3
In the tls1_check_sig_alg() helper function, we loop through the list of
"signature_algorithms_cert" values received from the client and attempt
to look up each one in turn in our internal table that maps wire
codepoint to string-form name, digest and/or signature NID, etc., in
order to compare the signature scheme from the peer's list against what
is used to sign the certificates in the certificate chain we're
checking. Unfortunately, when the peer sends a value that we don't
support, the lookup returns NULL, but we unconditionally dereference the
lookup result for the comparison, leading to an application crash
triggerable by an unauthenticated client.
Since we will not be able to say anything about algorithms we don't
recognize, treat NULL return from lookup as "does not match".
We currently only apply the "signature_algorithm_cert" checks on TLS 1.3
connections, so previous TLS versions are unaffected. SSL_check_chain()
is not called directly from libssl, but may be used by the application
inside a callback (e.g., client_hello or cert callback) to verify that a
candidate certificate chain will be acceptable to the client.
CVE-2020-1967
Reviewed-by: Matt Caswell <matt at openssl.org>
commit 3656c08ab4b1b892730cb5e808b6f4298b08a2e6
Author: Benjamin Kaduk <kaduk at mit.edu>
Date: Fri Apr 10 12:27:28 2020 -0700
Add test for CVE-2020-1967
Add to test_sslsigalgs a TLSProxy test that injects a
"signature_algorithms_cert" extension that contains an unallocated
codepoint.
The test currently fails, since s_server segfaults instead of
ignoring the unrecognized value.
Since "signature_algorithms" and "signature_algorithms_cert" are very
similar, also add the analogous test for "signature_algorithms".
Reviewed-by: Matt Caswell <matt at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
ssl/t1_lib.c | 2 +-
test/recipes/70-test_sslsigalgs.t | 66 +++++++++++++++++++++++++++++++++++++--
2 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index b9b3a60252..79f66601be 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2259,7 +2259,7 @@ static int tls1_check_sig_alg(SSL *s, X509 *x, int default_nid)
sigalg = use_pc_sigalgs
? tls1_lookup_sigalg(s->s3.tmp.peer_cert_sigalgs[i])
: s->shared_sigalgs[i];
- if (sig_nid == sigalg->sigandhash)
+ if (sigalg != NULL && sig_nid == sigalg->sigandhash)
return 1;
}
return 0;
diff --git a/test/recipes/70-test_sslsigalgs.t b/test/recipes/70-test_sslsigalgs.t
index 98482079b3..85f4381b4a 100644
--- a/test/recipes/70-test_sslsigalgs.t
+++ b/test/recipes/70-test_sslsigalgs.t
@@ -44,7 +44,9 @@ use constant {
COMPAT_SIGALGS => 6,
SIGALGS_CERT_ALL => 7,
SIGALGS_CERT_PKCS => 8,
- SIGALGS_CERT_INVALID => 9
+ SIGALGS_CERT_INVALID => 9,
+ UNRECOGNIZED_SIGALGS_CERT => 10,
+ UNRECOGNIZED_SIGALG => 11
};
#Note: Throughout this test we override the default ciphersuites where TLSv1.2
@@ -53,7 +55,7 @@ use constant {
#Test 1: Default sig algs should succeed
$proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
-plan tests => 24;
+plan tests => 26;
ok(TLSProxy::Message->success, "Default sigalgs");
my $testtype;
@@ -282,6 +284,39 @@ SKIP: {
ok(TLSProxy::Message->fail, "No matching certificate for sigalgs_cert");
}
+SKIP: {
+ skip "TLS 1.3 disabled", 2 if disabled("tls1_3");
+ #Test 25: Send an unrecognized signature_algorithms_cert
+ # We should be able to skip over the unrecognized value and use a
+ # valid one that appears later in the list.
+ $proxy->clear();
+ $proxy->filter(\&inject_unrecognized_sigalg);
+ $proxy->clientflags("-tls1_3");
+ # Use -xcert to get SSL_check_chain() to run in the cert_cb. This is
+ # needed to trigger (e.g.) CVE-2020-1967
+ $proxy->serverflags("" .
+ " -xcert " . srctop_file("test", "certs", "servercert.pem") .
+ " -xkey " . srctop_file("test", "certs", "serverkey.pem") .
+ " -xchain " . srctop_file("test", "certs", "rootcert.pem"));
+ $testtype = UNRECOGNIZED_SIGALGS_CERT;
+ $proxy->start();
+ ok(TLSProxy::Message->success(), "Unrecognized sigalg_cert in ClientHello");
+
+ #Test 26: Send an unrecognized signature_algorithms
+ # We should be able to skip over the unrecognized value and use a
+ # valid one that appears later in the list.
+ $proxy->clear();
+ $proxy->filter(\&inject_unrecognized_sigalg);
+ $proxy->clientflags("-tls1_3");
+ $proxy->serverflags("" .
+ " -xcert " . srctop_file("test", "certs", "servercert.pem") .
+ " -xkey " . srctop_file("test", "certs", "serverkey.pem") .
+ " -xchain " . srctop_file("test", "certs", "rootcert.pem"));
+ $testtype = UNRECOGNIZED_SIGALG;
+ $proxy->start();
+ ok(TLSProxy::Message->success(), "Unrecognized sigalg in ClientHello");
+}
+
sub sigalgs_filter
@@ -427,3 +462,30 @@ sub modify_cert_verify_sigalg
}
}
}
+
+sub inject_unrecognized_sigalg
+{
+ my $proxy = shift;
+ my $type;
+
+ # We're only interested in the initial ClientHello
+ if ($proxy->flight != 0) {
+ return;
+ }
+ if ($testtype == UNRECOGNIZED_SIGALGS_CERT) {
+ $type = TLSProxy::Message::EXT_SIG_ALGS_CERT;
+ } elsif ($testtype == UNRECOGNIZED_SIGALG) {
+ $type = TLSProxy::Message::EXT_SIG_ALGS;
+ } else {
+ return;
+ }
+
+ my $ext = pack "C8",
+ 0x00, 0x06, #Extension length
+ 0xfe, 0x18, #private use
+ 0x04, 0x01, #rsa_pkcs1_sha256
+ 0x08, 0x04; #rsa_pss_rsae_sha256;
+ my $message = ${$proxy->message_list}[0];
+ $message->set_extension($type, $ext);
+ $message->repack;
+}
More information about the openssl-commits
mailing list