Bug 1689987 - P1: Don't mess up TRR telemetry when ODoH is used r=necko-reviewers,valentin

Differential Revision: https://phabricator.services.mozilla.com/D104799
2021-02-18 12:50:02 +00:00 · 2021-02-18 12:50:02 +00:00 · a2532cb1f3
--- a/netwerk/dns/DNS.cpp
+++ b/netwerk/dns/DNS.cpp
@ -342,18 +342,22 @@ AddrInfo::AddrInfo(const nsACString& host, const PRAddrInfo* prAddrInfo,
 }

 AddrInfo::AddrInfo(const nsACString& host, const nsACString& cname,
-                   unsigned int aTRR, nsTArray<NetAddr>&& addresses)
+                   DNSResolverType aResolverType, unsigned int aTRRType,
+                   nsTArray<NetAddr>&& addresses)
    : mHostName(host),
      mCanonicalName(cname),
-      mFromTRR(aTRR),
+      mResolverType(aResolverType),
+      mTRRType(aTRRType),
      mAddresses(std::move(addresses)) {}

-AddrInfo::AddrInfo(const nsACString& host, unsigned int aTRR,
-                   nsTArray<NetAddr>&& addresses, uint32_t aTTL)
+AddrInfo::AddrInfo(const nsACString& host, DNSResolverType aResolverType,
+                   unsigned int aTRRType, nsTArray<NetAddr>&& addresses,
+                   uint32_t aTTL)
    : ttl(aTTL),
      mHostName(host),
      mCanonicalName(),
-      mFromTRR(aTRR),
+      mResolverType(aResolverType),
+      mTRRType(aTRRType),
      mAddresses(std::move(addresses)) {}

 // deep copy constructor
