From a587690d980298b2dff8967f5495262ef42a027d Mon Sep 17 00:00:00 2001 From: criss Date: Thu, 2 Dec 2021 11:57:08 +0200 Subject: [PATCH] Backed out 9 changesets (bug 1737198, bug 1743122) by dev request. CLOSED TREE Backed out changeset 8ea0830f0ebc (bug 1743122) Backed out changeset 5d68e2b664cc (bug 1737198) Backed out changeset c43e8d579121 (bug 1737198) Backed out changeset 7c257276a971 (bug 1737198) Backed out changeset 05e67f464ee1 (bug 1737198) Backed out changeset 04bae7f14cec (bug 1737198) Backed out changeset b703cf81d197 (bug 1737198) Backed out changeset 6f1e88c3daf3 (bug 1737198) Backed out changeset 960ecb376a56 (bug 1737198) --- modules/libpref/init/StaticPrefList.yaml | 20 +- netwerk/dns/ChildDNSService.cpp | 7 +- netwerk/dns/PTRRService.ipdl | 1 - netwerk/dns/TRR.cpp | 23 +- netwerk/dns/TRR.h | 6 +- netwerk/dns/TRRService.cpp | 101 ++--- netwerk/dns/TRRService.h | 6 +- netwerk/dns/TRRServiceParent.cpp | 6 - netwerk/dns/TRRServiceParent.h | 3 - netwerk/dns/nsHostRecord.cpp | 82 +--- netwerk/dns/nsHostRecord.h | 8 - netwerk/dns/nsHostResolver.cpp | 76 +--- netwerk/dns/nsHostResolver.h | 5 - netwerk/docs/dns/dns-over-https-trr.rst | 16 +- netwerk/protocol/http/TRRServiceChannel.cpp | 28 +- netwerk/protocol/http/nsHttpConnectionMgr.cpp | 29 -- netwerk/protocol/http/nsHttpConnectionMgr.h | 4 - netwerk/test/unit/head_trr.js | 10 - netwerk/test/unit/test_odoh.js | 2 - netwerk/test/unit/test_trr.js | 2 - netwerk/test/unit/trr_common.js | 413 ++++-------------- testing/xpcshell/moz-http2/moz-http2.js | 59 +-- toolkit/components/telemetry/Histograms.json | 49 +-- toolkit/components/telemetry/Scalars.yaml | 19 - .../telemetry/app/TelemetryEnvironment.jsm | 1 - 25 files changed, 206 insertions(+), 770 deletions(-) diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index e6472e6d160f..7ae8448262b7 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -9868,19 +9868,9 @@ value: "https://mozilla.cloudflare-dns.com/dns-query" mirror: never -# If true, don't fallback to native DNS upon network errors. - name: network.trr.strict_native_fallback type: RelaxedAtomicBool - value: @IS_NIGHTLY_BUILD@ - mirror: always - -# If false, the temporary blocklisting feature is disabled. -# This is useful for tests to prevent bleeding extra reqs -# between tasks, since we may attempt to look up the -# parent domain in the background when blocklisting a host. -- name: network.trr.temp_blocklist - type: RelaxedAtomicBool - value: true + value: false mirror: always # Single TRR request timeout, in milliseconds @@ -9931,6 +9921,14 @@ value: true mirror: always +# If this pref is false, a task will be dispatched to remove the file from the +# disk and the pref will be set to true. +# It can probably be removed after a few releases. +- name: network.trr.blocklist_cleanup_done + type: RelaxedAtomicBool + value: false + mirror: always + # If we should wait for captive portal confirmation before enabling TRR - name: network.trr.wait-for-portal type: RelaxedAtomicBool diff --git a/netwerk/dns/ChildDNSService.cpp b/netwerk/dns/ChildDNSService.cpp index 4ec34a66884e..aea8fc126625 100644 --- a/netwerk/dns/ChildDNSService.cpp +++ b/netwerk/dns/ChildDNSService.cpp @@ -322,12 +322,7 @@ ChildDNSService::GetCurrentTrrMode(nsIDNSService::ResolverMode* aMode) { NS_IMETHODIMP ChildDNSService::GetCurrentTrrConfirmationState(uint32_t* aConfirmationState) { - if (!mTRRServiceParent) { - return NS_ERROR_NOT_AVAILABLE; - } - - *aConfirmationState = mTRRServiceParent->GetConfirmationState(); - return NS_OK; + return NS_ERROR_NOT_IMPLEMENTED; } NS_IMETHODIMP diff --git a/netwerk/dns/PTRRService.ipdl b/netwerk/dns/PTRRService.ipdl index ad90ad39beb9..dd96755fbf20 100644 --- a/netwerk/dns/PTRRService.ipdl +++ b/netwerk/dns/PTRRService.ipdl @@ -19,7 +19,6 @@ namespace net { parent: async NotifyNetworkConnectivityServiceObservers(nsCString aTopic); async InitTRRConnectionInfo(); - async SetConfirmationState(uint32_t aNewState); child: async __delete__(); diff --git a/netwerk/dns/TRR.cpp b/netwerk/dns/TRR.cpp index 723df5081297..6be4f034b7a1 100644 --- a/netwerk/dns/TRR.cpp +++ b/netwerk/dns/TRR.cpp @@ -85,15 +85,14 @@ TRR::TRR(AHostResolver* aResolver, bool aPB) // to verify a domain TRR::TRR(AHostResolver* aResolver, nsACString& aHost, enum TrrType aType, - const nsACString& aOriginSuffix, bool aPB, bool aUseFreshConnection) + const nsACString& aOriginSuffix, bool aPB) : mozilla::Runnable("TRR"), mHost(aHost), mRec(nullptr), mHostResolver(aResolver), mType(aType), mPB(aPB), - mOriginSuffix(aOriginSuffix), - mUseFreshConnection(aUseFreshConnection) { + mOriginSuffix(aOriginSuffix) { MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess() || XRE_IsSocketProcess(), "TRR must be in parent or socket process"); } @@ -168,10 +167,8 @@ bool TRR::MaybeBlockRequest() { return true; } - if (!StaticPrefs::network_trr_strict_native_fallback() && - UseDefaultServer() && - TRRService::Get()->IsTemporarilyBlocked(mHost, mOriginSuffix, mPB, - true)) { + if (UseDefaultServer() && TRRService::Get()->IsTemporarilyBlocked( + mHost, mOriginSuffix, mPB, true)) { if (mType == TRRTYPE_A) { // count only blocklist for A records to avoid double counts Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED3, @@ -267,15 +264,9 @@ nsresult TRR::SendHTTPRequest() { return rv; } - auto loadFlags = nsIRequest::LOAD_ANONYMOUS | nsIRequest::INHIBIT_CACHING | - nsIRequest::LOAD_BYPASS_CACHE | - nsIChannel::LOAD_BYPASS_URL_CLASSIFIER; - if (mUseFreshConnection) { - // Causes TRRServiceChannel to tell the connection manager - // to clear out any connection with the current conn info. - loadFlags |= nsIRequest::LOAD_FRESH_CONNECTION; - } - channel->SetLoadFlags(loadFlags); + channel->SetLoadFlags( + nsIRequest::LOAD_ANONYMOUS | nsIRequest::INHIBIT_CACHING | + nsIRequest::LOAD_BYPASS_CACHE | nsIChannel::LOAD_BYPASS_URL_CLASSIFIER); NS_ENSURE_SUCCESS(rv, rv); rv = channel->SetNotificationCallbacks(this); diff --git a/netwerk/dns/TRR.h b/netwerk/dns/TRR.h index 09b2fc426e2f..e56d43d94f22 100644 --- a/netwerk/dns/TRR.h +++ b/netwerk/dns/TRR.h @@ -54,8 +54,7 @@ class TRR : public Runnable, explicit TRR(AHostResolver* aResolver, bool aPB); // to verify a domain explicit TRR(AHostResolver* aResolver, nsACString& aHost, enum TrrType aType, - const nsACString& aOriginSuffix, bool aPB, - bool aUseFreshConnection); + const nsACString& aOriginSuffix, bool aPB); NS_IMETHOD Run() override; void Cancel(nsresult aStatus); @@ -145,9 +144,6 @@ class TRR : public Runnable, // keep a copy of the originSuffix for the cases where mRec == nullptr */ const nsCString mOriginSuffix; - - // If true, we set LOAD_FRESH_CONNECTION on our channel's load flags. - bool mUseFreshConnection = false; }; } // namespace net diff --git a/netwerk/dns/TRRService.cpp b/netwerk/dns/TRRService.cpp index 8a4b7b137001..a206b44da730 100644 --- a/netwerk/dns/TRRService.cpp +++ b/netwerk/dns/TRRService.cpp @@ -3,6 +3,7 @@ * 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/. */ +#include "nsAppDirectoryServiceDefs.h" #include "nsCharSeparatedTokenizer.h" #include "nsComponentManagerUtils.h" #include "nsDirectoryServiceUtils.h" @@ -118,6 +119,32 @@ bool TRRService::CheckCaptivePortalIsPassed() { return result; } +static void RemoveTRRBlocklistFile() { + MOZ_ASSERT(NS_IsMainThread(), "Getting the profile dir on the main thread"); + + nsCOMPtr file; + nsresult rv = + NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(file)); + if (NS_FAILED(rv)) { + return; + } + + rv = file->AppendNative("TRRBlacklist.txt"_ns); + if (NS_FAILED(rv)) { + return; + } + + // Dispatch an async task that removes the blocklist file from the profile. + rv = NS_DispatchBackgroundTask( + NS_NewRunnableFunction("RemoveTRRBlocklistFile::Remove", + [file] { file->Remove(false); }), + NS_DISPATCH_EVENT_MAY_BLOCK); + if (NS_FAILED(rv)) { + return; + } + Preferences::SetBool("network.trr.blocklist_cleanup_done", true); +} + static void EventTelemetryPrefChanged(const char* aPref, void* aData) { Telemetry::SetEventRecordingEnabled( "network.dns"_ns, @@ -167,6 +194,15 @@ nsresult TRRService::Init() { } sTRRBackgroundThread = thread; + + if (!StaticPrefs::network_trr_blocklist_cleanup_done()) { + // Dispatch an idle task to the main thread that gets the profile dir + // then attempts to delete the blocklist file on a background thread. + Unused << NS_DispatchToMainThreadQueue( + NS_NewCancelableRunnableFunction("RemoveTRRBlocklistFile::GetDir", + [] { RemoveTRRBlocklistFile(); }), + EventQueuePriority::Idle); + } } mODoHService = new ODoHService(); @@ -663,16 +699,6 @@ void TRRService::RebuildSuffixList(nsTArray&& aSuffixList) { } } -void TRRService::ConfirmationContext::SetState( - enum ConfirmationState aNewState) { - mState = aNewState; - TRRServiceChild* child = TRRServiceChild::GetSingleton(); - if (child && child->CanSend()) { - LOG(("TRRService::SendSetConfirmationState")); - Unused << child->SendSetConfirmationState(mState); - } -} - void TRRService::ConfirmationContext::HandleEvent(ConfirmationEvent aEvent) { MutexAutoLock lock(OwningObject()->mLock); HandleEvent(aEvent, lock); @@ -696,26 +722,26 @@ void TRRService::ConfirmationContext::HandleEvent(ConfirmationEvent aEvent, if (TRR_DISABLED(mode)) { LOG(("TRR is disabled. mConfirmation.mState -> CONFIRM_OFF")); - SetState(CONFIRM_OFF); + mState = CONFIRM_OFF; return; } if (mode == nsIDNSService::MODE_TRRONLY) { LOG(("TRR_ONLY_MODE. mConfirmation.mState -> CONFIRM_DISABLED")); - SetState(CONFIRM_DISABLED); + mState = CONFIRM_DISABLED; return; } if (service->mConfirmationNS.Equals("skip"_ns)) { LOG(( "mConfirmationNS == skip. mConfirmation.mState -> CONFIRM_DISABLED")); - SetState(CONFIRM_DISABLED); + mState = CONFIRM_DISABLED; return; } // The next call to maybeConfirm will transition to CONFIRM_TRYING_OK LOG(("mConfirmation.mState -> CONFIRM_OK")); - SetState(CONFIRM_OK); + mState = CONFIRM_OK; }; auto maybeConfirm = [&](const char* aReason) { @@ -739,10 +765,10 @@ void TRRService::ConfirmationContext::HandleEvent(ConfirmationEvent aEvent, if (mState == CONFIRM_FAILED) { LOG(("mConfirmation.mState -> CONFIRM_TRYING_FAILED")); - SetState(CONFIRM_TRYING_FAILED); + mState = CONFIRM_TRYING_FAILED; } else { LOG(("mConfirmation.mState -> CONFIRM_TRYING_OK")); - SetState(CONFIRM_TRYING_OK); + mState = CONFIRM_TRYING_OK; } nsCOMPtr timer = std::move(mTimer); @@ -752,9 +778,8 @@ void TRRService::ConfirmationContext::HandleEvent(ConfirmationEvent aEvent, MOZ_ASSERT(mode == nsIDNSService::MODE_TRRFIRST, "Should only confirm in TRR first mode"); - // Set aUseFreshConnection if we are in strict fallback mode. - mTask = new TRR(service, service->mConfirmationNS, TRRTYPE_NS, ""_ns, false, - StaticPrefs::network_trr_strict_native_fallback()); + mTask = + new TRR(service, service->mConfirmationNS, TRRTYPE_NS, ""_ns, false); mTask->SetTimeout(StaticPrefs::network_trr_confirmation_timeout_ms()); mTask->SetPurpose(TRR::Confirmation); @@ -792,10 +817,6 @@ void TRRService::ConfirmationContext::HandleEvent(ConfirmationEvent aEvent, MOZ_ASSERT(mState == CONFIRM_OK); maybeConfirm("failed-lookups"); break; - case ConfirmationEvent::StrictMode: - MOZ_ASSERT(mState == CONFIRM_OK); - maybeConfirm("strict-mode"); - break; case ConfirmationEvent::URIChange: resetConfirmation(); maybeConfirm("uri-change"); @@ -817,13 +838,13 @@ void TRRService::ConfirmationContext::HandleEvent(ConfirmationEvent aEvent, } break; case ConfirmationEvent::ConfirmOK: - SetState(CONFIRM_OK); + mState = CONFIRM_OK; mTask = nullptr; break; case ConfirmationEvent::ConfirmFail: MOZ_ASSERT(mState == CONFIRM_TRYING_OK || mState == CONFIRM_TRYING_FAILED); - SetState(CONFIRM_FAILED); + mState = CONFIRM_FAILED; mTask = nullptr; // retry failed NS confirmation @@ -899,11 +920,6 @@ bool TRRService::IsTemporarilyBlocked(const nsACString& aHost, bool aPrivateBrowsing, bool aParentsToo) // false if domain { - if (!StaticPrefs::network_trr_temp_blocklist()) { - LOG(("TRRService::IsTemporarilyBlocked temp blocklist disabled by pref")); - return false; - } - if (mMode == nsIDNSService::MODE_TRRONLY) { return false; // might as well try } @@ -985,11 +1001,6 @@ bool TRRService::IsExcludedFromTRR_unlocked(const nsACString& aHost) { void TRRService::AddToBlocklist(const nsACString& aHost, const nsACString& aOriginSuffix, bool privateBrowsing, bool aParentsToo) { - if (!StaticPrefs::network_trr_temp_blocklist()) { - LOG(("TRRService::AddToBlocklist temp blocklist disabled by pref")); - return; - } - LOG(("TRR blocklist %s\n", nsCString(aHost).get())); nsAutoCString hashkey(aHost + aOriginSuffix); @@ -1019,8 +1030,8 @@ void TRRService::AddToBlocklist(const nsACString& aHost, LOG(("TRR: verify if '%s' resolves as NS\n", check.get())); // check if there's an NS entry for this name - RefPtr trr = new TRR(this, check, TRRTYPE_NS, aOriginSuffix, - privateBrowsing, false); + RefPtr trr = + new TRR(this, check, TRRTYPE_NS, aOriginSuffix, privateBrowsing); trr->SetPurpose(TRR::Blocklist); DispatchTRRRequest(trr); } @@ -1084,13 +1095,6 @@ static char StatusToChar(nsresult aLookupStatus, nsresult aChannelStatus) { return '?'; } -void TRRService::StrictModeConfirm() { - if (mConfirmation.State() == CONFIRM_OK) { - LOG(("TRRService::StrictModeConfirm triggering confirmation")); - mConfirmation.HandleEvent(ConfirmationEvent::StrictMode); - } -} - void TRRService::RecordTRRStatus(nsresult aChannelStatus) { MOZ_ASSERT_IF(XRE_IsParentProcess(), NS_IsMainThread() || IsOnTRRThread()); MOZ_ASSERT_IF(XRE_IsSocketProcess(), NS_IsMainThread()); @@ -1121,15 +1125,6 @@ void TRRService::ConfirmationContext::RecordTRRStatus(nsresult aChannelStatus) { return; } - // In strict mode, nsHostResolver will trigger Confirmation immediately - // upon a lookup failure, so nothing to be done here. nsHostResolver - // can assess the success of the lookup considering all the involved - // results (A, AAAA) so we let it tell us when to re-Confirm. - if (StaticPrefs::network_trr_strict_native_fallback()) { - LOG(("TRRService not counting failures in strict mode")); - return; - } - mFailureReasons[mTRRFailures % ConfirmationContext::RESULTS_SIZE] = StatusToChar(NS_OK, aChannelStatus); uint32_t fails = ++mTRRFailures; diff --git a/netwerk/dns/TRRService.h b/netwerk/dns/TRRService.h index dea8e302f31e..cf4b61153384 100644 --- a/netwerk/dns/TRRService.h +++ b/netwerk/dns/TRRService.h @@ -50,7 +50,6 @@ class TRRService : public TRRServiceBase, void GetURI(nsACString& result) override; nsresult GetCredentials(nsCString& result); uint32_t GetRequestTimeout(); - void StrictModeConfirm(); LookupStatus CompleteLookup(nsHostRecord*, nsresult, mozilla::net::AddrInfo*, bool pb, const nsACString& aOriginSuffix, @@ -148,7 +147,6 @@ class TRRService : public TRRServiceBase, PrefChange, Retry, FailedLookups, - StrictMode, URIChange, CaptivePortalConnectivity, NetworkUp, @@ -156,7 +154,7 @@ class TRRService : public TRRServiceBase, ConfirmFail, }; - // (FailedLookups/StrictMode/URIChange/NetworkUp) + // (FailedLookups/URIChange/NetworkUp) // +-------------------------+ // +-----------+ | | // | (Init) | +------v---------+ +-+--+ @@ -235,8 +233,6 @@ class TRRService : public TRRServiceBase, // confirmation. nsCString mFailedLookups; - void SetState(enum ConfirmationState aNewState); - public: // Called when a confirmation completes successfully or when the // confirmation context changes. diff --git a/netwerk/dns/TRRServiceParent.cpp b/netwerk/dns/TRRServiceParent.cpp index b753ff2f32ae..928f14c59cb2 100644 --- a/netwerk/dns/TRRServiceParent.cpp +++ b/netwerk/dns/TRRServiceParent.cpp @@ -202,11 +202,5 @@ mozilla::ipc::IPCResult TRRServiceParent::RecvInitTRRConnectionInfo() { return IPC_OK(); } -mozilla::ipc::IPCResult TRRServiceParent::RecvSetConfirmationState( - uint32_t aNewState) { - mConfirmationState = aNewState; - return IPC_OK(); -} - } // namespace net } // namespace mozilla diff --git a/netwerk/dns/TRRServiceParent.h b/netwerk/dns/TRRServiceParent.h index fd610e6ea01e..32fffe6cbbde 100644 --- a/netwerk/dns/TRRServiceParent.h +++ b/netwerk/dns/TRRServiceParent.h @@ -34,15 +34,12 @@ class TRRServiceParent : public TRRServiceBase, mozilla::ipc::IPCResult RecvNotifyNetworkConnectivityServiceObservers( const nsCString& aTopic); mozilla::ipc::IPCResult RecvInitTRRConnectionInfo(); - mozilla::ipc::IPCResult RecvSetConfirmationState(uint32_t aNewState); - uint32_t GetConfirmationState() { return mConfirmationState; } private: virtual ~TRRServiceParent(); virtual void ActorDestroy(ActorDestroyReason why) override; void prefsChanged(const char* aName); void SetDefaultTRRConnectionInfo(nsHttpConnectionInfo* aConnInfo) override; - uint32_t mConfirmationState = 0; }; } // namespace net diff --git a/netwerk/dns/nsHostRecord.cpp b/netwerk/dns/nsHostRecord.cpp index 673b60999460..229c6728cd43 100644 --- a/netwerk/dns/nsHostRecord.cpp +++ b/netwerk/dns/nsHostRecord.cpp @@ -7,8 +7,6 @@ #include "TRRQuery.h" // Put DNSLogging.h at the end to avoid LOG being overwritten by other headers. #include "DNSLogging.h" -#include "TRRSkippedReason.h" -#include "mozilla/StaticPrefs_network.h" #include "mozilla/Telemetry.h" #include "TRRService.h" @@ -263,15 +261,6 @@ bool AddrHostRecord::RemoveOrRefresh(bool aTrrToo) { return true; } -void AddrHostRecord::NotifyRetryingTrr() { - MOZ_ASSERT(mFirstTRRSkippedReason == - mozilla::net::TRRSkippedReason::TRR_UNSET); - - // Save the skip reason of our first attempt for recording telemetry later. - mFirstTRRSkippedReason = mTRRSkippedReason; - mTRRSkippedReason = mozilla::net::TRRSkippedReason::TRR_UNSET; -} - void AddrHostRecord::ResolveComplete() { if (LoadNativeUsed()) { if (mNativeSuccess) { @@ -313,60 +302,28 @@ void AddrHostRecord::ResolveComplete() { if (nsHostResolver::Mode() == nsIDNSService::MODE_TRRFIRST) { MOZ_ASSERT(mTRRSkippedReason != mozilla::net::TRRSkippedReason::TRR_UNSET); + Telemetry::Accumulate(Telemetry::TRR_SKIP_REASON_TRR_FIRST2, + TRRService::ProviderKey(), + static_cast(mTRRSkippedReason)); - if (StaticPrefs::network_trr_strict_native_fallback()) { - nsAutoCString telemetryKey(TRRService::ProviderKey()); + if (!mTRRSuccess) { + Telemetry::Accumulate( + mNativeSuccess ? Telemetry::TRR_SKIP_REASON_NATIVE_SUCCESS + : Telemetry::TRR_SKIP_REASON_NATIVE_FAILED, + TRRService::ProviderKey(), static_cast(mTRRSkippedReason)); + } - if (mFirstTRRSkippedReason != mozilla::net::TRRSkippedReason::TRR_UNSET) { - telemetryKey.AppendLiteral("|"); - telemetryKey.AppendInt(static_cast(mFirstTRRSkippedReason)); - - Telemetry::Accumulate(mTRRSuccess - ? Telemetry::TRR_SKIP_REASON_RETRY_SUCCESS - : Telemetry::TRR_SKIP_REASON_RETRY_FAILED, - TRRService::ProviderKey(), - static_cast(mFirstTRRSkippedReason)); - } - - Telemetry::Accumulate(Telemetry::TRR_SKIP_REASON_STRICT_MODE, - telemetryKey, - static_cast(mTRRSkippedReason)); - - if (mTRRSuccess) { - Telemetry::Accumulate(Telemetry::TRR_ATTEMPT_COUNT, - TRRService::ProviderKey(), mTrrAttempts); - } else if (LoadNativeUsed()) { - Telemetry::Accumulate(mNativeSuccess - ? Telemetry::TRR_SKIP_REASON_NATIVE_SUCCESS - : Telemetry::TRR_SKIP_REASON_NATIVE_FAILED, - TRRService::ProviderKey(), - static_cast(mTRRSkippedReason)); - } - } else { - Telemetry::Accumulate(Telemetry::TRR_SKIP_REASON_TRR_FIRST2, + if (IsRelevantTRRSkipReason(mTRRSkippedReason)) { + Telemetry::Accumulate(Telemetry::TRR_RELEVANT_SKIP_REASON_TRR_FIRST, TRRService::ProviderKey(), static_cast(mTRRSkippedReason)); + if (!mTRRSuccess) { - Telemetry::Accumulate(mNativeSuccess - ? Telemetry::TRR_SKIP_REASON_NATIVE_SUCCESS - : Telemetry::TRR_SKIP_REASON_NATIVE_FAILED, - TRRService::ProviderKey(), - static_cast(mTRRSkippedReason)); - } - - if (IsRelevantTRRSkipReason(mTRRSkippedReason)) { - Telemetry::Accumulate(Telemetry::TRR_RELEVANT_SKIP_REASON_TRR_FIRST, - TRRService::ProviderKey(), - static_cast(mTRRSkippedReason)); - - if (!mTRRSuccess && LoadNativeUsed()) { - Telemetry::Accumulate( - mNativeSuccess - ? Telemetry::TRR_RELEVANT_SKIP_REASON_NATIVE_SUCCESS - : Telemetry::TRR_RELEVANT_SKIP_REASON_NATIVE_FAILED, - TRRService::ProviderKey(), - static_cast(mTRRSkippedReason)); - } + Telemetry::Accumulate( + mNativeSuccess ? Telemetry::TRR_RELEVANT_SKIP_REASON_NATIVE_SUCCESS + : Telemetry::TRR_RELEVANT_SKIP_REASON_NATIVE_FAILED, + TRRService::ProviderKey(), + static_cast(mTRRSkippedReason)); } } } @@ -412,9 +369,8 @@ void AddrHostRecord::ResolveComplete() { break; } - if (mResolverType == DNSResolverType::TRR && - !StaticPrefs::network_trr_strict_native_fallback() && !mTRRSuccess && - mNativeSuccess && TRRService::Get()) { + if (mResolverType == DNSResolverType::TRR && !mTRRSuccess && mNativeSuccess && + TRRService::Get()) { TRRService::Get()->AddToBlocklist(nsCString(host), originSuffix, pb, true); } } diff --git a/netwerk/dns/nsHostRecord.h b/netwerk/dns/nsHostRecord.h index bd4d7cf3bec0..92e83396bd4c 100644 --- a/netwerk/dns/nsHostRecord.h +++ b/netwerk/dns/nsHostRecord.h @@ -186,7 +186,6 @@ class nsHostRecord : public mozilla::LinkedListElement>, nsIRequest::TRRMode mEffectiveTRRMode = nsIRequest::TRR_DEFAULT_MODE; TRRSkippedReason mTRRSkippedReason = TRRSkippedReason::TRR_UNSET; - TRRSkippedReason mFirstTRRSkippedReason = TRRSkippedReason::TRR_UNSET; TRRSkippedReason mTRRAFailReason = TRRSkippedReason::TRR_UNSET; TRRSkippedReason mTRRAAAAFailReason = TRRSkippedReason::TRR_UNSET; @@ -195,10 +194,6 @@ class nsHostRecord : public mozilla::LinkedListElement>, // counter of outstanding resolving calls mozilla::Atomic mResolving{0}; - // Number of times we've attempted TRR. Reset when we refresh. - // TRR is attempted at most twice - first attempt and retry. - mozilla::Atomic mTrrAttempts{0}; - // True if this record is a cache of a failed lookup. Negative cache // entries are valid just like any other (though never for more than 60 // seconds), but a use of that negative entry forces an asynchronous refresh. @@ -272,9 +267,6 @@ class AddrHostRecord final : public nsHostRecord { bool RemoveOrRefresh(bool aTrrToo); // Mark records currently being resolved // as needed to resolve again. - // Saves the skip reason of a first-attempt TRR lookup and clears - // it to prepare for a retry attempt. - void NotifyRetryingTrr(); void ResolveComplete(); static DnsPriority GetPriority(uint16_t aFlags); diff --git a/netwerk/dns/nsHostResolver.cpp b/netwerk/dns/nsHostResolver.cpp index 75da84cbcd32..f0bf79e1f216 100644 --- a/netwerk/dns/nsHostResolver.cpp +++ b/netwerk/dns/nsHostResolver.cpp @@ -3,8 +3,6 @@ * 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/. */ -#include "TRRSkippedReason.h" -#include "nsHostRecord.h" #if defined(HAVE_RES_NINIT) # include # include @@ -941,7 +939,6 @@ nsresult nsHostResolver::TrrLookup(nsHostRecord* aRec, } rec->mResolving++; - rec->mTrrAttempts++; return NS_OK; } @@ -1082,10 +1079,8 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec, } // Make sure we reset the reason each time we attempt to do a new lookup - // so we don't wrongly report the reason for the previous one. + // so we don't wronly report the reason for the previous one. rec->mTRRSkippedReason = TRRSkippedReason::TRR_UNSET; - rec->mFirstTRRSkippedReason = TRRSkippedReason::TRR_UNSET; - rec->mTrrAttempts = 0; ComputeEffectiveTRRMode(rec); @@ -1302,63 +1297,6 @@ void nsHostResolver::AddToEvictionQ(nsHostRecord* rec, mQueue.AddToEvictionQ(rec, mMaxCacheEntries, mRecordDB, aLock); } -// After a first lookup attempt with TRR in mode 2, we may: -// - If we are not in strict mode, retry with native. -// - If we are in strict mode: -// - Retry with native if the first attempt failed because we got NXDOMAIN, an -// unreachable address (TRR_DISABLED_FLAG), or we skipped TRR because -// Confirmation failed. -// - Trigger a "strict mode" Confirmation which will start a fresh -// connection for TRR, and then retry the lookup with TRR. -// Returns true if we retried with either TRR or Native. -bool nsHostResolver::MaybeRetryTRRLookup( - AddrHostRecord* aAddrRec, nsresult aFirstAttemptStatus, - TRRSkippedReason aFirstAttemptSkipReason, const MutexAutoLock& aLock) { - if (NS_SUCCEEDED(aFirstAttemptStatus) || - aAddrRec->mEffectiveTRRMode != nsIRequest::TRR_FIRST_MODE || - aFirstAttemptStatus == NS_ERROR_DEFINITIVE_UNKNOWN_HOST) { - return false; - } - - MOZ_ASSERT(!aAddrRec->mResolving); - if (!StaticPrefs::network_trr_strict_native_fallback()) { - LOG(("nsHostResolver::MaybeRetryTRRLookup retrying with native")); - NativeLookup(aAddrRec, aLock); - return true; - } - - if (aFirstAttemptSkipReason == TRRSkippedReason::TRR_NXDOMAIN || - aFirstAttemptSkipReason == TRRSkippedReason::TRR_DISABLED_FLAG || - aFirstAttemptSkipReason == TRRSkippedReason::TRR_NOT_CONFIRMED) { - LOG( - ("nsHostResolver::MaybeRetryTRRLookup retrying with native in strict " - "mode, skip reason was %d", - static_cast(aFirstAttemptSkipReason))); - NativeLookup(aAddrRec, aLock); - return true; - } - - if (aAddrRec->mTrrAttempts > 1) { - LOG(("nsHostResolver::MaybeRetryTRRLookup mTrrAttempts>1, not retrying.")); - return false; - } - - LOG( - ("nsHostResolver::MaybeRetryTRRLookup triggering Confirmation and " - "retrying with TRR, skip reason was %d", - static_cast(aFirstAttemptSkipReason))); - TRRService::Get()->StrictModeConfirm(); - - { - // Clear out the old query - auto trrQuery = aAddrRec->mTRRQuery.Lock(); - trrQuery.ref() = nullptr; - } - aAddrRec->NotifyRetryingTrr(); - TrrLookup(aAddrRec, aLock, nullptr /* pushedTRR */); - return true; -} - // // CompleteLookup() checks if the resolving should be redone and if so it // returns LOOKUP_RESOLVEAGAIN, but only if 'status' is not NS_ERROR_ABORT. @@ -1428,7 +1366,17 @@ nsHostResolver::LookupStatus nsHostResolver::CompleteLookupLocked( addrRec->RecordReason(TRRSkippedReason::TRR_OK); } - if (MaybeRetryTRRLookup(addrRec, status, aReason, aLock)) { + bool shouldAttemptNative = + !StaticPrefs::network_trr_strict_native_fallback() || + aReason == TRRSkippedReason::TRR_NXDOMAIN || + aReason == TRRSkippedReason::TRR_DISABLED_FLAG || + aReason == TRRSkippedReason::TRR_NOT_CONFIRMED; + + if (NS_FAILED(status) && + addrRec->mEffectiveTRRMode == nsIRequest::TRR_FIRST_MODE && + status != NS_ERROR_DEFINITIVE_UNKNOWN_HOST && shouldAttemptNative) { + MOZ_ASSERT(!addrRec->mResolving); + NativeLookup(addrRec, aLock); MOZ_ASSERT(addrRec->mResolving); return LOOKUP_OK; } diff --git a/netwerk/dns/nsHostResolver.h b/netwerk/dns/nsHostResolver.h index ac59ab20b9ed..0af6a2e7a3be 100644 --- a/netwerk/dns/nsHostResolver.h +++ b/netwerk/dns/nsHostResolver.h @@ -217,11 +217,6 @@ class nsHostResolver : public nsISupports, public AHostResolver { uint32_t defaultGracePeriod); virtual ~nsHostResolver(); - bool MaybeRetryTRRLookup( - AddrHostRecord* aAddrRec, nsresult aFirstAttemptStatus, - mozilla::net::TRRSkippedReason aFirstAttemptSkipReason, - const mozilla::MutexAutoLock& aLock); - LookupStatus CompleteLookupLocked(nsHostRecord*, nsresult, mozilla::net::AddrInfo*, bool pb, const nsACString& aOriginsuffix, diff --git a/netwerk/docs/dns/dns-over-https-trr.rst b/netwerk/docs/dns/dns-over-https-trr.rst index eaa6e68b2810..bb55b49d8a50 100644 --- a/netwerk/docs/dns/dns-over-https-trr.rst +++ b/netwerk/docs/dns/dns-over-https-trr.rst @@ -18,29 +18,17 @@ that normally listens on port 53. DoH Rollout ----------- -**DoH Rollout** refers to the frontend code that decides whether TRR will +**DoH Rollout** refers to the webextension code that decides whether TRR will be enabled automatically for users in the `rollout population `_. The functioning of this module is described `here `_. -The code lives in `browser/components/doh `_. - Implementation -------------- When enabled TRR may work in two modes, TRR-first (2) and TRR-only (3). These are controlled by the **network.trr.mode** or **doh-rollout.mode** prefs. The difference is that when a DoH request fails in TRR-first mode, we then fallback to **Do53**. -For TRR-first mode, we have a strict-fallback setting which can be enabled by setting network.trr.strict_native_fallback to true. -With this, while we will still completely skip TRR for certain requests (like captive portal detection, bootstrapping the TRR provider, etc.) -we will only fall back after a TRR failure to **Do53** for three possible reasons: -1. We detected, via Confirmation, that TRR is currently out of service on the network. This could mean the provider is down or blocked. -2. The address successfully resolved via TRR could not be connected to. -3. TRR result is NXDOMAIN. - -In other cases, instead of falling back, we will trigger a fresh Confirmation (which will start us on a fresh connection to the provider) and -retry the lookup with TRR again. We only retry once. - DNS name resolutions are performed in nsHostResolver::ResolveHost. If a cached response for the request could not be found, nsHostResolver::NameLookup will trigger either 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. @@ -84,8 +72,6 @@ The confirmation state has one of the following values: The state machine for the confirmation is defined in the `HandleConfirmationEvent` method in `TRRService.cpp`. -If strict fallback mode is enabled, Confirmation will set a flag to refresh our connection to the provider. - Excluded domains ---------------- diff --git a/netwerk/protocol/http/TRRServiceChannel.cpp b/netwerk/protocol/http/TRRServiceChannel.cpp index e6791d2eb751..cde57bb686c2 100644 --- a/netwerk/protocol/http/TRRServiceChannel.cpp +++ b/netwerk/protocol/http/TRRServiceChannel.cpp @@ -26,7 +26,6 @@ #include "TRRLoadInfo.h" #include "ReferrerInfo.h" #include "TRR.h" -#include "TRRService.h" namespace mozilla { namespace net { @@ -455,6 +454,22 @@ nsresult TRRServiceChannel::BeginConnect() { } } + // Force-Reload should reset the persistent connection pool for this host + if (mLoadFlags & LOAD_FRESH_CONNECTION) { + // just the initial document resets the whole pool + if (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI) { + gHttpHandler->AltServiceCache()->ClearAltServiceMappings(); + rv = gHttpHandler->ConnMgr()->DoShiftReloadConnectionCleanupWithConnInfo( + mConnectionInfo); + if (NS_FAILED(rv)) { + LOG(( + "TRRServiceChannel::BeginConnect " + "DoShiftReloadConnectionCleanupWithConnInfo failed: %08x [this=%p]", + static_cast(rv), this)); + } + } + } + if (mCanceled) { return mStatus; } @@ -496,17 +511,6 @@ nsresult TRRServiceChannel::ContinueOnBeforeConnect() { mConnectionInfo->SetIPv4Disabled(mCaps & NS_HTTP_DISABLE_IPV4); mConnectionInfo->SetIPv6Disabled(mCaps & NS_HTTP_DISABLE_IPV6); - if (mLoadFlags & LOAD_FRESH_CONNECTION) { - Telemetry::ScalarAdd( - Telemetry::ScalarID::NETWORKING_TRR_CONNECTION_CYCLE_COUNT, 1); - nsresult rv = - gHttpHandler->ConnMgr()->DoSingleConnectionCleanup(mConnectionInfo); - LOG( - ("TRRServiceChannel::BeginConnect " - "DoSingleConnectionCleanup succeeded=%d %08x [this=%p]", - NS_SUCCEEDED(rv), static_cast(rv), this)); - } - return Connect(); } diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp index 9421ad4686b5..0acf649339e5 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp +++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ -399,16 +399,6 @@ nsresult nsHttpConnectionMgr::DoShiftReloadConnectionCleanupWithConnInfo( ci); } -nsresult nsHttpConnectionMgr::DoSingleConnectionCleanup( - nsHttpConnectionInfo* aCI) { - if (!aCI) { - return NS_ERROR_INVALID_ARG; - } - - RefPtr ci = aCI->Clone(); - return PostEvent(&nsHttpConnectionMgr::OnMsgDoSingleConnectionCleanup, 0, ci); -} - class SpeculativeConnectArgs : public ARefBase { public: SpeculativeConnectArgs() = default; @@ -2272,25 +2262,6 @@ void nsHttpConnectionMgr::OnMsgDoShiftReloadConnectionCleanup(int32_t, if (ci) ResetIPFamilyPreference(ci); } -void nsHttpConnectionMgr::OnMsgDoSingleConnectionCleanup(int32_t, - ARefBase* param) { - LOG(("nsHttpConnectionMgr::OnMsgDoSingleConnectionCleanup\n")); - MOZ_ASSERT(OnSocketThread(), "not on socket thread"); - - nsHttpConnectionInfo* ci = static_cast(param); - - if (!ci) { - return; - } - - ConnectionEntry* entry = mCT.GetWeak(ci->HashKey()); - if (entry) { - entry->ClosePersistentConnections(); - } - - ResetIPFamilyPreference(ci); -} - void nsHttpConnectionMgr::OnMsgReclaimConnection(HttpConnectionBase* conn) { MOZ_ASSERT(OnSocketThread(), "not on socket thread"); diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.h b/netwerk/protocol/http/nsHttpConnectionMgr.h index e9580661fa5a..80717fb043fa 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.h +++ b/netwerk/protocol/http/nsHttpConnectionMgr.h @@ -85,9 +85,6 @@ class nsHttpConnectionMgr final : public HttpConnectionMgrShell, [[nodiscard]] nsresult CloseIdleConnection(nsHttpConnection*); [[nodiscard]] nsresult RemoveIdleConnection(nsHttpConnection*); - // Close a single connection and prevent it from being reused. - [[nodiscard]] nsresult DoSingleConnectionCleanup(nsHttpConnectionInfo*); - // The connection manager needs to know when a normal HTTP connection has been // upgraded to SPDY because the dispatch and idle semantics are a little // bit different. @@ -320,7 +317,6 @@ class nsHttpConnectionMgr final : public HttpConnectionMgrShell, void OnMsgCompleteUpgrade(int32_t, ARefBase*); void OnMsgUpdateParam(int32_t, ARefBase*); void OnMsgDoShiftReloadConnectionCleanup(int32_t, ARefBase*); - void OnMsgDoSingleConnectionCleanup(int32_t, ARefBase*); void OnMsgProcessFeedback(int32_t, ARefBase*); void OnMsgProcessAllSpdyPendingQ(int32_t, ARefBase*); void OnMsgUpdateRequestTokenBucket(int32_t, ARefBase*); diff --git a/netwerk/test/unit/head_trr.js b/netwerk/test/unit/head_trr.js index b1a652c76b3c..17f90e7399bc 100644 --- a/netwerk/test/unit/head_trr.js +++ b/netwerk/test/unit/head_trr.js @@ -57,14 +57,6 @@ function trr_test_setup() { ); addCertFromFile(certdb, "http2-ca.pem", "CTu,u,u"); - // Turn off strict fallback mode for most tests, it is tested specifically. - Services.prefs.setBoolPref("network.trr.strict_native_fallback", false); - - // Turn off temp blocklist feature in tests. When enabled we may issue a - // lookup to resolve a parent name when blocklisting, which may bleed into - // and interfere with subsequent tasks. - Services.prefs.setBoolPref("network.trr.temp_blocklist", false); - // We intentionally don't set the TRR mode. Each test should set it // after setup in the first test. @@ -99,8 +91,6 @@ function trr_clear_prefs() { Services.prefs.clearUserPref( "network.trr.send_empty_accept-encoding_headers" ); - Services.prefs.clearUserPref("network.trr.strict_native_fallback"); - Services.prefs.clearUserPref("network.trr.temp_blocklist"); } /// This class sends a DNS query and can be awaited as a promise to get the diff --git a/netwerk/test/unit/test_odoh.js b/netwerk/test/unit/test_odoh.js index 388fdb41a94c..a454dc5646ee 100644 --- a/netwerk/test/unit/test_odoh.js +++ b/netwerk/test/unit/test_odoh.js @@ -261,5 +261,3 @@ add_task(test_ipv6_trr_fallback); add_task(test_ipv4_trr_fallback); add_task(test_no_retry_without_doh); - -add_task(test_connection_reuse_and_cycling).skip(); // Bug 1742743 diff --git a/netwerk/test/unit/test_trr.js b/netwerk/test/unit/test_trr.js index cd25a6102d9f..b00f2acbfc78 100644 --- a/netwerk/test/unit/test_trr.js +++ b/netwerk/test/unit/test_trr.js @@ -893,5 +893,3 @@ add_task(async function test_padding() { "1.1.0.160" ); }); - -add_task(test_connection_reuse_and_cycling); diff --git a/netwerk/test/unit/trr_common.js b/netwerk/test/unit/trr_common.js index b65576e957a2..7e8b3a0400ba 100644 --- a/netwerk/test/unit/trr_common.js +++ b/netwerk/test/unit/trr_common.js @@ -348,7 +348,7 @@ async function test_strict_native_fallback() { setModeAndURI(2, "doh?responseIP=2.2.2.2&corruptedAnswer=true"); await new TRRDNSListener("bar.example.com", "127.0.0.1"); // Should fallback - Services.prefs.setBoolPref("network.trr.strict_native_fallback", false); + Services.prefs.clearUserPref("network.trr.strict_native_fallback"); Services.prefs.clearUserPref("network.trr.request_timeout_ms"); Services.prefs.clearUserPref("network.trr.request_timeout_mode_trronly_ms"); } @@ -359,18 +359,6 @@ async function test_no_answers_fallback() { setModeAndURI(2, "doh?responseIP=none"); // TRR-first await new TRRDNSListener("confirm.example.com", "127.0.0.1"); - - info("Now in strict mode - no fallback"); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", true); - dns.clearCache(true); - let { inStatus } = await new TRRDNSListener("confirm.example.com", { - expectedSuccess: false, - }); - Assert.ok( - !Components.isSuccessCode(inStatus), - `${inStatus} should be an error code` - ); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", false); } async function test_404_fallback() { @@ -379,18 +367,6 @@ async function test_404_fallback() { setModeAndURI(2, "404"); // TRR-first await new TRRDNSListener("test404.example.com", "127.0.0.1"); - - info("Now in strict mode - no fallback"); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", true); - dns.clearCache(true); - let { inStatus } = await new TRRDNSListener("test404.example.com", { - expectedSuccess: false, - }); - Assert.ok( - !Components.isSuccessCode(inStatus), - `${inStatus} should be an error code` - ); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", false); } async function test_mode_1_and_4() { @@ -483,53 +459,37 @@ async function test_mode_2() { Services.prefs.setCharPref("network.trr.builtin-excluded-domains", ""); await new TRRDNSListener("bar.example.com", "192.192.192.192"); - - info("Now in strict mode"); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", true); - dns.clearCache(true); - await new TRRDNSListener("bar.example.com", "192.192.192.192"); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", false); } async function test_excluded_domains() { info("Checking that Do53 is used for names in excluded-domains list"); - for (let strictMode of [true, false]) { - info("Strict mode: " + strictMode); - Services.prefs.setBoolPref( - "network.trr.strict_native_fallback", - strictMode - ); - dns.clearCache(true); - setModeAndURI(2, "doh?responseIP=192.192.192.192"); - Services.prefs.setCharPref( - "network.trr.excluded-domains", - "bar.example.com" - ); + dns.clearCache(true); + setModeAndURI(2, "doh?responseIP=192.192.192.192"); + Services.prefs.setCharPref("network.trr.excluded-domains", "bar.example.com"); - await new TRRDNSListener("bar.example.com", "127.0.0.1"); // Do53 result + await new TRRDNSListener("bar.example.com", "127.0.0.1"); // Do53 result - dns.clearCache(true); - Services.prefs.setCharPref("network.trr.excluded-domains", "example.com"); + dns.clearCache(true); + Services.prefs.setCharPref("network.trr.excluded-domains", "example.com"); - await new TRRDNSListener("bar.example.com", "127.0.0.1"); + await new TRRDNSListener("bar.example.com", "127.0.0.1"); - dns.clearCache(true); - Services.prefs.setCharPref( - "network.trr.excluded-domains", - "foo.test.com, bar.example.com" - ); - await new TRRDNSListener("bar.example.com", "127.0.0.1"); + dns.clearCache(true); + Services.prefs.setCharPref( + "network.trr.excluded-domains", + "foo.test.com, bar.example.com" + ); + await new TRRDNSListener("bar.example.com", "127.0.0.1"); - dns.clearCache(true); - Services.prefs.setCharPref( - "network.trr.excluded-domains", - "bar.example.com, foo.test.com" - ); + dns.clearCache(true); + Services.prefs.setCharPref( + "network.trr.excluded-domains", + "bar.example.com, foo.test.com" + ); - await new TRRDNSListener("bar.example.com", "127.0.0.1"); + await new TRRDNSListener("bar.example.com", "127.0.0.1"); - Services.prefs.clearUserPref("network.trr.excluded-domains"); - } + Services.prefs.clearUserPref("network.trr.excluded-domains"); } function topicObserved(topic) { @@ -549,51 +509,44 @@ function topicObserved(topic) { async function test_captiveportal_canonicalURL() { info("Check that captivedetect.canonicalURL is resolved via native DNS"); - for (let strictMode of [true, false]) { - info("Strict mode: " + strictMode); - Services.prefs.setBoolPref( - "network.trr.strict_native_fallback", - strictMode - ); - dns.clearCache(true); - setModeAndURI(2, "doh?responseIP=2.2.2.2"); + dns.clearCache(true); + setModeAndURI(2, "doh?responseIP=2.2.2.2"); - const cpServer = new HttpServer(); - cpServer.registerPathHandler("/cp", function handleRawData( - request, - response - ) { - response.setHeader("Content-Type", "text/plain", false); - response.setHeader("Cache-Control", "no-cache", false); - response.bodyOutputStream.write("data", 4); - }); - cpServer.start(-1); - cpServer.identity.setPrimary( - "http", - "detectportal.firefox.com", - cpServer.identity.primaryPort - ); - let cpPromise = topicObserved("captive-portal-login"); + const cpServer = new HttpServer(); + cpServer.registerPathHandler("/cp", function handleRawData( + request, + response + ) { + response.setHeader("Content-Type", "text/plain", false); + response.setHeader("Cache-Control", "no-cache", false); + response.bodyOutputStream.write("data", 4); + }); + cpServer.start(-1); + cpServer.identity.setPrimary( + "http", + "detectportal.firefox.com", + cpServer.identity.primaryPort + ); + let cpPromise = topicObserved("captive-portal-login"); - Services.prefs.setCharPref( - "captivedetect.canonicalURL", - `http://detectportal.firefox.com:${cpServer.identity.primaryPort}/cp` - ); - Services.prefs.setBoolPref("network.captive-portal-service.testMode", true); - Services.prefs.setBoolPref("network.captive-portal-service.enabled", true); + Services.prefs.setCharPref( + "captivedetect.canonicalURL", + `http://detectportal.firefox.com:${cpServer.identity.primaryPort}/cp` + ); + Services.prefs.setBoolPref("network.captive-portal-service.testMode", true); + Services.prefs.setBoolPref("network.captive-portal-service.enabled", true); - // The captive portal has to have used native DNS, otherwise creating - // a socket to a non-local IP would trigger a crash. - await cpPromise; - // Simply resolving the captive portal domain should still use TRR - await new TRRDNSListener("detectportal.firefox.com", "2.2.2.2"); + // The captive portal has to have used native DNS, otherwise creating + // a socket to a non-local IP would trigger a crash. + await cpPromise; + // Simply resolving the captive portal domain should still use TRR + await new TRRDNSListener("detectportal.firefox.com", "2.2.2.2"); - Services.prefs.clearUserPref("network.captive-portal-service.enabled"); - Services.prefs.clearUserPref("network.captive-portal-service.testMode"); - Services.prefs.clearUserPref("captivedetect.canonicalURL"); + Services.prefs.clearUserPref("network.captive-portal-service.enabled"); + Services.prefs.clearUserPref("network.captive-portal-service.testMode"); + Services.prefs.clearUserPref("captivedetect.canonicalURL"); - await new Promise(resolve => cpServer.stop(resolve)); - } + await new Promise(resolve => cpServer.stop(resolve)); } async function test_parentalcontrols() { @@ -603,50 +556,34 @@ async function test_parentalcontrols() { await SetParentalControlEnabled(true); await new TRRDNSListener("www.example.com", "127.0.0.1"); await SetParentalControlEnabled(false); - - info("Now in strict mode"); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", true); - dns.clearCache(true); - setModeAndURI(2, "doh?responseIP=2.2.2.2"); - await SetParentalControlEnabled(true); - await new TRRDNSListener("www.example.com", "127.0.0.1"); - await SetParentalControlEnabled(false); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", false); } async function test_builtin_excluded_domains() { info("Verifying Do53 is used for domains in builtin-excluded-domians list"); - for (let strictMode of [true, false]) { - info("Strict mode: " + strictMode); - Services.prefs.setBoolPref( - "network.trr.strict_native_fallback", - strictMode - ); - dns.clearCache(true); - setModeAndURI(2, "doh?responseIP=2.2.2.2"); + dns.clearCache(true); + setModeAndURI(2, "doh?responseIP=2.2.2.2"); - Services.prefs.setCharPref("network.trr.excluded-domains", ""); - Services.prefs.setCharPref( - "network.trr.builtin-excluded-domains", - "bar.example.com" - ); - await new TRRDNSListener("bar.example.com", "127.0.0.1"); + Services.prefs.setCharPref("network.trr.excluded-domains", ""); + Services.prefs.setCharPref( + "network.trr.builtin-excluded-domains", + "bar.example.com" + ); + await new TRRDNSListener("bar.example.com", "127.0.0.1"); - dns.clearCache(true); - Services.prefs.setCharPref( - "network.trr.builtin-excluded-domains", - "example.com" - ); - await new TRRDNSListener("bar.example.com", "127.0.0.1"); + dns.clearCache(true); + Services.prefs.setCharPref( + "network.trr.builtin-excluded-domains", + "example.com" + ); + await new TRRDNSListener("bar.example.com", "127.0.0.1"); - dns.clearCache(true); - Services.prefs.setCharPref( - "network.trr.builtin-excluded-domains", - "foo.test.com, bar.example.com" - ); - await new TRRDNSListener("bar.example.com", "127.0.0.1"); - await new TRRDNSListener("foo.test.com", "127.0.0.1"); - } + dns.clearCache(true); + Services.prefs.setCharPref( + "network.trr.builtin-excluded-domains", + "foo.test.com, bar.example.com" + ); + await new TRRDNSListener("bar.example.com", "127.0.0.1"); + await new TRRDNSListener("foo.test.com", "127.0.0.1"); } async function test_excluded_domains_mode3() { @@ -871,20 +808,14 @@ async function test_fetch_time() { await new TRRDNSListener("bar_time1.example.com", "127.0.0.1", true, 0); // check an excluded domain. It should fall back to regular DNS. The TRR timing should be 0. + dns.clearCache(true); Services.prefs.setCharPref( "network.trr.excluded-domains", "bar_time2.example.com" ); - for (let strictMode of [true, false]) { - info("Strict mode: " + strictMode); - Services.prefs.setBoolPref( - "network.trr.strict_native_fallback", - strictMode - ); - dns.clearCache(true); - setModeAndURI(2, "doh?responseIP=2.2.2.2&delayIPv4=20"); - await new TRRDNSListener("bar_time2.example.com", "127.0.0.1", true, 0); - } + setModeAndURI(2, "doh?responseIP=2.2.2.2&delayIPv4=20"); + + await new TRRDNSListener("bar_time2.example.com", "127.0.0.1", true, 0); Services.prefs.setCharPref("network.trr.excluded-domains", ""); @@ -932,16 +863,6 @@ async function test_ipv6_trr_fallback() { setModeAndURI(2, "doh?responseIP=none"); await new TRRDNSListener("ipv6.host.com", "1:1::2"); - info("In strict mode, the lookup should fail when both reqs fail."); - dns.clearCache(true); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", true); - setModeAndURI(2, "doh?responseIP=none"); - let { inStatus: inStatus2 } = await new TRRDNSListener("ipv6.host.com", { - expectedSuccess: false, - }); - equal(inStatus2, Cr.NS_ERROR_UNKNOWN_HOST); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", false); - override.clearOverrides(); } @@ -968,16 +889,6 @@ async function test_ipv4_trr_fallback() { setModeAndURI(2, "doh?responseIP=none"); await new TRRDNSListener("ipv4.host.com", "3.4.5.6"); - // No fallback with strict mode. - dns.clearCache(true); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", true); - setModeAndURI(2, "doh?responseIP=none"); - let { inStatus: inStatus2 } = await new TRRDNSListener("ipv4.host.com", { - expectedSuccess: false, - }); - equal(inStatus2, Cr.NS_ERROR_UNKNOWN_HOST); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", false); - override.clearOverrides(); } @@ -1021,168 +932,6 @@ async function test_no_retry_without_doh() { ); } - for (let strictMode of [true, false]) { - info("Strict mode: " + strictMode); - Services.prefs.setBoolPref( - "network.trr.strict_native_fallback", - strictMode - ); - await test(`http://unknown.ipv4.stuff:666/path`, "0.0.0.0"); - await test(`http://unknown.ipv6.stuff:666/path`, "::"); - } -} - -async function test_connection_reuse_and_cycling() { - dns.clearCache(true); - Services.prefs.setIntPref("network.trr.request_timeout_ms", 500); - Services.prefs.setIntPref("network.trr.request_timeout_mode_trronly_ms", 500); - - setModeAndURI(2, `doh?responseIP=9.8.7.6`); - Services.prefs.setBoolPref("network.trr.strict_native_fallback", true); - Services.prefs.setCharPref("network.trr.confirmationNS", "example.com"); - await TestUtils.waitForCondition( - // 2 => CONFIRM_OK - () => dns.currentTrrConfirmationState == 2, - `Timed out waiting for confirmation success. Currently ${dns.currentTrrConfirmationState}`, - 1, - 5000 - ); - - // Setting conncycle=true in the URI. Server will start logging reqs. - // We will do a specific sequence of lookups, then fetch the log from - // the server and check that it matches what we'd expect. - setModeAndURI(2, `doh?responseIP=9.8.7.6&conncycle=true`); - await TestUtils.waitForCondition( - // 2 => CONFIRM_OK - () => dns.currentTrrConfirmationState == 2, - `Timed out waiting for confirmation success. Currently ${dns.currentTrrConfirmationState}`, - 1, - 5000 - ); - // Confirmation upon uri-change will have created one req. - - // Two reqs for each bar1 and bar2 - A + AAAA. - await new TRRDNSListener("bar1.example.org.", "9.8.7.6"); - await new TRRDNSListener("bar2.example.org.", "9.8.7.6"); - // Total so far: (1) + 2 + 2 = 5 - - // Two reqs that fail, one Confirmation req, two retried reqs that succeed. - await new TRRDNSListener("newconn.example.org.", "9.8.7.6"); - await TestUtils.waitForCondition( - // 2 => CONFIRM_OK - () => dns.currentTrrConfirmationState == 2, - `Timed out waiting for confirmation success. Currently ${dns.currentTrrConfirmationState}`, - 1, - 5000 - ); - // Total so far: (5) + 2 + 1 + 2 = 10 - - // Two reqs for each bar3 and bar4 . - await new TRRDNSListener("bar3.example.org.", "9.8.7.6"); - await new TRRDNSListener("bar4.example.org.", "9.8.7.6"); - // Total so far: (10) + 2 + 2 = 14. - - // Two reqs that fail, one Confirmation req, two retried reqs that succeed. - await new TRRDNSListener("newconn2.example.org.", "9.8.7.6"); - await TestUtils.waitForCondition( - // 2 => CONFIRM_OK - () => dns.currentTrrConfirmationState == 2, - `Timed out waiting for confirmation success. Currently ${dns.currentTrrConfirmationState}`, - 1, - 5000 - ); - // Total so far: (14) + 2 + 1 + 2 = 19 - - // Two reqs for each bar5 and bar6 . - await new TRRDNSListener("bar5.example.org.", "9.8.7.6"); - await new TRRDNSListener("bar6.example.org.", "9.8.7.6"); - // Total so far: (19) + 2 + 2 = 23 - - let chan = makeChan( - `https://foo.example.com:${h2Port}/get-doh-req-port-log`, - Ci.nsIRequest.TRR_DISABLED_MODE - ); - let dohReqPortLog = await new Promise(resolve => - chan.asyncOpen( - new ChannelListener((stuff, buffer) => { - resolve(JSON.parse(buffer)); - }) - ) - ); - - // Since the actual ports seen will vary at runtime, we use placeholders - // instead in our expected output definition. For example, if two entries - // both have "port1", it means they both should have the same port in the - // server's log. - // For reqs that fail and trigger a Confirmation + retry, the retried reqs - // might not re-use the new connection created for Confirmation due to a - // race, so we have an extra alternate expected port for them. This lets - // us test that they use *a* new port even if it's not *the* new port. - // Subsequent lookups are not affected, they will use the same conn as - // the Confirmation req. - let expectedLogTemplate = [ - ["example.com", "port1"], - ["bar1.example.org", "port1"], - ["bar1.example.org", "port1"], - ["bar2.example.org", "port1"], - ["bar2.example.org", "port1"], - ["newconn.example.org", "port1"], - ["newconn.example.org", "port1"], - ["example.com", "port2"], - ["newconn.example.org", "port2"], - ["newconn.example.org", "port2"], - ["bar3.example.org", "port2"], - ["bar3.example.org", "port2"], - ["bar4.example.org", "port2"], - ["bar4.example.org", "port2"], - ["newconn2.example.org", "port2"], - ["newconn2.example.org", "port2"], - ["example.com", "port3"], - ["newconn2.example.org", "port3"], - ["newconn2.example.org", "port3"], - ["bar5.example.org", "port3"], - ["bar5.example.org", "port3"], - ["bar6.example.org", "port3"], - ["bar6.example.org", "port3"], - ]; - - if (expectedLogTemplate.length != dohReqPortLog.length) { - // This shouldn't happen, and if it does, we'll fail the assertion - // below. But first dump the whole server-side log to help with - // debugging should we see a failure. Most likely cause would be - // that another consumer of TRR happened to make a request while - // the test was running and polluted the log. - info(dohReqPortLog); - } - - equal( - expectedLogTemplate.length, - dohReqPortLog.length, - "Correct number of req log entries" - ); - - let seenPorts = new Set(); - // This is essentially a symbol table - as we iterate through the log - // we will assign the actual seen port numbers to the placeholders. - let seenPortsByExpectedPort = new Map(); - - for (let i = 0; i < expectedLogTemplate.length; i++) { - let expectedName = expectedLogTemplate[i][0]; - let expectedPort = expectedLogTemplate[i][1]; - let seenName = dohReqPortLog[i][0]; - let seenPort = dohReqPortLog[i][1]; - info(`Checking log entry. Name: ${seenName}, Port: ${seenPort}`); - equal(expectedName, seenName, "Name matches for entry " + i); - if (!seenPortsByExpectedPort.has(expectedPort)) { - ok(!seenPorts.has(seenPort), "Port should not have been previously used"); - seenPorts.add(seenPort); - seenPortsByExpectedPort.set(expectedPort, seenPort); - } else { - equal( - seenPort, - seenPortsByExpectedPort.get(expectedPort), - "Connection was reused as expected" - ); - } - } + await test(`http://unknown.ipv4.stuff:666/path`, "0.0.0.0"); + await test(`http://unknown.ipv6.stuff:666/path`, "::"); } diff --git a/testing/xpcshell/moz-http2/moz-http2.js b/testing/xpcshell/moz-http2/moz-http2.js index 793d9523942f..8f80d8754af8 100644 --- a/testing/xpcshell/moz-http2/moz-http2.js +++ b/testing/xpcshell/moz-http2/moz-http2.js @@ -234,9 +234,6 @@ var didRst = false; var rstConnection = null; var illegalheader_conn = null; -var gDoHPortsLog = []; -var gDoHNewConnLog = {}; - // eslint-disable-next-line complexity function handleRequest(req, res) { // We do this first to ensure nothing goes wonky in our tests that don't want @@ -513,22 +510,14 @@ function handleRequest(req, res) { zlib.gzip(buffer, function(err, result) { resp.setHeader("Content-Encoding", "gzip"); resp.setHeader("Content-Length", result.length); - try { - resp.writeHead(200); - resp.end(result); - } catch (e) { - // connection was closed by the time we started writing. - } + resp.writeHead(200); + res.end(result); }); } else { resp.setHeader("Content-Length", buffer.length); - try { - resp.writeHead(200); - resp.write(buffer); - resp.end(""); - } catch (e) { - // connection was closed by the time we started writing. - } + resp.writeHead(200); + resp.write(buffer); + resp.end(""); } } @@ -894,13 +883,6 @@ function handleRequest(req, res) { emitResponse(res, payload); }); return; - } else if (u.pathname == "/get-doh-req-port-log") { - let rContent = JSON.stringify(gDoHPortsLog); - res.setHeader("Content-Type", "text/plain"); - res.setHeader("Content-Length", rContent.length); - res.writeHead(400); - res.end(rContent); - return; } else if (u.pathname == "/doh") { let responseIP = u.query.responseIP; if (!responseIP) { @@ -980,8 +962,8 @@ function handleRequest(req, res) { let payload = Buffer.from(""); - function emitResponse(response, requestPayload, decodedPacket, delay) { - let packet = decodedPacket || dnsPacket.decode(requestPayload); + function emitResponse(response, requestPayload) { + let packet = dnsPacket.decode(requestPayload); let answer = createDNSAnswer( response, packet, @@ -994,7 +976,7 @@ function handleRequest(req, res) { writeDNSResponse( response, answer, - delay || getDelayFromPacket(packet, responseType(packet, responseIP)), + getDelayFromPacket(packet, responseType(packet, responseIP)), "application/dns-message" ); } @@ -1011,30 +993,7 @@ function handleRequest(req, res) { req.on("end", function finishedData() { // parload is empty when we send redirect response. if (payload.length) { - let packet = dnsPacket.decode(payload); - let delay; - if (u.query.conncycle) { - let name = packet.questions[0].name; - if (name.startsWith("newconn")) { - // If we haven't seen a req for this newconn name before, - // or if we've seen one for the same name on the same port, - // synthesize a timeout. - if ( - !gDoHNewConnLog[name] || - gDoHNewConnLog[name] == req.remotePort - ) { - delay = 1000; - } - if (!gDoHNewConnLog[name]) { - gDoHNewConnLog[name] = req.remotePort; - } - } - gDoHPortsLog.push([packet.questions[0].name, req.remotePort]); - } else { - gDoHPortsLog = []; - gDoHNewConnLog = {}; - } - emitResponse(res, payload, packet, delay); + emitResponse(res, payload); } }); return; diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 2ea3d800debb..e5f615eac669 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -4678,54 +4678,7 @@ "releaseChannelCollection": "opt-out", "description": "When in TRR-first mode, if TRR was skipped and native failed, it lists the reason we may have skipped TRR, keyed by the provider. Does not include requests that intentionally skip TRR." }, - "TRR_SKIP_REASON_STRICT_MODE": { - "record_in_processes": ["main", "socket"], - "products": ["firefox"], - "alert_emails": ["necko@mozilla.com", "nhnt11@mozilla.com"], - "expires_in_version": "never", - "kind": "enumerated", - "keyed": true, - "n_values": 50, - "bug_numbers": [1737198], - "releaseChannelCollection": "opt-out", - "description": "When in TRR-first mode, it lists the reason we may have skipped TRR. The key is like `|` or just `` used when there was no second attempt." - }, - "TRR_SKIP_REASON_RETRY_SUCCESS": { - "record_in_processes": ["main", "socket"], - "products": ["firefox"], - "alert_emails": ["necko@mozilla.com", "nhnt11@mozilla.com"], - "expires_in_version": "never", - "kind": "enumerated", - "keyed": true, - "n_values": 50, - "bug_numbers": [1737198], - "releaseChannelCollection": "opt-out", - "description": "When in TRR-first mode, if TRR failed once and was successfully retried, it lists the reason for the first failure, keyed by the provider." - }, - "TRR_SKIP_REASON_RETRY_FAILED": { - "record_in_processes": ["main", "socket"], - "products": ["firefox"], - "alert_emails": ["necko@mozilla.com", "nhnt11@mozilla.com"], - "expires_in_version": "never", - "kind": "enumerated", - "keyed": true, - "n_values": 50, - "bug_numbers": [1737198], - "releaseChannelCollection": "opt-out", - "description": "When in TRR-first mode, if TRR failed once and retried unsuccessfully, it lists the reason for the first failure, keyed by the provider." - }, - "TRR_ATTEMPT_COUNT": { - "record_in_processes": ["main", "socket"], - "products": ["firefox"], - "alert_emails": ["necko@mozilla.com", "nhnt11@mozilla.com"], - "expires_in_version": "never", - "kind": "enumerated", - "keyed": true, - "n_values": 10, - "bug_numbers": [1737198], - "releaseChannelCollection": "opt-out", - "description": "Number of times we attempted TRR for a successful lookup in TRR-first mode. Keyed by provider." - }, + "DNS_TRR_FIRST4": { "record_in_processes": ["main", "socket"], "products": ["firefox", "fennec"], diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index 25a6983db97e..12ebced6fd88 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/toolkit/components/telemetry/Scalars.yaml @@ -6094,25 +6094,6 @@ networking: record_in_processes: - 'main' - trr_connection_cycle_count: - bug_numbers: - - 1737198 - description: > - Number of times we cycled the TRR connection during a subsession. - Keyed by TRR provider URL. - expires: never - keyed: true - kind: uint - release_channel_collection: opt-out - notification_emails: - - necko@mozilla.com - - nhnt11@mozilla.com - products: - - 'firefox' - record_in_processes: - - 'main' - - 'socket' - blocklist: lastModified_rs_addons: bug_numbers: diff --git a/toolkit/components/telemetry/app/TelemetryEnvironment.jsm b/toolkit/components/telemetry/app/TelemetryEnvironment.jsm index 28469433f966..8be2a3748037 100644 --- a/toolkit/components/telemetry/app/TelemetryEnvironment.jsm +++ b/toolkit/components/telemetry/app/TelemetryEnvironment.jsm @@ -325,7 +325,6 @@ const DEFAULT_ENVIRONMENT_PREFS = new Map([ ["network.proxy.http", { what: RECORD_PREF_STATE }], ["network.proxy.ssl", { what: RECORD_PREF_STATE }], ["network.trr.mode", { what: RECORD_PREF_VALUE }], - ["network.trr.strict_native_fallback", { what: RECORD_PREF_VALUE }], ["pdfjs.disabled", { what: RECORD_PREF_VALUE }], ["places.history.enabled", { what: RECORD_PREF_VALUE }], ["plugins.show_infobar", { what: RECORD_PREF_VALUE }],