Bug 1083058 - Add a pref to control TLS version fallback. r=keeler, a=lsblakk
authorMartin Thomson <martin.thomson@gmail.com>
Thu, 02 Oct 2014 16:36:48 -0700
changeset 225785 ae15f14a1db1
parent 225784 27b0655c1385
child 225786 79560a3c511f
push id4015
push userryanvm@gmail.com
push date2014-10-23 01:19 +0000
treeherdermozilla-beta@a80d4ca56309 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler, lsblakk
bugs1083058
milestone34.0
Bug 1083058 - Add a pref to control TLS version fallback. r=keeler, a=lsblakk
netwerk/base/public/security-prefs.js
security/manager/ssl/src/nsNSSIOLayer.cpp
security/manager/ssl/src/nsNSSIOLayer.h
security/manager/ssl/tests/gtest/TLSIntoleranceTest.cpp
--- a/netwerk/base/public/security-prefs.js
+++ b/netwerk/base/public/security-prefs.js
@@ -1,14 +1,15 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 pref("security.tls.version.min", 1);
 pref("security.tls.version.max", 3);
+pref("security.tls.version.fallback-limit", 1);
 
 pref("security.ssl.allow_unrestricted_renego_everywhere__temporarily_available_pref", false);
 pref("security.ssl.renego_unrestricted_hosts", "");
 pref("security.ssl.treat_unsafe_negotiation_as_broken", false);
 pref("security.ssl.require_safe_negotiation",  false);
 pref("security.ssl.warn_missing_rfc5746",  1);
 pref("security.ssl.enable_ocsp_stapling", true);
 pref("security.ssl.enable_false_start", true);
--- a/security/manager/ssl/src/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/src/nsNSSIOLayer.cpp
@@ -812,17 +812,17 @@ nsSSLIOLayerHelpers::rememberIntolerantA
                                                  uint16_t minVersion,
                                                  uint16_t intolerant)
 {
   nsCString key;
   getSiteKey(hostName, port, key);
 
   MutexAutoLock lock(mutex);
 
-  if (intolerant <= minVersion) {
+  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.AssertInvariant();
       mTLSIntoleranceInfo.Put(key, entry);
     }
@@ -1261,16 +1261,17 @@ nsSSLIOLayerPoll(PRFileDesc* fd, int16_t
 
 nsSSLIOLayerHelpers::nsSSLIOLayerHelpers()
   : mRenegoUnrestrictedSites(nullptr)
   , mTreatUnsafeNegotiationAsBroken(false)
   , mWarnLevelMissingRFC5746(1)
   , mTLSIntoleranceInfo()
   , mFalseStartRequireNPN(true)
   , mFalseStartRequireForwardSecrecy(false)
+  , mVersionFallbackLimit(SSL_LIBRARY_VERSION_TLS_1_0)
   , mutex("nsSSLIOLayerHelpers.mutex")
 {
 }
 
 static int
 _PSM_InvalidInt(void)
 {
     PR_ASSERT(!"I/O method is invalid");
@@ -1477,16 +1478,18 @@ PrefObserver::Observe(nsISupports* aSubj
     } else if (prefName.EqualsLiteral("security.ssl.false_start.require-npn")) {
       mOwner->mFalseStartRequireNPN =
         Preferences::GetBool("security.ssl.false_start.require-npn",
                              FALSE_START_REQUIRE_NPN_DEFAULT);
     } else if (prefName.EqualsLiteral("security.ssl.false_start.require-forward-secrecy")) {
       mOwner->mFalseStartRequireForwardSecrecy =
         Preferences::GetBool("security.ssl.false_start.require-forward-secrecy",
                              FALSE_START_REQUIRE_FORWARD_SECRECY_DEFAULT);
+    } else if (prefName.EqualsLiteral("security.tls.version.fallback-limit")) {
+      mOwner->loadVersionFallbackLimit();
     }
   }
   return NS_OK;
 }
 
 static int32_t
 PlaintextRecv(PRFileDesc* fd, void* buf, int32_t amount, int flags,
               PRIntervalTime timeout)
@@ -1585,32 +1588,48 @@ nsSSLIOLayerHelpers::Init()
   setWarnLevelMissingRFC5746(warnLevel);
 
   mFalseStartRequireNPN =
     Preferences::GetBool("security.ssl.false_start.require-npn",
                          FALSE_START_REQUIRE_NPN_DEFAULT);
   mFalseStartRequireForwardSecrecy =
     Preferences::GetBool("security.ssl.false_start.require-forward-secrecy",
                          FALSE_START_REQUIRE_FORWARD_SECRECY_DEFAULT);
+  loadVersionFallbackLimit();
 
   mPrefObserver = new PrefObserver(this);
   Preferences::AddStrongObserver(mPrefObserver,
                                  "security.ssl.renego_unrestricted_hosts");
   Preferences::AddStrongObserver(mPrefObserver,
                                  "security.ssl.treat_unsafe_negotiation_as_broken");
   Preferences::AddStrongObserver(mPrefObserver,
                                  "security.ssl.warn_missing_rfc5746");
   Preferences::AddStrongObserver(mPrefObserver,
                                  "security.ssl.false_start.require-npn");
   Preferences::AddStrongObserver(mPrefObserver,
                                  "security.ssl.false_start.require-forward-secrecy");
+  Preferences::AddStrongObserver(mPrefObserver,
+                                 "security.tls.version.fallback-limit");
   return NS_OK;
 }
 
 void
