Bug 1286694 - Part 3: Change the fallback sequence to reduce a redundant fallback when TLS 1.3 is enabled. r?keeler draft
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Wed, 20 Jul 2016 07:40:14 +0900
changeset 389727 734558698df8b4ed2a939e9f7c6cb98941802294
parent 389661 9bab9be8e9343ac18eaa641592aeaaf5ce6735f6
child 389728 f8c6b72de3a019c12ab417bea897d2e2bfaa2b02
push id23500
push userVYV03354@nifty.ne.jp
push dateTue, 19 Jul 2016 22:44:05 +0000
reviewerskeeler
bugs1286694
milestone50.0a1
Bug 1286694 - Part 3: Change the fallback sequence to reduce a redundant fallback when TLS 1.3 is enabled. r?keeler MozReview-Commit-ID: BUf0tY6bitW
security/manager/ssl/nsNSSIOLayer.cpp
security/manager/ssl/tests/unit/test_fallback_cipher.js
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -1049,18 +1049,21 @@ retryDueToTLSIntolerance(PRErrorCode err
                               socketInfo->GetPort());
 
     return false;
   }
 
   if ((err == SSL_ERROR_NO_CYPHER_OVERLAP || err == PR_END_OF_FILE_ERROR ||
        err == PR_CONNECT_RESET_ERROR) &&
       nsNSSComponent::AreAnyFallbackCiphersEnabled()) {
+    // There is no point in trying fallback cipher suites with TLS 1.3
+    // because TLS 1.3 only supports AEAD cipher suites.
     if (helpers.rememberStrongCiphersFailed(socketInfo->GetHostName(),
-                                            socketInfo->GetPort(), err)) {
+                                            socketInfo->GetPort(), err) &&
+        range.max < SSL_LIBRARY_VERSION_TLS_1_3) {
       return true;
     }
   }
 
   // When not using a proxy we'll see a connection reset error.
   // When using a proxy, we'll see an end of file error.
 
   // Don't allow STARTTLS connections to fall back on connection resets or
--- a/security/manager/ssl/tests/unit/test_fallback_cipher.js
+++ b/security/manager/ssl/tests/unit/test_fallback_cipher.js
@@ -89,16 +89,18 @@ function startServer(cert, fallbackOnly)
   tlsServer.setRequestClientCertificate(Ci.nsITLSServerSocket.REQUEST_NEVER);
   if (fallbackOnly) {
     let cipherSuites = [
       TLS_DHE_RSA_WITH_AES_128_CBC_SHA,
       TLS_DHE_RSA_WITH_AES_256_CBC_SHA
     ];
     tlsServer.setCipherSuites(cipherSuites, cipherSuites.length);
   }
