Bug 1088950 - Fix handling of inappropriate_fallback alert. r=keeler
authorMartin Thomson <martin.thomson@gmail.com>
Mon, 27 Oct 2014 17:47:00 +0100
changeset 213170 e41f03a6b32585bed66bfa1939a637bdadc6f2bc
parent 213169 c13d91ae9f04451636b11910ae13d41568e6f62f
child 213171 484e85ba2134dacba7b54ab4c020ac1fc7aaae59
push id27742
push userryanvm@gmail.com
push dateThu, 30 Oct 2014 20:15:35 +0000
treeherdermozilla-central@e0b505a37b1c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1088950
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1088950 - Fix handling of inappropriate_fallback alert. r=keeler
security/manager/ssl/src/nsNSSIOLayer.cpp
security/manager/ssl/src/nsNSSIOLayer.h
--- a/security/manager/ssl/src/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/src/nsNSSIOLayer.cpp
@@ -861,43 +861,55 @@ nsSSLIOLayerHelpers::rememberTolerantAtV
     entry.intoleranceReason = 0;
   }
 
   entry.AssertInvariant();
 
   mTLSIntoleranceInfo.Put(key, entry);
 }
 
+void nsSSLIOLayerHelpers::forgetIntolerance(const nsACString& hostName,
+                                            int16_t port)
+{
+  nsCString key;
+  getSiteKey(hostName, port, key);
+
+  MutexAutoLock lock(mutex);
+
+  IntoleranceEntry entry;
+  if (mTLSIntoleranceInfo.Get(key, &entry)) {
+    entry.AssertInvariant();
+
+    entry.intolerant = 0;
+    entry.intoleranceReason = 0;
+
+    entry.AssertInvariant();
+    mTLSIntoleranceInfo.Put(key, entry);
+  }
+}
+
 // returns true if we should retry the handshake
 bool
 nsSSLIOLayerHelpers::rememberIntolerantAtVersion(const nsACString& hostName,
                                                  int16_t port,
                                                  uint16_t minVersion,
                                                  uint16_t intolerant,
                                                  PRErrorCode intoleranceReason)
 {
+  if (intolerant <= minVersion || intolerant <= mVersionFallbackLimit) {
+    // We can't fall back any further. Assume that intolerance isn't the issue.
+    forgetIntolerance(hostName, port);
+    return false;
+  }
+
   nsCString key;
   getSiteKey(hostName, port, key);
 
   MutexAutoLock lock(mutex);
 
-  if (intolerant <= minVersion || intolerant <= mVersionFallbackLimit) {
-    // We can't fall back any further. Assume that intolerance isn't the issue.
-    IntoleranceEntry entry;
-    if (mTLSIntoleranceInfo.Get(key, &entry)) {
-      entry.AssertInvariant();
-      entry.intolerant = 0;
-      entry.intoleranceReason = 0;
-      entry.AssertInvariant();
-      mTLSIntoleranceInfo.Put(key, entry);
-    }
-
-    return false;
-  }
-
   IntoleranceEntry entry;
   if (mTLSIntoleranceInfo.Get(key, &entry)) {
     entry.AssertInvariant();
     if (intolerant <= entry.tolerant) {
       // We already know the server is tolerant at an equal or higher version.
       return false;
     }
     if ((entry.intolerant != 0 && intolerant >= entry.intolerant)) {
@@ -1137,32 +1149,29 @@ retryDueToTLSIntolerance(PRErrorCode err
   // This function is supposed to decide which error codes should
   // be used to conclude server is TLS intolerant.
   // Note this only happens during the initial SSL handshake.
 
   SSLVersionRange range = socketInfo->GetTLSVersionRange();
 
   if (err == SSL_ERROR_INAPPROPRIATE_FALLBACK_ALERT) {
     // This is a clear signal that we've fallen back too many versions.  Treat
-    // this as a hard failure now, but also mark the next higher version as
-    // being tolerant so that later attempts don't use this version (i.e.,
-    // range.max), which makes the error unrecoverable without a full restart.
+    // this as a hard failure, but forget any intolerance so that later attempts
+    // don't use this version (i.e., range.max) and trigger the error again.
 
     // First, track the original cause of the version fallback.  This uses the
     // same buckets as the telemetry below, except that bucket 0 will include
     // all cases where there wasn't an original reason.
     PRErrorCode originalReason = socketInfo->SharedState().IOLayerHelpers()
       .getIntoleranceReason(socketInfo->GetHostName(), socketInfo->GetPort());
     Telemetry::Accumulate(Telemetry::SSL_VERSION_FALLBACK_INAPPROPRIATE,
                           tlsIntoleranceTelemetryBucket(originalReason));
 
     socketInfo->SharedState().IOLayerHelpers()
-      .rememberTolerantAtVersion(socketInfo->GetHostName(),
-                                 socketInfo->GetPort(),
-                                 range.max + 1);
+      .forgetIntolerance(socketInfo->GetHostName(), socketInfo->GetPort());
 
     return false;
   }
 
   // When not using a proxy we'll see a connection reset error.
   // When using a proxy, we'll see an end of file error.
   // In addition check for some error codes where it is reasonable
   // to retry without TLS.
--- a/security/manager/ssl/src/nsNSSIOLayer.h
+++ b/security/manager/ssl/src/nsNSSIOLayer.h
@@ -211,16 +211,17 @@ private:
   };
   nsDataHashtable<nsCStringHashKey, IntoleranceEntry> mTLSIntoleranceInfo;
 public:
   void rememberTolerantAtVersion(const nsACString& hostname, int16_t port,
                                  uint16_t tolerant);
   bool rememberIntolerantAtVersion(const nsACString& hostname, int16_t port,
                                    uint16_t intolerant, uint16_t minVersion,
                                    PRErrorCode intoleranceReason);
+  void forgetIntolerance(const nsACString& hostname, int16_t port);
   void adjustForTLSIntolerance(const nsACString& hostname, int16_t port,
                                /*in/out*/ SSLVersionRange& range);
   PRErrorCode getIntoleranceReason(const nsACString& hostname, int16_t port);
 
   void setRenegoUnrestrictedSites(const nsCString& str);
   bool isRenegoUnrestrictedSite(const nsCString& str);
   void clearStoredData();
   void loadVersionFallbackLimit();