Skip to content

Commit

Permalink
[Private Network Access] Disable secure context restriction on webview.
Browse files Browse the repository at this point in the history
Per go/finch-killswitch#merge-policy, a kill switch must be followed up
with a merge to the affected channel (here webview stable) to disable
the feature by default.

ntfschr@ also points out that the kill switch might not work for the
devices affected by b/201865749, which might all run for the first time.

This CL makes use of the existing ContentBrowserClient interface to
selectively disable the deprecation on Webview only. Returning true
from ShouldAllowPrivateNetworkRequests results in the content/ code
ignoring the value of the BlockInsecurePrivateNetworkRequests Finch
feature flag. This means that even though the flag may be set to true
by default for webview, it is ignored and the deprecation is disabled.

Bug: chromium:1255675
Change-Id: I21c60acbea2bf3b7598b659ecbbac0f447fb5d1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3199763
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Auto-Submit: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Weilun Shi <sweilun@chromium.org>
Cr-Commit-Position: refs/heads/main@{#928260}
  • Loading branch information
letitz authored and Chromium LUCI CQ committed Oct 5, 2021
1 parent b2536a8 commit 8aaf96f
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 0 deletions.
10 changes: 10 additions & 0 deletions android_webview/browser/aw_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,16 @@ bool AwContentBrowserClient::IsOriginTrialRequiredForAppCache(
return false;
}

bool AwContentBrowserClient::ShouldAllowInsecurePrivateNetworkRequests(
content::BrowserContext* browser_context,
const url::Origin& origin) {
// Webview does not implement support for deprecation trials, so webview apps
// broken by Private Network Access restrictions cannot help themselves by
// registering for the trial.
// See crbug.com/1255675.
return true;
}

content::SpeechRecognitionManagerDelegate*
AwContentBrowserClient::CreateSpeechRecognitionManagerDelegate() {
return new AwSpeechRecognitionManagerDelegate();
Expand Down
3 changes: 3 additions & 0 deletions android_webview/browser/aw_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
blink::mojom::WebFeature feature) override;
bool IsOriginTrialRequiredForAppCache(
content::BrowserContext* browser_text) override;
bool ShouldAllowInsecurePrivateNetworkRequests(
content::BrowserContext* browser_context,
const url::Origin& origin) override;
content::SpeechRecognitionManagerDelegate*
CreateSpeechRecognitionManagerDelegate() override;
bool HasErrorPage(int http_status_code) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,68 @@ public void testInsertNullVisualStateCallback() {
}
}

// This test verifies that Private Network Access' secure context
// restriction (feature flag BlockInsecurePrivateNetworkRequests) does not
// apply to Webview: insecure private network requests are allowed.
//
// This is a regression test for crbug.com/1255675.
@Test
@Feature({"AndroidWebView"})
@CommandLineFlags.Add(ContentSwitches.HOST_RESOLVER_RULES + "=MAP * 127.0.0.1")
@SmallTest
public void testInsecurePrivateNetworkAccess() throws Throwable {
mActivityTestRule.startBrowserProcess();
final AwTestContainerView testContainer =
mActivityTestRule.createAwTestContainerViewOnMainSync(mContentsClient);
final AwContents awContents = testContainer.getAwContents();

AwActivityTestRule.enableJavaScriptOnUiThread(awContents);

// This SettableFuture and its accompanying injected object allows us to
// synchronize on the fetch result.
final SettableFuture<Boolean> fetchResultFuture = SettableFuture.create();
Object injectedObject = new Object() {
@JavascriptInterface
public void success() {
fetchResultFuture.set(true);
}
@JavascriptInterface
public void error() {
fetchResultFuture.set(false);
}
};
AwActivityTestRule.addJavascriptInterfaceOnUiThread(
awContents, injectedObject, "injectedObject");

EmbeddedTestServer testServer = EmbeddedTestServer.createAndStartServer(
InstrumentationRegistry.getInstrumentation().getContext());

// Need to avoid http://localhost, which is considered secure, so we
// use http://foo.test, which resolves to 127.0.0.1 thanks to the
// host resolver rules command-line flag.
//
// The resulting document is a non-secure context in the public IP
// address space. If the secure context restriction were applied, it
// would not be allowed to fetch subresources from localhost.
String url = testServer.getURLWithHostName(
"foo.test", "/set-header?Content-Security-Policy: treat-as-public-address");

mActivityTestRule.loadUrlSync(awContents, mContentsClient.getOnPageFinishedHelper(), url);

// Fetch a subresource from the same server, whose IP address is still
// 127.0.0.1, thus belonging to the local IP address space.
// This should succeed.
mActivityTestRule.executeJavaScriptAndWaitForResult(awContents, mContentsClient,
"fetch('/defaultresponse')"
+ ".then(() => { injectedObject.success() })"
+ ".catch((err) => { "
+ " console.log(err); "
+ " injectedObject.error(); "
+ "})");

Assert.assertTrue(AwActivityTestRule.waitForFuture(fetchResultFuture));
}

private static final String HELLO_WORLD_URL = "/android_webview/test/data/hello_world.html";
private static final String HELLO_WORLD_TITLE = "Hello, World!";
private static final String WEBUI_URL = "chrome://safe-browsing";
Expand Down

0 comments on commit 8aaf96f

Please sign in to comment.