+nsSSLIOLayerHelpers::loadVersionFallbackLimit()
+{
+  // see nsNSSComponent::setEnabledTLSVersions for pref handling rules
+  int32_t limit = 1;   // 1 = TLS 1.0
+  Preferences::GetInt("security.tls.version.fallback-limit", &limit);
+  limit += SSL_LIBRARY_VERSION_3_0;
+  mVersionFallbackLimit = (uint16_t)limit;
+  if (limit != (int32_t)mVersionFallbackLimit) { // overflow check
+    mVersionFallbackLimit = SSL_LIBRARY_VERSION_TLS_1_0;
+  }
+}
+
+void
 nsSSLIOLayerHelpers::clearStoredData()
 {
   mRenegoUnrestrictedSites->Clear();
   mTLSIntoleranceInfo.Clear();
 }
 
 void
 nsSSLIOLayerHelpers::setRenegoUnrestrictedSites(const nsCString& str)
@@ -1795,26 +1814,26 @@ loser:
 // strings, and determine in which mode it is in. We currently use ASK mode for
 // UI apps and AUTO mode for UI-less apps without really asking for
 // preferences.
 nsresult
 nsGetUserCertChoice(SSM_UserCertChoice* certChoice)
 {
   char* mode = nullptr;
   nsresult ret;
-  
+
   NS_ENSURE_ARG_POINTER(certChoice);
-  
+
   nsCOMPtr<nsIPrefBranch> pref = do_GetService(NS_PREFSERVICE_CONTRACTID);
-  
+
   ret = pref->GetCharPref("security.default_personal_cert", &mode);
   if (NS_FAILED(ret)) {
     goto loser;
   }
-  
+
   if (PL_strcmp(mode, "Select Automatically") == 0) {
     *certChoice = AUTO;
   } else if (PL_strcmp(mode, "Ask Every Time") == 0) {
     *certChoice = ASK;
   } else {
     // Most likely we see a nickname from a migrated cert.
     // We do not currently support that, ask the user which cert to use.
     *certChoice = ASK;
--- a/security/manager/ssl/src/nsNSSIOLayer.h
+++ b/security/manager/ssl/src/nsNSSIOLayer.h
@@ -197,19 +197,21 @@ public:
   bool rememberIntolerantAtVersion(const nsACString& hostname, int16_t port,
                                    uint16_t intolerant, uint16_t minVersion);
   void adjustForTLSIntolerance(const nsACString& hostname, int16_t port,
                                /*in/out*/ SSLVersionRange& range);
 
   void setRenegoUnrestrictedSites(const nsCString& str);
   bool isRenegoUnrestrictedSite(const nsCString& str);
   void clearStoredData();
+  void loadVersionFallbackLimit();
 
   bool mFalseStartRequireNPN;
   bool mFalseStartRequireForwardSecrecy;
+  uint16_t mVersionFallbackLimit;
 private:
   mozilla::Mutex mutex;
   nsCOMPtr<nsIObserver> mPrefObserver;
 };
 
 nsresult nsSSLIOLayerNewSocket(int32_t family,
                                const char* host,
                                int32_t port,
--- a/security/manager/ssl/tests/gtest/TLSIntoleranceTest.cpp
+++ b/security/manager/ssl/tests/gtest/TLSIntoleranceTest.cpp
@@ -15,16 +15,18 @@ const int16_t PORT = 443;
 class TLSIntoleranceTest : public ::testing::Test
 {
 protected:
   nsSSLIOLayerHelpers helpers;
 };
 
 TEST_F(TLSIntoleranceTest, Test_1_2_through_3_0)
 {
+  helpers.mVersionFallbackLimit = SSL_LIBRARY_VERSION_3_0;
+
   // No adjustment made when there is no entry for the site.
   {
     SSLVersionRange range = { SSL_LIBRARY_VERSION_3_0,
                               SSL_LIBRARY_VERSION_TLS_1_2 };
     helpers.adjustForTLSIntolerance(HOST, PORT, range);
     ASSERT_EQ(SSL_LIBRARY_VERSION_3_0, range.min);
     ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, range.max);
 
@@ -73,50 +75,98 @@ TEST_F(TLSIntoleranceTest, Test_1_2_thro
     helpers.adjustForTLSIntolerance(HOST, PORT, range);
     ASSERT_EQ(SSL_LIBRARY_VERSION_3_0, range.min);
     // When rememberIntolerantAtVersion returns false, it also resets the
     // intolerance information for the server.
     ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, range.max);
   }
 }
 
+TEST_F(TLSIntoleranceTest, Test_Disable_Fallback_With_High_Limit)
+{
+  // this value disables version fallback entirely: with this value, all efforts
+  // to mark an origin as version intolerant fail
+  helpers.mVersionFallbackLimit = SSL_LIBRARY_VERSION_TLS_1_2;
+  ASSERT_FALSE(helpers.rememberIntolerantAtVersion(HOST, PORT,
+                                                   SSL_LIBRARY_VERSION_3_0,
+                                                   SSL_LIBRARY_VERSION_TLS_1_2));
+  ASSERT_FALSE(helpers.rememberIntolerantAtVersion(HOST, PORT,
+                                                   SSL_LIBRARY_VERSION_3_0,
+                                                   SSL_LIBRARY_VERSION_TLS_1_1));
+  ASSERT_FALSE(helpers.rememberIntolerantAtVersion(HOST, PORT,
+                                                   SSL_LIBRARY_VERSION_3_0,
+                                                   SSL_LIBRARY_VERSION_TLS_1_0));
+}
+
+TEST_F(TLSIntoleranceTest, Test_Fallback_Limit_Default)
+{
+  // the default limit prevents SSL 3.0 fallback
+  ASSERT_EQ(helpers.mVersionFallbackLimit, SSL_LIBRARY_VERSION_TLS_1_0);
+  ASSERT_TRUE(helpers.rememberIntolerantAtVersion(HOST, PORT,
+                                                  SSL_LIBRARY_VERSION_3_0,
+                                                  SSL_LIBRARY_VERSION_TLS_1_1));
+  ASSERT_FALSE(helpers.rememberIntolerantAtVersion(HOST, PORT,
+                                                   SSL_LIBRARY_VERSION_3_0,
+                                                   SSL_LIBRARY_VERSION_TLS_1_0));
+}
+
+TEST_F(TLSIntoleranceTest, Test_Fallback_Limit_Below_Min)
+{
+  // check that we still respect the minimum version,
+  // when it is higher than the fallback limit
+  ASSERT_TRUE(helpers.rememberIntolerantAtVersion(HOST, PORT,
+                                                  SSL_LIBRARY_VERSION_TLS_1_1,
+                                                  SSL_LIBRARY_VERSION_TLS_1_2));
+  {
+    SSLVersionRange range = { SSL_LIBRARY_VERSION_3_0,
+                              SSL_LIBRARY_VERSION_TLS_1_2 };
+    helpers.adjustForTLSIntolerance(HOST, PORT, range);
+    ASSERT_EQ(SSL_LIBRARY_VERSION_3_0, range.min);
+    ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_1, range.max);
+  }
+
+  ASSERT_FALSE(helpers.rememberIntolerantAtVersion(HOST, PORT,
+                                                   SSL_LIBRARY_VERSION_TLS_1_1,
+                                                   SSL_LIBRARY_VERSION_TLS_1_1));
+}
+
 TEST_F(TLSIntoleranceTest, Test_Tolerant_Overrides_Intolerant_1)
 {
   ASSERT_TRUE(helpers.rememberIntolerantAtVersion(HOST, PORT,
                                                   SSL_LIBRARY_VERSION_3_0,
-                                                  SSL_LIBRARY_VERSION_TLS_1_0));
-  helpers.rememberTolerantAtVersion(HOST, PORT, SSL_LIBRARY_VERSION_TLS_1_0);
-  SSLVersionRange range = { SSL_LIBRARY_VERSION_3_0,
-                            SSL_LIBRARY_VERSION_TLS_1_2 };
-  helpers.adjustForTLSIntolerance(HOST, PORT, range);
-  ASSERT_EQ(SSL_LIBRARY_VERSION_3_0, range.min);
-  ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_0, range.max);
-}
-
-TEST_F(TLSIntoleranceTest, Test_Tolerant_Overrides_Intolerant_2)
-{
-  ASSERT_TRUE(helpers.rememberIntolerantAtVersion(HOST, PORT,
-                                                  SSL_LIBRARY_VERSION_3_0,
-                                                  SSL_LIBRARY_VERSION_TLS_1_0));
+                                                  SSL_LIBRARY_VERSION_TLS_1_1));
   helpers.rememberTolerantAtVersion(HOST, PORT, SSL_LIBRARY_VERSION_TLS_1_1);
   SSLVersionRange range = { SSL_LIBRARY_VERSION_3_0,
                             SSL_LIBRARY_VERSION_TLS_1_2 };
   helpers.adjustForTLSIntolerance(HOST, PORT, range);
   ASSERT_EQ(SSL_LIBRARY_VERSION_3_0, range.min);
   ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_1, range.max);
 }
 
