Bug 1279479 - Hide DHE cipher suites from the first handshake. r=keeler
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Thu, 23 Jun 2016 03:48:02 +0900
changeset 343136 1a1d7ef3cb0e6076f98088b57ebf7c2946e4c52f
parent 343135 bcd10b16cb335edf3dcb653b2e3db91176ecd8e0
child 343137 231f18c06694da3d6cda786e4055f262a04e6a3c
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1279479
milestone50.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 1279479 - Hide DHE cipher suites from the first handshake. r=keeler MozReview-Commit-ID: BXZm6EMaLy2
dom/flyweb/HttpServer.cpp
security/manager/ssl/LocalCertService.cpp
security/manager/ssl/nsILocalCertService.idl
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/nsNSSComponent.h
security/manager/ssl/nsNSSIOLayer.cpp
security/manager/ssl/tests/unit/test_fallback_cipher.js
security/manager/ssl/tests/unit/test_weak_crypto.js
security/manager/ssl/tests/unit/xpcshell.ini
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/histogram-whitelists.json
--- a/dom/flyweb/HttpServer.cpp
+++ b/dom/flyweb/HttpServer.cpp
@@ -52,17 +52,18 @@ HttpServer::Init(int32_t aPort, bool aHt
   mListener = aListener;
 
   nsCOMPtr<nsIPrefBranch> prefService;
   prefService = do_GetService(NS_PREFSERVICE_CONTRACTID);
 
   if (mHttps) {
     nsCOMPtr<nsILocalCertService> lcs =
       do_CreateInstance("@mozilla.org/security/local-cert-service;1");
-    nsresult rv = lcs->GetOrCreateCert(NS_LITERAL_CSTRING("flyweb"), this);
+    nsresult rv = lcs->GetOrCreateCert(NS_LITERAL_CSTRING("flyweb"), this,
+                                       nsILocalCertService::KEY_TYPE_EC);
     if (NS_FAILED(rv)) {
       NotifyStarted(rv);
     }
   } else {
     // Make sure to always have an async step before notifying callbacks
     HandleCert(nullptr, NS_OK);
   }
 }
--- a/security/manager/ssl/LocalCertService.cpp
+++ b/security/manager/ssl/LocalCertService.cpp
@@ -66,20 +66,22 @@ protected:
 
   nsCString mNickname;
 };
 
 class LocalCertGetTask final : public LocalCertTask
 {
 public:
   LocalCertGetTask(const nsACString& aNickname,
-                   nsILocalCertGetCallback* aCallback)
+                   nsILocalCertGetCallback* aCallback,
+                   uint32_t aKeyType)
     : LocalCertTask(aNickname)
     , mCallback(new nsMainThreadPtrHolder<nsILocalCertGetCallback>(aCallback))
     , mCert(nullptr)