@ -361,7 +365,8 @@ AddrInfo::AddrInfo(const AddrInfo* src) {
  mHostName = src->mHostName;
  mCanonicalName = src->mCanonicalName;
  ttl = src->ttl;
-  mFromTRR = src->mFromTRR;
+  mResolverType = src->mResolverType;
+  mTRRType = src->mTRRType;
  mTrrFetchDuration = src->mTrrFetchDuration;
  mTrrFetchDurationNetworkOnly = src->mTrrFetchDurationNetworkOnly;

--- a/netwerk/dns/DNS.h
+++ b/netwerk/dns/DNS.h
@ -183,6 +183,8 @@ struct ObliviousDoHMessage {
  nsTArray<uint8_t> mEncryptedMessage;
 };

+enum class DNSResolverType : uint32_t { Native = 0, TRR, ODoH };
+
 class AddrInfo {
  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(AddrInfo)

@ -197,17 +199,24 @@ class AddrInfo {
  // Creates a basic AddrInfo object (initialize only the host, cname and TRR
  // type).
  explicit AddrInfo(const nsACString& host, const nsACString& cname,
-                    unsigned int aTRR, nsTArray<NetAddr>&& addresses);
+                    DNSResolverType aResolverType, unsigned int aTRRType,
+                    nsTArray<NetAddr>&& addresses);

  // Creates a basic AddrInfo object (initialize only the host and TRR status).
-  explicit AddrInfo(const nsACString& host, unsigned int aTRR,
-                    nsTArray<NetAddr>&& addresses, uint32_t aTTL = NO_TTL_DATA);
+  explicit AddrInfo(const nsACString& host, DNSResolverType aResolverType,
+                    unsigned int aTRRType, nsTArray<NetAddr>&& addresses,
+                    uint32_t aTTL = NO_TTL_DATA);

  explicit AddrInfo(const AddrInfo* src);  // copy

  size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;

-  unsigned int IsTRR() { return mFromTRR; }
+  bool IsTRROrODoH() const {
+    return mResolverType == DNSResolverType::TRR ||
+           mResolverType == DNSResolverType::ODoH;
+  }
+  DNSResolverType ResolverType() const { return mResolverType; }
+  unsigned int TRRType() { return mTRRType; }

  double GetTrrFetchDuration() { return mTrrFetchDuration; }
  double GetTrrFetchDurationNetworkOnly() {
@ -255,8 +264,8 @@ class AddrInfo {

  nsCString mHostName;
  nsCString mCanonicalName;
-
-  unsigned int mFromTRR = 0;
+  DNSResolverType mResolverType = DNSResolverType::Native;
+  unsigned int mTRRType = 0;
  double mTrrFetchDuration = 0;
  double mTrrFetchDurationNetworkOnly = 0;

--- a/netwerk/dns/GetAddrInfo.cpp
+++ b/netwerk/dns/GetAddrInfo.cpp
@ -186,8 +186,8 @@ _DNSQuery_A_SingleLabel(const nsACString& aCanonHost, uint16_t aAddressFamily,
  if (addresses.IsEmpty()) {
    return NS_ERROR_UNKNOWN_HOST;
  }
-  RefPtr<AddrInfo> ai(
-      new AddrInfo(aCanonHost, canonName, 0, std::move(addresses)));
+  RefPtr<AddrInfo> ai(new AddrInfo(
+      aCanonHost, canonName, DNSResolverType::Native, 0, std::move(addresses)));
  ai.forget(aAddrInfo);

  return NS_OK;
@ -318,9 +318,10 @@ bool FindAddrOverride(const nsACString& aHost, uint16_t aAddressFamily,
  }

  if (!cname) {
-    ai = new AddrInfo(aHost, 0, std::move(addresses));
+    ai = new AddrInfo(aHost, DNSResolverType::Native, 0, std::move(addresses));
  } else {
-    ai = new AddrInfo(aHost, *cname, 0, std::move(addresses));
+    ai = new AddrInfo(aHost, *cname, DNSResolverType::Native, 0,
+                      std::move(addresses));
  }

  ai.forget(aAddrInfo);
--- a/netwerk/dns/ODoH.h
+++ b/netwerk/dns/ODoH.h
@ -30,6 +30,19 @@ class ODoH final : public TRR {
  virtual const char* ContentType() const override {
    return "application/oblivious-dns-message";
  }
+  virtual DNSResolverType ResolverType() const override {
+    return DNSResolverType::ODoH;
+  }
+  virtual bool MaybeBlockRequest() override {
+    // TODO: check excluded list
+    return false;
+  };
+  virtual void RecordProcessingTime(nsIChannel* aChannel) override {
+    // TODO: record processing time for ODoH.
+  }
+  virtual void ReportStatus(nsresult aStatusCode) override {
+    // TODO: record status in ODoHService.
+  }
 };

 }  // namespace net
--- a/netwerk/dns/TRR.cpp
+++ b/netwerk/dns/TRR.cpp
@ -161,17 +161,7 @@ nsresult TRR::CreateQueryURI(nsIURI** aOutURI) {
  return NS_OK;
 }

-nsresult TRR::SendHTTPRequest() {
-  // This is essentially the "run" method - created from nsHostResolver
-
-  if ((mType != TRRTYPE_A) && (mType != TRRTYPE_AAAA) &&
-      (mType != TRRTYPE_NS) && (mType != TRRTYPE_TXT) &&
-      (mType != TRRTYPE_HTTPSSVC)) {
-    // limit the calling interface because nsHostResolver has explicit slots for
-    // these types
-    return NS_ERROR_FAILURE;
-  }
-
+bool TRR::MaybeBlockRequest() {
  if (((mType == TRRTYPE_A) || (mType == TRRTYPE_AAAA)) &&
      mRec->mEffectiveTRRMode != nsIRequest::TRR_ONLY_MODE) {
    // let NS resolves skip the blocklist check
@ -188,12 +178,12 @@ nsresult TRR::SendHTTPRequest() {

      RecordReason(nsHostRecord::TRR_HOST_BLOCKED_TEMPORARY);
      // not really an error but no TRR is issued
-      return NS_ERROR_UNKNOWN_HOST;
+      return true;
    }

    if (gTRRService->IsExcludedFromTRR(mHost)) {
      RecordReason(nsHostRecord::TRR_EXCLUDED);
-      return NS_ERROR_UNKNOWN_HOST;
+      return true;
    }

    if (UseDefaultServer() && (mType == TRRTYPE_A)) {
@ -202,6 +192,24 @@ nsresult TRR::SendHTTPRequest() {
    }
  }

+  return false;
+}
+
+nsresult TRR::SendHTTPRequest() {
+  // This is essentially the "run" method - created from nsHostResolver
+
+  if ((mType != TRRTYPE_A) && (mType != TRRTYPE_AAAA) &&
+      (mType != TRRTYPE_NS) && (mType != TRRTYPE_TXT) &&
+      (mType != TRRTYPE_HTTPSSVC)) {
+    // limit the calling interface because nsHostResolver has explicit slots for
+    // these types
+    return NS_ERROR_FAILURE;
+  }
+
+  if (MaybeBlockRequest()) {
+    return NS_ERROR_UNKNOWN_HOST;
+  }
+
  LOG(("TRR::SendHTTPRequest resolve %s type %u\n", mHost.get(), mType));

  nsAutoCString body;
@ -326,7 +334,7 @@ nsresult TRR::SendHTTPRequest() {
  // use the TRR connection.
  RefPtr<AddrHostRecord> addrRec = do_QueryObject(mRec);
  if (addrRec) {
-    addrRec->mTRRUsed = true;
+    addrRec->mResolverType = ResolverType();
  }

  NS_NewTimerWithCallback(
@ -619,7 +627,7 @@ void TRR::SaveAdditionalRecords(
           nsCString(iter.Key()).get()));
      continue;
    }
-    RefPtr<AddrInfo> ai(new AddrInfo(iter.Key(), TRRTYPE_A,
+    RefPtr<AddrInfo> ai(new AddrInfo(iter.Key(), ResolverType(), TRRTYPE_A,
                                     std::move(iter.Data()->mAddresses),
                                     iter.Data()->mTtl));
    mHostResolver->MaybeRenewHostRecord(hostRecord);
@ -661,8 +669,8 @@ void TRR::StoreIPHintAsDNSRecord(const struct SVCB& aSVCBRecord) {
  mHostResolver->MaybeRenewHostRecord(hostRecord);

  uint32_t ttl = AddrInfo::NO_TTL_DATA;
-  RefPtr<AddrInfo> ai(new AddrInfo(aSVCBRecord.mSvcDomainName, TRRTYPE_A,
-                                   std::move(addresses), ttl));
+  RefPtr<AddrInfo> ai(new AddrInfo(aSVCBRecord.mSvcDomainName, ResolverType(),
+                                   TRRTYPE_A, std::move(addresses), ttl));

  // Since we're not actually calling NameLookup for this record, we need
  // to set these fields to avoid assertions in CompleteLookup.
@ -679,8 +687,8 @@ void TRR::StoreIPHintAsDNSRecord(const struct SVCB& aSVCBRecord) {
 nsresult TRR::ReturnData(nsIChannel* aChannel) {
  if (mType != TRRTYPE_TXT && mType != TRRTYPE_HTTPSSVC) {
    // create and populate an AddrInfo instance to pass on
-    RefPtr<AddrInfo> ai(
-        new AddrInfo(mHost, mType, nsTArray<NetAddr>(), mDNS.mTtl));
+    RefPtr<AddrInfo> ai(new AddrInfo(mHost, ResolverType(), mType,
+                                     nsTArray<NetAddr>(), mDNS.mTtl));
    auto builder = ai->Build();
    builder.SetAddresses(std::move(mDNS.mAddresses));
    builder.SetCanonicalHostname(mCname);
@ -730,7 +738,8 @@ nsresult TRR::FailData(nsresult error) {
    // create and populate an TRR AddrInfo instance to pass on to signal that
    // this comes from TRR
    nsTArray<NetAddr> noAddresses;
-    RefPtr<AddrInfo> ai = new AddrInfo(mHost, mType, std::move(noAddresses));
+    RefPtr<AddrInfo> ai =
+        new AddrInfo(mHost, ResolverType(), mType, std::move(noAddresses));

    (void)mHostResolver->CompleteLookup(mRec, error, ai, mPB, mOriginSuffix,
                                        mTRRSkippedReason, this);
@ -833,7 +842,7 @@ nsresult TRR::On200Response(nsIChannel* aChannel) {
  return FollowCname(aChannel);
 }

-static void RecordProcessingTime(nsIChannel* aChannel) {
+void TRR::RecordProcessingTime(nsIChannel* aChannel) {
  // This method records the time it took from the last received byte of the
  // DoH response until we've notified the consumer with a host record.
  nsCOMPtr<nsITimedChannel> timedChan = do_QueryInterface(aChannel);
@ -855,6 +864,16 @@ static void RecordProcessingTime(nsIChannel* aChannel) {
       (TimeStamp::Now() - end).ToMilliseconds()));
 }

+void TRR::ReportStatus(nsresult aStatusCode) {
+  // If the TRR was cancelled by nsHostResolver, then we don't need to report
+  // it as failed; otherwise it can cause the confirmation to fail.
+  if (UseDefaultServer() && aStatusCode != NS_ERROR_ABORT) {
+    // Bad content is still considered "okay" if the HTTP response is okay
+    gTRRService->TRRIsOkay(NS_SUCCEEDED(aStatusCode) ? TRRService::OKAY_NORMAL
+                                                     : TRRService::OKAY_BAD);
+  }
+}
+
 NS_IMETHODIMP
 TRR::OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) {
  // The dtor will be run after the function returns
@ -874,13 +893,7 @@ TRR::OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) {
    }
  }

-  // If the TRR was cancelled by nsHostResolver, then we don't need to report
-  // it as failed; otherwise it can cause the confirmation to fail.
-  if (UseDefaultServer() && aStatusCode != NS_ERROR_ABORT) {
-    // Bad content is still considered "okay" if the HTTP response is okay
-    gTRRService->TRRIsOkay(NS_SUCCEEDED(aStatusCode) ? TRRService::OKAY_NORMAL
-                                                     : TRRService::OKAY_BAD);
-  }
+  ReportStatus(aStatusCode);

  nsresult rv = NS_OK;
  // if status was "fine", parse the response and pass on the answer
--- a/netwerk/dns/TRR.h
+++ b/netwerk/dns/TRR.h
@ -110,6 +110,10 @@ class TRR : public Runnable,
  virtual DNSPacket* GetOrCreateDNSPacket();
  virtual nsresult CreateQueryURI(nsIURI** aOutURI);
  virtual const char* ContentType() const { return "application/dns-message"; }
+  virtual DNSResolverType ResolverType() const { return DNSResolverType::TRR; }
+  virtual bool MaybeBlockRequest();
+  virtual void RecordProcessingTime(nsIChannel* aChannel);
+  virtual void ReportStatus(nsresult aStatusCode);
  nsresult SendHTTPRequest();
  nsresult ReturnData(nsIChannel* aChannel);

--- a/netwerk/dns/TRRQuery.cpp
+++ b/netwerk/dns/TRRQuery.cpp
@ -170,16 +170,17 @@ AHostResolver::LookupStatus TRRQuery::CompleteLookup(
  }

  RefPtr<AddrInfo> newRRSet(aNewRRSet);
+  DNSResolverType resolverType = newRRSet->ResolverType();
  bool pendingARequest = false;
  bool pendingAAAARequest = false;
  {
    MutexAutoLock trrlock(mTrrLock);
-    if (newRRSet->IsTRR() == TRRTYPE_A) {
+    if (newRRSet->TRRType() == TRRTYPE_A) {
      MOZ_ASSERT(mTrrA);
      mTRRAFailReason = aReason;
      mTrrA = nullptr;
      mTrrAUsed = NS_SUCCEEDED(status) ? OK : FAILED;
-    } else if (newRRSet->IsTRR() == TRRTYPE_AAAA) {
+    } else if (newRRSet->TRRType() == TRRTYPE_AAAA) {
      MOZ_ASSERT(mTrrAAAA);
      mTRRAAAAFailReason = aReason;
      mTrrAAAA = nullptr;
@ -250,24 +251,26 @@ AHostResolver::LookupStatus TRRQuery::CompleteLookup(
    }
  }

-  if (mTrrAUsed == OK) {
-    AccumulateCategoricalKeyed(
-        TRRService::AutoDetectedKey(),
-        Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAOK);
-  } else if (mTrrAUsed == FAILED) {
-    AccumulateCategoricalKeyed(
-        TRRService::AutoDetectedKey(),
-        Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAFail);
-  }
+  if (resolverType == DNSResolverType::TRR) {
+    if (mTrrAUsed == OK) {
+      AccumulateCategoricalKeyed(
+          TRRService::AutoDetectedKey(),
+          Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAOK);
+    } else if (mTrrAUsed == FAILED) {
+      AccumulateCategoricalKeyed(
+          TRRService::AutoDetectedKey(),
+          Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAFail);
+    }

-  if (mTrrAAAAUsed == OK) {
-    AccumulateCategoricalKeyed(
-        TRRService::AutoDetectedKey(),
-        Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAAAAOK);
-  } else if (mTrrAAAAUsed == FAILED) {
-    AccumulateCategoricalKeyed(
-        TRRService::AutoDetectedKey(),
-        Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAAAAFail);
+    if (mTrrAAAAUsed == OK) {
+      AccumulateCategoricalKeyed(
+          TRRService::AutoDetectedKey(),
+          Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAAAAOK);
+    } else if (mTrrAAAAUsed == FAILED) {
+      AccumulateCategoricalKeyed(
+          TRRService::AutoDetectedKey(),
+          Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAAAAFail);
+    }
  }

  return mHostResolver->CompleteLookup(rec, status, newRRSet, pb, aOriginsuffix,
--- a/netwerk/dns/TRRService.cpp
+++ b/netwerk/dns/TRRService.cpp
@ -1054,7 +1054,7 @@ AHostResolver::LookupStatus TRRService::CompleteLookup(
  MOZ_ASSERT(!rec);

  RefPtr<AddrInfo> newRRSet(aNewRRSet);
-  MOZ_ASSERT(newRRSet && newRRSet->IsTRR() == TRRTYPE_NS);
+  MOZ_ASSERT(newRRSet && newRRSet->TRRType() == TRRTYPE_NS);

 #ifdef DEBUG
  {
--- a/netwerk/dns/nsDNSService2.cpp
+++ b/netwerk/dns/nsDNSService2.cpp
@ -130,7 +130,9 @@ NS_IMETHODIMP
 nsDNSRecord::IsTRR(bool* retval) {
  MutexAutoLock lock(mHostRecord->addr_info_lock);
  if (mHostRecord->addr_info) {
-    *retval = mHostRecord->addr_info->IsTRR();
+    // TODO: Let the consumers of nsIDNSRecord be unaware of the difference of
+    // TRR and ODoH. Will let them know the truth when needed.
+    *retval = mHostRecord->addr_info->IsTRROrODoH();
  } else {
    *retval = false;
  }
@ -140,7 +142,7 @@ nsDNSRecord::IsTRR(bool* retval) {
 NS_IMETHODIMP
 nsDNSRecord::GetTrrFetchDuration(double* aTime) {
  MutexAutoLock lock(mHostRecord->addr_info_lock);
-  if (mHostRecord->addr_info && mHostRecord->addr_info->IsTRR()) {
+  if (mHostRecord->addr_info && mHostRecord->addr_info->IsTRROrODoH()) {
    *aTime = mHostRecord->addr_info->GetTrrFetchDuration();
  } else {
    *aTime = 0;
@ -151,7 +153,7 @@ nsDNSRecord::GetTrrFetchDuration(double* aTime) {
 NS_IMETHODIMP
 nsDNSRecord::GetTrrFetchDurationNetworkOnly(double* aTime) {
  MutexAutoLock lock(mHostRecord->addr_info_lock);
-  if (mHostRecord->addr_info && mHostRecord->addr_info->IsTRR()) {
+  if (mHostRecord->addr_info && mHostRecord->addr_info->IsTRROrODoH()) {
    *aTime = mHostRecord->addr_info->GetTrrFetchDurationNetworkOnly();
  } else {
    *aTime = 0;
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@ -295,7 +295,7 @@ AddrHostRecord::AddrHostRecord(const nsHostKey& key)
      addr_info_gencnt(0),
      addr_info(nullptr),
      addr(nullptr),
-      mTRRUsed(false),
+      mResolverType(DNSResolverType::Native),
      mTRRSuccess(0),
      mNativeSuccess(0) {}

@ -381,7 +381,7 @@ bool AddrHostRecord::HasUsableResultInternal() const {
 bool AddrHostRecord::RemoveOrRefresh(bool aTrrToo) {
  // no need to flush TRRed names, they're not resolved "locally"
  MutexAutoLock lock(addr_info_lock);
-  if (addr_info && !aTrrToo && addr_info->IsTRR()) {
+  if (addr_info && !aTrrToo && addr_info->IsTRROrODoH()) {
    return false;
  }
  if (LoadNative()) {
@ -411,7 +411,14 @@ void AddrHostRecord::ResolveComplete() {
                       : Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::osFail);
  }

-  if (mTRRUsed) {
+  if (mResolverType == DNSResolverType::ODoH) {
+    // TODO:
+    // 1. Put ODoH related probes here.
+    // 2. Consider adding the failed host name into a blocklist.
+    return;
+  }
+
+  if (mResolverType == DNSResolverType::TRR) {
    if (mTRRSuccess) {
      uint32_t millis = static_cast<uint32_t>(mTrrDuration.ToMilliseconds());
      Telemetry::Accumulate(Telemetry::DNS_TRR_LOOKUP_TIME2,
@ -443,7 +450,7 @@ void AddrHostRecord::ResolveComplete() {
        AccumulateCategoricalKeyed(TRRService::AutoDetectedKey(),
                                   Telemetry::LABELS_DNS_TRR_FIRST3::TRR);
      } else if (mNativeSuccess) {
-        if (mTRRUsed) {
+        if (mResolverType == DNSResolverType::TRR) {
          AccumulateCategoricalKeyed(
              TRRService::AutoDetectedKey(),
              Telemetry::LABELS_DNS_TRR_FIRST3::NativeAfterTRR);
@ -474,7 +481,8 @@ void AddrHostRecord::ResolveComplete() {
      break;
  }

-  if (mTRRUsed && !mTRRSuccess && mNativeSuccess && gTRRService) {
+  if (mResolverType == DNSResolverType::TRR && !mTRRSuccess && mNativeSuccess &&
+      gTRRService) {
    gTRRService->AddToBlocklist(nsCString(host), originSuffix, pb, true);
  }
 }
@ -925,7 +933,8 @@ already_AddRefed<nsHostRecord> nsHostResolver::InitLoopbackRecord(
    addresses.AppendElement(NetAddr(&prAddr));
  }

-  RefPtr<AddrInfo> ai = new AddrInfo(rec->host, 0, std::move(addresses));
+  RefPtr<AddrInfo> ai =
+      new AddrInfo(rec->host, DNSResolverType::Native, 0, std::move(addresses));

  RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
  MutexAutoLock lock(addrRec->addr_info_lock);
@ -1154,7 +1163,8 @@ nsresult nsHostResolver::ResolveHost(const nsACString& aHost,
                addrRec->addr_info = new AddrInfo(
                    addrUnspecRec->addr_info->Hostname(),
                    addrUnspecRec->addr_info->CanonicalHostname(),
-                    addrUnspecRec->addr_info->IsTRR(), std::move(addresses));
+                    addrUnspecRec->addr_info->ResolverType(),
+                    addrUnspecRec->addr_info->TRRType(), std::move(addresses));
                addrRec->addr_info_gencnt++;
                rec->CopyExpirationTimesAndFlagsFrom(unspecRec);
              }
@ -1601,7 +1611,7 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec) {
    MOZ_ASSERT(addrRec);

    addrRec->StoreNativeUsed(false);
-    addrRec->mTRRUsed = false;
+    addrRec->mResolverType = DNSResolverType::Native;
    addrRec->mNativeSuccess = false;
  }

@ -1645,7 +1655,7 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec) {
    // Even if we did call TrrLookup above, the fact that it failed sync-ly
    // means that we didn't actually succeed in opening the channel.
    RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
-    MOZ_ASSERT(addrRec && !addrRec->mTRRUsed);
+    MOZ_ASSERT(addrRec && addrRec->mResolverType == DNSResolverType::Native);
 #endif

    rv = NativeLookup(rec);
@ -1772,7 +1782,7 @@ void nsHostResolver::PrepareRecordExpirationAddrRecord(
  unsigned int grace = mDefaultGracePeriod;

  unsigned int ttl = mDefaultCacheLifetime;
-  if (sGetTtlEnabled || rec->addr_info->IsTRR()) {
+  if (sGetTtlEnabled || rec->addr_info->IsTRROrODoH()) {
    if (rec->addr_info && rec->addr_info->TTL() != AddrInfo::NO_TTL_DATA) {
      ttl = rec->addr_info->TTL();
    }
@ -1792,7 +1802,11 @@ static bool different_rrset(AddrInfo* rrset1, AddrInfo* rrset2) {

  LOG(("different_rrset %s\n", rrset1->Hostname().get()));

-  if (rrset1->IsTRR() != rrset2->IsTRR()) {
+  if (rrset1->ResolverType() != rrset2->ResolverType()) {
+    return true;
+  }
+
+  if (rrset1->TRRType() != rrset2->TRRType()) {
    return true;
  }

@ -1878,12 +1892,15 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup(
  RefPtr<AddrInfo> newRRSet(aNewRRSet);
  MOZ_ASSERT(NS_FAILED(status) || newRRSet->Addresses().Length() > 0);

-  bool trrResult = newRRSet && newRRSet->IsTRR();
+  DNSResolverType type =
+      newRRSet ? newRRSet->ResolverType() : DNSResolverType::Native;
+
  if (NS_FAILED(status)) {
    newRRSet = nullptr;
  }

-  if (addrRec->LoadResolveAgain() && (status != NS_ERROR_ABORT) && !trrResult) {
+  if (addrRec->LoadResolveAgain() && (status != NS_ERROR_ABORT) &&
+      type == DNSResolverType::Native) {
    LOG(("nsHostResolver record %p resolve again due to flushcache\n",
         addrRec.get()));
    addrRec->StoreResolveAgain(false);
@ -1892,19 +1909,18 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup(

  MOZ_ASSERT(addrRec->mResolving);
  addrRec->mResolving--;
-  LOG(("nsHostResolver::CompleteLookup %s %p %X trr=%d stillResolving=%d\n",
-       addrRec->host.get(), aNewRRSet, (unsigned int)status,
-       aNewRRSet ? aNewRRSet->IsTRR() : 0, int(addrRec->mResolving)));
+  LOG((
+      "nsHostResolver::CompleteLookup %s %p %X resolver=%d stillResolving=%d\n",
+      addrRec->host.get(), aNewRRSet, (unsigned int)status, type,
+      int(addrRec->mResolving)));

-  if (trrResult) {
+  if (type != DNSResolverType::Native) {
    if (NS_FAILED(status) && status != NS_ERROR_UNKNOWN_HOST &&
        status != NS_ERROR_DEFINITIVE_UNKNOWN_HOST) {
      // the errors are not failed resolves, that means
      // something else failed, consider this as *TRR not used*
      // for actually trying to resolve the host
-      addrRec->mTRRUsed = false;
-    } else {
-      addrRec->mTRRUsed = true;
+      addrRec->mResolverType = DNSResolverType::Native;
    }

    if (NS_FAILED(status)) {
@ -2024,7 +2040,7 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookup(
  bool hasNativeResult = false;
  {
    MutexAutoLock lock(addrRec->addr_info_lock);
-    if (addrRec->addr_info && !addrRec->addr_info->IsTRR()) {
+    if (addrRec->addr_info && !addrRec->addr_info->IsTRROrODoH()) {
      hasNativeResult = true;
    }
  }
@ -2312,7 +2328,7 @@ void nsHostResolver::GetDNSCacheEntries(nsTArray<DNSCacheEntries>* args) {
          info.hostaddr.AppendElement(buf);
        }
      }
-      info.TRR = addrRec->addr_info->IsTRR();
+      info.TRR = addrRec->addr_info->IsTRROrODoH();
    }

    info.originAttributesSuffix = iter.Key().originSuffix;
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@ -288,9 +288,10 @@ class AddrHostRecord final : public nsHostRecord {
  mozilla::TimeDuration mTrrDuration;
  mozilla::TimeDuration mNativeDuration;

-  mozilla::Atomic<bool> mTRRUsed;  // TRR was used on this record
-  uint8_t mTRRSuccess;             // number of successful TRR responses
-  uint8_t mNativeSuccess;          // number of native lookup responses
+  // TRR or ODoH was used on this record
+  mozilla::Atomic<mozilla::net::DNSResolverType> mResolverType;
+  uint8_t mTRRSuccess;     // number of successful TRR responses
+  uint8_t mNativeSuccess;  // number of native lookup responses

  // clang-format off
  MOZ_ATOMIC_BITFIELDS(mAtomicBitfields, 8, (