Skip to content

Commit

Permalink
[WebView] Add header verification for Web content restrictions
Browse files Browse the repository at this point in the history 
Header verifications are performed on every redirect and the final URL.

Bug: 1376958
Change-Id: I3f216e2f4ddecd9dcd6e7e009359d9b177e58779
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4209208
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Commit-Queue: Susanne Westphal <swestphal@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1107399}
  • Loading branch information
@s-westphal
s-westphal authored and Chromium LUCI CQ committed Feb 20, 2023
1 parent 42d44b3 commit dedb857
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 41 deletions.
123 changes: 99 additions & 24 deletions ...bview/javatests/src/org/chromium/android_webview/test/AwRestrictSensitiveContentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ public void setUp() throws Exception {
mAwContents = mTestContainerView.getAwContents();

mWebServer = TestWebServer.start();

final Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
TestThreadUtils.runOnUiThreadBlocking(
()
-> AwOriginVerificationScheduler.init(context.getPackageName(),
mActivityTestRule.getAwBrowserContext(), context));
}

@After
Expand All @@ -80,15 +86,23 @@ public void tearDown() {
}

private String addPageToTestServer(TestWebServer webServer, String httpPath, String html) {
return addPageToTestServer(
webServer, httpPath, html, new ArrayList<Pair<String, String>>());
}

private String addPageToTestServer(TestWebServer webServer, String httpPath, String html,
List<Pair<String, String>> additionalHeaders) {
List<Pair<String, String>> headers = new ArrayList<Pair<String, String>>();
headers.add(Pair.create("Content-Type", "text/html"));
headers.add(Pair.create("Cache-Control", "no-store"));
headers.addAll(additionalHeaders);
return webServer.setResponse(httpPath, html, headers);
}

private String addAboutPageToTestServer(TestWebServer webServer) {
return addPageToTestServer(
webServer, "/" + CommonResources.ABOUT_FILENAME, CommonResources.ABOUT_HTML);
private String addAboutPageToTestServer(
TestWebServer webServer, List<Pair<String, String>> additionalHeaders) {
return addPageToTestServer(webServer, "/" + CommonResources.ABOUT_FILENAME,
CommonResources.ABOUT_HTML, additionalHeaders);
}

private String addAssetListToTestServer(TestWebServer webServer, String fingerprint) {
Expand All @@ -100,14 +114,10 @@ private String addAssetListToTestServer(TestWebServer webServer, String fingerpr
@SmallTest
@Feature({"AndroidWebView"})
@CommandLineFlags.Add({"disable-features=WebViewRestrictSensitiveContent"})
public void disablingFeatureDoesValidatePendingOrigins() throws Throwable {
final String aboutPageUrl = addAboutPageToTestServer(mWebServer);
final Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();

TestThreadUtils.runOnUiThreadBlocking(
()
-> AwOriginVerificationScheduler.init(context.getPackageName(),
mActivityTestRule.getAwBrowserContext(), context));
public void disablingFeatureDoesBlockOrRunValidation() throws Throwable {
List<Pair<String, String>> headers = new ArrayList<Pair<String, String>>();
headers.add(Pair.create("X-Embedder-Ancestors", "none"));
final String aboutPageUrl = addAboutPageToTestServer(mWebServer, headers);

AwOriginVerificationScheduler scheduler = AwOriginVerificationScheduler.getInstance();

Expand All @@ -118,6 +128,7 @@ public void disablingFeatureDoesValidatePendingOrigins() throws Throwable {
() -> mTestContainerView.getAwContents().loadUrl(aboutPageUrl, null));
mContentsClient.waitForFullLoad();

Assert.assertEquals(CommonResources.ABOUT_TITLE, mAwContents.getTitle());
Assert.assertEquals(2, scheduler.getPendingOriginsForTesting().size());
Assert.assertTrue(
scheduler.getPendingOriginsForTesting().contains(Origin.create(aboutPageUrl)));
Expand Down Expand Up @@ -147,20 +158,18 @@ public void testInitAndScheduleAll() throws Throwable {
@SmallTest
@Feature({"AndroidWebView"})
@CommandLineFlags.Add({"enable-features=WebViewRestrictSensitiveContent"})
public void testDoesNotBlockVerifiedContent() throws Throwable {
final String aboutPageUrl = addAboutPageToTestServer(mWebServer);
public void doesNotBlockDALVerifiedContent() throws Throwable {
final Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();

List<Pair<String, String>> headers = new ArrayList<Pair<String, String>>();
headers.add(Pair.create("X-Embedder-Ancestors", "none"));
final String aboutPageUrl = addAboutPageToTestServer(mWebServer, headers);

List<String> mSignatureFingerprints =
PackageUtils.getCertificateSHA256FingerprintForPackage(context.getPackageName());
final String assetLinksUrl =
addAssetListToTestServer(mWebServer, mSignatureFingerprints.get(0));

mActivityTestRule.runOnUiThread(
()
-> AwOriginVerificationScheduler.init(context.getPackageName(),
mActivityTestRule.getAwBrowserContext(), context));

AwOriginVerificationScheduler scheduler = AwOriginVerificationScheduler.getInstance();

AwOriginVerificationScheduler.getInstance().addPendingOriginForTesting(
Expand All @@ -184,15 +193,81 @@ public void testDoesNotBlockVerifiedContent() throws Throwable {
@SmallTest
@Feature({"AndroidWebView"})
@CommandLineFlags.Add({"enable-features=WebViewRestrictSensitiveContent"})
public void doesBlockAccessForFailedValidation() throws Throwable {
public void doesNotBlockHeaderVerifiedContent() throws Throwable {
final String webpageNotAvailable = "Webpage not available";
List<Pair<String, String>> headers = new ArrayList<Pair<String, String>>();
headers.add(Pair.create("X-Embedder-Ancestors", "*"));
final String aboutPageUrl = addAboutPageToTestServer(mWebServer, headers);

InstrumentationRegistry.getInstrumentation().runOnMainSync(
() -> mTestContainerView.getAwContents().loadUrl(aboutPageUrl, null));
mContentsClient.waitForFullLoad();

Assert.assertEquals(CommonResources.ABOUT_TITLE, mAwContents.getTitle());
}

@Test
@SmallTest
@Feature({"AndroidWebView"})
@CommandLineFlags.Add({"enable-features=WebViewRestrictSensitiveContent"})
public void headerCanBlockRedirects() throws Throwable {
final String webpageNotAvailable = "Webpage not available";

final String aboutPageUrl = addAboutPageToTestServer(mWebServer);
List<Pair<String, String>> headers = new ArrayList<Pair<String, String>>();
headers.add(Pair.create("X-Embedder-Ancestors", "*"));
final String aboutPageUrl = addAboutPageToTestServer(mWebServer, headers);

String redirect_path = "/redirect.html";
List<Pair<String, String>> blocking_headers = new ArrayList<Pair<String, String>>();
blocking_headers.add(Pair.create("X-Embedder-Ancestors", "none"));
final String initialUrl =
mWebServer.setRedirect(redirect_path, aboutPageUrl, blocking_headers);

InstrumentationRegistry.getInstrumentation().runOnMainSync(
() -> mTestContainerView.getAwContents().loadUrl(initialUrl, null));
mContentsClient.waitForFullLoad();
Assert.assertEquals(webpageNotAvailable, mAwContents.getTitle());
}

@Test
@SmallTest
@Feature({"AndroidWebView"})
@CommandLineFlags.Add({"enable-features=WebViewRestrictSensitiveContent"})
public void allowDALVerifiedRedirects() throws Throwable {
final Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
mActivityTestRule.runOnUiThread(
()
-> AwOriginVerificationScheduler.init(context.getPackageName(),
mActivityTestRule.getAwBrowserContext(), context));

List<Pair<String, String>> headers = new ArrayList<Pair<String, String>>();
headers.add(Pair.create("X-Embedder-Ancestors", "*"));
final String aboutPageUrl = addAboutPageToTestServer(mWebServer, headers);

List<String> mSignatureFingerprints =
PackageUtils.getCertificateSHA256FingerprintForPackage(context.getPackageName());
final String assetLinksUrl =
addAssetListToTestServer(mWebServer, mSignatureFingerprints.get(0));

String redirect_path = "/redirect.html";
List<Pair<String, String>> blocking_headers = new ArrayList<Pair<String, String>>();
blocking_headers.add(Pair.create("X-Embedder-Ancestors", "none"));
final String initialUrl =
mWebServer.setRedirect(redirect_path, aboutPageUrl, blocking_headers);
AwOriginVerificationScheduler.getInstance().addPendingOriginForTesting(
Origin.create(initialUrl));

InstrumentationRegistry.getInstrumentation().runOnMainSync(
() -> mTestContainerView.getAwContents().loadUrl(initialUrl, null));
mContentsClient.waitForFullLoad();
Assert.assertEquals(CommonResources.ABOUT_TITLE, mAwContents.getTitle());
}

@Test
@SmallTest
@Feature({"AndroidWebView"})
@CommandLineFlags.Add({"enable-features=WebViewRestrictSensitiveContent"})
public void doesBlockForNotVerifiedContent() throws Throwable {
final String webpageNotAvailable = "Webpage not available";
List<Pair<String, String>> headers = new ArrayList<Pair<String, String>>();
headers.add(Pair.create("X-Embedder-Ancestors", "none"));
final String aboutPageUrl = addAboutPageToTestServer(mWebServer, headers);

Set<Origin> pendingOrigins =
AwOriginVerificationScheduler.getInstance().getPendingOriginsForTesting();
Expand Down
4 changes: 2 additions & 2 deletions components/digital_asset_links/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

source_set("digital_asset_links") {
sources = [
"browser_url_loader_throttle.cc",
"browser_url_loader_throttle.h",
"digital_asset_links_constants.cc",
"digital_asset_links_constants.h",
"digital_asset_links_handler.cc",
Expand All @@ -25,6 +23,8 @@ source_set("digital_asset_links") {
]
if (is_android) {
sources += [
"browser_url_loader_throttle.cc",
"browser_url_loader_throttle.h",
"origin_verifier.cc",
"origin_verifier.h",
]
Expand Down
58 changes: 47 additions & 11 deletions components/digital_asset_links/browser_url_loader_throttle.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@

#include "components/digital_asset_links/browser_url_loader_throttle.h"

#include "base/android/build_info.h"
#include "base/check_op.h"
#include "base/functional/bind.h"
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_functions.h"
#include "base/trace_event/trace_event.h"
#include "components/digital_asset_links/digital_asset_links_constants.h"
#include "components/digital_asset_links/response_header_verifier.h"
#include "content/public/browser/browser_task_traits.h"
#include "net/log/net_log_event_type.h"
#include "net/url_request/redirect_info.h"
Expand Down Expand Up @@ -39,38 +41,72 @@ BrowserURLLoaderThrottle::BrowserURLLoaderThrottle(

BrowserURLLoaderThrottle::~BrowserURLLoaderThrottle() = default;

bool BrowserURLLoaderThrottle::VerifyHeader(
const network::mojom::URLResponseHead& response_head) {
std::string header_value;
response_head.headers->GetNormalizedHeader(kEmbedderAncestorHeader,
&header_value);
return digital_asset_links::ResponseHeaderVerifier::Verify(
base::android::BuildInfo::GetInstance()->host_package_name(),
header_value);
}

void BrowserURLLoaderThrottle::WillStartRequest(
network::ResourceRequest* request,
bool* defer) {
url_ = request->url;
}

void BrowserURLLoaderThrottle::WillRedirectRequest(
net::RedirectInfo* redirect_info,
const network::mojom::URLResponseHead& response_head,
bool* defer,
std::vector<std::string>* to_be_removed_request_headers,
net::HttpRequestHeaders* modified_request_headers,
net::HttpRequestHeaders* modified_cors_exempt_request_headers) {
DCHECK(delegate_);

*defer = true;
content::GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce(&OriginVerificationSchedulerBridge::Verify,
base::Unretained(bridge_), url_.spec(),
base::BindOnce(&BrowserURLLoaderThrottle::OnCompleteCheck,
weak_factory_.GetWeakPtr(), url_.spec(),
VerifyHeader(response_head))));
url_ = redirect_info->new_url;
}

void BrowserURLLoaderThrottle::WillProcessResponse(
const GURL& response_url,
network::mojom::URLResponseHead* response_head,
bool* defer) {
// TODO(crbug.com/1376958): Check the headers in |response_head| for CSP.

DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
DCHECK(delegate_);

// Do the verification here, as redirected urls are not verified, only the
// final url.
*defer = true;
content::GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce(
&OriginVerificationSchedulerBridge::Verify, base::Unretained(bridge_),
response_url.spec(),
base::BindOnce(&BrowserURLLoaderThrottle::OnCompleteCheck,
weak_factory_.GetWeakPtr(), response_url.spec())));
weak_factory_.GetWeakPtr(), response_url.spec(),
VerifyHeader(*response_head))));
}

void BrowserURLLoaderThrottle::OnCompleteCheck(std::string url, bool verified) {
void BrowserURLLoaderThrottle::OnCompleteCheck(std::string url,
bool header_verification_result,
bool dal_verified) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
DCHECK(delegate_);

if (verified) {
if (dal_verified || header_verification_result) {
delegate_->Resume();
} else {
// TODO(crbug.com/1376958): Show an interstitial for blocked content.
delegate_->CancelWithError(kNetErrorCodeForDigitalAssetLinks,
kCustomCancelReasonForURLLoader);
return;
}
delegate_->CancelWithError(kNetErrorCodeForDigitalAssetLinks,
kCustomCancelReasonForURLLoader);
}

const char* BrowserURLLoaderThrottle::NameForLoggingWillProcessResponse() {
Expand Down
22 changes: 19 additions & 3 deletions components/digital_asset_links/browser_url_loader_throttle.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ namespace digital_asset_links {
// be bad.
class BrowserURLLoaderThrottle : public blink::URLLoaderThrottle {
public:
// Inherited classes must not define member variables since the
// |weak_factory_| needs to be the last member.
class OriginVerificationSchedulerBridge {
public:
using OriginVerifierCallback = base::OnceCallback<void(bool /*verified*/)>;
Expand All @@ -58,6 +56,18 @@ class BrowserURLLoaderThrottle : public blink::URLLoaderThrottle {
~BrowserURLLoaderThrottle() override;

// blink::URLLoaderThrottle implementation.

void WillStartRequest(network::ResourceRequest* request,
bool* defer) override;

void WillRedirectRequest(
net::RedirectInfo* redirect_info,
const network::mojom::URLResponseHead& response_head,
bool* defer,
std::vector<std::string>* to_be_removed_request_headers,
net::HttpRequestHeaders* modified_request_headers,
net::HttpRequestHeaders* modified_cors_exempt_request_headers) override;

void WillProcessResponse(const GURL& response_url,
network::mojom::URLResponseHead* response_head,
bool* defer) override;
Expand All @@ -66,10 +76,16 @@ class BrowserURLLoaderThrottle : public blink::URLLoaderThrottle {
private:
explicit BrowserURLLoaderThrottle(OriginVerificationSchedulerBridge* bridge);

void OnCompleteCheck(std::string url, bool verified);
void OnCompleteCheck(std::string url,
bool header_verification_result,
bool dal_verified);

bool VerifyHeader(const network::mojom::URLResponseHead& response_head);

raw_ptr<OriginVerificationSchedulerBridge> bridge_;

GURL url_;

base::WeakPtrFactory<BrowserURLLoaderThrottle> weak_factory_{this};
};

Expand Down
16 changes: 15 additions & 1 deletion net/test/android/javatests/src/org/chromium/net/test/util/TestWebServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,21 @@ public String setResponseWithRunnableAction(String requestPath, String responseS
* response.
*/
public String setRedirect(String requestPath, String targetLocation) {
List<Pair<String, String>> responseHeaders = new ArrayList<Pair<String, String>>();
return setRedirect(requestPath, targetLocation, new ArrayList<>());
}

/**
* Sets a redirect with optional headers.
*
* @param requestPath The path to respond to.
* @param targetLocation The path (or absolute URL) to redirect to.
* @param responseHeaders Any additional headers that should be returned along with the
* response (null is acceptable).
* @return The full URL including the path that should be requested to get the expected
* response.
*/
public String setRedirect(
String requestPath, String targetLocation, List<Pair<String, String>> responseHeaders) {
responseHeaders.add(Pair.create("Location", targetLocation));

return setResponseInternal(requestPath, ApiCompatibilityUtils.getBytesUtf8(targetLocation),
Expand Down

0 comments on commit dedb857

Please sign in to comment.