Low CVSS 7.5 firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
7.5
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionIn certain cases, SNI could have been sent unencrypted even when encrypted DNS was enabled.
ComponentNetworking
Bug ClassLogic Error
Tracker1910298
Fix commit3cd5b5927d09 (firefox) +57/-64
CWECWE-319
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N
CISA KEVNot listed
Creditedxiulou
Disclosed2025-05-27

Background

ECH (Encrypted Client Hello)
a TLS extension that encrypts the ClientHello (including the SNI server name) using an echConfig published via DNS (HTTPS RR), hiding which host the client is contacting.
`nsHttpConnectionInfo`
the key object (netwerk/protocol/http/nsHttpConnectionInfo.cpp) describing a connection target; its HashKey() is used to locate/coalesce reusable connections. It carries an mEchConfig field.
Connection coalescing / reuse
HTTP/2 and HTTP/3 let multiple origins share one connection, and Firefox reuses existing connections whose HashKey() matches. If the key doesn’t account for ECH, a connection opened without ECH (cleartext SNI) can be reused for a request that should have used ECH.
`ConnectionEntry::MaybeUpdateEchConfig`
the old (removed) mechanism that mutated an existing entry’s mConnInfo echConfig in place and tore down sockets, attempting to retrofit a new echConfig onto an existing entry.

Root Cause Analysis

The old design tried to keep ECH current by mutating connection-info objects in place (ConnectionEntry::MaybeUpdateEchConfig, invoked from ProcessNewTransaction and DoSpeculativeConnection) without the echConfig participating in the connection HashKey(). Because BuildHashKey() ignored mEchConfig, two connection-info objects that differed only in ECH status produced the same key, so connection lookup/coalescing (FindCoalescableConnection, GetOrCreateConnectionEntry) could hand back a connection that had already been established without ECH — sending the SNI in cleartext even though encrypted DNS/ECH was enabled for the request (CWE-319, cleartext transmission). The in-place update was racy and could not retroactively make an already-open cleartext connection encrypted.

The patch moves ECH into the connection identity. BuildHashKey() now appends a SHA-1 digest of mEchConfig ({ech...}) when non-empty, so connections with different (or absent) echConfigs get distinct keys and cannot be coalesced together. SetEchConfig becomes a real method that rebuilds the hash key when the config changes, and RebuildHashKey additionally re-applies SetTlsFlags. A new HasEchConfig() predicate is used to forbid coalescing/wildcard/anonymous-inversion reuse for ECH connections: FindCoalescableConnection bails when ci->HasEchConfig(), and GetOrCreateConnectionEntry skips the anonymous-inverted and wildcard reuse paths for ECH entries. The now-unnecessary MaybeUpdateEchConfig and its two call sites are deleted. A components.conf change additionally allows the crypto-hash component in the socket process (where the hashing now runs). The advisory’s CWE-319/“SNI sent unencrypted” is accurate; the diff shows the concrete fault is a connection-identity logic error (echConfig omitted from the hash key / improper reuse), so the operative class is LogicError even though the impact is cleartext transmission.

Key insight
echConfig was not part of the connection HashKey, so Firefox could reuse or coalesce an already-open, non-ECH connection for a request that required ECH, leaking the SNI in cleartext; the fix folds the echConfig into the key and bars coalescing of ECH connections.

Attack Path

  1. Precondition ECH/encrypted DNS is enabled and the victim has or opens a non-ECH connection to a host (e.g. before the HTTPS RR/echConfig is applied).
  2. Request to ECH-eligible origin the victim’s browser looks up a reusable connection; because the hash key ignored echConfig, the existing non-ECH connection matches.
  3. Reuse / coalescing FindCoalescableConnection or GetOrCreateConnectionEntry returns the non-ECH connection (or coalesces the new origin onto it).
  4. Cleartext SNI the ClientHello for that connection was sent without ECH, so the SNI (target hostname) is visible to a network observer.
  5. Primitive a passive on-path observer learns the hostname the victim is visiting despite ECH being configured (confidentiality leak; C:H, no integrity/availability impact).

Impact Assessment

This is a networking-layer logic/privacy defect: with ECH enabled, SNI could still traverse the network in cleartext under specific connection-reuse conditions, defeating ECH’s confidentiality guarantee. It is observed by a passive network adversary (no code execution, no process compromise); the gain is disclosure of the visited hostname (CVSS 6.5, C:H). Preconditions are the existence of a reusable non-ECH connection and the specific coalescing/reuse paths the patch now blocks.

Changed Functions

FunctionChangeNotes
if
netwerk/protocol/http/nsHttpConnectionInfo.cpp
modified
if
netwerk/protocol/http/nsHttpConnectionMgr.cpp
modified

