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

