Bug 1552176 - Captive portal domain should not be automatically excluded from TRR r=mayhemer

Previously we had no way from excluding just one channel from TRR mode3.
The solution was to add the captive portal domain to the exclusion list.
Now the captive portal channel is marked with nsIRequest.DISABLE_TRR_MODE so
the exclusion is not necessary anymore.

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Valentin Gosu 2020-01-07 19:32:32 +00:00
parent c7d9b630cb
commit cbc2554d2d
3 changed files with 88 additions and 24 deletions

View File

@ -20,7 +20,6 @@ static const char kOpenCaptivePortalLoginEvent[] = "captive-portal-login";
static const char kClearPrivateData[] = "clear-private-data";
static const char kPurge[] = "browser:purge-session-history";
static const char kDisableIpv6Pref[] = "network.dns.disableIPv6";
static const char kCaptivedetectCanonicalURL[] = "captivedetect.canonicalURL";
static const char kPrefSkipTRRParentalControl[] =
"network.dns.skipTRR-when-parental-control-enabled";
@ -82,7 +81,6 @@ nsresult TRRService::Init() {
if (prefBranch) {
prefBranch->AddObserver(TRR_PREF_PREFIX, this, true);
prefBranch->AddObserver(kDisableIpv6Pref, this, true);
prefBranch->AddObserver(kCaptivedetectCanonicalURL, this, true);
prefBranch->AddObserver(kPrefSkipTRRParentalControl, this, true);
}
nsCOMPtr<nsICaptivePortalService> captivePortalService =
@ -316,8 +314,7 @@ nsresult TRRService::ReadPrefs(const char* name) {
}
}
if (!name || !strcmp(name, TRR_PREF("excluded-domains")) ||
!strcmp(name, TRR_PREF("builtin-excluded-domains")) ||
!strcmp(name, kCaptivedetectCanonicalURL)) {
!strcmp(name, TRR_PREF("builtin-excluded-domains"))) {
MutexAutoLock lock(mLock);
mExcludedDomains.Clear();
@ -340,19 +337,6 @@ nsresult TRRService::ReadPrefs(const char* name) {
parseExcludedDomains(TRR_PREF("excluded-domains"));
parseExcludedDomains(TRR_PREF("builtin-excluded-domains"));
clearEntireCache = true;
nsAutoCString canonicalSiteURL;
Preferences::GetCString(kCaptivedetectCanonicalURL, canonicalSiteURL);
nsCOMPtr<nsIURI> uri;
nsresult rv = NS_NewURI(getter_AddRefs(uri), canonicalSiteURL,
UTF_8_ENCODING, nullptr);
if (NS_SUCCEEDED(rv)) {
nsAutoCString host;
uri->GetHost(host);
LOG(("TRRService::ReadPrefs captive portal URL:[%s]\n", host.get()));
mExcludedDomains.PutEntry(host);
}
}
if (!name || !strcmp(name, kPrefSkipTRRParentalControl)) {

View File

@ -1446,6 +1446,10 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec) {
}
nsIRequest::TRRMode effectiveRequestMode = rec->EffectiveTRRMode();
LOG(("NameLookup: %s effectiveTRRmode: %d", rec->host.get(),
effectiveRequestMode));
if (effectiveRequestMode != nsIRequest::TRR_DISABLED_MODE &&
!((rec->flags & RES_DISABLE_TRR))) {
rv = TrrLookup(rec);

View File

@ -881,15 +881,61 @@ add_task(async function test24e() {
await new DNSListener("bar.example.com", "127.0.0.1");
});
function observerPromise(topic) {
return new Promise(resolve => {
let observer = {
QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver]),
observe(aSubject, aTopic, aData) {
dump(`observe: ${aSubject}, ${aTopic}, ${aData} \n`);
if (aTopic == topic) {
Services.obs.removeObserver(observer, topic);
resolve(aData);
}
},
};
Services.obs.addObserver(observer, topic);
});
}
// TRR-first check that captivedetect.canonicalURL is resolved via native DNS
add_task(async function test24f() {
dns.clearCache(true);
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 = observerPromise("captive-portal-login");
Services.prefs.setCharPref(
"captivedetect.canonicalURL",
"http://test.detectportal.com/success.txt"
`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);
await new DNSListener("test.detectportal.com", "127.0.0.1");
// 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 DNSListener("detectportal.firefox.com", "192.192.192.192");
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));
});
// TRR-first check that a domain is resolved via native DNS when parental control is enabled.
@ -1002,16 +1048,46 @@ add_task(async function test25d() {
add_task(async function test25e() {
dns.clearCache(true);
Services.prefs.setIntPref("network.trr.mode", 3); // TRR-only
Services.prefs.setCharPref(
"captivedetect.canonicalURL",
"http://test.detectportal.com/success.txt"
);
Services.prefs.setCharPref(
"network.trr.uri",
`https://foo.example.com:${h2Port}/doh?responseIP=192.192.192.192`
);
await new DNSListener("test.detectportal.com", "127.0.0.1");
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 = observerPromise("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);
// 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 DNSListener("detectportal.firefox.com", "192.192.192.192");
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));
});
// TRR-only check that a domain is resolved via native DNS when parental control is enabled.