Bug 1960834 - Don't use CORS cache if DNS entry has changed, a=pascalc

Original Revision: https://phabricator.services.mozilla.com/D251141

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

netwerk/dns/DNSRequestChild.cpp

@@ -58,6 +58,7 @@ class ChildDNSRecord : public nsIDNSAddrRecord {
   nsIRequest::TRRMode mEffectiveTRRMode = nsIRequest::TRR_DEFAULT_MODE;
   nsITRRSkipReason::value mTRRSkipReason = nsITRRSkipReason::TRR_UNSET;
   uint32_t mTTL = 0;
+  TimeStamp mLastUpdate = mozilla::TimeStamp::NowLoRes();
 };
 
 NS_IMPL_ISUPPORTS(ChildDNSRecord, nsIDNSRecord, nsIDNSAddrRecord)
@@ -78,6 +79,7 @@ ChildDNSRecord::ChildDNSRecord(const DNSRecord& reply,
   const nsTArray<NetAddr>& addrs = reply.addrs();
   mAddresses = addrs.Clone();
   mTTL = reply.ttl();
+  mLastUpdate = reply.lastUpdate();
 }
 
 //-----------------------------------------------------------------------------
@@ -208,6 +210,12 @@ ChildDNSRecord::GetTtl(uint32_t* aTtl) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+ChildDNSRecord::GetLastUpdate(TimeStamp* aLastUpdate) {
+  *aLastUpdate = mLastUpdate;
+  return NS_OK;
+}
+
 class ChildDNSByTypeRecord : public nsIDNSByTypeRecord,
                              public nsIDNSTXTRecord,
                              public nsIDNSHTTPSSVCRecord,

netwerk/dns/DNSRequestParent.cpp

@@ -141,10 +141,14 @@ DNSRequestHandler::OnLookupComplete(nsICancelable* request,
     uint32_t ttl = 0;
     rec->GetTtl(&ttl);
 
+    TimeStamp lastUpdate;
+    rec->GetLastUpdate(&lastUpdate);
+
     SendLookupCompletedHelper(
-        mIPCActor, DNSRequestResponse(DNSRecord(cname, array, trrFetchDuration,
+        mIPCActor,
-                                                trrFetchDurationNetworkOnly,
+        DNSRequestResponse(DNSRecord(cname, array, trrFetchDuration,
-                                                isTRR, effectiveTRRMode, ttl)));
+                                     trrFetchDurationNetworkOnly, isTRR,
+                                     effectiveTRRMode, ttl, lastUpdate)));
   } else {
     SendLookupCompletedHelper(mIPCActor, DNSRequestResponse(status));
   }

netwerk/dns/PDNSRequestParams.ipdlh

@@ -8,6 +8,7 @@
 using mozilla::net::NetAddr from "mozilla/net/DNS.h";
 using mozilla::net::IPCTypeRecord from "mozilla/net/DNSByTypeRecord.h";
 using nsIRequest::TRRMode from "nsIRequest.h";
+using class mozilla::TimeStamp from "mozilla/TimeStamp.h";
 
 namespace mozilla {
 namespace net {
@@ -25,6 +26,7 @@ struct DNSRecord
   bool isTRR;
   TRRMode effectiveTRRMode;
   uint32_t ttl;
+  TimeStamp lastUpdate;
 };
 
 union DNSRequestResponse

netwerk/dns/nsDNSService2.cpp

@@ -364,6 +364,12 @@ NS_IMETHODIMP nsDNSRecord::GetTrrSkipReason(
 NS_IMETHODIMP
 nsDNSRecord::GetTtl(uint32_t* aTtl) { return mHostRecord->GetTtl(aTtl); }
 
+NS_IMETHODIMP
+nsDNSRecord::GetLastUpdate(mozilla::TimeStamp* aLastUpdate) {
+  MutexAutoLock lock(mHostRecord->addr_info_lock);
+  return mHostRecord->GetLastUpdate(aLastUpdate);
+}
+
 class nsDNSByTypeRecord : public nsIDNSByTypeRecord,
                           public nsIDNSTXTRecord,
                           public nsIDNSHTTPSSVCRecord {
@@ -566,18 +572,53 @@ nsDNSAsyncRequest::Cancel(nsresult reason) {
 
 //-----------------------------------------------------------------------------
 
-class nsDNSSyncRequest : public nsResolveHostCallback {
+class DNSCacheRequest : public nsResolveHostCallback {
+ public:
   NS_DECL_THREADSAFE_ISUPPORTS
+
+  DNSCacheRequest() = default;
+
+  void OnResolveHostComplete(nsHostResolver* resolver, nsHostRecord* hostRecord,
+                             nsresult status) override {
+    mStatus = status;
+    mHostRecord = hostRecord;
+  }
+
+  bool EqualsAsyncListener(nsIDNSListener* aListener) override {
+    // Sync request: no listener to compare
+    return false;
+  }
+
+  size_t SizeOfIncludingThis(
+      mozilla::MallocSizeOf mallocSizeOf) const override {
+    size_t n = mallocSizeOf(this);
+
+    // The following fields aren't measured.
+    // - mHostRecord, because it's a non-owning pointer
+
+    // Measurement of the following members may be added later if DMD finds it
+    // is worthwhile:
+    // - nsDNSSyncRequest::mMonitor
+
+    return n;
+  }
+
+  nsresult mStatus = NS_OK;
+  RefPtr<nsHostRecord> mHostRecord;
+
+ protected:
+  virtual ~DNSCacheRequest() = default;
+};
+
+NS_IMPL_ISUPPORTS0(DNSCacheRequest)
+
+class nsDNSSyncRequest : public DNSCacheRequest {
  public:
   explicit nsDNSSyncRequest(PRMonitor* mon) : mMonitor(mon) {}
 
   void OnResolveHostComplete(nsHostResolver*, nsHostRecord*, nsresult) override;
-  bool EqualsAsyncListener(nsIDNSListener* aListener) override;
-  size_t SizeOfIncludingThis(mozilla::MallocSizeOf) const override;
 
   bool mDone = false;
-  nsresult mStatus = NS_OK;
-  RefPtr<nsHostRecord> mHostRecord;
 
  private:
   virtual ~nsDNSSyncRequest() = default;
@@ -585,38 +626,17 @@ class nsDNSSyncRequest : public nsResolveHostCallback {
   PRMonitor* mMonitor = nullptr;
 };
 
-NS_IMPL_ISUPPORTS0(nsDNSSyncRequest)
-
 void nsDNSSyncRequest::OnResolveHostComplete(nsHostResolver* resolver,
                                              nsHostRecord* hostRecord,
                                              nsresult status) {
   // store results, and wake up nsDNSService::Resolve to process results.
   PR_EnterMonitor(mMonitor);
   mDone = true;
-  mStatus = status;
+  DNSCacheRequest::OnResolveHostComplete(resolver, hostRecord, status);
-  mHostRecord = hostRecord;
   PR_Notify(mMonitor);
   PR_ExitMonitor(mMonitor);
 }
 
-bool nsDNSSyncRequest::EqualsAsyncListener(nsIDNSListener* aListener) {
-  // Sync request: no listener to compare
-  return false;
-}
-
-size_t nsDNSSyncRequest::SizeOfIncludingThis(MallocSizeOf mallocSizeOf) const {
-  size_t n = mallocSizeOf(this);
-
-  // The following fields aren't measured.
-  // - mHostRecord, because it's a non-owning pointer
-
-  // Measurement of the following members may be added later if DMD finds it
-  // is worthwhile:
-  // - mMonitor
-
-  return n;
-}
-
 class NotifyDNSResolution : public Runnable {
  public:
   explicit NotifyDNSResolution(const nsACString& aHostname)
@@ -1185,8 +1205,10 @@ nsDNSService::ResolveNative(const nsACString& aHostname,
                             nsIDNSService::DNSFlags flags,
                             const OriginAttributes& aOriginAttributes,
                             nsIDNSRecord** result) {
-  // Synchronous resolution is not available on the main thread.
+  // Synchronous resolution is not allowed on the main thread.
-  if (NS_IsMainThread()) {
+  // However, if RESOLVE_OFFLINE is set, we're only reading from the DNS cache,
+  // so it's safe to allow this on the main thread.
+  if (NS_IsMainThread() && !(flags & nsIDNSService::RESOLVE_OFFLINE)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
@@ -1233,6 +1255,20 @@ nsresult nsDNSService::ResolveInternal(
     return NS_ERROR_UNKNOWN_PROXY_HOST;
   }
 
+  //  Since RESOLVE_OFFLINE is set, we can use DNSCacheRequest to retrieve the
+  //  cached result directly.
+  if (flags & RESOLVE_OFFLINE) {
+    RefPtr<DNSCacheRequest> req = new DNSCacheRequest();
+    uint16_t af = GetAFForLookup(hostname, flags);
+    rv = res->ResolveHost(hostname, ""_ns, -1, RESOLVE_TYPE_DEFAULT,
+                          aOriginAttributes, flags, af, req);
+    if (NS_SUCCEEDED(rv)) {
+      RefPtr<nsDNSRecord> rec = new nsDNSRecord(req->mHostRecord);
+      rec.forget(result);
+    }
+    return rv;
+  }
+
   //
   // sync resolve: since the host resolver only works asynchronously, we need
   // to use a mutex and a condvar to wait for the result.  however, since the

netwerk/dns/nsHostRecord.cpp

@@ -447,6 +447,12 @@ nsresult AddrHostRecord::GetTtl(uint32_t* aResult) {
   return NS_OK;
 }
 
+nsresult AddrHostRecord::GetLastUpdate(mozilla::TimeStamp* aLastUpdate) {
+  addr_info_lock.AssertCurrentThreadOwns();
+  *aLastUpdate = mLastUpdate;
+  return NS_OK;
+}
+
 //----------------------------------------------------------------------------
 // TypeHostRecord
 //----------------------------------------------------------------------------

netwerk/dns/nsHostRecord.h

@@ -187,6 +187,8 @@ class nsHostRecord : public mozilla::LinkedListElement<RefPtr<nsHostRecord>>,
   // true if pending and on the queue (not yet given to getaddrinfo())
   bool onQueue() { return LoadNative() && isInList(); }
 
+  mozilla::TimeStamp mLastUpdate = mozilla::TimeStamp::NowLoRes();
+
   // When the record began being valid. Used mainly for bookkeeping.
   mozilla::TimeStamp mValidStart;
 
@@ -303,6 +305,7 @@ class AddrHostRecord final : public nsHostRecord {
   nsITRRSkipReason::value TrrSkipReason() const { return mTRRSkippedReason; }
 
   nsresult GetTtl(uint32_t* aResult);
+  nsresult GetLastUpdate(mozilla::TimeStamp* aLastUpdate);
 
  private:
   friend class nsHostResolver;

netwerk/dns/nsHostResolver.cpp

@@ -425,6 +425,9 @@ already_AddRefed<nsHostRecord> nsHostResolver::InitLoopbackRecord(
                          StaticPrefs::network_dnsCacheExpiration(),
                          StaticPrefs::network_dnsCacheExpirationGracePeriod());
   addrRec->negative = false;
+  // Use the oldest possible timestamp, since the contents of this record never
+  // change.
+  addrRec->mLastUpdate = TimeStamp::ProcessCreation();
 
   *aRv = NS_OK;
   return rec.forget();
@@ -1587,6 +1590,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookupLocked(
       old_addr_info = addrRec->addr_info;
       addrRec->addr_info = std::move(newRRSet);
       addrRec->addr_info_gencnt++;
+      addrRec->mLastUpdate = TimeStamp::NowLoRes();
     } else {
       if (addrRec->addr_info && newRRSet) {
         auto builder = addrRec->addr_info->Build();

netwerk/dns/nsIDNSRecord.idl

@@ -8,6 +8,7 @@
 
 %{ C++
 namespace mozilla {
+class TimeStamp;
 namespace net {
 union NetAddr;
 }
@@ -16,6 +17,7 @@ union NetAddr;
 %}
 native NetAddr(mozilla::net::NetAddr);
 [ref] native nsNetAddrTArrayRef(nsTArray<mozilla::net::NetAddr>);
+native TimeStamp(mozilla::TimeStamp);
 interface nsINetAddr;
 
 /**
@@ -151,4 +153,9 @@ interface nsIDNSAddrRecord : nsIDNSRecord
      * Returns the ttl of this record.
      */
     readonly attribute uint32_t ttl;
+
+     /**
+     * Returns the timestamp when this record is updated.
+     */
+    [noscript] readonly attribute TimeStamp lastUpdate;
 };

netwerk/protocol/http/nsCORSListenerProxy.cpp

@@ -28,6 +28,7 @@
 #include "nsGkAtoms.h"
 #include "nsWhitespaceTokenizer.h"
 #include "nsIChannelEventSink.h"
+#include "nsIDNSService.h"
 #include "nsIAsyncVerifyRedirectCallback.h"
 #include "nsCharSeparatedTokenizer.h"
 #include "nsAsyncRedirectVerifyHelper.h"
@@ -210,12 +211,16 @@ struct CORSCacheEntry : public LinkedListElement<CORSCacheEntry>,
   nsCOMPtr<nsIPrincipal> mPrincipal;
   bool mWithCredentials;
   nsCString mKey;  // serialized key
+  const TimeStamp mCreationTime{TimeStamp::NowLoRes()};
+  bool mDoomed{false};
 
   nsTArray<nsPreflightCache::TokenTime> mMethods;
   nsTArray<nsPreflightCache::TokenTime> mHeaders;
 
  private:
   virtual ~CORSCacheEntry() = default;
+
+  bool CheckDNSCache();
 };
 
 NS_IMPL_ISUPPORTS(nsPreflightCache, nsICORSPreflightCache)
@@ -333,10 +338,51 @@ void CORSCacheEntry::PurgeExpired(TimeStamp now) {
   }
 }
 
+bool CORSCacheEntry::CheckDNSCache() {
+  nsCOMPtr<nsIDNSService> dns;
+  dns = mozilla::components::DNS::Service();
+  if (!dns) {
+    return false;
+  }
+
+  nsAutoCString host;
+  if (NS_FAILED(mURI->GetAsciiHost(host))) {
+    return false;
+  }
+
+  nsCOMPtr<nsIDNSRecord> record;
+  nsresult rv = dns->ResolveNative(host, nsIDNSService::RESOLVE_OFFLINE, mOA,
+                                   getter_AddRefs(record));
+  if (NS_FAILED(rv) || !record) {
+    return false;
+  }
+
+  nsCOMPtr<nsIDNSAddrRecord> addrRec = do_QueryInterface(record);
+  if (!addrRec) {
+    return false;
+  }
+
+  TimeStamp lastUpdate;
+  Unused << addrRec->GetLastUpdate(&lastUpdate);
+
+  if (lastUpdate > mCreationTime) {
+    return false;
+  }
+
+  return true;
+}
+
 bool CORSCacheEntry::CheckRequest(const nsCString& aMethod,
                                   const nsTArray<nsCString>& aHeaders) {
   PurgeExpired(TimeStamp::NowLoRes());
 
+  if (!CheckDNSCache()) {
+    mMethods.Clear();
+    mHeaders.Clear();
+    mDoomed = true;
+    return false;
+  }
+
   if (!aMethod.EqualsLiteral("GET") && !aMethod.EqualsLiteral("POST")) {
     struct CheckToken {
       bool Equals(const nsPreflightCache::TokenTime& e,
@@ -377,11 +423,16 @@ already_AddRefed<CORSCacheEntry> nsPreflightCache::GetEntry(
 
   RefPtr<CORSCacheEntry> existingEntry = nullptr;
   if ((existingEntry = mTable.Get(key))) {
-    // Entry already existed so just return it. Also update the LRU list.
+    if (existingEntry->mDoomed) {
-    // Move to the head of the list.
+      existingEntry->removeFrom(mList);
-    existingEntry->removeFrom(mList);
+      mTable.Remove(key);
-    mList.insertFront(existingEntry);
+    } else {
-    return existingEntry.forget();
+      // Entry already existed so just return it. Also update the LRU list.
+      // Move to the head of the list.
+      existingEntry->removeFrom(mList);
+      mList.insertFront(existingEntry);
+      return existingEntry.forget();
+    }
   }
 
   if (!aCreate) {