Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
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
CISA KEVNot listed
Creditedxiulou
Disclosed2025-05-27

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
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) {
Loading diff…