Files Changed

  • netwerk/protocol/http/ConnectionEntry.cpp
  • netwerk/protocol/http/ConnectionEntry.h
  • netwerk/protocol/http/nsHttpConnectionInfo.cpp
  • netwerk/protocol/http/nsHttpConnectionInfo.h
  • netwerk/protocol/http/nsHttpConnectionMgr.cpp
  • security/manager/ssl/components.conf

Audit Directions

  • Hash-key completeness
    review BuildHashKey for any other security-relevant attribute (client-cert, TLS flags, proxy identity) that, if omitted, could allow reuse of a connection with weaker security properties.
  • Coalescing predicates
    audit all FindCoalescableConnection/GetH2orH3ActiveConn callers for additional reuse paths (origin-frame, wildcard, anonymous inversion) that might still bypass the new ECH guard.
  • Speculative/preconnect flows
    check speculative and HTTPS-RR-driven preconnect code for other places that formerly relied on MaybeUpdateEchConfig semantics and might now open non-ECH connections prematurely.
diff --git a/netwerk/protocol/http/ConnectionEntry.cpp b/netwerk/protocol/http/ConnectionEntry.cpp
index 7312e29fae8..0de6786156c 100644
--- a/netwerk/protocol/http/ConnectionEntry.cpp
+++ b/netwerk/protocol/http/ConnectionEntry.cpp
@@ -957,28 +957,6 @@ bool ConnectionEntry::RemoveTransFromPendingQ(nsHttpTransaction* aTrans) {
   return true;
 }
 