+    , mKeyType(aKeyType)
   {
   }
 
 private:
   virtual nsresult CalculateResult() override
   {
     // Try to lookup an existing cert in the DB
     nsresult rv = GetFromDB();
@@ -124,34 +126,53 @@ private:
     // Generate a new cert
     NS_NAMED_LITERAL_CSTRING(commonNamePrefix, "CN=");
     nsAutoCString subjectNameStr(commonNamePrefix + mNickname);
     UniqueCERTName subjectName(CERT_AsciiToName(subjectNameStr.get()));
     if (!subjectName) {
       return mozilla::psm::GetXPCOMFromNSSError(PR_GetError());
     }
 
-    // Use the well-known NIST P-256 curve
-    SECOidData* curveOidData = SECOID_FindOIDByTag(SEC_OID_SECG_EC_SECP256R1);
-    if (!curveOidData) {
-      return mozilla::psm::GetXPCOMFromNSSError(PR_GetError());
+    UniqueSECKEYPrivateKey privateKey;
+
+    SECOidTag algTag;
+    SECKEYPublicKey* tempPublicKey;
+    if (mKeyType == nsILocalCertService::KEY_TYPE_RSA) {
+      // Use RSA key params
+      PK11RSAGenParams rsaParams;
+      rsaParams.keySizeInBits = 2048;
+      rsaParams.pe = 65537;
+      algTag = SEC_OID_PKCS1_SHA256_WITH_RSA_ENCRYPTION;
+
+      // Generate cert key pair
+      privateKey.reset(
+        PK11_GenerateKeyPair(slot.get(), CKM_RSA_PKCS_KEY_PAIR_GEN, &rsaParams,
+                             &tempPublicKey, true /* token */,
+                             true /* sensitive */, nullptr));
+    } else {
+      // Use the well-known NIST P-256 curve
+      SECOidData* curveOidData = SECOID_FindOIDByTag(SEC_OID_SECG_EC_SECP256R1);
+      if (!curveOidData) {
+        return mozilla::psm::GetXPCOMFromNSSError(PR_GetError());
+      }
+
+      // Get key params from the curve
+      ScopedAutoSECItem keyParams(2 + curveOidData->oid.len);
+      keyParams.data[0] = SEC_ASN1_OBJECT_ID;
+      keyParams.data[1] = curveOidData->oid.len;
+      memcpy(keyParams.data + 2, curveOidData->oid.data, curveOidData->oid.len);
+      algTag = SEC_OID_ANSIX962_ECDSA_SHA256_SIGNATURE;
+
+      // Generate cert key pair
+      privateKey.reset(
+        PK11_GenerateKeyPair(slot.get(), CKM_EC_KEY_PAIR_GEN, &keyParams,
+                             &tempPublicKey, true /* token */,
+                             true /* sensitive */, nullptr));
     }
 
-    // Get key params from the curve
-    ScopedAutoSECItem keyParams(2 + curveOidData->oid.len);
-    keyParams.data[0] = SEC_ASN1_OBJECT_ID;
-    keyParams.data[1] = curveOidData->oid.len;
-    memcpy(keyParams.data + 2, curveOidData->oid.data, curveOidData->oid.len);
-
-    // Generate cert key pair
-    SECKEYPublicKey* tempPublicKey;
-    UniqueSECKEYPrivateKey privateKey(
-      PK11_GenerateKeyPair(slot.get(), CKM_EC_KEY_PAIR_GEN, &keyParams,
-                           &tempPublicKey, true /* token */,
-                           true /* sensitive */, nullptr));
     UniqueSECKEYPublicKey publicKey(tempPublicKey);
     tempPublicKey = nullptr;
     if (!privateKey || !publicKey) {
       return mozilla::psm::GetXPCOMFromNSSError(PR_GetError());
     }
 
     // Create subject public key info and cert request
     UniqueCERTSubjectPublicKeyInfo spki(
@@ -205,33 +226,31 @@ private:
     cert->version.len = 1;
 
     // Set cert signature algorithm
     PLArenaPool* arena = cert->arena;
     if (!arena) {
       return NS_ERROR_INVALID_POINTER;
     }
     rv = MapSECStatus(
-           SECOID_SetAlgorithmID(arena, &cert->signature,
-                                 SEC_OID_ANSIX962_ECDSA_SHA256_SIGNATURE, 0));
+           SECOID_SetAlgorithmID(arena, &cert->signature, algTag, 0));
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     // Encode and self-sign the cert
     UniqueSECItem certDER(
       SEC_ASN1EncodeItem(nullptr, nullptr, cert.get(),
                          SEC_ASN1_GET(CERT_CertificateTemplate)));
     if (!certDER) {
       return mozilla::psm::GetXPCOMFromNSSError(PR_GetError());
     }
     rv = MapSECStatus(
            SEC_DerSignData(arena, &cert->derCert, certDER->data, certDER->len,
-                           privateKey.get(),
-                           SEC_OID_ANSIX962_ECDSA_SHA256_SIGNATURE));
+                           privateKey.get(), algTag));
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     // Create a CERTCertificate from the signed data
     UniqueCERTCertificate certFromDER(
       CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &cert->derCert, nullptr,
                               true /* perm */, true /* copyDER */));
@@ -321,16 +340,17 @@ private:
 
   virtual void CallCallback(nsresult rv) override
   {
     (void) mCallback->HandleCert(mCert, rv);
   }
 
   nsMainThreadPtrHandle<nsILocalCertGetCallback> mCallback;
   nsCOMPtr<nsIX509Cert> mCert; // out
+  uint32_t mKeyType;
 };
 
 class LocalCertRemoveTask final : public LocalCertTask
 {
 public:
   LocalCertRemoveTask(const nsACString& aNickname,
                       nsILocalCertCallback* aCallback)
     : LocalCertTask(aNickname)
@@ -400,33 +420,39 @@ LocalCertService::LoginToKeySlot()
     return keyToken->Login(false /* force */);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 LocalCertService::GetOrCreateCert(const nsACString& aNickname,
-                                  nsILocalCertGetCallback* aCallback)
+                                  nsILocalCertGetCallback* aCallback,
+                                  uint32_t aKeyType)
 {
   if (NS_WARN_IF(aNickname.IsEmpty())) {
     return NS_ERROR_INVALID_ARG;
   }
   if (NS_WARN_IF(!aCallback)) {
     return NS_ERROR_INVALID_POINTER;
   }
+  if (aKeyType != nsILocalCertService::KEY_TYPE_EC &&
+      aKeyType != nsILocalCertService::KEY_TYPE_RSA) {
+    return NS_ERROR_INVALID_ARG;
+  }
 
   // Before sending off the task, login to key slot if needed
   nsresult rv = LoginToKeySlot();
   if (NS_FAILED(rv)) {
     aCallback->HandleCert(nullptr, rv);
     return NS_OK;
   }
 
-  RefPtr<LocalCertGetTask> task(new LocalCertGetTask(aNickname, aCallback));
+  RefPtr<LocalCertGetTask> task(new LocalCertGetTask(aNickname, aCallback,
+                                                     aKeyType));
   return task->Dispatch("LocalCertGet");
 }
 
 NS_IMETHODIMP
 LocalCertService::RemoveCert(const nsACString& aNickname,
                              nsILocalCertCallback* aCallback)
 {
   if (NS_WARN_IF(aNickname.IsEmpty())) {
--- a/security/manager/ssl/nsILocalCertService.idl
+++ b/security/manager/ssl/nsILocalCertService.idl
@@ -6,28 +6,34 @@
 
 interface nsIX509Cert;
 interface nsILocalCertGetCallback;
 interface nsILocalCertCallback;
 
 [scriptable, uuid(9702fdd4-4c2c-439c-ba2e-19cda018eb99)]
 interface nsILocalCertService : nsISupports
 {
+  const unsigned long KEY_TYPE_EC = 0;
+  const unsigned long KEY_TYPE_RSA = 1;
+
   /**
    * Get or create a new self-signed X.509 cert to represent this device over a
    * secure transport, like TLS.
    *
    * The cert is stored permanently in the profile's key store after first use,
    * and is valid for 1 year.  If an expired or otherwise invalid cert is found
    * with the nickname supplied here, it is removed and a new one is made.
    *
    * @param nickname Nickname that identifies the cert
    * @param cb       Callback to be notified with the result
+   * @param keyType  A KEY_TYPE_* constant that specifies the key type.
+   *                 KEY_TYPE_EC if absent.
    */
-  void getOrCreateCert(in ACString nickname, in nsILocalCertGetCallback cb);
+  void getOrCreateCert(in ACString nickname, in nsILocalCertGetCallback cb,
+                       [optional] in unsigned long keyType);
 
   /**
    * Remove a X.509 cert with the given nickname.
    *
    * @param nickname Nickname that identifies the cert
    * @param cb       Callback to be notified with the result
    */
   void removeCert(in ACString nickname, in nsILocalCertCallback cb);
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -1277,17 +1277,17 @@ nsNSSComponent::InitializePIPNSSBundle()
   return rv;
 }
 
 // Table of pref names and SSL cipher ID
 typedef struct {
   const char* pref;
   long id;
   bool enabledByDefault;
-  bool weak;
+  bool fallback;
 } CipherPref;
 
 // Update the switch statement in AccumulateCipherSuite in nsNSSCallbacks.cpp
 // when you add/remove cipher suites here.
 static const CipherPref sCipherPrefs[] = {
  { "security.ssl3.ecdhe_rsa_aes_128_gcm_sha256",
    TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, true },
  { "security.ssl3.ecdhe_ecdsa_aes_128_gcm_sha256",
@@ -1309,20 +1309,20 @@ static const CipherPref sCipherPrefs[] =
    TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, true },
 
  { "security.ssl3.ecdhe_rsa_aes_256_sha",
    TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, true },
  { "security.ssl3.ecdhe_ecdsa_aes_256_sha",
    TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, true },
 
  { "security.ssl3.dhe_rsa_aes_128_sha",
-   TLS_DHE_RSA_WITH_AES_128_CBC_SHA, true },
+   TLS_DHE_RSA_WITH_AES_128_CBC_SHA, true, true },
 
  { "security.ssl3.dhe_rsa_aes_256_sha",
-   TLS_DHE_RSA_WITH_AES_256_CBC_SHA, true },
+   TLS_DHE_RSA_WITH_AES_256_CBC_SHA, true, true },
 
  { "security.ssl3.ecdhe_psk_aes_128_gcm_sha256",
    TLS_ECDHE_PSK_WITH_AES_128_GCM_SHA256, true },
  { "security.ssl3.ecdhe_psk_chacha20_poly1305_sha256",
    TLS_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256, true },
  { "security.ssl3.ecdhe_psk_aes_256_gcm_sha384",
    TLS_ECDHE_PSK_WITH_AES_256_GCM_SHA384, true },
 
@@ -1333,36 +1333,36 @@ static const CipherPref sCipherPrefs[] =
  { "security.ssl3.rsa_des_ede3_sha",
    TLS_RSA_WITH_3DES_EDE_CBC_SHA, true }, // deprecated (RSA key exchange, 3DES)
 
  // All the rest are disabled
 
  { nullptr, 0 } // end marker
 };
 
-// Bit flags indicating what weak ciphers are enabled.
+// Bit flags indicating what fallback ciphers are enabled.
 // The bit index will correspond to the index in sCipherPrefs.
 // Wrtten by the main thread, read from any threads.
-static Atomic<uint32_t> sEnabledWeakCiphers;
+static Atomic<uint32_t> sEnabledFallbackCiphers;
 static_assert(MOZ_ARRAY_LENGTH(sCipherPrefs) - 1 <= sizeof(uint32_t) * CHAR_BIT,
               "too many cipher suites");
 
 /*static*/ bool
-nsNSSComponent::AreAnyWeakCiphersEnabled()
+nsNSSComponent::AreAnyFallbackCiphersEnabled()
 {
-  return !!sEnabledWeakCiphers;
+  return !!sEnabledFallbackCiphers;
 }
 
 /*static*/ void
-nsNSSComponent::UseWeakCiphersOnSocket(PRFileDesc* fd)
+nsNSSComponent::UseFallbackCiphersOnSocket(PRFileDesc* fd)
 {
-  const uint32_t enabledWeakCiphers = sEnabledWeakCiphers;
+  const uint32_t enabledFallbackCiphers = sEnabledFallbackCiphers;
   const CipherPref* const cp = sCipherPrefs;
   for (size_t i = 0; cp[i].pref; ++i) {
-    if (enabledWeakCiphers & ((uint32_t)1 << i)) {
+    if (enabledFallbackCiphers & ((uint32_t)1 << i)) {
       SSL_CipherPrefSet(fd, cp[i].id, true);
     }
   }
 }
 
 // This function will convert from pref values like 1, 2, ...
 // to the internal values of SSL_LIBRARY_VERSION_TLS_1_0,
 // SSL_LIBRARY_VERSION_TLS_1_1, ...
@@ -1465,28 +1465,28 @@ CipherSuiteChangeObserver::Observe(nsISu
   if (nsCRT::strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID) == 0) {
     NS_ConvertUTF16toUTF8  prefName(someData);
     // Look through the cipher table and set according to pref setting
     const CipherPref* const cp = sCipherPrefs;
     for (size_t i = 0; cp[i].pref; ++i) {
       if (prefName.Equals(cp[i].pref)) {
         bool cipherEnabled = Preferences::GetBool(cp[i].pref,
                                                   cp[i].enabledByDefault);
-        if (cp[i].weak) {
-          // Weak ciphers will not be used by default even if they
+        if (cp[i].fallback) {
+          // Fallback ciphers will not be used by default even if they
           // are enabled in prefs. They are only used on specific
           // sockets as a part of a fallback mechanism.
-          // Only the main thread will change sEnabledWeakCiphers.
-          uint32_t enabledWeakCiphers = sEnabledWeakCiphers;
+          // Only the main thread will change sEnabledFallbackCiphers.
+          uint32_t enabledFallbackCiphers = sEnabledFallbackCiphers;
           if (cipherEnabled) {
-            enabledWeakCiphers |= ((uint32_t)1 << i);
+            enabledFallbackCiphers |= ((uint32_t)1 << i);
           } else {
-            enabledWeakCiphers &= ~((uint32_t)1 << i);
+            enabledFallbackCiphers &= ~((uint32_t)1 << i);
           }
-          sEnabledWeakCiphers = enabledWeakCiphers;
+          sEnabledFallbackCiphers = enabledFallbackCiphers;
         } else {
           SSL_CipherPrefSetDefault(cp[i].id, cipherEnabled);
           SSL_ClearSessionCache();
         }
         break;
       }
     }
   } else if (nsCRT::strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
@@ -2335,32 +2335,32 @@ InitializeCipherSuite()
 
   // Disable any ciphers that NSS might have enabled by default
   for (uint16_t i = 0; i < SSL_NumImplementedCiphers; ++i) {
     uint16_t cipher_id = SSL_ImplementedCiphers[i];
     SSL_CipherPrefSetDefault(cipher_id, false);
   }
 
   // Now only set SSL/TLS ciphers we knew about at compile time
-  uint32_t enabledWeakCiphers = 0;
+  uint32_t enabledFallbackCiphers = 0;
   const CipherPref* const cp = sCipherPrefs;
   for (size_t i = 0; cp[i].pref; ++i) {
     bool cipherEnabled = Preferences::GetBool(cp[i].pref,
                                               cp[i].enabledByDefault);
-    if (cp[i].weak) {
-      // Weak ciphers are not used by default. See the comment
+    if (cp[i].fallback) {
+      // Fallback ciphers are not used by default. See the comment
       // in CipherSuiteChangeObserver::Observe for details.
       if (cipherEnabled) {
-        enabledWeakCiphers |= ((uint32_t)1 << i);
+        enabledFallbackCiphers |= ((uint32_t)1 << i);
       }
     } else {
       SSL_CipherPrefSetDefault(cp[i].id, cipherEnabled);
     }
   }
-  sEnabledWeakCiphers = enabledWeakCiphers;
+  sEnabledFallbackCiphers = enabledFallbackCiphers;
 
   // Enable ciphers for PKCS#12
   SEC_PKCS12EnableCipher(PKCS12_RC4_40, 1);
   SEC_PKCS12EnableCipher(PKCS12_RC4_128, 1);
   SEC_PKCS12EnableCipher(PKCS12_RC2_CBC_40, 1);
   SEC_PKCS12EnableCipher(PKCS12_RC2_CBC_128, 1);
   SEC_PKCS12EnableCipher(PKCS12_DES_56, 1);
   SEC_PKCS12EnableCipher(PKCS12_DES_EDE3_168, 1);
--- a/security/manager/ssl/nsNSSComponent.h
+++ b/security/manager/ssl/nsNSSComponent.h
@@ -155,18 +155,18 @@ public:
 #ifdef XP_WIN
   NS_IMETHOD GetEnterpriseRoots(nsIX509CertList** enterpriseRoots) override;
 #endif
 
   ::already_AddRefed<mozilla::psm::SharedCertVerifier>
     GetDefaultCertVerifier() override;
 
   // The following two methods are thread-safe.
-  static bool AreAnyWeakCiphersEnabled();
-  static void UseWeakCiphersOnSocket(PRFileDesc* fd);
+  static bool AreAnyFallbackCiphersEnabled();
+  static void UseFallbackCiphersOnSocket(PRFileDesc* fd);
 
   static void FillTLSVersionRange(SSLVersionRange& rangeOut,
                                   uint32_t minFromPrefs,
                                   uint32_t maxFromPrefs,
                                   SSLVersionRange defaults);
 
 protected:
   virtual ~nsNSSComponent();
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -1092,35 +1092,22 @@ retryDueToTLSIntolerance(PRErrorCode err
                           tlsIntoleranceTelemetryBucket(originalReason));
 
     helpers.forgetIntolerance(socketInfo->GetHostName(),
                               socketInfo->GetPort());
 
     return false;
   }
 
-  // Disallow PR_CONNECT_RESET_ERROR if fallback limit reached.
-  bool fallbackLimitReached =
-    helpers.fallbackLimitReached(socketInfo->GetHostName(), range.max);
-  if (err == PR_CONNECT_RESET_ERROR && fallbackLimitReached) {
-    return false;
-  }
-
   if ((err == SSL_ERROR_NO_CYPHER_OVERLAP || err == PR_END_OF_FILE_ERROR ||
        err == PR_CONNECT_RESET_ERROR) &&
-       nsNSSComponent::AreAnyWeakCiphersEnabled()) {
-    if (helpers.isInsecureFallbackSite(socketInfo->GetHostName()) ||
-        helpers.mUnrestrictedRC4Fallback) {
-      if (helpers.rememberStrongCiphersFailed(socketInfo->GetHostName(),
-                                              socketInfo->GetPort(), err)) {
-        Telemetry::Accumulate(Telemetry::SSL_WEAK_CIPHERS_FALLBACK,
-                              tlsIntoleranceTelemetryBucket(err));
-        return true;
-      }
-      Telemetry::Accumulate(Telemetry::SSL_WEAK_CIPHERS_FALLBACK, 0);
+      nsNSSComponent::AreAnyFallbackCiphersEnabled()) {
+    if (helpers.rememberStrongCiphersFailed(socketInfo->GetHostName(),
+                                            socketInfo->GetPort(), err)) {
+      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
   // EOF.
@@ -2454,25 +2441,25 @@ nsSSLIOLayerSetOptions(PRFileDesc* fd, b
   StrongCipherStatus strongCiphersStatus = StrongCipherStatusUnknown;
   infoObject->SharedState().IOLayerHelpers()
     .adjustForTLSIntolerance(infoObject->GetHostName(), infoObject->GetPort(),
                              range, strongCiphersStatus);
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
          ("[%p] nsSSLIOLayerSetOptions: using TLS version range (0x%04x,0x%04x)%s\n",
           fd, static_cast<unsigned int>(range.min),
               static_cast<unsigned int>(range.max),
-          strongCiphersStatus == StrongCiphersFailed ? " with weak ciphers" : ""));
+          strongCiphersStatus == StrongCiphersFailed ? " with fallback ciphers" : ""));
 
   if (SSL_VersionRangeSet(fd, &range) != SECSuccess) {
     return NS_ERROR_FAILURE;
   }
   infoObject->SetTLSVersionRange(range);
 
   if (strongCiphersStatus == StrongCiphersFailed) {
-    nsNSSComponent::UseWeakCiphersOnSocket(fd);
+    nsNSSComponent::UseFallbackCiphersOnSocket(fd);
   }
 
   // when adjustForTLSIntolerance tweaks the maximum version downward,
   // we tell the server using this SCSV so they can detect a downgrade attack
   if (range.max < maxEnabledVersion) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
            ("[%p] nsSSLIOLayerSetOptions: enabling TLS_FALLBACK_SCSV\n", fd));
     // Some servers will choke if we send the fallback SCSV with TLS 1.2.
rename from security/manager/ssl/tests/unit/test_weak_crypto.js
rename to security/manager/ssl/tests/unit/test_fallback_cipher.js
--- a/security/manager/ssl/tests/unit/test_weak_crypto.js
+++ b/security/manager/ssl/tests/unit/test_fallback_cipher.js
@@ -1,50 +1,57 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 // Tests the weak crypto override
 
-const TLS_RSA_WITH_RC4_128_MD5         = 0x0004;
-const TLS_RSA_WITH_RC4_128_SHA         = 0x0005;
-const TLS_ECDHE_ECDSA_WITH_RC4_128_SHA = 0xC007;
-const TLS_ECDHE_RSA_WITH_RC4_128_SHA   = 0xC011;
+const TLS_DHE_RSA_WITH_AES_128_CBC_SHA     = 0x0033;
+const TLS_DHE_RSA_WITH_AES_256_CBC_SHA     = 0x0039;
+
+const fallback_cipher_prefs = [
+  "security.ssl3.dhe_rsa_aes_128_sha",
+  "security.ssl3.dhe_rsa_aes_256_sha"
+];
 
 // Need profile dir to store the key / cert
 do_get_profile();
 // Ensure PSM is initialized
 Cc["@mozilla.org/psm;1"].getService(Ci.nsISupports);
 
 const certService = Cc["@mozilla.org/security/local-cert-service;1"]
                     .getService(Ci.nsILocalCertService);
 const certOverrideService = Cc["@mozilla.org/security/certoverride;1"]
                             .getService(Ci.nsICertOverrideService);
 const weakCryptoOverride = Cc["@mozilla.org/security/weakcryptooverride;1"]
                            .getService(Ci.nsIWeakCryptoOverride);
 const socketTransportService =
   Cc["@mozilla.org/network/socket-transport-service;1"]
   .getService(Ci.nsISocketTransportService);
 
-function getCert() {
+function getCert(keyType = Ci.nsILocalCertService.KEY_TYPE_EC) {
   let deferred = Promise.defer();
-  certService.getOrCreateCert("tls-test", {
+  let nickname = "tls-test";
+  if (keyType == Ci.nsILocalCertService.KEY_TYPE_RSA) {
+    nickname = "tls-test-rsa";
+  }
+  certService.getOrCreateCert(nickname, {
     handleCert: function(c, rv) {
       if (rv) {
         deferred.reject(rv);
         return;
       }
       deferred.resolve(c);
     }
-  });
+  }, keyType);
   return deferred.promise;
 }
 
-function startServer(cert, rc4only) {
+function startServer(cert, fallbackOnly) {
   let tlsServer = Cc["@mozilla.org/network/tls-server-socket;1"]
                   .createInstance(Ci.nsITLSServerSocket);
   tlsServer.init(-1, true, -1);
   tlsServer.serverCert = cert;
 
   let input, output;
 
   let listener = {
@@ -56,58 +63,56 @@ function startServer(cert, rc4only) {
       input = transport.openInputStream(0, 0, 0);
       output = transport.openOutputStream(0, 0, 0);
     },
     onHandshakeDone: function(socket, status) {
       do_print("TLS handshake done");
 
       equal(status.tlsVersionUsed, Ci.nsITLSClientStatus.TLS_VERSION_1_2,
             "Using TLS 1.2");
-      let expectedCipher = rc4only ? "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA"
-                                   : "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256";
+      let expectedCipher = fallbackOnly ? "TLS_DHE_RSA_WITH_AES_128_CBC_SHA"
+                                        : "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256";
       equal(status.cipherName, expectedCipher,
             "Using expected cipher");
       equal(status.keyLength, 128, "Using 128-bit key");
-      equal(status.macLength, rc4only ? 160 : 128, "Using MAC of expected length");
+      equal(status.macLength, fallbackOnly ? 160 : 128, "Using MAC of expected length");
 
       input.asyncWait({
         onInputStreamReady: function(input) {
           NetUtil.asyncCopy(input, output);
         }
       }, 0, 0, Services.tm.currentThread);
     },
     onStopListening: function() {}
   };
 
   tlsServer.setSessionCache(false);
   tlsServer.setSessionTickets(false);
   tlsServer.setRequestClientCertificate(Ci.nsITLSServerSocket.REQUEST_NEVER);
-  if (rc4only) {
+  if (fallbackOnly) {
     let cipherSuites = [
-      TLS_ECDHE_RSA_WITH_RC4_128_SHA,
-      TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
-      TLS_RSA_WITH_RC4_128_SHA,
-      TLS_RSA_WITH_RC4_128_MD5
+      TLS_DHE_RSA_WITH_AES_128_CBC_SHA,
+      TLS_DHE_RSA_WITH_AES_256_CBC_SHA
     ];
     tlsServer.setCipherSuites(cipherSuites, cipherSuites.length);
   }
 
   tlsServer.asyncListen(listener);
 
   return tlsServer.port;
 }
 
 function storeCertOverride(port, cert) {
   let overrideBits = Ci.nsICertOverrideService.ERROR_UNTRUSTED |
                      Ci.nsICertOverrideService.ERROR_MISMATCH;
   certOverrideService.rememberValidityOverride("127.0.0.1", port, cert,
                                                overrideBits, true);
 }
 
-function startClient(port, expectedResult, options = {}) {
+function startClient(name, port, expectedResult, options = {}) {
   let transport =
     socketTransportService.createTransport(["ssl"], 1, "127.0.0.1", port, null);
   if (options.isPrivate) {
     transport.connectionFlags |= Ci.nsISocketTransport.NO_PERMANENT_STORAGE;
   }
   let input;
   let output;
 
@@ -119,18 +124,18 @@ function startClient(port, expectedResul
       if (status === Ci.nsISocketTransport.STATUS_CONNECTED_TO) {
         output.asyncWait(handler, 0, 0, Services.tm.currentThread);
       }
     },
 
     onInputStreamReady: function(input) {
       try {
         let data = NetUtil.readInputStreamToString(input, input.available());
-        equal(Cr.NS_OK, expectedResult, "Connection should succeed");
-        equal(data, "HELLO", "Echoed data received");
+        equal(Cr.NS_OK, expectedResult, name + ": Connection should succeed");
+        equal(data, "HELLO", name + ": Echoed data received");
       } catch (e) {
         if (!((e.result == Cr.NS_ERROR_NET_RESET) && options.allowReset) &&
             (e.result != expectedResult)) {
           deferred.reject(e);
         }
       }
       input.close();
       output.close();
@@ -144,23 +149,24 @@ function startClient(port, expectedResul
         } catch (e) {
           if (e.result == Cr.NS_BASE_STREAM_WOULD_BLOCK) {
             output.asyncWait(handler, 0, 0, Services.tm.currentThread);
             return;
           }
           if (e.result != Cr.NS_OK) {
             ok((e.result === expectedResult) ||
                (options.allowReset && (e.result === Cr.NS_ERROR_NET_RESET)),
-               "Actual and expected connection result should match");
+               name + ": Actual and expected connection result should match:" +
+               e.result + " === " + expectedResult);
             output.close();
             deferred.resolve();
             return;
           }
         }
-        do_print("Output to server written");
+        do_print(name + ": Output to server written");
         input = transport.openInputStream(0, 0, 0);
         input.asyncWait(handler, 0, 0, Services.tm.currentThread);
       } catch (e) {
         deferred.reject(e);
       }
     }
 
   };
@@ -178,94 +184,56 @@ function run_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(port, Cr.NS_OK);
-  yield startClient(port, Cr.NS_OK, {isPrivate: true});
+  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();
+  let cert = yield getCert(Ci.nsILocalCertService.KEY_TYPE_RSA);
   ok(!!cert, "Got self-signed cert");
   let port = startServer(cert, true);
   storeCertOverride(port, cert);
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
+
+  // disable fallback cipher suites
+  for (let pref of fallback_cipher_prefs) {
+    Services.prefs.setBoolPref(pref, false);
+  }
+
+  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});
 
-  weakCryptoOverride.addWeakCryptoOverride("127.0.0.1", true);
-  // private browsing should not affect the permanent storage.
-  equal(Services.prefs.getCharPref("security.tls.insecure_fallback_hosts"),
-        "");
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
+  // enable fallback cipher suites
+  for (let pref of fallback_cipher_prefs) {
+    Services.prefs.setBoolPref(pref, 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(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {isPrivate: true, allowReset: true});
-  // retry manually to simulate the HTTP layer
-  yield startClient(port, Cr.NS_OK, {isPrivate: true});
-
-  weakCryptoOverride.removeWeakCryptoOverride("127.0.0.1", port, true);
-  equal(Services.prefs.getCharPref("security.tls.insecure_fallback_hosts"),
-        "");
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {isPrivate: true});
-
-  weakCryptoOverride.addWeakCryptoOverride("127.0.0.1", false, true);
-  // temporary override should not change the pref.
-  equal(Services.prefs.getCharPref("security.tls.insecure_fallback_hosts"),
-        "");
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
+  yield startClient("server: fallback only, client: fallback enabled, public",
+                    port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
                     {allowReset: true});
-  yield startClient(port, Cr.NS_OK);
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {isPrivate: true});
-
-  weakCryptoOverride.removeWeakCryptoOverride("127.0.0.1", port, false);
-  equal(Services.prefs.getCharPref("security.tls.insecure_fallback_hosts"),
-        "");
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {isPrivate: true});
-
-  weakCryptoOverride.addWeakCryptoOverride("127.0.0.1", false);
-  // permanent override should change the pref.
-  equal(Services.prefs.getCharPref("security.tls.insecure_fallback_hosts"),
-        "127.0.0.1");
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {allowReset: true});
-  yield startClient(port, Cr.NS_OK);
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {isPrivate: true});
-
-  weakCryptoOverride.removeWeakCryptoOverride("127.0.0.1", port, false);
-  equal(Services.prefs.getCharPref("security.tls.insecure_fallback_hosts"),
-        "");
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {isPrivate: true});
-
-  // add a host to the pref to prepare the next test
-  weakCryptoOverride.addWeakCryptoOverride("127.0.0.1", false);
-  yield startClient(port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP),
-                    {allowReset: true});
-  yield startClient(port, Cr.NS_OK);
-  equal(Services.prefs.getCharPref("security.tls.insecure_fallback_hosts"),
-        "127.0.0.1");
+  // retry manually to simulate the HTTP layer
+  yield startClient("server: fallback only, client: fallback enabled retry, 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});
 });
 
