Bug 1321909 - Remove the "security.ssl.false_start.require-npn" pref. r?keeler
MozReview-Commit-ID: 1RQlxQb2IJJ
--- a/netwerk/base/security-prefs.js
+++ b/netwerk/base/security-prefs.js
@@ -7,17 +7,16 @@ pref("security.tls.version.max", 4);
pref("security.tls.version.fallback-limit", 3);
pref("security.tls.insecure_fallback_hosts", "");
pref("security.tls.enable_0rtt_data", false);
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_alpn", true);
pref("security.ssl3.ecdhe_rsa_aes_128_gcm_sha256", true);
pref("security.ssl3.ecdhe_ecdsa_aes_128_gcm_sha256", true);
pref("security.ssl3.ecdhe_ecdsa_chacha20_poly1305_sha256", true);
pref("security.ssl3.ecdhe_rsa_chacha20_poly1305_sha256", true);
pref("security.ssl3.ecdhe_ecdsa_aes_256_gcm_sha384", true);
pref("security.ssl3.ecdhe_rsa_aes_256_gcm_sha384", true);
--- a/security/manager/ssl/nsNSSCallbacks.cpp
+++ b/security/manager/ssl/nsNSSCallbacks.cpp
@@ -43,17 +43,16 @@ extern LazyLogModule gPIPNSSLog;
static void AccumulateCipherSuite(Telemetry::ID probe,
const SSLChannelInfo& channelInfo);
namespace {
// Bits in bit mask for SSL_REASONS_FOR_NOT_FALSE_STARTING telemetry probe
// 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;
} // namespace
class nsHTTPDownloadEvent : public Runnable {
public:
@@ -942,18 +941,16 @@ CanFalseStartCallback(PRFileDesc* fd, vo
if (SSL_GetCipherSuiteInfo(channelInfo.cipherSuite, &cipherInfo,
sizeof (cipherInfo)) != SECSuccess) {
MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("CanFalseStartCallback [%p] failed - "
" KEA %d\n", fd,
static_cast<int32_t>(channelInfo.keaType)));
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) {
MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("CanFalseStartCallback [%p] failed - "
"SSL Version must be TLS 1.2, was %x\n", fd,
static_cast<int32_t>(channelInfo.protocolVersion)));
reasonsForNotFalseStarting |= POSSIBLE_VERSION_DOWNGRADE;
}
@@ -978,28 +975,16 @@ CanFalseStartCallback(PRFileDesc* fd, vo
}
// XXX: An attacker can choose which protocols are advertised in the
// NPN 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;
- }
- }
-
Telemetry::Accumulate(Telemetry::SSL_REASONS_FOR_NOT_FALSE_STARTING,
reasonsForNotFalseStarting);
if (reasonsForNotFalseStarting == 0) {
*canFalseStart = PR_TRUE;
infoObject->SetFalseStarted();
infoObject->NoteTimeUntilReady();
MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("CanFalseStartCallback [%p] ok\n", fd));
--- a/security/manager/ssl/nsNSSIOLayer.cpp
+++ b/security/manager/ssl/nsNSSIOLayer.cpp
@@ -65,23 +65,16 @@ void
getSiteKey(const nsACString& hostName, uint16_t port,
/*out*/ nsCSubstring& key)
{
key = hostName;
key.AppendASCII(":");
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;
nsNSSSocketInfo::nsNSSSocketInfo(SharedSSLState& aState, uint32_t providerFlags)
: mFd(nullptr),
mCertVerificationState(before_cert_verification),
mSharedState(aState),
@@ -1286,17 +1279,16 @@ nsSSLIOLayerPoll(PRFileDesc* fd, int16_t
MOZ_LOG(gPIPNSSLog, LogLevel::Verbose,
("[%p] poll SSL socket returned %d\n", (void*) fd, (int) result));
return result;
}
nsSSLIOLayerHelpers::nsSSLIOLayerHelpers()
: mTreatUnsafeNegotiationAsBroken(false)
, mTLSIntoleranceInfo()
- , mFalseStartRequireNPN(false)
, mVersionFallbackLimit(SSL_LIBRARY_VERSION_TLS_1_0)
, mutex("nsSSLIOLayerHelpers.mutex")
{
}
static int
_PSM_InvalidInt(void)
{
@@ -1496,20 +1488,16 @@ PrefObserver::Observe(nsISupports* aSubj
{
if (nsCRT::strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID) == 0) {
NS_ConvertUTF16toUTF8 prefName(someData);
if (prefName.EqualsLiteral("security.ssl.treat_unsafe_negotiation_as_broken")) {
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")) {
// Changes to the whitelist on the public side will update the pref.
// Don't propagate the changes to the private side.
if (mOwner->isPublic()) {
mOwner->initInsecureFallbackSites();
}
@@ -1539,18 +1527,16 @@ PlaintextRecv(PRFileDesc* fd, void* buf,
nsSSLIOLayerHelpers::~nsSSLIOLayerHelpers()
{
// mPrefObserver will only be set if this->Init was called. The GTest tests
// do not call Init.
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,
"security.tls.insecure_fallback_hosts");
}
}
nsresult
nsSSLIOLayerHelpers::Init()
@@ -1596,28 +1582,23 @@ nsSSLIOLayerHelpers::Init()
nsSSLPlaintextLayerMethods = *PR_GetDefaultIOMethods();
nsSSLPlaintextLayerMethods.recv = PlaintextRecv;
}
bool enabled = false;
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();
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,
"security.tls.insecure_fallback_hosts");
return NS_OK;
}
void
nsSSLIOLayerHelpers::loadVersionFallbackLimit()
--- a/security/manager/ssl/nsNSSIOLayer.h
+++ b/security/manager/ssl/nsNSSIOLayer.h
@@ -213,17 +213,16 @@ public:
void clearStoredData();
void loadVersionFallbackLimit();
void setInsecureFallbackSites(const nsCString& str);
void initInsecureFallbackSites();
bool isPublic() const;
void removeInsecureFallbackSite(const nsACString& hostname, uint16_t port);
bool isInsecureFallbackSite(const nsACString& hostname);
- bool mFalseStartRequireNPN;
uint16_t mVersionFallbackLimit;
private:
mozilla::Mutex mutex;
nsCOMPtr<nsIObserver> mPrefObserver;
};
nsresult nsSSLIOLayerNewSocket(int32_t family,
const char* host,