-void ConnectionEntry::MaybeUpdateEchConfig(nsHttpConnectionInfo* aConnInfo) {
-  if (!mConnInfo->HashKey().Equals(aConnInfo->HashKey())) {
-    return;
-  }
-
-  const nsCString& echConfig = aConnInfo->GetEchConfig();
-  if (mConnInfo->GetEchConfig().Equals(echConfig)) {
-    return;
-  }
-
-  LOG(("ConnectionEntry::MaybeUpdateEchConfig [ci=%s]\n",
-       mConnInfo->HashKey().get()));
-
-  mConnInfo->SetEchConfig(echConfig);
-
-  // If echConfig is changed, we should close all DnsAndConnectSockets and idle
-  // connections. This is to make sure the new echConfig will be used for the
-  // next connection.
-  CloseAllDnsAndConnectSockets();
-  CloseIdleConnections();
-}
-
 bool ConnectionEntry::MaybeProcessCoalescingKeys(nsIDNSAddrRecord* dnsRecord,
                                                  bool aIsHttp3) {
   if (!mConnInfo || !mConnInfo->EndToEndSSL() || (!aIsHttp3 && !AllowHttp2()) ||
diff --git a/netwerk/protocol/http/ConnectionEntry.h b/netwerk/protocol/http/ConnectionEntry.h
index d6516f185b0..cbe6a28889e 100644
--- a/netwerk/protocol/http/ConnectionEntry.h
+++ b/netwerk/protocol/http/ConnectionEntry.h
@@ -202,8 +202,6 @@ class ConnectionEntry {
 
   bool RemoveTransFromPendingQ(nsHttpTransaction* aTrans);
 
-  void MaybeUpdateEchConfig(nsHttpConnectionInfo* aConnInfo);
-
   bool AllowToRetryDifferentIPFamilyForHttp3(nsresult aError);
   void SetRetryDifferentIPFamilyForHttp3(uint16_t aIPFamily);
 
diff --git a/netwerk/protocol/http/nsHttpConnectionInfo.cpp b/netwerk/protocol/http/nsHttpConnectionInfo.cpp
index 954c78d7fb1..d2b3c689e03 100644
--- a/netwerk/protocol/http/nsHttpConnectionInfo.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionInfo.cpp
@@ -26,19 +26,19 @@
 #include "nsProxyInfo.h"
 #include "prnetdb.h"
 
-static nsresult SHA256(const char* aPlainText, nsAutoCString& aResult) {
-  nsresult rv;
-  nsCOMPtr<nsICryptoHash> hasher =
-      do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv);
+static nsresult ComputeHash(uint32_t aAlgorithm, const uint8_t* aInput,
+                            uint32_t aLen, nsAutoCString& aResult) {
+  nsCOMPtr<nsICryptoHash> hasher;
+  nsresult rv = NS_NewCryptoHash(aAlgorithm, getter_AddRefs(hasher));
+
   if (NS_FAILED(rv)) {
     LOG(("nsHttpDigestAuth: no crypto hash!\n"));
     return rv;
   }
-  rv = hasher->Init(nsICryptoHash::SHA256);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = hasher->Update((unsigned char*)aPlainText, strlen(aPlainText));
+
+  rv = hasher->Update(aInput, aLen);
   NS_ENSURE_SUCCESS(rv, rv);
-  return hasher->Finish(false, aResult);
+  return hasher->Finish(true, aResult);
 }
 
 namespace mozilla {
@@ -202,9 +202,12 @@ void nsHttpConnectionInfo::BuildHashKey() {
     mHashKey.Append(ProxyUsername());
     mHashKey.Append(':');
     const char* password = ProxyPassword();
-    if (strlen(password) > 0) {
+    uint32_t len = strlen(password);
+    if (len > 0) {
       nsAutoCString digestedPassword;
-      nsresult rv = SHA256(password, digestedPassword);
+      nsresult rv = ComputeHash(nsICryptoHash::SHA256,
+                                reinterpret_cast<const uint8_t*>(password), len,
+                                digestedPassword);
       if (rv == NS_OK) {
         mHashKey.Append(digestedPassword);
       }
@@ -262,6 +265,20 @@ void nsHttpConnectionInfo::BuildHashKey() {
     mHashKey.AppendLiteral("}");
   }
 
+  // Make sure when echConfig is changed, we don't reuse the old connection.
+  if (!mEchConfig.IsEmpty()) {
+    nsAutoCString digestedEch;
+    nsresult rv =
+        ComputeHash(nsICryptoHash::SHA1,
+                    reinterpret_cast<const uint8_t*>(mEchConfig.BeginReading()),
+                    mEchConfig.Length(), digestedEch);
+    if (NS_SUCCEEDED(rv)) {
+      mHashKey.AppendLiteral("{ech");
+      mHashKey.Append(digestedEch);
+      mHashKey.AppendLiteral("}");
+    }
+  }
+
   nsAutoCString originAttributes;
   mOriginAttributes.CreateSuffix(originAttributes);
   mHashKey.Append(originAttributes);
@@ -287,6 +304,7 @@ void nsHttpConnectionInfo::RebuildHashKey() {
   SetBeConservative(isBeConservative);
   SetAnonymousAllowClientCert(isAnonymousAllowClientCert);
   SetFallbackConnection(isFallback);
+  SetTlsFlags(mTlsFlags);
 }
 
 void nsHttpConnectionInfo::SetOriginServer(const nsACString& host,
@@ -557,6 +575,13 @@ void nsHttpConnectionInfo::SetWebTransportId(uint64_t id) {
   }
 }
 
+void nsHttpConnectionInfo::SetEchConfig(const nsACString& aEchConfig) {
+  if (!mEchConfig.Equals(aEchConfig)) {
+    mEchConfig = aEchConfig;
+    RebuildHashKey();
+  }
+}
+
 void nsHttpConnectionInfo::SetTlsFlags(uint32_t aTlsFlags) {
   mTlsFlags = aTlsFlags;
   const uint32_t tlsFlagsLength = 8;
diff --git a/netwerk/protocol/http/nsHttpConnectionInfo.h b/netwerk/protocol/http/nsHttpConnectionInfo.h
index f2fb38be600..9ab314136b9 100644
--- a/netwerk/protocol/http/nsHttpConnectionInfo.h
+++ b/netwerk/protocol/http/nsHttpConnectionInfo.h
@@ -273,8 +273,9 @@ class nsHttpConnectionInfo final : public ARefBase {
   void SetHasIPHintAddress(bool aHasIPHint) { mHasIPHintAddress = aHasIPHint; }
   bool HasIPHintAddress() const { return mHasIPHintAddress; }
 
-  void SetEchConfig(const nsACString& aEchConfig) { mEchConfig = aEchConfig; }
+  void SetEchConfig(const nsACString& aEchConfig);
   const nsCString& GetEchConfig() const { return mEchConfig; }
+  bool HasEchConfig() const { return !mEchConfig.IsEmpty(); }
 
  private:
   void Init(const nsACString& host, int32_t port, const nsACString& npnToken,
diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
index 73a6aea2a22..ddeafaf208a 100644
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -804,8 +804,8 @@ HttpConnectionBase* nsHttpConnectionMgr::FindCoalescableConnection(
   nsHttpConnectionInfo* ci = ent->mConnInfo;
   LOG(("FindCoalescableConnection %s\n", ci->HashKey().get()));
 
-  if (ci->GetWebTransport()) {
-    LOG(("Don't coalesce a WebTransport conn "));
+  if (ci->GetWebTransport() || ci->HasEchConfig()) {
+    LOG(("Don't coalesce a WebTransport/EchConfig conn"));
     return nullptr;
   }
   // First try and look it up by origin frame
@@ -1826,10 +1826,6 @@ nsresult nsHttpConnectionMgr::ProcessNewTransaction(nsHttpTransaction* trans) {
       trans->Caps() & NS_HTTP_DISALLOW_HTTP3, &isWildcard);
   MOZ_ASSERT(ent);
 
-  if (gHttpHandler->EchConfigEnabled(ci->IsHttp3())) {
-    ent->MaybeUpdateEchConfig(ci);
-  }
-
   ReportProxyTelemetry(ent);
 
   // Check if the transaction already has a sticky reference to a connection.
@@ -3457,20 +3453,22 @@ ConnectionEntry* nsHttpConnectionMgr::GetOrCreateConnectionEntry(
   // step 1 repeated for an inverted anonymous flag; we return an entry
   // only when it has an h2 established connection that is not authenticated
   // with a client certificate.
-  RefPtr<nsHttpConnectionInfo> anonInvertedCI(specificCI->Clone());
-  anonInvertedCI->SetAnonymous(!specificCI->GetAnonymous());
-  ConnectionEntry* invertedEnt = mCT.GetWeak(anonInvertedCI->HashKey());
-  if (invertedEnt) {
-    HttpConnectionBase* h2orh3conn =
-        GetH2orH3ActiveConn(invertedEnt, aNoHttp2, aNoHttp3);
-    if (h2orh3conn && h2orh3conn->IsExperienced() &&
-        h2orh3conn->NoClientCertAuth()) {
-      MOZ_ASSERT(h2orh3conn->UsingSpdy() || h2orh3conn->UsingHttp3());
-      LOG(
-          ("GetOrCreateConnectionEntry is coalescing h2/3 an/onymous "
-           "connections, ent=%p",
-           invertedEnt));
-      return invertedEnt;
+  if (!specificCI->HasEchConfig()) {
+    RefPtr<nsHttpConnectionInfo> anonInvertedCI(specificCI->Clone());
+    anonInvertedCI->SetAnonymous(!specificCI->GetAnonymous());
+    ConnectionEntry* invertedEnt = mCT.GetWeak(anonInvertedCI->HashKey());
+    if (invertedEnt) {
+      HttpConnectionBase* h2orh3conn =
+          GetH2orH3ActiveConn(invertedEnt, aNoHttp2, aNoHttp3);
+      if (h2orh3conn && h2orh3conn->IsExperienced() &&
+          h2orh3conn->NoClientCertAuth()) {
+        MOZ_ASSERT(h2orh3conn->UsingSpdy() || h2orh3conn->UsingHttp3());
+        LOG(
+            ("GetOrCreateConnectionEntry is coalescing h2/3 an/onymous "
+             "connections, ent=%p",
+             invertedEnt));
+        return invertedEnt;
+      }
     }
   }
 
@@ -3479,7 +3477,7 @@ ConnectionEntry* nsHttpConnectionMgr::GetOrCreateConnectionEntry(
   }
 
   // step 2
-  if (!prohibitWildCard && aNoHttp3) {
+  if (!prohibitWildCard && aNoHttp3 && !specificCI->HasEchConfig()) {
     RefPtr<nsHttpConnectionInfo> wildCardProxyCI;
     DebugOnly<nsresult> rv =
         specificCI->CreateWildCard(getter_AddRefs(wildCardProxyCI));
@@ -3512,14 +3510,6 @@ void nsHttpConnectionMgr::DoSpeculativeConnection(
   ConnectionEntry* ent = GetOrCreateConnectionEntry(
       aTrans->ConnectionInfo(), false, aTrans->Caps() & NS_HTTP_DISALLOW_SPDY,
       aTrans->Caps() & NS_HTTP_DISALLOW_HTTP3, &isWildcard);
-  if (!aFetchHTTPSRR &&
-      gHttpHandler->EchConfigEnabled(aTrans->ConnectionInfo()->IsHttp3())) {
-    // This happens when this is called from
-    // SpeculativeTransaction::OnHTTPSRRAvailable. We have to update this
-    // entry's echConfig so that the newly created connection can use the latest
-    // echConfig.
-    ent->MaybeUpdateEchConfig(aTrans->ConnectionInfo());
-  }
   DoSpeculativeConnectionInternal(ent, aTrans, aFetchHTTPSRR);
 }
 
diff --git a/security/manager/ssl/components.conf b/security/manager/ssl/components.conf
index 788527eb03d..aed5fc88eff 100644
--- a/security/manager/ssl/components.conf
+++ b/security/manager/ssl/components.conf
@@ -74,6 +74,7 @@ Classes = [
         'contract_ids': ['@mozilla.org/security/hash;1'],
         'headers': ['/security/manager/ssl/crypto_hash/crypto_hash.h'],
         'legacy_constructor': 'crypto_hash_constructor',
+        'processes': ProcessSelector.ALLOW_IN_SOCKET_PROCESS,
     },
     {
         'cid': '{45a5fe2f-c350-4b86-962d-02d5aaaa955a}',
Loading diff…