-add_task(function* () {
-  let cert = yield getCert();
-  ok(!!cert, "Got self-signed cert");
-  let port = startServer(cert, false);
-  storeCertOverride(port, cert);
-  yield startClient(port, Cr.NS_OK);
-  // Successful strong cipher will remove the host from the pref.
-  equal(Services.prefs.getCharPref("security.tls.insecure_fallback_hosts"),
-        "");
+do_register_cleanup(function() {
+  do_print("reset modified prefs");
+  for (let pref of fallback_cipher_prefs) {
+    Services.prefs.clearUserPref(pref);
+  }
 });
--- a/security/manager/ssl/tests/unit/xpcshell.ini
+++ b/security/manager/ssl/tests/unit/xpcshell.ini
@@ -56,16 +56,18 @@ run-sequentially = hardcoded ports
 skip-if = toolkit == 'android' || buildapp == 'b2g'
 [test_constructX509FromBase64.js]
 [test_content_signing.js]
 [test_datasignatureverifier.js]
 [test_enterprise_roots.js]
 skip-if = os != 'win' # tests a Windows-specific feature
 [test_ev_certs.js]
 run-sequentially = hardcoded ports
+[test_fallback_cipher.js]
+firefox-appdir = browser
 [test_getchain.js]
 [test_hash_algorithms.js]
 [test_hash_algorithms_wrap.js]
 # bug 1124289 - run_test_in_child violates the sandbox on b2g and android
 skip-if = toolkit == 'android' || toolkit == 'gonk'
 [test_hmac.js]
 [test_intermediate_basic_usage_constraints.js]
 [test_js_cert_override_service.js]
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -8244,23 +8244,16 @@
   },
   "SSL_FALLBACK_LIMIT_REACHED": {
     "alert_emails": ["seceng-telemetry@mozilla.com"],
     "expires_in_version": "default",
     "kind": "enumerated",
     "n_values": 16,
     "description": "TLS/SSL version fallback reached the minimum version (1=TLS 1.0, 2=TLS 1.1, 3=TLS 1.2) or the fallback limit (4=TLS 1.0, 8=TLS 1.1, 12=TLS 1.2), stopped the fallback"
   },
-  "SSL_WEAK_CIPHERS_FALLBACK": {
-    "alert_emails": ["seceng-telemetry@mozilla.com"],
-    "expires_in_version": "never",
-    "kind": "enumerated",
-    "n_values": 64,
-    "description": "Fallback attempted when server did not support any strong cipher suites"
-  },
   "SSL_CIPHER_SUITE_FULL": {
     "alert_emails": ["seceng-telemetry@mozilla.com"],
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 128,
     "description": "Negotiated cipher suite in full handshake (see key in HandshakeCallback in nsNSSCallbacks.cpp)"
   },
   "SSL_CIPHER_SUITE_RESUMED": {
--- a/toolkit/components/telemetry/histogram-whitelists.json
+++ b/toolkit/components/telemetry/histogram-whitelists.json
@@ -909,17 +909,16 @@
     "SSL_TIME_UNTIL_READY",
     "SSL_TLS10_INTOLERANCE_REASON_POST",
     "SSL_TLS10_INTOLERANCE_REASON_PRE",
     "SSL_TLS11_INTOLERANCE_REASON_POST",
     "SSL_TLS11_INTOLERANCE_REASON_PRE",
     "SSL_TLS12_INTOLERANCE_REASON_POST",
     "SSL_TLS12_INTOLERANCE_REASON_PRE",
     "SSL_VERSION_FALLBACK_INAPPROPRIATE",
-    "SSL_WEAK_CIPHERS_FALLBACK",
     "STARTUP_CACHE_AGE_HOURS",
     "STARTUP_CRASH_DETECTED",
     "STARTUP_MEASUREMENT_ERRORS",
     "STS_NUMBER_OF_ONSOCKETREADY_CALLS",
     "STS_NUMBER_OF_PENDING_EVENTS",
     "STS_NUMBER_OF_PENDING_EVENTS_IN_THE_LAST_CYCLE",
     "STS_POLL_AND_EVENTS_CYCLE",
     "STS_POLL_AND_EVENT_THE_LAST_CYCLE",
@@ -2109,17 +2108,16 @@
     "SSL_TIME_UNTIL_READY",
     "SSL_TLS10_INTOLERANCE_REASON_POST",
     "SSL_TLS10_INTOLERANCE_REASON_PRE",
     "SSL_TLS11_INTOLERANCE_REASON_POST",
     "SSL_TLS11_INTOLERANCE_REASON_PRE",
     "SSL_TLS12_INTOLERANCE_REASON_POST",
     "SSL_TLS12_INTOLERANCE_REASON_PRE",
     "SSL_VERSION_FALLBACK_INAPPROPRIATE",
-    "SSL_WEAK_CIPHERS_FALLBACK",
     "STARTUP_CACHE_AGE_HOURS",
     "STARTUP_CACHE_INVALID",
     "STARTUP_CRASH_DETECTED",
     "STARTUP_MEASUREMENT_ERRORS",
     "STS_NUMBER_OF_ONSOCKETREADY_CALLS",
     "STS_NUMBER_OF_PENDING_EVENTS",
     "STS_NUMBER_OF_PENDING_EVENTS_IN_THE_LAST_CYCLE",
     "STS_POLL_AND_EVENTS_CYCLE",