Bug 1720118 - always use the TLS token cache r=kershaw,necko-reviewers,ci-and-tooling,jmaher

Differential Revision: https://phabricator.services.mozilla.com/D158792

Author: mozkeeler

modules/libpref/init/StaticPrefList.yaml

@@ -11035,12 +11035,6 @@
   value: true
   mirror: always
 
-# Whether to cache SSL resumption tokens in necko.
-- name: network.ssl_tokens_cache_enabled
-  type: RelaxedAtomicBool
-  value: true
-  mirror: always
-
 # Capacity of the above cache, in kilobytes.
 - name: network.ssl_tokens_cache_capacity
   type: RelaxedAtomicUint32

netwerk/base/SSLTokensCache.cpp

@@ -510,10 +510,6 @@ SSLTokensCache::CollectReports(nsIHandleReportCallback* aHandleReport,
 // static
 void SSLTokensCache::Clear() {
   LOG(("SSLTokensCache::Clear"));
-  if (!StaticPrefs::network_ssl_tokens_cache_enabled() &&
-      !StaticPrefs::network_http_http3_enable_0rtt()) {
-    return;
-  }
 
   StaticMutexAutoLock lock(sLock);
   if (!gInstance) {

netwerk/base/nsIOService.cpp

@@ -217,7 +217,6 @@ static const char* gCallbackPrefs[] = {
 static const char* gCallbackPrefsForSocketProcess[] = {
     WEBRTC_PREF_PREFIX,
     NETWORK_DNS_PREF,
-    "network.ssl_tokens_cache_enabled",
     "network.send_ODA_to_content_directly",
     "network.trr.",
     "doh-rollout.",

netwerk/test/unit/test_http3_0rtt.js

@@ -5,7 +5,6 @@
 var { setTimeout } = ChromeUtils.import("resource://gre/modules/Timer.jsm");
 
 registerCleanupFunction(async () => {
-  Services.prefs.clearUserPref("network.ssl_tokens_cache_enabled");
   http3_clear_prefs();
 });
 
@@ -68,14 +67,8 @@ async function test_first_conn_no_resumed() {
   await chanPromise(chan, listener);
 }
 
-async function test_0RTT(enable_ssl_tokens_cache, enable_0rtt, resumed) {
-  info(
-    `enable_ssl_tokens_cache=${enable_ssl_tokens_cache} enable_0rtt=${enable_0rtt} resumed=${resumed}`
-  );
-  Services.prefs.setBoolPref(
-    "network.ssl_tokens_cache_enabled",
-    enable_ssl_tokens_cache
-  );
+async function test_0RTT(enable_0rtt, resumed) {
+  info(`enable_0rtt=${enable_0rtt} resumed=${resumed}`);
   Services.prefs.setBoolPref("network.http.http3.enable_0rtt", enable_0rtt);
 
   // Make sure the h3 connection created by the previous test is cleared.
@@ -92,16 +85,10 @@ async function test_0RTT(enable_ssl_tokens_cache, enable_0rtt, resumed) {
 
 add_task(async function test_0RTT_setups() {
   await test_first_conn_no_resumed();
-  // http3.0RTT and network.ssl_tokens_cache_enabled enabled
-  // ssl_tokens_cache
-  await test_0RTT(true, true, true);
 
-  // http3.0RTT enabled and network.ssl_tokens_cache_enabled disabled
-  await test_0RTT(false, true, true);
+  // http3.0RTT enabled
+  await test_0RTT(true, true);
 
-  // http3.0RTT disabled and network.ssl_tokens_cache_enabled enabled
-  await test_0RTT(true, false, false);
-
-  // http3.0RTT disabled and network.ssl_tokens_cache_enabled disabled
-  await test_0RTT(false, false, false);
+  // http3.0RTT disabled
+  await test_0RTT(false, false);
 });

netwerk/test/unit/test_http3_prio_disabled.js

@@ -15,7 +15,6 @@ load("../unit/test_http3_prio_helpers.js");
 // otherwise the wrapper will handle
 if (!inChildProcess()) {
   registerCleanupFunction(async () => {
-    Services.prefs.clearUserPref("network.ssl_tokens_cache_enabled");
     Services.prefs.clearUserPref("network.http.http3.priority");
     http3_clear_prefs();
   });

netwerk/test/unit/test_http3_prio_enabled.js

@@ -15,7 +15,6 @@ load("../unit/test_http3_prio_helpers.js");
 // otherwise the wrapper will handle
 if (!inChildProcess()) {
   registerCleanupFunction(async () => {
-    Services.prefs.clearUserPref("network.ssl_tokens_cache_enabled");
     Services.prefs.clearUserPref("network.http.http3.priority");
     http3_clear_prefs();
   });

netwerk/test/unit_ipc/test_http3_prio_disabled_wrap.js

@@ -3,7 +3,6 @@
 //
 
 registerCleanupFunction(async () => {
-  Services.prefs.clearUserPref("network.ssl_tokens_cache_enabled");
   Services.prefs.clearUserPref("network.http.http3.priority");
   http3_clear_prefs();
 });

netwerk/test/unit_ipc/test_http3_prio_enabled_wrap.js

@@ -3,7 +3,6 @@
 //
 
 registerCleanupFunction(async () => {
-  Services.prefs.clearUserPref("network.ssl_tokens_cache_enabled");
   Services.prefs.clearUserPref("network.http.http3.priority");
   http3_clear_prefs();
 });

security/manager/ssl/nsNSSCallbacks.cpp

@@ -1008,126 +1008,6 @@ static void AccumulateCipherSuite(Telemetry::HistogramID probe,
   Telemetry::Accumulate(probe, value);
 }
 
-// In the case of session resumption, the AuthCertificate hook has been bypassed
-// (because we've previously successfully connected to our peer). That being the
-// case, we unfortunately don't know what the verified certificate chain was, if
-// the peer's server certificate verified as extended validation, or what its CT
-// status is (if enabled). To address this, we attempt to build a certificate
-// chain here using as much of the original context as possible (e.g. stapled
-// OCSP responses, SCTs, the hostname, the first party domain, etc.). Note that
-// because we are on the socket thread, this must not cause any network
-// requests, hence the use of FLAG_LOCAL_ONLY.
-static void RebuildVerifiedCertificateInformation(PRFileDesc* fd,
-                                                  nsNSSSocketInfo* infoObject) {
-  MOZ_ASSERT(fd);
-  MOZ_ASSERT(infoObject);
-
-  if (!fd || !infoObject) {
-    return;
-  }
-
-  UniqueCERTCertificate cert(SSL_PeerCertificate(fd));
-  MOZ_ASSERT(cert, "SSL_PeerCertificate failed in TLS handshake callback?");
-  if (!cert) {
-    return;
-  }
-
-  Maybe<nsTArray<nsTArray<uint8_t>>> maybePeerCertsBytes;
-  UniqueCERTCertList peerCertChain(SSL_PeerCertificateChain(fd));
-  if (!peerCertChain) {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-            ("RebuildVerifiedCertificateInformation: failed to get peer "
-             "certificate chain"));
-  } else {
-    nsTArray<nsTArray<uint8_t>> peerCertsBytes;
-    for (CERTCertListNode* n = CERT_LIST_HEAD(peerCertChain);
-         !CERT_LIST_END(n, peerCertChain); n = CERT_LIST_NEXT(n)) {
-      // Don't include the end-entity certificate.
-      if (n == CERT_LIST_HEAD(peerCertChain)) {
-        continue;
-      }
-      nsTArray<uint8_t> certBytes;
-      certBytes.AppendElements(n->cert->derCert.data, n->cert->derCert.len);
-      peerCertsBytes.AppendElement(std::move(certBytes));
-    }
-    maybePeerCertsBytes.emplace(std::move(peerCertsBytes));
-  }
-
-  RefPtr<SharedCertVerifier> certVerifier(GetDefaultCertVerifier());
-  MOZ_ASSERT(certVerifier,
-             "Certificate verifier uninitialized in TLS handshake callback?");
-  if (!certVerifier) {
-    return;
-  }
-
-  // We don't own these pointers.
-  const SECItemArray* stapledOCSPResponses = SSL_PeerStapledOCSPResponses(fd);
-  Maybe<nsTArray<uint8_t>> stapledOCSPResponse;
-  // we currently only support single stapled responses
-  if (stapledOCSPResponses && stapledOCSPResponses->len == 1) {
-    stapledOCSPResponse.emplace();
-    stapledOCSPResponse->SetCapacity(stapledOCSPResponses->items[0].len);
-    stapledOCSPResponse->AppendElements(stapledOCSPResponses->items[0].data,
-                                        stapledOCSPResponses->items[0].len);
-  }
-
-  Maybe<nsTArray<uint8_t>> sctsFromTLSExtension;
-  const SECItem* sctsFromTLSExtensionSECItem = SSL_PeerSignedCertTimestamps(fd);
-  if (sctsFromTLSExtensionSECItem) {
-    sctsFromTLSExtension.emplace();
-    sctsFromTLSExtension->SetCapacity(sctsFromTLSExtensionSECItem->len);
-    sctsFromTLSExtension->AppendElements(sctsFromTLSExtensionSECItem->data,
-                                         sctsFromTLSExtensionSECItem->len);
-  }
-
-  int flags = mozilla::psm::CertVerifier::FLAG_LOCAL_ONLY;
-  if (!infoObject->SharedState().IsOCSPStaplingEnabled() ||
-      !infoObject->SharedState().IsOCSPMustStapleEnabled()) {
-    flags |= CertVerifier::FLAG_TLS_IGNORE_STATUS_REQUEST;
-  }
-
-  EVStatus evStatus;
-  CertificateTransparencyInfo certificateTransparencyInfo;
-  nsTArray<nsTArray<uint8_t>> builtChainCertBytes;
-  nsTArray<uint8_t> certBytes(cert->derCert.data, cert->derCert.len);
-  bool isBuiltCertChainRootBuiltInRoot = false;
-  mozilla::pkix::Result rv = certVerifier->VerifySSLServerCert(
-      certBytes, mozilla::pkix::Now(), infoObject, infoObject->GetHostName(),
-      builtChainCertBytes, flags, maybePeerCertsBytes, stapledOCSPResponse,
-      sctsFromTLSExtension, Nothing(), infoObject->GetOriginAttributes(),
-      &evStatus,
-      nullptr,  // OCSP stapling telemetry
-      nullptr,  // key size telemetry
-      nullptr,  // pinning telemetry
-      &certificateTransparencyInfo, &isBuiltCertChainRootBuiltInRoot);
-
-  if (rv != Success) {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-            ("HandshakeCallback: couldn't rebuild verified certificate info"));
-  }
-
-  nsCOMPtr<nsIX509Cert> x509Cert(new nsNSSCertificate(cert.get()));
-  if (rv == Success && evStatus == EVStatus::EV) {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-            ("HandshakeCallback using NEW cert (is EV)"));
-    infoObject->SetServerCert(x509Cert, EVStatus::EV);
-  } else {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-            ("HandshakeCallback using NEW cert (is not EV)"));
-    infoObject->SetServerCert(x509Cert, EVStatus::NotEV);
-  }
-
-  if (rv == Success) {
-    uint16_t status =
-        TransportSecurityInfo::ConvertCertificateTransparencyInfoToStatus(
-            certificateTransparencyInfo);
-    infoObject->SetCertificateTransparencyStatus(status);
-    infoObject->SetSucceededCertChain(std::move(builtChainCertBytes));
-    infoObject->SetIsBuiltCertChainRootBuiltInRoot(
-        isBuiltCertChainRootBuiltInRoot);
-  }
-}
-
 void HandshakeCallback(PRFileDesc* fd, void* client_data) {
   SECStatus rv;
 
@@ -1265,11 +1145,7 @@ void HandshakeCallback(PRFileDesc* fd, void* client_data) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
             ("HandshakeCallback KEEPING existing cert\n"));
   } else {
-    if (StaticPrefs::network_ssl_tokens_cache_enabled()) {
-      infoObject->RebuildCertificateInfoFromSSLTokenCache();
-    } else {
-      RebuildVerifiedCertificateInformation(fd, infoObject);
-    }
+    infoObject->RebuildCertificateInfoFromSSLTokenCache();
   }
 
   nsITransportSecurityInfo::OverridableErrorCategory overridableErrorCategory;

security/manager/ssl/nsNSSIOLayer.cpp

@@ -774,9 +774,7 @@ PRStatus nsNSSSocketInfo::CloseSocketAndDestroy() {
 
   // We need to clear the callback to make sure the ssl layer cannot call the
   // callback after mFD is nulled.
-  if (StaticPrefs::network_ssl_tokens_cache_enabled()) {
-    SSL_SetResumptionTokenCallback(mFd, nullptr, nullptr);
-  }
+  SSL_SetResumptionTokenCallback(mFd, nullptr, nullptr);
 
   PRStatus status = mFd->methods->close(mFd);
 
@@ -901,10 +899,6 @@ nsNSSSocketInfo::GetPeerId(nsACString& aResult) {
 }
 
 nsresult nsNSSSocketInfo::SetResumptionTokenFromExternalCache() {
-  if (!StaticPrefs::network_ssl_tokens_cache_enabled()) {
-    return NS_OK;
-  }
-
   if (!mFd) {
     return NS_ERROR_FAILURE;
   }
@@ -2255,13 +2249,11 @@ nsresult nsSSLIOLayerAddToSocket(int32_t family, const char* host, int32_t port,
 
   infoObject->SharedState().NoteSocketCreated();
 
-  if (StaticPrefs::network_ssl_tokens_cache_enabled()) {
-    rv = infoObject->SetResumptionTokenFromExternalCache();
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-    SSL_SetResumptionTokenCallback(sslSock, &StoreResumptionToken, infoObject);
+  rv = infoObject->SetResumptionTokenFromExternalCache();
+  if (NS_FAILED(rv)) {
+    return rv;
   }
+  SSL_SetResumptionTokenCallback(sslSock, &StoreResumptionToken, infoObject);
 
   return NS_OK;
 loser: