Bug 1587875 - Clear TRR cache entries when changing DoH servers r=kershaw

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

--HG--
extra : moz-landing-system : lando

modules/libpref/init/all.js

@@ -4277,6 +4277,9 @@ pref("network.trr.max-fails", 5);
 // Comma separated list of domains that we should not use TRR for
 pref("network.trr.excluded-domains", "");
 pref("network.trr.builtin-excluded-domains", "localhost,local");
+// When true, the DNS+TRR cache will be cleared when a relevant TRR pref
+// changes. (uri, bootstrapAddress, excluded-domains)
+pref("network.trr.clear-cache-on-pref-change", true);
 
 pref("captivedetect.canonicalURL", "http://detectportal.firefox.com/success.txt");
 pref("captivedetect.canonicalContent", "success\n");

netwerk/dns/TRRService.cpp

@@ -142,6 +142,11 @@ void TRRService::GetPrefBranch(nsIPrefBranch** result) {
 
 nsresult TRRService::ReadPrefs(const char* name) {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
+
+  // Whenever a pref change occurs that would cause us to clear the cache
+  // we set this to true then do it at the end of the method.
+  bool clearEntireCache = false;
+
   if (!name || !strcmp(name, TRR_PREF("mode"))) {
     // 0 - off, 1 - reserved, 2 - TRR first, 3 - TRR only, 4 - reserved,
     // 5 - explicit off
@@ -210,6 +215,7 @@ nsresult TRRService::ReadPrefs(const char* name) {
     if (!old.IsEmpty() && !mPrivateURI.Equals(old)) {
       mClearTRRBLStorage = true;
       LOG(("TRRService clearing blacklist because of change is uri service\n"));
+      clearEntireCache = true;
     }
   }
   if (!name || !strcmp(name, TRR_PREF("credentials"))) {
@@ -229,6 +235,7 @@ nsresult TRRService::ReadPrefs(const char* name) {
   if (!name || !strcmp(name, TRR_PREF("bootstrapAddress"))) {
     MutexAutoLock lock(mLock);
     Preferences::GetCString(TRR_PREF("bootstrapAddress"), mBootstrapAddr);
+    clearEntireCache = true;
   }
   if (!name || !strcmp(name, TRR_PREF("wait-for-portal"))) {
     // Wait for captive portal?
@@ -319,6 +326,7 @@ nsresult TRRService::ReadPrefs(const char* name) {
 
     parseExcludedDomains(TRR_PREF("excluded-domains"));
     parseExcludedDomains(TRR_PREF("builtin-excluded-domains"));
+    clearEntireCache = true;
 
     nsAutoCString canonicalSiteURL;
     Preferences::GetCString(kCaptivedetectCanonicalURL, canonicalSiteURL);
@@ -334,6 +342,20 @@ nsresult TRRService::ReadPrefs(const char* name) {
     }
   }
 
+  // if name is null, then we're just now initializing. In that case we don't
+  // need to clear the cache.
+  if (name && clearEntireCache) {
+    bool tmp;
+    if (NS_SUCCEEDED(Preferences::GetBool(
+            TRR_PREF("clear-cache-on-pref-change"), &tmp)) &&
+        tmp) {
+      nsCOMPtr<nsIDNSService> dns = do_GetService(NS_DNSSERVICE_CONTRACTID);
+      if (dns) {
+        dns->ClearCache(true);
+      }
+    }
+  }
+
   return NS_OK;
 }
 

netwerk/test/unit/test_trr.js

@@ -58,6 +58,9 @@ add_task(function setup() {
   Services.prefs.setBoolPref("network.trr.wait-for-A-and-AAAA", true);
   // don't confirm that TRR is working, just go!
   Services.prefs.setCharPref("network.trr.confirmationNS", "skip");
+  // some tests rely on the cache not being cleared on pref change.
+  // we specifically test that this works
+  Services.prefs.setBoolPref("network.trr.clear-cache-on-pref-change", false);
 
   // The moz-http2 cert is for foo.example.com and is signed by http2-ca.pem
   // so add that cert to the trust list as a signing cert.  // the foo.example.com domain name.
@@ -87,6 +90,7 @@ registerCleanupFunction(() => {
   Services.prefs.clearUserPref("network.trr.wait-for-A-and-AAAA");
   Services.prefs.clearUserPref("network.trr.excluded-domains");
   Services.prefs.clearUserPref("network.trr.builtin-excluded-domains");
+  Services.prefs.clearUserPref("network.trr.clear-cache-on-pref-change");
   Services.prefs.clearUserPref("captivedetect.canonicalURL");
 
   Services.prefs.clearUserPref("network.http.spdy.enabled");
@@ -1055,6 +1059,27 @@ add_task(async function test_connection_closed_trr_first() {
   await new DNSListener("bar2.example.com", "9.9.9.9");
 });
 
+add_task(async function test_clearCacheOnURIChange() {
+  dns.clearCache(true);
+  Services.prefs.setBoolPref("network.trr.clear-cache-on-pref-change", true);
+  Services.prefs.setIntPref("network.trr.mode", 2); // TRR-first
+  Services.prefs.setCharPref(
+    "network.trr.uri",
+    `https://localhost:${h2Port}/doh?responseIP=7.7.7.7`
+  );
+
+  await new DNSListener("bar.example.com", "7.7.7.7");
+
+  // The TRR cache should be cleared by this pref change.
+  Services.prefs.setCharPref(
+    "network.trr.uri",
+    `https://localhost:${h2Port}/doh?responseIP=8.8.8.8`
+  );
+
+  await new DNSListener("bar.example.com", "8.8.8.8");
+  Services.prefs.setBoolPref("network.trr.clear-cache-on-pref-change", false);
+});
+
 add_task(async function test_dnsSuffix() {
   async function checkDnsSuffixInMode(mode) {
     dns.clearCache(true);