Bug 1665878 - Reset exclusion list when all target names are all excluded, r=dragana,necko-reviewers

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

modules/libpref/init/StaticPrefList.yaml

@@ -8579,6 +8579,12 @@
   value: true
   mirror: always
 
+# When true, reset the exclusion list when all records are excluded.
+- name: network.dns.httpssvc.reset_exclustion_list
+  type: RelaxedAtomicBool
+  value: true
+  mirror: always
+
 #---------------------------------------------------------------------------
 # Prefs starting with "nglayout."
 #---------------------------------------------------------------------------

netwerk/dns/ChildDNSService.cpp

@@ -435,6 +435,11 @@ ChildDNSService::IsSVCDomainNameFailed(const nsACString& aOwnerName,
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+NS_IMETHODIMP
+ChildDNSService::ResetExcludedSVCDomainName(const nsACString& aOwnerName) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 //-----------------------------------------------------------------------------
 // ChildDNSService::nsIObserver
 //-----------------------------------------------------------------------------

netwerk/dns/nsDNSService2.cpp

@@ -1449,3 +1449,10 @@ nsDNSService::IsSVCDomainNameFailed(const nsACString& aOwnerName,
   *aResult = failedList->Contains(aSVCDomainName);
   return NS_OK;
 }
+
+NS_IMETHODIMP
+nsDNSService::ResetExcludedSVCDomainName(const nsACString& aOwnerName) {
+  MutexAutoLock lock(mLock);
+  mFailedSVCDomainNames.Remove(aOwnerName);
+  return NS_OK;
+}

netwerk/dns/nsIDNSService.idl

@@ -212,6 +212,13 @@ interface nsIDNSService : nsISupports
     [noscript] boolean IsSVCDomainNameFailed(in ACString aOwnerName,
                                              in ACString aSVCDomainName);
 
+    /**
+     * Reset the exclusion list.
+     * @param aOwnerName
+     *        The owner name of this HTTPS RRs.
+     */
+    [noscript] void ResetExcludedSVCDomainName(in ACString aOwnerName);
+
     /**
      * Returns a string containing the URI currently used by the TRR service.
      */

netwerk/protocol/http/nsHttpTransaction.cpp

@@ -16,6 +16,7 @@
 #include "TunnelUtils.h"
 #include "base/basictypes.h"
 #include "mozilla/Tokenizer.h"
+#include "mozilla/StaticPrefs_network.h"
 #include "nsCRT.h"
 #include "nsComponentManagerUtils.h"  // do_CreateInstance
 #include "nsHttpBasicAuth.h"
@@ -2911,6 +2912,7 @@ NS_IMETHODIMP nsHttpTransaction::OnLookupComplete(nsICancelable* aRequest,
       Some(hasIPAddress ? HTTPSSVC_WITH_IPHINT_RECEIVED_STAGE_1
                         : HTTPSSVC_WITHOUT_IPHINT_RECEIVED_STAGE_1);
 
+  nsCOMPtr<nsIDNSService> dns = do_GetService(NS_DNSSERVICE_CONTRACTID);
   nsCOMPtr<nsISVCBRecord> svcbRecord;
   if (NS_FAILED(record->GetServiceModeRecord(mCaps & NS_HTTP_DISALLOW_SPDY,
                                              mCaps & NS_HTTP_DISALLOW_HTTP3,
@@ -2922,8 +2924,18 @@ NS_IMETHODIMP nsHttpTransaction::OnLookupComplete(nsICancelable* aRequest,
                           allRecordsExcluded
                               ? HTTPSSVC_CONNECTION_ALL_RECORDS_EXCLUDED
                               : HTTPSSVC_CONNECTION_NO_USABLE_RECORD);
+    if (allRecordsExcluded &&
+        StaticPrefs::network_dns_httpssvc_reset_exclustion_list() && dns) {
+      Unused << dns->ResetExcludedSVCDomainName(mConnInfo->GetOrigin());
+      if (NS_FAILED(record->GetServiceModeRecord(mCaps & NS_HTTP_DISALLOW_SPDY,
+                                                 mCaps & NS_HTTP_DISALLOW_HTTP3,
+                                                 getter_AddRefs(svcbRecord)))) {
         return NS_ERROR_FAILURE;
       }
+    } else {
+      return NS_ERROR_FAILURE;
+    }
+  }
 
   // Remember this RR set. In the case that the connection establishment failed,
   // we will use other records to retry.
@@ -2941,7 +2953,6 @@ NS_IMETHODIMP nsHttpTransaction::OnLookupComplete(nsICancelable* aRequest,
   }
 
   // Prefetch the A/AAAA records of the target name.
-  nsCOMPtr<nsIDNSService> dns = do_GetService(NS_DNSSERVICE_CONTRACTID);
   if (dns) {
     uint32_t flags =
         nsIDNSService::GetFlagsFromTRRMode(mConnInfo->GetTRRMode());

netwerk/test/unit/test_trr_https_fallback.js

@@ -88,6 +88,8 @@ registerCleanupFunction(() => {
   prefs.clearUserPref("network.dns.use_https_rr_as_altsvc");
   prefs.clearUserPref("network.dns.echconfig.enabled");
   prefs.clearUserPref("network.dns.echconfig.fallback_to_origin");
+  prefs.clearUserPref("network.dns.httpssvc.reset_exclustion_list");
+  trrServer.stop();
 });
 
 class DNSListener {
@@ -591,3 +593,104 @@ add_task(async function testFallbackToTheOrigin3() {
 
   await trrServer.stop();
 });
+
+add_task(async function testResetExclusionList() {
+  trrServer = new TRRServer();
+  await trrServer.start();
+  Services.prefs.setIntPref("network.trr.mode", 3);
+  Services.prefs.setCharPref(
+    "network.trr.uri",
+    `https://foo.example.com:${trrServer.port}/dns-query`
+  );
+  Services.prefs.setBoolPref(
+    "network.dns.httpssvc.reset_exclustion_list",
+    false
+  );
+
+  await trrServer.registerDoHAnswers("test.reset.com", "HTTPS", [
+    {
+      name: "test.reset.com",
+      ttl: 55,
+      type: "HTTPS",
+      flush: false,
+      data: {
+        priority: 1,
+        name: "test.reset1.com",
+        values: [
+          { key: "alpn", value: "h2,h3" },
+          { key: "port", value: h2Port },
+          { key: "echconfig", value: "456..." },
+        ],
+      },
+    },
+    {
+      name: "test.reset.com",
+      ttl: 55,
+      type: "HTTPS",
+      flush: false,
+      data: {
+        priority: 2,
+        name: "test.reset2.com",
+        values: [
+          { key: "alpn", value: "h2,h3" },
+          { key: "echconfig", value: "456..." },
+        ],
+      },
+    },
+  ]);
+
+  let listener = new DNSListener();
+
+  let request = dns.asyncResolve(
+    "test.reset.com",
+    dns.RESOLVE_TYPE_HTTPSSVC,
+    0,
+    null, // resolverInfo
+    listener,
+    mainThread,
+    defaultOriginAttributes
+  );
+
+  let [inRequest, inRecord, inStatus] = await listener;
+  Assert.equal(inRequest, request, "correct request was used");
+  Assert.equal(inStatus, Cr.NS_OK, "status OK");
+
+  certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
+    true
+  );
+
+  // After this request, test.reset1.com and test.reset2.com should be both in
+  // the exclusion list.
+  let chan = makeChan(`https://test.reset.com:${h2Port}/server-timing`);
+  await channelOpenPromise(chan, CL_EXPECT_LATE_FAILURE | CL_ALLOW_UNKNOWN_CL);
+
+  // This request should be also failed, because all records are excluded.
+  chan = makeChan(`https://test.reset.com:${h2Port}/server-timing`);
+  await channelOpenPromise(chan, CL_EXPECT_LATE_FAILURE | CL_ALLOW_UNKNOWN_CL);
+
+  await trrServer.registerDoHAnswers("test.reset1.com", "A", [
+    {
+      name: "test.reset1.com",
+      ttl: 55,
+      type: "A",
+      flush: false,
+      data: "127.0.0.1",
+    },
+  ]);
+
+  Services.prefs.setBoolPref(
+    "network.dns.httpssvc.reset_exclustion_list",
+    true
+  );
+
+  // After enable network.dns.httpssvc.reset_exclustion_list and register
+  // A record for test.reset1.com, this request should be succeeded.
+  chan = makeChan(`https://test.reset.com:${h2Port}/server-timing`);
+  await channelOpenPromise(chan);
+
+  certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
+    false
+  );
+
+  await trrServer.stop();
+});