From 0e928af6c39bbc5f63b25202e1329ff9bbff972c Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Sun, 24 Nov 2019 14:54:02 +0000 Subject: [PATCH] Bug 1565008 - TRR: Check for Proxy on Windows to use platform DNS r=mayhemer Differential Revision: https://phabricator.services.mozilla.com/D54092 --HG-- extra : moz-landing-system : lando --- modules/libpref/init/StaticPrefList.yaml | 13 +++++ netwerk/base/nsINetworkLinkService.idl | 9 +++- netwerk/dns/TRRService.cpp | 32 +++++++----- netwerk/dns/TRRService.h | 4 +- .../android/nsAndroidNetworkLinkService.cpp | 3 +- netwerk/system/linux/nsNetworkLinkService.cpp | 3 +- netwerk/system/mac/nsNetworkLinkService.mm | 4 +- netwerk/system/win32/nsNotifyAddrListener.cpp | 49 +++++++++++++++++-- netwerk/system/win32/nsNotifyAddrListener.h | 5 +- netwerk/test/unit/test_trr.js | 5 +- 10 files changed, 99 insertions(+), 28 deletions(-) diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index c436b127f76c..14de20e5376f 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -6855,6 +6855,12 @@ value: false mirror: always +# This pref controls if TRR will still be enabled when a proxy is detected +- name: network.trr.enable_when_proxy_detected + type: RelaxedAtomicBool + value: false + mirror: always + # Allow the network changed event to get sent when a network topology or setup # change is noticed while running. - name: network.notify.changed @@ -6878,6 +6884,13 @@ value: true mirror: always +# Whether to check the registry for proxies on network changes that indicate +# that TRR should not be used. +- name: network.notify.checkForProxies + type: RelaxedAtomicBool + value: true + mirror: always + #--------------------------------------------------------------------------- # Prefs starting with "nglayout." #--------------------------------------------------------------------------- diff --git a/netwerk/base/nsINetworkLinkService.idl b/netwerk/base/nsINetworkLinkService.idl index 770b8b4041c5..3fca78108216 100644 --- a/netwerk/base/nsINetworkLinkService.idl +++ b/netwerk/base/nsINetworkLinkService.idl @@ -57,10 +57,15 @@ interface nsINetworkLinkService : nsISupports */ readonly attribute Array dnsSuffixList; + const unsigned long NONE_DETECTED = 0; + const unsigned long VPN_DETECTED = 1 << 0; + const unsigned long PROXY_DETECTED = 1 << 1; + /** - * Whether a VPN was detected on at least one active network interface. + * A bitfield that encodes the platform attributes we detected which + * indicate that we should only use DNS, not TRR. */ - readonly attribute boolean vpnDetected; + readonly attribute unsigned long platformDNSIndications; }; %{C++ diff --git a/netwerk/dns/TRRService.cpp b/netwerk/dns/TRRService.cpp index 36706c3d1797..02bcd56c764b 100644 --- a/netwerk/dns/TRRService.cpp +++ b/netwerk/dns/TRRService.cpp @@ -49,7 +49,7 @@ TRRService::TRRService() mUseGET(false), mDisableECS(true), mDisableAfterFails(5), - mVPNDetected(false), + mPlatformDisabledTRR(false), mClearTRRBLStorage(false), mConfirmationState(CONFIRM_INIT), mRetryConfirmInterval(1000), @@ -456,13 +456,12 @@ TRRService::Observe(nsISupports* aSubject, const char* aTopic, LOG(("TRRService link-event")); nsCOMPtr link = do_QueryInterface(aSubject); RebuildSuffixList(link); - CheckVPNStatus(link); + CheckPlatformDNSStatus(link); } return NS_OK; } void TRRService::RebuildSuffixList(nsINetworkLinkService* aLinkService) { - mDNSSuffixDomains.Clear(); // The network link service notification normally passes itself as the // subject, but some unit tests will sometimes pass a null subject. if (!aLinkService) { @@ -471,21 +470,28 @@ void TRRService::RebuildSuffixList(nsINetworkLinkService* aLinkService) { nsTArray suffixList; aLinkService->GetDnsSuffixList(suffixList); + + MutexAutoLock lock(mLock); + mDNSSuffixDomains.Clear(); for (const auto& item : suffixList) { LOG(("TRRService adding %s to suffix list", item.get())); mDNSSuffixDomains.PutEntry(item); } } -void TRRService::CheckVPNStatus(nsINetworkLinkService* aLinkService) { +void TRRService::CheckPlatformDNSStatus(nsINetworkLinkService* aLinkService) { if (!aLinkService) { return; } - bool vpnDetected = false; - aLinkService->GetVpnDetected(&vpnDetected); - mVPNDetected = vpnDetected; - LOG(("TRRService vpnDetected=%d", vpnDetected)); + uint32_t platformIndications = nsINetworkLinkService::NONE_DETECTED; + aLinkService->GetPlatformDNSIndications(&platformIndications); + LOG(("TRRService platformIndications=%u", platformIndications)); + mPlatformDisabledTRR = + (!StaticPrefs::network_trr_enable_when_vpn_detected() && + (platformIndications & nsINetworkLinkService::VPN_DETECTED)) || + (!StaticPrefs::network_trr_enable_when_proxy_detected() && + (platformIndications & nsINetworkLinkService::PROXY_DETECTED)); } void TRRService::MaybeConfirm() { @@ -648,14 +654,16 @@ bool TRRService::IsExcludedFromTRR(const nsACString& aHost) { } bool TRRService::IsExcludedFromTRR_unlocked(const nsACString& aHost) { - if (mVPNDetected && !StaticPrefs::network_trr_enable_when_vpn_detected()) { - LOG(("%s is excluded from TRR because of VPN", aHost.BeginReading())); - return true; - } if (!NS_IsMainThread()) { mLock.AssertCurrentThreadOwns(); } + if (mPlatformDisabledTRR) { + LOG(("%s is excluded from TRR because of platform indications", + aHost.BeginReading())); + return true; + } + int32_t dot = 0; // iteratively check the sub-domain of |aHost| while (dot < static_cast(aHost.Length())) { diff --git a/netwerk/dns/TRRService.h b/netwerk/dns/TRRService.h index f7d5abc811bd..808d83985dbe 100644 --- a/netwerk/dns/TRRService.h +++ b/netwerk/dns/TRRService.h @@ -78,7 +78,7 @@ class TRRService : public nsIObserver, bool IsExcludedFromTRR_unlocked(const nsACString& aHost); void RebuildSuffixList(nsINetworkLinkService* aLinkService); - void CheckVPNStatus(nsINetworkLinkService* aLinkService); + void CheckPlatformDNSStatus(nsINetworkLinkService* aLinkService); bool mInitialized; Atomic mMode; @@ -105,7 +105,7 @@ class TRRService : public nsIObserver, Atomic mDisableECS; // disable EDNS Client Subnet in requests Atomic mDisableAfterFails; // this many fails in a row means failed TRR service - Atomic mVPNDetected; + Atomic mPlatformDisabledTRR; // TRR Blacklist storage // mTRRBLStorage is only modified on the main thread, but we query whether it diff --git a/netwerk/system/android/nsAndroidNetworkLinkService.cpp b/netwerk/system/android/nsAndroidNetworkLinkService.cpp index a4c2344415c3..9cdfd450c7bf 100644 --- a/netwerk/system/android/nsAndroidNetworkLinkService.cpp +++ b/netwerk/system/android/nsAndroidNetworkLinkService.cpp @@ -153,7 +153,8 @@ nsAndroidNetworkLinkService::GetDnsSuffixList( } NS_IMETHODIMP -nsAndroidNetworkLinkService::GetVpnDetected(bool* aHasVPN) { +nsAndroidNetworkLinkService::GetPlatformDNSIndications( + uint32_t* aPlatformDNSIndications) { return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/netwerk/system/linux/nsNetworkLinkService.cpp b/netwerk/system/linux/nsNetworkLinkService.cpp index d860b515c5cb..8e252e5f4e47 100644 --- a/netwerk/system/linux/nsNetworkLinkService.cpp +++ b/netwerk/system/linux/nsNetworkLinkService.cpp @@ -69,7 +69,8 @@ nsNetworkLinkService::GetDnsSuffixList(nsTArray& aDnsSuffixList) { } NS_IMETHODIMP -nsNetworkLinkService::GetVpnDetected(bool* aHasVPN) { +nsNetworkLinkService::GetPlatformDNSIndications( + uint32_t* aPlatformDNSIndications) { return NS_ERROR_NOT_IMPLEMENTED; } diff --git a/netwerk/system/mac/nsNetworkLinkService.mm b/netwerk/system/mac/nsNetworkLinkService.mm index 68afabc0433d..95756b304dd4 100644 --- a/netwerk/system/mac/nsNetworkLinkService.mm +++ b/netwerk/system/mac/nsNetworkLinkService.mm @@ -119,7 +119,9 @@ nsNetworkLinkService::GetNetworkID(nsACString& aNetworkID) { } NS_IMETHODIMP -nsNetworkLinkService::GetVpnDetected(bool* aHasVPN) { return NS_ERROR_NOT_IMPLEMENTED; } +nsNetworkLinkService::GetPlatformDNSIndications(uint32_t* aPlatformDNSIndications) { + return NS_ERROR_NOT_IMPLEMENTED; +} // Note that this function is copied from xpcom/io/nsLocalFileUnix.cpp. static nsresult CFStringReftoUTF8(CFStringRef aInStrRef, nsACString& aOutStr) { diff --git a/netwerk/system/win32/nsNotifyAddrListener.cpp b/netwerk/system/win32/nsNotifyAddrListener.cpp index 9ae16fae5f51..29ba46252614 100644 --- a/netwerk/system/win32/nsNotifyAddrListener.cpp +++ b/netwerk/system/win32/nsNotifyAddrListener.cpp @@ -66,7 +66,7 @@ nsNotifyAddrListener::nsNotifyAddrListener() mMutex("nsNotifyAddrListener::mMutex"), mCheckEvent(nullptr), mShutdown(false), - mFoundVPN(false), + mPlatformDNSIndications(NONE_DETECTED), mIPInterfaceChecksum(0), mCoalescingActive(false) {} @@ -116,8 +116,9 @@ nsNotifyAddrListener::GetDnsSuffixList(nsTArray& aDnsSuffixList) { } NS_IMETHODIMP -nsNotifyAddrListener::GetVpnDetected(bool* aHasVPN) { - *aHasVPN = mFoundVPN; +nsNotifyAddrListener::GetPlatformDNSIndications( + uint32_t* aPlatformDNSIndications) { + *aPlatformDNSIndications = mPlatformDNSIndications; return NS_OK; } @@ -445,7 +446,7 @@ nsNotifyAddrListener::CheckAdaptersAddresses(void) { ULONG sumAll = 0; nsTArray dnsSuffixList; - mFoundVPN = false; + uint32_t platformDNSIndications = NONE_DETECTED; if (ret == ERROR_SUCCESS) { bool linkUp = false; ULONG sum = 0; @@ -460,7 +461,7 @@ nsNotifyAddrListener::CheckAdaptersAddresses(void) { if (adapter->IfType == IF_TYPE_PPP) { LOG(("VPN connection found")); - mFoundVPN = true; + platformDNSIndications |= VPN_DETECTED; } sum <<= 2; @@ -539,9 +540,47 @@ nsNotifyAddrListener::CheckAdaptersAddresses(void) { }; checkRegistry(); + } + if (StaticPrefs::network_notify_checkForProxies()) { + auto registryChildCount = [](const nsAString& aRegPath) -> uint32_t { + nsresult rv; + nsCOMPtr regKey = + do_CreateInstance("@mozilla.org/windows-registry-key;1", &rv); + if (NS_FAILED(rv)) { + LOG((" creating nsIWindowsRegKey failed\n")); + return 0; + } + rv = regKey->Open(nsIWindowsRegKey::ROOT_KEY_LOCAL_MACHINE, aRegPath, + nsIWindowsRegKey::ACCESS_READ); + if (NS_FAILED(rv)) { + LOG((" opening registry key failed\n")); + return 0; + } + + uint32_t count = 0; + rv = regKey->GetChildCount(&count); + if (NS_FAILED(rv)) { + return 0; + } + + return count; + }; + + if (registryChildCount( + NS_LITERAL_STRING("SYSTEM\\CurrentControlSet\\Services\\Dnscache\\" + "Parameters\\DnsConnections")) > 0 || + registryChildCount( + NS_LITERAL_STRING("SYSTEM\\CurrentControlSet\\Services\\Dnscache\\" + "Parameters\\DnsConnectionsProxies")) > 0) { + platformDNSIndications |= PROXY_DETECTED; + } + } + + { MutexAutoLock lock(mMutex); mDnsSuffixList.SwapElements(dnsSuffixList); + mPlatformDNSIndications = platformDNSIndications; } calculateNetworkId(); diff --git a/netwerk/system/win32/nsNotifyAddrListener.h b/netwerk/system/win32/nsNotifyAddrListener.h index 39c37e05504b..aaa603d91bf1 100644 --- a/netwerk/system/win32/nsNotifyAddrListener.h +++ b/netwerk/system/win32/nsNotifyAddrListener.h @@ -72,8 +72,9 @@ class nsNotifyAddrListener : public nsINetworkLinkService, // set true when mCheckEvent means shutdown bool mShutdown; - // If a VPN was detected on at least one active network interface. - mozilla::Atomic mFoundVPN; + // Contains a set of flags that codify the reasons for which + // the platform indicates DNS should be used instead of TRR. + mozilla::Atomic mPlatformDNSIndications; // This is a checksum of various meta data for all network interfaces // considered UP at last check. diff --git a/netwerk/test/unit/test_trr.js b/netwerk/test/unit/test_trr.js index 3a9bb5ca07b1..5d41a1a7bce0 100644 --- a/netwerk/test/unit/test_trr.js +++ b/netwerk/test/unit/test_trr.js @@ -1149,7 +1149,7 @@ add_task(async function test_vpnDetection() { await new DNSListener("push.example.org", "2018::2018"); let networkLinkService = { - vpnDetected: true, + platformDNSIndications: Ci.nsINetworkLinkService.VPN_DETECTED, QueryInterface: ChromeUtils.generateQI([Ci.nsINetworkLinkService]), }; @@ -1175,7 +1175,8 @@ add_task(async function test_vpnDetection() { Services.prefs.clearUserPref("network.trr.bootstrapAddress"); // Attempt to clean up, just in case - networkLinkService.vpnDetected = false; + networkLinkService.platformDNSIndications = + Ci.nsINetworkLinkService.NONE_DETECTED; Services.obs.notifyObservers( networkLinkService, "network:link-status-changed",