diff --git a/netwerk/base/security-prefs.js b/netwerk/base/security-prefs.js index d0adccd879..1c0bc7b27e 100644 --- a/netwerk/base/security-prefs.js +++ b/netwerk/base/security-prefs.js @@ -13,8 +13,6 @@ pref("security.ssl.treat_unsafe_negotiation_as_broken", false); pref("security.ssl.require_safe_negotiation", false); pref("security.ssl.enable_ocsp_stapling", true); pref("security.ssl.enable_false_start", true); -pref("security.ssl.false_start.require-npn", false); -pref("security.ssl.enable_npn", true); pref("security.ssl.enable_alpn", true); // TLS 1.3 cipher suites diff --git a/security/manager/ssl/nsNSSCallbacks.cpp b/security/manager/ssl/nsNSSCallbacks.cpp index 4696d7c862..b879942bb9 100644 --- a/security/manager/ssl/nsNSSCallbacks.cpp +++ b/security/manager/ssl/nsNSSCallbacks.cpp @@ -43,7 +43,6 @@ namespace { // These bits are numbered so that the least subtle issues have higher values. // This should make it easier for us to interpret the results. -const uint32_t NPN_NOT_NEGOTIATED = 64; const uint32_t POSSIBLE_VERSION_DOWNGRADE = 4; const uint32_t POSSIBLE_CIPHER_SUITE_DOWNGRADE = 2; const uint32_t KEA_NOT_SUPPORTED = 1; @@ -1012,8 +1011,6 @@ CanFalseStartCallback(PRFileDesc* fd, void* client_data, PRBool *canFalseStart) return SECSuccess; } - nsSSLIOLayerHelpers& helpers = infoObject->SharedState().IOLayerHelpers(); - // Prevent version downgrade attacks from TLS 1.2, and avoid False Start for // TLS 1.3 and later. See Bug 861310 for all the details as to why. if (channelInfo.protocolVersion != SSL_LIBRARY_VERSION_TLS_1_2) { @@ -1043,23 +1040,11 @@ CanFalseStartCallback(PRFileDesc* fd, void* client_data, PRBool *canFalseStart) } // XXX: An attacker can choose which protocols are advertised in the - // NPN extension. TODO(Bug 861311): We should restrict the ability + // ALPN extension. TODO(Bug 861311): We should restrict the ability // of an attacker leverage this capability by restricting false start // to the same protocol we previously saw for the server, after the // first successful connection to the server. - // Enforce NPN to do false start if policy requires it. Do this as an - // indicator if server compatibility. - if (helpers.mFalseStartRequireNPN) { - nsAutoCString negotiatedNPN; - if (NS_FAILED(infoObject->GetNegotiatedNPN(negotiatedNPN)) || - !negotiatedNPN.Length()) { - MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("CanFalseStartCallback [%p] failed - " - "NPN cannot be verified\n", fd)); - reasonsForNotFalseStarting |= NPN_NOT_NEGOTIATED; - } - } - if (reasonsForNotFalseStarting == 0) { *canFalseStart = PR_TRUE; infoObject->SetFalseStarted(); diff --git a/security/manager/ssl/nsNSSComponent.cpp b/security/manager/ssl/nsNSSComponent.cpp index bc660f8a55..5c27cf02d0 100644 --- a/security/manager/ssl/nsNSSComponent.cpp +++ b/security/manager/ssl/nsNSSComponent.cpp @@ -1440,8 +1440,7 @@ nsNSSComponent::FillTLSVersionRange(SSLVersionRange& rangeOut, static const int32_t OCSP_ENABLED_DEFAULT = 1; static const bool REQUIRE_SAFE_NEGOTIATION_DEFAULT = false; static const bool FALSE_START_ENABLED_DEFAULT = true; -static const bool NPN_ENABLED_DEFAULT = true; -static const bool ALPN_ENABLED_DEFAULT = false; +static const bool ALPN_ENABLED_DEFAULT = true; static const bool ENABLED_0RTT_DATA_DEFAULT = false; static const bool TLS13_COMPAT_MODE_DEFAULT = false; static const bool HELLO_DOWNGRADE_CHECK_DEFAULT = true; @@ -1869,13 +1868,10 @@ nsNSSComponent::InitializeNSS() Preferences::GetBool("security.ssl.enable_false_start", FALSE_START_ENABLED_DEFAULT)); - // SSL_ENABLE_NPN and SSL_ENABLE_ALPN also require calling - // SSL_SetNextProtoNego in order for the extensions to be negotiated. - // WebRTC does not do that so it will not use NPN or ALPN even when these - // preferences are true. - SSL_OptionSetDefault(SSL_ENABLE_NPN, - Preferences::GetBool("security.ssl.enable_npn", - NPN_ENABLED_DEFAULT)); + // SSL_ENABLE_ALPN also requires calling SSL_SetNextProtoNego in order for + // the extensions to be negotiated. + // WebRTC does not do that so it will not use ALPN even when this preference + // is true. SSL_OptionSetDefault(SSL_ENABLE_ALPN, Preferences::GetBool("security.ssl.enable_alpn", ALPN_ENABLED_DEFAULT)); @@ -2066,10 +2062,6 @@ nsNSSComponent::Observe(nsISupports* aSubject, const char* aTopic, SSL_OptionSetDefault(SSL_ENABLE_FALSE_START, Preferences::GetBool("security.ssl.enable_false_start", FALSE_START_ENABLED_DEFAULT)); - } else if (prefName.EqualsLiteral("security.ssl.enable_npn")) { - SSL_OptionSetDefault(SSL_ENABLE_NPN, - Preferences::GetBool("security.ssl.enable_npn", - NPN_ENABLED_DEFAULT)); } else if (prefName.EqualsLiteral("security.ssl.enable_alpn")) { SSL_OptionSetDefault(SSL_ENABLE_ALPN, Preferences::GetBool("security.ssl.enable_alpn", diff --git a/security/manager/ssl/nsNSSIOLayer.cpp b/security/manager/ssl/nsNSSIOLayer.cpp index 337ef8b8e6..482b9f3b00 100644 --- a/security/manager/ssl/nsNSSIOLayer.cpp +++ b/security/manager/ssl/nsNSSIOLayer.cpp @@ -69,13 +69,6 @@ getSiteKey(const nsACString& hostName, uint16_t port, key.AppendInt(port); } -// Historically, we have required that the server negotiate ALPN or NPN in -// order to false start, as a compatibility hack to work around -// implementations that just stop responding during false start. However, now -// false start is resricted to modern crypto (TLS 1.2 and AEAD cipher suites) -// so it is less likely that requring NPN or ALPN is still necessary. -static const bool FALSE_START_REQUIRE_NPN_DEFAULT = false; - } // unnamed namespace extern LazyLogModule gPIPNSSLog; @@ -1260,7 +1253,6 @@ nsSSLIOLayerPoll(PRFileDesc* fd, int16_t in_flags, int16_t* out_flags) nsSSLIOLayerHelpers::nsSSLIOLayerHelpers() : mTreatUnsafeNegotiationAsBroken(false) , mTLSIntoleranceInfo() - , mFalseStartRequireNPN(false) , mUnrestrictedRC4Fallback(false) , mVersionFallbackLimit(SSL_LIBRARY_VERSION_TLS_1_0) , mutex("nsSSLIOLayerHelpers.mutex") @@ -1471,10 +1463,6 @@ PrefObserver::Observe(nsISupports* aSubject, const char* aTopic, bool enabled; Preferences::GetBool("security.ssl.treat_unsafe_negotiation_as_broken", &enabled); mOwner->setTreatUnsafeNegotiationAsBroken(enabled); - } 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.tls.version.fallback-limit")) { mOwner->loadVersionFallbackLimit(); } else if (prefName.EqualsLiteral("security.tls.insecure_fallback_hosts")) { @@ -1516,8 +1504,6 @@ nsSSLIOLayerHelpers::~nsSSLIOLayerHelpers() if (mPrefObserver) { Preferences::RemoveObserver(mPrefObserver, "security.ssl.treat_unsafe_negotiation_as_broken"); - Preferences::RemoveObserver(mPrefObserver, - "security.ssl.false_start.require-npn"); Preferences::RemoveObserver(mPrefObserver, "security.tls.version.fallback-limit"); Preferences::RemoveObserver(mPrefObserver, @@ -1576,9 +1562,6 @@ nsSSLIOLayerHelpers::Init() Preferences::GetBool("security.ssl.treat_unsafe_negotiation_as_broken", &enabled); setTreatUnsafeNegotiationAsBroken(enabled); - mFalseStartRequireNPN = - Preferences::GetBool("security.ssl.false_start.require-npn", - FALSE_START_REQUIRE_NPN_DEFAULT); loadVersionFallbackLimit(); initInsecureFallbackSites(); mUnrestrictedRC4Fallback = @@ -1587,8 +1570,6 @@ nsSSLIOLayerHelpers::Init() mPrefObserver = new PrefObserver(this); Preferences::AddStrongObserver(mPrefObserver, "security.ssl.treat_unsafe_negotiation_as_broken"); - Preferences::AddStrongObserver(mPrefObserver, - "security.ssl.false_start.require-npn"); Preferences::AddStrongObserver(mPrefObserver, "security.tls.version.fallback-limit"); Preferences::AddStrongObserver(mPrefObserver, diff --git a/security/manager/ssl/nsNSSIOLayer.h b/security/manager/ssl/nsNSSIOLayer.h index b0b6281a89..06e8bc6e65 100644 --- a/security/manager/ssl/nsNSSIOLayer.h +++ b/security/manager/ssl/nsNSSIOLayer.h @@ -229,7 +229,6 @@ public: void removeInsecureFallbackSite(const nsACString& hostname, uint16_t port); bool isInsecureFallbackSite(const nsACString& hostname); - bool mFalseStartRequireNPN; bool mUnrestrictedRC4Fallback; uint16_t mVersionFallbackLimit; private: