Bug 1413634, If TLS server has no signature algorithm overlap with the client hello list, the NSS server sends an incorrect TLS alert, r=mt
authorKai Engert <kaie@kuix.de>
Thu, 18 Jan 2018 13:16:49 +0100
changeset 14221 010767c7c33946bc34ae89587c81e239f10828df
parent 14220 729db9fa15f08e2a0c5cb74558f3bb8e22d94a46
child 14225 39c74bc63a1e4f8af741bae99891ce1a9c46b8fe
push id2964
push userkaie@kuix.de
push dateThu, 18 Jan 2018 12:16:22 +0000
reviewersmt
bugs1413634
Bug 1413634, If TLS server has no signature algorithm overlap with the client hello list, the NSS server sends an incorrect TLS alert, r=mt
gtests/ssl_gtest/ssl_extension_unittest.cc
lib/ssl/ssl3con.c
lib/ssl/ssl3exthandle.c
--- a/gtests/ssl_gtest/ssl_extension_unittest.cc
+++ b/gtests/ssl_gtest/ssl_extension_unittest.cc
@@ -419,17 +419,26 @@ TEST_P(TlsExtensionTest12Plus, Signature
   ClientHelloErrorTest(std::make_shared<TlsExtensionReplacer>(
       ssl_signature_algorithms_xtn, extension));
 }
 
 TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsEmpty) {
   const uint8_t val[] = {0x00, 0x00};
   DataBuffer extension(val, sizeof(val));
   ClientHelloErrorTest(std::make_shared<TlsExtensionReplacer>(
-      ssl_signature_algorithms_xtn, extension));
+                           ssl_signature_algorithms_xtn, extension),
+                       kTlsAlertHandshakeFailure);
+}
+
+TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsNoOverlap) {
+  const uint8_t val[] = {0x00, 0x02, 0xff, 0xff};
+  DataBuffer extension(val, sizeof(val));
+  ClientHelloErrorTest(std::make_shared<TlsExtensionReplacer>(
+                           ssl_signature_algorithms_xtn, extension),
+                       kTlsAlertHandshakeFailure);
 }
 
 TEST_P(TlsExtensionTest12Plus, SignatureAlgorithmsOddLength) {
   const uint8_t val[] = {0x00, 0x01, 0x04};
   DataBuffer extension(val, sizeof(val));
   ClientHelloErrorTest(std::make_shared<TlsExtensionReplacer>(
       ssl_signature_algorithms_xtn, extension));
 }
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -8106,16 +8106,19 @@ ssl3_HandleClientHello(sslSocket *ss, PR
             goto alert_loser;
         }
     }
 
     /* Now parse the rest of the extensions. */
     rv = ssl3_HandleParsedExtensions(ss, ssl_hs_client_hello);
     ssl3_DestroyRemoteExtensions(&ss->ssl3.hs.remoteExtensions);
     if (rv != SECSuccess) {
+        if (PORT_GetError() == SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM) {
+            errCode = SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM;
+        }
         goto loser; /* malformed */
     }
 
     /* If the ClientHello version is less than our maximum version, check for a
      * TLS_FALLBACK_SCSV and reject the connection if found. */
     if (ss->vrange.max > ss->version) {
         for (i = 0; i + 1 < suites.len; i += 2) {
             PRUint16 suite_i = (suites.data[i] << 8) | suites.data[i + 1];
--- a/lib/ssl/ssl3exthandle.c
+++ b/lib/ssl/ssl3exthandle.c
@@ -1647,21 +1647,26 @@ ssl3_HandleSigAlgsXtn(const sslSocket *s
     if (xtnData->sigSchemes) {
         PORT_Free(xtnData->sigSchemes);
         xtnData->sigSchemes = NULL;
     }
     rv = ssl_ParseSignatureSchemes(ss, NULL,
                                    &xtnData->sigSchemes,
                                    &xtnData->numSigSchemes,
                                    &data->data, &data->len);
-    if (rv != SECSuccess || xtnData->numSigSchemes == 0) {
+    if (rv != SECSuccess) {
         ssl3_ExtSendAlert(ss, alert_fatal, decode_error);
         PORT_SetError(SSL_ERROR_RX_MALFORMED_CLIENT_HELLO);
         return SECFailure;
     }
+    if (xtnData->numSigSchemes == 0) {
+        ssl3_ExtSendAlert(ss, alert_fatal, handshake_failure);
+        PORT_SetError(SSL_ERROR_UNSUPPORTED_SIGNATURE_ALGORITHM);
+        return SECFailure;
+    }
     /* Check for trailing data. */
     if (data->len != 0) {
         ssl3_ExtSendAlert(ss, alert_fatal, decode_error);
         PORT_SetError(SSL_ERROR_RX_MALFORMED_CLIENT_HELLO);
         return SECFailure;
     }
 
     /* Keep track of negotiated extensions. */