115 lines
4.5 KiB
Diff
115 lines
4.5 KiB
Diff
|
From 96728bb2bc55246d2bb3d98e4c1ab4b5b58a5c41 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Daiki Ueno <dueno@redhat.com>
|
||
|
|
Date: Thu, 22 Nov 2018 10:55:20 +0100
|
||
|
|
Subject: [PATCH 441/489] 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 66f55d3..5c7a2e3 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 93a8c54..3a52ac2 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<TlsZeroCertificateRequestSigAlgsFilter>(server_);
|
||
|
|
auto capture_cert_verify = MakeTlsFilter<TlsHandshakeRecorder>(
|
||
|
|
@@ -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 d7e8452..225f4f6 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);
|
||
|
|
--
|
||
|
|
1.7.12.4
|
||
|
|
|