+TEST_F(TLSIntoleranceTest, Test_Tolerant_Overrides_Intolerant_2)
+{
+  ASSERT_TRUE(helpers.rememberIntolerantAtVersion(HOST, PORT,
+                                                  SSL_LIBRARY_VERSION_3_0,
+                                                  SSL_LIBRARY_VERSION_TLS_1_1));
+  helpers.rememberTolerantAtVersion(HOST, PORT, SSL_LIBRARY_VERSION_TLS_1_2);
+  SSLVersionRange range = { SSL_LIBRARY_VERSION_3_0,
+                            SSL_LIBRARY_VERSION_TLS_1_2 };
+  helpers.adjustForTLSIntolerance(HOST, PORT, range);
+  ASSERT_EQ(SSL_LIBRARY_VERSION_3_0, range.min);
+  ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, range.max);
+}
+
 TEST_F(TLSIntoleranceTest, Test_Intolerant_Does_Not_Override_Tolerant)
 {
   // No adjustment made when there is no entry for the site.
-  helpers.rememberTolerantAtVersion(HOST, PORT, SSL_LIBRARY_VERSION_TLS_1_0);
+  helpers.rememberTolerantAtVersion(HOST, PORT, SSL_LIBRARY_VERSION_TLS_1_1);
   // false because we reached the floor set by rememberTolerantAtVersion.
   ASSERT_FALSE(helpers.rememberIntolerantAtVersion(HOST, PORT,
                                                    SSL_LIBRARY_VERSION_3_0,
-                                                   SSL_LIBRARY_VERSION_TLS_1_0));
+                                                   SSL_LIBRARY_VERSION_TLS_1_1));
   SSLVersionRange range = { SSL_LIBRARY_VERSION_3_0,
                             SSL_LIBRARY_VERSION_TLS_1_2 };
   helpers.adjustForTLSIntolerance(HOST, PORT, range);
   ASSERT_EQ(SSL_LIBRARY_VERSION_3_0, range.min);
   ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, range.max);
 }
 
 TEST_F(TLSIntoleranceTest, Test_Port_Is_Relevant)