Skip to content

Commit

Permalink
[M115] [HTTPS-First Mode] Fix handling non-unique hostnames that don'…
Browse files Browse the repository at this point in the history
…t resolve

This change exempts non-unique hostnames from preferring to show the
net error page for "potentially transient" network errors (like
NXDOMAIN). This fixes an issue where internal single-label hostnames
would never fallback to HTTP (when these hostnames only ever resolve
on HTTP due to proxy configuration).

(cherry picked from commit 22dd8be)

Bug: 1451040
Change-Id: If6c6d2521de33e6414a4810fbb8888799a216221
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4583555
Reviewed-by: Carlos IL <carlosil@chromium.org>
Commit-Queue: Chris Thompson <cthomp@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1152952}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4590522
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Chris Thompson <cthomp@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#380}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
christhompson authored and Chromium LUCI CQ committed Jun 5, 2023
1 parent 5a96d56 commit 457947e
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
40 changes: 40 additions & 0 deletions chrome/browser/ssl/https_upgrades_browsertest.cc
Expand Up @@ -1011,6 +1011,46 @@ IN_PROC_BROWSER_TEST_P(HttpsUpgradesBrowserTest,
EXPECT_EQ(nonexistent_domain, contents->GetLastCommittedURL().host());
}

// If the upgraded HTTPS URL is not available due to a potentially-transient
// exempted net error but the hostname is non-unique, don't show the net error
// page and instead just fallback to HTTP and the HTTPS-First Mode interstitial.
// Otherwise, the user can be stuck on the net error page when the HTTP version
// of the host would have resolved, such as for corp single-label hostnames.
// (Regression test for crbug.com/1451040.)
IN_PROC_BROWSER_TEST_P(
HttpsUpgradesBrowserTest,
ExemptNetErrorOnUpgrade_NonUniqueHostname_ShouldFallback) {
// This test is only interesting when HTTPS-First Mode is enabled.
if (!IsHttpsFirstModePrefEnabled()) {
return;
}

GURL http_url = http_server()->GetURL("blorp", "/simple.html");
GURL https_url = https_server()->GetURL("blorp", "/simple.html");
auto* contents = browser()->tab_strip_model()->GetActiveWebContents();

// Set up an interceptor that will return ERR_NAME_NOT_RESOLVED. Navigating
// to the HTTP URL should get upgraded to HTTPS, and then fallback to HTTP
// and the HFM interstitial.
auto dns_failure_interceptor =
std::make_unique<content::URLLoaderInterceptor>(base::BindRepeating(
[](content::URLLoaderInterceptor::RequestParams* params) {
params->client->OnComplete(
network::URLLoaderCompletionStatus(net::ERR_NAME_NOT_RESOLVED));
return true;
}));
EXPECT_FALSE(content::NavigateToURL(contents, http_url));
EXPECT_TRUE(chrome_browser_interstitials::IsShowingInterstitial(contents));
ProceedThroughInterstitial(contents);

// Should now be on the HTTP URL and it should be allowlisted.
EXPECT_EQ(http_url, contents->GetLastCommittedURL());
Profile* profile = Profile::FromBrowserContext(contents->GetBrowserContext());
content::SSLHostStateDelegate* state = profile->GetSSLHostStateDelegate();
EXPECT_TRUE(state->IsHttpAllowedForHost(
http_url.host(), contents->GetPrimaryMainFrame()->GetStoragePartition()));
}

// Navigations in subframes should not get upgraded by HTTPS-Only Mode. They
// should be blocked as mixed content.
IN_PROC_BROWSER_TEST_P(HttpsUpgradesBrowserTest,
Expand Down
8 changes: 7 additions & 1 deletion chrome/browser/ssl/https_upgrades_interceptor.cc
Expand Up @@ -587,8 +587,14 @@ bool HttpsUpgradesInterceptor::MaybeCreateLoaderForResponse(
// enabled, because otherwise these would cause the interstitial to be shown
// which is confusing. HTTPS-Upgrades will silently fall back to HTTP for
// these errors.
//
// However, if this is a request to a non-unique hostname, don't prefer
// net error as it is likely non-recoverable -- we want to fallback to HTTP
// and the HTTPS-First Mode interstitial in this case. (In particular, this
// avoids breaking corporate single-label hostnames.)
if (IsInterstitialEnabled(*interstitial_state_) &&
IsHttpsFirstModeExemptedError(status.error_code)) {
IsHttpsFirstModeExemptedError(status.error_code) &&
!net::IsHostnameNonUnique(request.url.host())) {
tab_helper->set_is_exempt_error(true);
return false;
}
Expand Down

0 comments on commit 457947e

Please sign in to comment.