Bug 1688092 - Update ODoHConfigs when the TTL expires r=necko-reviewers,valentin

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

modules/libpref/init/StaticPrefList.yaml

@@ -8822,6 +8822,12 @@
   value: ""
   mirror: never
 
+# The minimum ttl of the DNS record that contains ODoHConfigs.
+- name: network.trr.odoh.min_ttl
+  type: RelaxedAtomicUint32
+  value: 60
+  mirror: always
+
 # Allow the network changed event to get sent when a network topology or setup
 # change is noticed while running.
 - name: network.notify.changed

netwerk/dns/DNSPacket.cpp

@@ -768,6 +768,8 @@ nsresult DNSPacket::DecodeInternal(
             auto& results = aTypeResult.as<TypeRecordHTTPSSVC>();
             results.AppendElement(parsed);
           }
+
+          aTTL = TTL;
           break;
         }
         default:

netwerk/dns/DNSRequestChild.cpp

@@ -326,6 +326,11 @@ ChildDNSByTypeRecord::GetResults(mozilla::net::TypeRecordResultType* aResults) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+ChildDNSByTypeRecord::GetTtl(uint32_t* aResult) {
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
 //-----------------------------------------------------------------------------
 // DNSRequestSender
 //-----------------------------------------------------------------------------

netwerk/dns/ODoHService.cpp

@@ -7,6 +7,7 @@
 
 #include "mozilla/net/SocketProcessChild.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/ScopeExit.h"
 #include "mozilla/StaticPrefs_network.h"
 #include "nsIDNSService.h"
 #include "nsIDNSByTypeRecord.h"
@@ -29,7 +30,7 @@ extern mozilla::LazyLogModule gHostResolverLog;
 #define LOG(args) MOZ_LOG(gHostResolverLog, mozilla::LogLevel::Debug, args)
 
 NS_IMPL_ISUPPORTS(ODoHService, nsIDNSListener, nsIObserver,
-                  nsISupportsWeakReference)
+                  nsISupportsWeakReference, nsITimerCallback)
 
 ODoHService::ODoHService()
     : mLock("net::ODoHService"), mQueryODoHConfigInProgress(false) {
@@ -52,6 +53,12 @@ bool ODoHService::Init() {
 
   ReadPrefs(nullptr);
 
+  nsCOMPtr<nsIObserverService> observerService =
+      mozilla::services::GetObserverService();
+  if (observerService) {
+    observerService->AddObserver(this, "xpcom-shutdown-threads", true);
+  }
+
   return true;
 }
 
@@ -65,6 +72,11 @@ ODoHService::Observe(nsISupports* aSubject, const char* aTopic,
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
   if (!strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID)) {
     ReadPrefs(NS_ConvertUTF16toUTF8(aData).get());
+  } else if (!strcmp(aTopic, "xpcom-shutdown-threads")) {
+    if (mTTLTimer) {
+      mTTLTimer->Cancel();
+      mTTLTimer = nullptr;
+    }
   }
 
   return NS_OK;
@@ -192,7 +204,10 @@ nsresult ODoHService::UpdateODoHConfig() {
 
   nsCOMPtr<nsICancelable> tmpOutstanding;
   nsCOMPtr<nsIEventTarget> target = gTRRService->MainThreadOrTRRThread();
-  uint32_t flags = nsIDNSService::RESOLVE_DISABLE_ODOH;
+  // We'd like to bypass the DNS cache, since ODoHConfigs will be updated
+  // manually by ODoHService.
+  uint32_t flags =
+      nsIDNSService::RESOLVE_DISABLE_ODOH | nsIDNSService::RESOLVE_BYPASS_CACHE;
   rv = dns->AsyncResolveNative(hostStr, nsIDNSService::RESOLVE_TYPE_HTTPSSVC,
                                flags, nullptr, this, target, OriginAttributes(),
                                getter_AddRefs(tmpOutstanding));
@@ -205,6 +220,23 @@ nsresult ODoHService::UpdateODoHConfig() {
   return rv;
 }
 
+void ODoHService::StartTTLTimer(uint32_t aTTL) {
+  if (mTTLTimer) {
+    mTTLTimer->Cancel();
+    mTTLTimer = nullptr;
+  }
+  LOG(("ODoHService::StartTTLTimer ttl=%d(s)", aTTL));
+  NS_NewTimerWithCallback(getter_AddRefs(mTTLTimer), this, aTTL * 1000,
+                          nsITimer::TYPE_ONE_SHOT);
+}
+
+NS_IMETHODIMP
+ODoHService::Notify(nsITimer* aTimer) {
+  MOZ_ASSERT(aTimer == mTTLTimer);
+  UpdateODoHConfig();
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 ODoHService::OnLookupComplete(nsICancelable* aRequest, nsIDNSRecord* aRec,
                               nsresult aStatus) {
@@ -212,7 +244,55 @@ ODoHService::OnLookupComplete(nsICancelable* aRequest, nsIDNSRecord* aRec,
                 NS_IsMainThread() || gTRRService->IsOnTRRThread());
   MOZ_ASSERT_IF(XRE_IsSocketProcess(), NS_IsMainThread());
 
+  nsCOMPtr<nsIDNSHTTPSSVCRecord> httpsRecord;
+  auto notifyActivation = MakeScopeExit([&]() {
+    // Let observers know whether ODoHService is activated or not.
+    bool hasODoHConfigs = mODoHConfigs && !mODoHConfigs->IsEmpty();
+    uint32_t ttl = 0;
+    if (httpsRecord) {
+      Unused << httpsRecord->GetTtl(&ttl);
+      if (ttl < StaticPrefs::network_trr_odoh_min_ttl()) {
+        ttl = StaticPrefs::network_trr_odoh_min_ttl();
+      }
+    }
+    auto task = [hasODoHConfigs, ttl]() {
+      MOZ_ASSERT(NS_IsMainThread());
+      if (XRE_IsSocketProcess()) {
+        SocketProcessChild::GetSingleton()->SendODoHServiceActivated(
+            hasODoHConfigs);
+      }
+
+      nsCOMPtr<nsIObserverService> observerService =
+          mozilla::services::GetObserverService();
+
+      if (observerService) {
+        observerService->NotifyObservers(nullptr, "odoh-service-activated",
+                                         hasODoHConfigs ? u"true" : u"false");
+      }
+
+      if (ttl) {
+        gODoHService->StartTTLTimer(ttl);
+      }
+    };
+
+    if (NS_IsMainThread()) {
+      task();
+    } else {
+      NS_DispatchToMainThread(
+          NS_NewRunnableFunction("ODoHService::Activated", std::move(task)));
+    }
+
+    if (!mPendingRequests.IsEmpty()) {
+      nsTArray<RefPtr<ODoH>> requests = std::move(mPendingRequests);
+      nsCOMPtr<nsIEventTarget> target = gTRRService->MainThreadOrTRRThread();
+      for (auto& query : requests) {
+        target->Dispatch(query.forget());
+      }
+    }
+  });
+
   mQueryODoHConfigInProgress = false;
+  mODoHConfigs.reset();
 
   LOG(("ODoHService::OnLookupComplete [aStatus=%" PRIx32 "]",
        static_cast<uint32_t>(aStatus)));
@@ -220,7 +300,7 @@ ODoHService::OnLookupComplete(nsICancelable* aRequest, nsIDNSRecord* aRec,
     return NS_OK;
   }
 
-  nsCOMPtr<nsIDNSHTTPSSVCRecord> httpsRecord = do_QueryInterface(aRec);
+  httpsRecord = do_QueryInterface(aRec);
   if (!httpsRecord) {
     return NS_OK;
   }
@@ -240,41 +320,7 @@ ODoHService::OnLookupComplete(nsICancelable* aRequest, nsIDNSRecord* aRec,
     return NS_OK;
   }
 
-  mODoHConfigs.reset();
   mODoHConfigs.emplace(std::move(configs));
-
-  // Let observers know whether ODoHService is activated or not.
-  bool hasODoHConfigs = !mODoHConfigs->IsEmpty();
-  auto task = [hasODoHConfigs]() {
-    MOZ_ASSERT(NS_IsMainThread());
-    if (XRE_IsSocketProcess()) {
-      SocketProcessChild::GetSingleton()->SendODoHServiceActivated(
-          hasODoHConfigs);
-    }
-
-    nsCOMPtr<nsIObserverService> observerService =
-        mozilla::services::GetObserverService();
-
-    if (observerService) {
-      observerService->NotifyObservers(nullptr, "odoh-service-activated",
-                                       hasODoHConfigs ? u"true" : u"false");
-    }
-  };
-
-  if (NS_IsMainThread()) {
-    task();
-  } else {
-    NS_DispatchToMainThread(
-        NS_NewRunnableFunction("ODoHService::Activated", std::move(task)));
-  }
-
-  if (!mPendingRequests.IsEmpty()) {
-    nsTArray<RefPtr<ODoH>> requests = std::move(mPendingRequests);
-    nsCOMPtr<nsIEventTarget> target = gTRRService->MainThreadOrTRRThread();
-    for (auto& query : requests) {
-      target->Dispatch(query.forget());
-    }
-  }
   return NS_OK;
 }
 

netwerk/dns/ODoHService.h

@@ -12,6 +12,7 @@
 #include "nsString.h"
 #include "nsIDNSListener.h"
 #include "nsIObserver.h"
+#include "nsITimer.h"
 #include "nsWeakReference.h"
 
 namespace mozilla {
@@ -21,11 +22,13 @@ class ODoH;
 
 class ODoHService : public nsIDNSListener,
                     public nsIObserver,
-                    public nsSupportsWeakReference {
+                    public nsSupportsWeakReference,
+                    public nsITimerCallback {
  public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIDNSLISTENER
   NS_DECL_NSIOBSERVER
+  NS_DECL_NSITIMERCALLBACK
 
   ODoHService();
   bool Init();
@@ -43,6 +46,7 @@ class ODoHService : public nsIDNSListener,
   nsresult ReadPrefs(const char* aName);
   void OnODoHPrefsChange(bool aInit);
   void BuildODoHRequestURI();
+  void StartTTLTimer(uint32_t aTTL);
 
   Mutex mLock;
   Atomic<bool, Relaxed> mQueryODoHConfigInProgress;
@@ -52,6 +56,8 @@ class ODoHService : public nsIDNSListener,
   nsCString mODoHRequestURI;
   Maybe<nsTArray<ObliviousDoHConfig>> mODoHConfigs;
   nsTArray<RefPtr<ODoH>> mPendingRequests;
+  // This timer is always touched on main thread to avoid race conditions.
+  nsCOMPtr<nsITimer> mTTLTimer;
 };
 
 extern ODoHService* gODoHService;

netwerk/dns/nsDNSService2.cpp

@@ -430,6 +430,9 @@ nsDNSByTypeRecord::GetResults(mozilla::net::TypeRecordResultType* aResults) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsDNSByTypeRecord::GetTtl(uint32_t* aTtl) { return mHostRecord->GetTtl(aTtl); }
+
 //-----------------------------------------------------------------------------
 
 class nsDNSAsyncRequest final : public nsResolveHostCallback,

netwerk/dns/nsHostResolver.cpp

@@ -240,6 +240,7 @@ void nsHostRecord::SetExpiration(const mozilla::TimeStamp& now,
   }
   mGraceStart = now + TimeDuration::FromSeconds(valid);
   mValidEnd = now + TimeDuration::FromSeconds(valid + grace);
+  mTtl = valid;
 }
 
 void nsHostRecord::CopyExpirationTimesAndFlagsFrom(
@@ -650,6 +651,13 @@ TypeHostRecord::GetAllRecordsExcluded(bool* aResult) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+TypeHostRecord::GetTtl(uint32_t* aResult) {
+  NS_ENSURE_ARG(aResult);
+  *aResult = mTtl;
+  return NS_OK;
+}
+
 //----------------------------------------------------------------------------
 
 static const char kPrefGetTtl[] = "network.dns.get-ttl";

netwerk/dns/nsHostResolver.h

@@ -152,6 +152,8 @@ class nsHostRecord : public mozilla::LinkedListElement<RefPtr<nsHostRecord>>,
   // but a request to refresh it will be made.
   mozilla::TimeStamp mGraceStart;
 
+  uint32_t mTtl = 0;
+
   // The computed TRR mode that is actually used by the request.
   // It is set in nsHostResolver::NameLookup and is based on the mode of the
   // default resolver and the TRRMode encoded in the flags.

netwerk/dns/nsIDNSByTypeRecord.idl

@@ -119,6 +119,11 @@ interface nsIDNSHTTPSSVCRecord : nsISupports
    */
   readonly attribute boolean allRecordsExcluded;
 
+  /**
+   * Returns the ttl of this record.
+   */
+  readonly attribute uint32_t ttl;
+
   Array<nsISVCBRecord> GetAllRecordsWithEchConfig(in boolean aNoHttp2,
                                                   in boolean aNoHttp3,
                                                   out boolean aAllRecordsHaveEchConfig,

netwerk/test/unit/test_odoh.js

@@ -248,6 +248,22 @@ class DNSListener {
   }
 }
 
+function observerPromise(topic) {
+  return new Promise(resolve => {
+    let observer = {
+      QueryInterface: ChromeUtils.generateQI(["nsIObserver"]),
+      observe(aSubject, aTopic, aData) {
+        dump(`observe: ${aSubject}, ${aTopic}, ${aData} \n`);
+        if (aTopic == topic) {
+          Services.obs.removeObserver(observer, topic);
+          resolve(aData);
+        }
+      },
+    };
+    Services.obs.addObserver(observer, topic);
+  });
+}
+
 add_task(async function testODoHConfig() {
   // use the h2 server as DOH provider
   prefs.setCharPref(
@@ -271,15 +287,80 @@ add_task(async function testODoHConfig() {
   Assert.equal(odohconfig.length, 46);
 });
 
+let testIndex = 0;
+
+async function ODoHConfigTest(query, ODoHHost, expectedResult = false) {
+  prefs.setCharPref(
+    "network.trr.uri",
+    "https://foo.example.com:" + h2Port + "/odohconfig?" + query
+  );
+
+  // Setting the pref "network.trr.odoh.target_host" will trigger the reload of
+  // the ODoHConfigs.
+  if (ODoHHost != undefined) {
+    prefs.setCharPref("network.trr.odoh.target_host", ODoHHost);
+  } else {
+    prefs.setCharPref(
+      "network.trr.odoh.target_host",
+      `https://odoh_host_${testIndex++}.com`
+    );
+  }
+
+  await observerPromise("odoh-service-activated");
+  Assert.equal(dns.ODoHActivated, expectedResult);
+}
+
+add_task(async function testODoHConfig1() {
+  await ODoHConfigTest("invalid=empty");
+});
+
+add_task(async function testODoHConfig2() {
+  await ODoHConfigTest("invalid=version");
+});
+
+add_task(async function testODoHConfig3() {
+  await ODoHConfigTest("invalid=configLength");
+});
+
+add_task(async function testODoHConfig4() {
+  await ODoHConfigTest("invalid=totalLength");
+});
+
+add_task(async function testODoHConfig5() {
+  await ODoHConfigTest("invalid=kemId");
+});
+
+add_task(async function testODoHConfig6() {
+  // Use a very short TTL.
+  prefs.setIntPref("network.trr.odoh.min_ttl", 1);
+  await ODoHConfigTest("invalid=kemId&ttl=1");
+
+  // This is triggered by the expiration of the TTL.
+  await observerPromise("odoh-service-activated");
+  Assert.ok(!dns.ODoHActivated);
+  prefs.clearUserPref("network.trr.odoh.min_ttl");
+});
+
+add_task(async function testODoHConfig7() {
+  dns.clearCache(true);
+  prefs.setIntPref("network.trr.mode", 2); // TRR-first
+  prefs.setBoolPref("network.trr.odoh.enabled", true);
+  // At this point, we've queried the ODoHConfig, but there is no usable config
+  // (kemId is not supported). So, we should see the DNS result is from the
+  // native resolver.
+  await new DNSListener("bar.example.com", "127.0.0.1");
+});
+
 // verify basic A record
 add_task(async function test1() {
   dns.clearCache(true);
   prefs.setIntPref("network.trr.mode", 2); // TRR-first
   prefs.setBoolPref("network.trr.odoh.enabled", true);
-  prefs.setCharPref(
-    "network.trr.odoh.target_host",
-    `https://odoh_host.example.com:${h2Port}`
-  );
+
+  // Make sure we have an usable ODoHConfig to use.
+  await ODoHConfigTest("", `https://odoh_host.example.com:${h2Port}`, true);
+
+  // Make sure we can connect to ODOH target successfully.
   prefs.setCharPref("network.dns.localDomains", "odoh_host.example.com");
 
   const expectedIP = "8.8.8.8";

testing/xpcshell/moz-http2/moz-http2.js

@@ -979,13 +979,40 @@ function handleRequest(req, res) {
     req.on("end", function finishedData() {
       let packet = dnsPacket.decode(payload);
       let answers = [];
-      let odohconfig = odoh.get_odoh_config();
+      let odohconfig;
+      if (u.query.invalid) {
+        if (u.query.invalid === "empty") {
+          odohconfig = Buffer.from("");
+        } else if (u.query.invalid === "version") {
+          odohconfig = Buffer.from(
+            "002cff030028002000010001002021c8c16355091b28d521cb196627297955c1b607a3dcf1f136534578460d077d",
+            "hex"
+          );
+        } else if (u.query.invalid === "configLength") {
+          odohconfig = Buffer.from(
+            "002cff040028002000010001002021c8c16355091b28d521cb196627297955c1b607a3dcf1f136534578460d07",
+            "hex"
+          );
+        } else if (u.query.invalid === "totalLength") {
+          odohconfig = Buffer.from(
+            "012cff030028002000010001002021c8c16355091b28d521cb196627297955c1b607a3dcf1f136534578460d077d",
+            "hex"
+          );
+        } else if (u.query.invalid === "kemId") {
+          odohconfig = Buffer.from(
+            "002cff040028002100010001002021c8c16355091b28d521cb196627297955c1b607a3dcf1f136534578460d077d",
+            "hex"
+          );
+        }
+      } else {
+        odohconfig = odoh.get_odoh_config();
+      }
       var b64encoded = Buffer.from(odohconfig).toString("base64");
       if (packet.questions[0].type == "HTTPS") {
         answers.push({
           name: packet.questions[0].name,
           type: packet.questions[0].type,
-          ttl: 55,
+          ttl: u.query.ttl ? u.query.ttl : 55,
           class: "IN",
           flush: false,
           data: {