From 96728bb2bc55246d2bb3d98e4c1ab4b5b58a5c41 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 22 Nov 2018 10:55:20 +0100 Subject: [PATCH] Bug 1412829, reject empty supported_signature_algorithms in CR in TLS 1.2, r=mt Summary: This basically reverts bug 1335069 to align with RFC 5246. Reviewers: mt Reviewed By: mt Bug #: 1412829 Differential Revision: https://phabricator.services.mozilla.com/D12563 --HG-- extra : amend_source : a87f98603e14841654948c7664dbde26ebaf04e4 --- gtests/nss_bogo_shim/config.json | 3 ++- gtests/ssl_gtest/ssl_auth_unittest.cc | 15 +++++---------- lib/ssl/ssl3con.c | 17 +++++++++-------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/gtests/nss_bogo_shim/config.json b/gtests/nss_bogo_shim/config.json index 66f55d38de..5c7a2e3481 100644 --- a/gtests/nss_bogo_shim/config.json +++ b/gtests/nss_bogo_shim/config.json @@ -64,7 +64,8 @@ "RequireAnyClientCertificate-TLS1*":"Bug 1339387", "SendExtensionOnClientCertificate-TLS13":"Bug 1339392", "ALPNClient-Mismatch-TLS13":"NSS sends alerts in response to errors in protected handshake messages in the clear", - "P224-Server":"NSS doesn't support P-224" + "P224-Server":"NSS doesn't support P-224", + "ClientAuth-SHA1-Fallback*":"Boring wants us to fall back to SHA-1 if supported_signature_algorithms in CR is empty." }, "ErrorMap" : { ":HANDSHAKE_FAILURE_ON_CLIENT_HELLO:":"SSL_ERROR_NO_CYPHER_OVERLAP", diff --git a/gtests/ssl_gtest/ssl_auth_unittest.cc b/gtests/ssl_gtest/ssl_auth_unittest.cc index 93a8c540ad..3a52ac20c5 100644 --- a/gtests/ssl_gtest/ssl_auth_unittest.cc +++ b/gtests/ssl_gtest/ssl_auth_unittest.cc @@ -386,9 +386,9 @@ class TlsZeroCertificateRequestSigAlgsFilter : public TlsHandshakeFilter { } }; -// Check that we fall back to SHA-1 when the server doesn't provide any +// Check that we send an alert when the server doesn't provide any // supported_signature_algorithms in the CertificateRequest message. -TEST_P(TlsConnectTls12, ClientAuthNoSigAlgsFallback) { +TEST_P(TlsConnectTls12, ClientAuthNoSigAlgs) { EnsureTlsSetup(); MakeTlsFilter(server_); auto capture_cert_verify = MakeTlsFilter( @@ -396,15 +396,10 @@ TEST_P(TlsConnectTls12, ClientAuthNoSigAlgsFallback) { client_->SetupClientAuth(); server_->RequestClientAuth(true); - ConnectExpectAlert(server_, kTlsAlertDecryptError); - - // We're expecting a bad signature here because we tampered with a handshake - // message (CertReq). Previously, without the SHA-1 fallback, we would've - // seen a malformed record alert. - server_->CheckErrorCode(SEC_ERROR_BAD_SIGNATURE); - client_->CheckErrorCode(SSL_ERROR_DECRYPT_ERROR_ALERT); + ConnectExpectAlert(client_, kTlsAlertHandshakeFailure); - CheckSigScheme(capture_cert_verify, 0, server_, ssl_sig_rsa_pkcs1_sha1, 1024); + server_->CheckErrorCode(SSL_ERROR_HANDSHAKE_FAILURE_ALERT); + client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM); } static const SSLSignatureScheme kSignatureSchemeEcdsaSha384[] = { diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index d7e8452314..225f4f679b 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -6171,16 +6171,12 @@ ssl_PickClientSignatureScheme(sslSocket *ss, const SSLSignatureScheme *schemes, PORT_Assert(pubKey); - if (!isTLS13 && numSchemes == 0) { - /* If the server didn't provide any signature algorithms - * then let's assume they support SHA-1. */ - rv = ssl_PickFallbackSignatureScheme(ss, pubKey); - SECKEY_DestroyPublicKey(pubKey); - return rv; + if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_2) { + /* We should have already checked that a signature scheme was + * listed in the request. */ + PORT_Assert(schemes && numSchemes > 0); } - PORT_Assert(schemes && numSchemes > 0); - if (!isTLS13 && (SECKEY_GetPublicKeyType(pubKey) == rsaKey || SECKEY_GetPublicKeyType(pubKey) == dsaKey) && @@ -7331,6 +7327,11 @@ ssl3_HandleCertificateRequest(sslSocket *ss, PRUint8 *b, PRUint32 length) PORT_SetError(SSL_ERROR_RX_MALFORMED_CERT_REQUEST); goto loser; /* malformed, alert has been sent */ } + if (signatureSchemeCount == 0) { + errCode = SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM; + desc = handshake_failure; + goto alert_loser; + } } rv = ssl3_ParseCertificateRequestCAs(ss, &b, &length, &ca_list);