+  tlsServer.setVersionRange(Ci.nsITLSClientStatus.TLS_VERSION_1,
+                            Ci.nsITLSClientStatus.TLS_VERSION_1_2);
 
   tlsServer.asyncListen(listener);
 
   return tlsServer.port;
 }
 
 function storeCertOverride(port, cert) {
   let overrideBits = Ci.nsICertOverrideService.ERROR_UNTRUSTED |
@@ -174,72 +176,88 @@ function startClient(name, port, expecte
   transport.setEventSink(handler, Services.tm.currentThread);
   output = transport.openOutputStream(Ci.nsITransport.OPEN_UNBUFFERED, 0, 0);
   output.QueryInterface(Ci.nsIAsyncOutputStream);
 
   return deferred.promise;
 }
 
 function run_test() {
-  Services.prefs.setBoolPref("security.tls.unrestricted_rc4_fallback", false);
   run_next_test();
 }
 
 // for sanity check
 add_task(function* () {
   let cert = yield getCert();
   ok(!!cert, "Got self-signed cert");
   let port = startServer(cert, false);
   storeCertOverride(port, cert);
   yield startClient("sanity check, public", port, Cr.NS_OK);
   yield startClient("sanity check, private", port, Cr.NS_OK, {isPrivate: true});
 });
 
 add_task(function* () {
   let cert = yield getCert(Ci.nsILocalCertService.KEY_TYPE_RSA);
   ok(!!cert, "Got self-signed cert");
-  let port = startServer(cert, true);
-  storeCertOverride(port, cert);
+  for (let versionPref of [3, 4]) {
+    let port = startServer(cert, true);
+    storeCertOverride(port, cert);
 
-  // disable fallback cipher suites
-  for (let pref of fallback_cipher_prefs) {
-    Services.prefs.setBoolPref(pref, false);
-  }
+    Services.prefs.setIntPref("security.tls.version.max", versionPref);
 
-  yield startClient("server: fallback only, client: fallback disabled, public",
-                    port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
-  yield startClient("server: fallback only, client: fallback disabled, private",
-                    port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {isPrivate: true});
+    // disable fallback cipher suites
+    for (let pref of fallback_cipher_prefs) {
+      Services.prefs.setBoolPref(pref, false);
+    }
+    if (versionPref >= 4) {
+      Services.prefs.setIntPref("security.tls.version.fallback-limit", 4);
+    }
 
-  // enable fallback cipher suites
-  for (let pref of fallback_cipher_prefs) {
-    Services.prefs.setBoolPref(pref, true);
-  }
+    yield startClient("server: fallback only, client: fallback disabled, public",
+                      port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
+    // make sure we do not remember the intolerance
+    yield startClient("server: fallback only, client: retry, fallback disabled, public",
+                      port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
+    yield startClient("server: fallback only, client: fallback disabled, private",
+                      port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
+                      {isPrivate: true});
+    // make sure we do not remember the intolerance
+    yield startClient("server: fallback only, client: retry, fallback disabled, private",
+                      port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
+                      {isPrivate: true});
 
-  // The auto-retry on connection reset is implemented in our HTTP layer.
-  // So we will see the crafted NS_ERROR_NET_RESET when we use
-  // nsISocketTransport directly.
-  yield startClient("server: fallback only, client: fallback enabled, public",
-                    port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {allowReset: true});
-  // retry manually to simulate the HTTP layer
-  yield startClient("server: fallback only, client: fallback enabled retry, public",
-                    port, Cr.NS_OK);
-  // make sure that we remember the TLS intolerance
-  yield startClient("server: fallback only, client: second try after fallback success, public",
-                    port, Cr.NS_OK);
-  yield startClient("server: fallback only, client: fallback enabled, private",
-                    port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {isPrivate: true, allowReset: true});
-  yield startClient("server: fallback only, client: fallback enabled retry, private",
-                    port, Cr.NS_OK, {isPrivate: true});
-  // make sure that we remember the TLS intolerance
-  yield startClient("server: fallback only, client: second try after fallback success, private",
-                    port, Cr.NS_OK, {isPrivate: true});
+    // enable fallback cipher suites
+    for (let pref of fallback_cipher_prefs) {
+      Services.prefs.setBoolPref(pref, true);
+    }
+    Services.prefs.clearUserPref("security.tls.version.fallback-limit");
+
+    // The auto-retry on connection reset is implemented in our HTTP layer.
+    // So we will see the crafted NS_ERROR_NET_RESET when we use
+    // nsISocketTransport directly.
+    yield startClient("server: fallback only, client: fallback enabled, public",
+                      port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
+                      {allowReset: true});
+    // retry manually to simulate the HTTP layer
+    yield startClient("server: fallback only, client: fallback enabled retry, public",
+                      port, Cr.NS_OK);
+    // make sure that we remember the TLS intolerance
+    yield startClient("server: fallback only, client: second try after fallback success, public",
+                      port, Cr.NS_OK);
+    yield startClient("server: fallback only, client: fallback enabled, private",
+                      port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
+                      {isPrivate: true, allowReset: true});
+    yield startClient("server: fallback only, client: fallback enabled retry, private",
+                      port, Cr.NS_OK, {isPrivate: true});
+    // make sure that we remember the TLS intolerance
+    yield startClient("server: fallback only, client: second try after fallback success, private",
+                      port, Cr.NS_OK, {isPrivate: true});
+  }
 });
 
 do_register_cleanup(function() {
   do_print("reset modified prefs");
   for (let pref of fallback_cipher_prefs) {
     Services.prefs.clearUserPref(pref);
   }
+  Services.prefs.clearUserPref("security.tls.version.max");
+  Services.prefs.clearUserPref("security.tls.version.fallback-limit");
 });