Bug 1743022 - Add tests for TRR temporary blocklisting r=necko-reviewers,kershaw

* DNSPacket::Decode now returns an error code for NS responses with a non-zero
  RCODE. Previously, when we'd do the check for the parent domain, we'd treat
  any DoH response as a valid NS, making the entire check for parents useless.
* Changes the documentation for this feature to mention the prefs used by this
  feature.
* I don't think we need to worry about clearing the blocklist when the DNS
  cache is cleared. For testing we can simply disable the blocklist. In real
  life the blocklist is only 60 seconds and it's unlikely to cause problems
  for users.

Depends on D136530

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

netwerk/dns/DNSPacket.cpp

@@ -1014,6 +1014,10 @@ nsresult DNSPacket::DecodeInternal(
     return NS_ERROR_ILLEGAL_VALUE;
   }
 
+  if (aType == TRRTYPE_NS && rcode != 0) {
+    return NS_ERROR_UNKNOWN_HOST;
+  }
+
   if ((aType != TRRTYPE_NS) && aCname.IsEmpty() && aResp.mAddresses.IsEmpty() &&
       aTypeResult.is<TypeRecordEmpty>()) {
     // no entries were stored!

netwerk/docs/dns/dns-over-https-trr.rst

@@ -46,7 +46,7 @@ a DoH or a Do53 request. First it checks the effective TRR mode of the request
 is as requests could have a different mode from the global one.
 If the request may use TRR, then we dispatch a request in nsHostResolver::TrrLookup.
 Since we usually reolve both IPv4 and IPv6 names, a **TRRQuery** object is
-created to perform and combine both responses. 
+created to perform and combine both responses.
 
 Once done, nsHostResolver::CompleteLookup is called. If the DoH server returned a
 valid response we use it, otherwise we report a failure in TRR-only mode, or
@@ -62,7 +62,9 @@ main thread.
 Dynamic Blocklist
 -----------------
 
-In order to improve performance TRR service manages a dynamic persistent blocklist for host names that can't be resolved with DoH but works with the native resolver. Blocklisted entries will not be retried over DoH for one minute.
+In order to improve performance TRR service manages a dynamic blocklist for host names that can't be resolved with DoH but work with the native resolver. Blocklisted entries will not be retried over DoH for one minute (See `network.trr.temp_blocklist_duration_sec` pref).
+When a domain is added to the blocklist, we also check if there is an NS record for its parent domain, in which case we add that to the blocklist.
+This feature is controlled by the `network.trr.temp_blocklist` pref.
 
 TRR confirmation
 ----------------

netwerk/test/unit/test_trr_blocklist.js

@@ -0,0 +1,81 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+const dns = Cc["@mozilla.org/network/dns-service;1"].getService(
+  Ci.nsIDNSService
+);
+const override = Cc["@mozilla.org/network/native-dns-override;1"].getService(
+  Ci.nsINativeDNSResolverOverride
+);
+
+function setup() {
+  trr_test_setup();
+  Services.prefs.setBoolPref("network.trr.temp_blocklist", true);
+}
+setup();
+
+add_task(async function checkBlocklisting() {
+  let trrServer = new TRRServer();
+  registerCleanupFunction(async () => {
+    await trrServer.stop();
+  });
+  await trrServer.start();
+  info(`port = ${trrServer.port}\n`);
+
+  dns.clearCache(true);
+  Services.prefs.setCharPref(
+    "network.trr.uri",
+    `https://foo.example.com:${trrServer.port}/dns-query`
+  );
+  Services.prefs.setIntPref("network.trr.mode", Ci.nsIDNSService.MODE_TRRFIRST);
+
+  await trrServer.registerDoHAnswers("top.test.com", "NS", {});
+
+  override.addIPOverride("sub.top.test.com", "2.2.2.2");
+  override.addIPOverride("sub2.top.test.com", "2.2.2.2");
+  await new TRRDNSListener("sub.top.test.com", {
+    expectedAnswer: "2.2.2.2",
+  });
+  equal(await trrServer.requestCount("sub.top.test.com", "A"), 1);
+
+  // Clear the cache so that we need to consult the blocklist and not simply
+  // return the cached DNS record.
+  dns.clearCache(true);
+  await new TRRDNSListener("sub.top.test.com", {
+    expectedAnswer: "2.2.2.2",
+  });
+  equal(
+    await trrServer.requestCount("sub.top.test.com", "A"),
+    1,
+    "Request should go directly to native because result is still in blocklist"
+  );
+
+  // XXX(valentin): if this ever starts intermittently failing we need to add
+  // a sleep here. But the check for the parent NS should normally complete
+  // before the second subdomain request.
+  equal(
+    await trrServer.requestCount("top.test.com", "NS"),
+    1,
+    "Should have checked parent domain"
+  );
+  await new TRRDNSListener("sub2.top.test.com", {
+    expectedAnswer: "2.2.2.2",
+  });
+  equal(await trrServer.requestCount("sub2.top.test.com", "A"), 0);
+
+  // The blocklist should instantly expire.
+  Services.prefs.setIntPref("network.trr.temp_blocklist_duration_sec", 0);
+  dns.clearCache(true);
+  await new TRRDNSListener("sub.top.test.com", {
+    expectedAnswer: "2.2.2.2",
+  });
+  // blocklist expired. Do another check.
+  equal(
+    await trrServer.requestCount("sub.top.test.com", "A"),
+    2,
+    "We should do another TRR request because the bloclist expired"
+  );
+});

netwerk/test/unit/xpcshell.ini

@@ -597,3 +597,4 @@ skip-if = os == "android"
 head = head_channels.js head_cache.js head_cookies.js head_trr.js trr_common.js
 skip-if = os == "android"
 run-sequentially = node server exceptions dont replay well
+[test_trr_blocklist.js]