Skip to content

Commit

Permalink
[WebEngine] Add Header verification for js-execution
Browse files Browse the repository at this point in the history
Change-Id: I5680cf9ac1bb39a7ad6db15e04db9f6d2903762a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4200665
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Commit-Queue: Susanne Westphal <swestphal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100585}
  • Loading branch information
s-westphal authored and Chromium LUCI CQ committed Feb 2, 2023
1 parent 0ef3d94 commit d9efaf8
Show file tree
Hide file tree
Showing 18 changed files with 259 additions and 20 deletions.
7 changes: 6 additions & 1 deletion components/digital_asset_links/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ source_set("digital_asset_links") {
"digital_asset_links_constants.h",
"digital_asset_links_handler.cc",
"digital_asset_links_handler.h",
"response_header_verifier.cc",
"response_header_verifier.h",
]

deps = [
Expand All @@ -33,7 +35,10 @@ source_set("digital_asset_links") {
source_set("unit_tests") {
testonly = true

sources = [ "digital_asset_links_handler_unittest.cc" ]
sources = [
"digital_asset_links_handler_unittest.cc",
"response_header_verifier_unittest.cc",
]

deps = [
":digital_asset_links",
Expand Down
49 changes: 49 additions & 0 deletions components/digital_asset_links/response_header_verifier.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/digital_asset_links/response_header_verifier.h"

#include <stdio.h>

#include "base/containers/contains.h"
#include "base/strings/string_split.h"

namespace {
const char kNormalizedHeaderDelimiter[] = ",";
} // namespace

namespace digital_asset_links {

const char kEmbedderAncestorHeader[] = "X-Embedder-Ancestors";

// TODO(crbug.com/1376958): Also support fingerprints.
bool ResponseHeaderVerifier::Verify(
const std::string& package_name,
const std::string& embedder_ancestors_header_value) {
// No embedder-ancestor-header defaults to verified.
if (embedder_ancestors_header_value.empty()) {
// TODO(crbug.com/1376958): Set to false if undecided content should be
// treated like explicitly unconsenting content.
return true;
}

if (embedder_ancestors_header_value == "*") {
return true;
}
if (embedder_ancestors_header_value == "none") {
return false;
}

std::vector<std::string> allowed_package_names =
SplitString(embedder_ancestors_header_value, kNormalizedHeaderDelimiter,
base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);

if (base::Contains(allowed_package_names, package_name)) {
return true;
}

return false;
}

} // namespace digital_asset_links
23 changes: 23 additions & 0 deletions components/digital_asset_links/response_header_verifier.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_DIGITAL_ASSET_LINKS_RESPONSE_HEADER_VERIFIER_H_
#define COMPONENTS_DIGITAL_ASSET_LINKS_RESPONSE_HEADER_VERIFIER_H_

#include <string>

namespace digital_asset_links {

class ResponseHeaderVerifier {
public:
// Verify if the provided |package_name| is verified via the embedder
// ancestor header.
static bool Verify(const std::string& package_name,
const std::string& embedder_ancestors_header_value);
};

extern const char kEmbedderAncestorHeader[];
} // namespace digital_asset_links

#endif // COMPONENTS_DIGITAL_ASSET_LINKS_RESPONSE_HEADER_VERIFIER_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/digital_asset_links/response_header_verifier.h"

#include "testing/gtest/include/gtest/gtest.h"

namespace digital_asset_links {

TEST(ResponseHeaderVerifier, VerifyEmptyHeader) {
EXPECT_TRUE(ResponseHeaderVerifier::Verify("any.package.name", ""));
}

TEST(ResponseHeaderVerifier, VerifyStar) {
EXPECT_TRUE(ResponseHeaderVerifier::Verify("any.package.name", "*"));
}

TEST(ResponseHeaderVerifier, VerifyNone) {
EXPECT_FALSE(ResponseHeaderVerifier::Verify("any.package.name", "none"));
}

TEST(ResponseHeaderVerifier, VerifyListOfPackageNames) {
EXPECT_TRUE(ResponseHeaderVerifier::Verify(
"one.package", "one.package, two.package, three.package"));
EXPECT_TRUE(ResponseHeaderVerifier::Verify(
"two.package", "one.package, two.package, three.package"));
EXPECT_TRUE(ResponseHeaderVerifier::Verify(
"three.package", "one.package, two.package, three.package"));

EXPECT_FALSE(ResponseHeaderVerifier::Verify(
"unknown.package", "one.package, two.package, three.package"));
EXPECT_FALSE(
ResponseHeaderVerifier::Verify("any.package", "any.package.name"));

// 'none' and '*' get ignored if package names are listed.
EXPECT_TRUE(ResponseHeaderVerifier::Verify("a.package", "none, a.package"));
EXPECT_FALSE(
ResponseHeaderVerifier::Verify("another.package", "*, a.package"));
}

} // namespace digital_asset_links
1 change: 1 addition & 0 deletions weblayer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ source_set("weblayer_lib_base") {
"//components/crash/content/browser",
"//components/crash/core/app",
"//components/crash/core/common",
"//components/digital_asset_links",
"//components/download/content/factory",
"//components/download/content/public",
"//components/download/public/background_service:public",
Expand Down
1 change: 1 addition & 0 deletions weblayer/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ include_rules = [
"+components/content_settings/core/common",
"+components/crash/content/browser",
"+components/crash/core/common",
"+components/digital_asset_links",
"+components/download/content/factory",
"+components/download/content/public",
"+components/download/public/background_service",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import static org.chromium.content_public.browser.test.util.TestThreadUtils.runOnUiThreadBlocking;

import android.util.Pair;

import androidx.test.filters.SmallTest;

import com.google.common.util.concurrent.FutureCallback;
Expand All @@ -24,6 +26,8 @@
import org.chromium.webengine.RestrictedAPIException;
import org.chromium.webengine.Tab;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;

/**
Expand All @@ -32,6 +36,10 @@
@DoNotBatch(reason = "Tests need separate Activities and WebFragments")
@RunWith(WebEngineJUnit4ClassRunner.class)
public class ExecuteScriptTest {
private static final String sRequestPath = "/page.html";
private static final String sResponseString =
"<html><head></head><body>contents!</body><script>window.foo = 42;</script></html>";

@Rule
public InstrumentationActivityTestRule mActivityTestRule =
new InstrumentationActivityTestRule();
Expand All @@ -47,9 +55,9 @@ public void setUp() throws Exception {
mServer = mDALServerRule.getServer();
mActivityTestRule.launchShell();

mDefaultUrl = mServer.setResponse("/page.html",
"<html><head></head><body>contents!</body><script>window.foo = 42;</script></html>",
null);
List<Pair<String, String>> embedderAncestorHeader = new ArrayList();
embedderAncestorHeader.add(new Pair("X-embedder-ancestors", "none"));
mDefaultUrl = mServer.setResponse(sRequestPath, sResponseString, embedderAncestorHeader);
}

@After
Expand Down Expand Up @@ -94,7 +102,7 @@ public void onFailure(Throwable thrown) {

@Test
@SmallTest
public void executeScriptSucceedsWithVerification() throws Exception {
public void executeScriptSucceedsWithDALVerification() throws Exception {
mDALServerRule.setUpDigitalAssetLinks();
Tab activeTab = navigate();

Expand All @@ -103,6 +111,31 @@ public void executeScriptSucceedsWithVerification() throws Exception {
Assert.assertEquals(future.get(), "2");
}

@Test
@SmallTest
public void executeScriptSucceedsWithHeaderVerification() throws Exception {
List<Pair<String, String>> embedderAncestorHeader = new ArrayList();
embedderAncestorHeader.add(
new Pair("X-embedder-ancestors", mActivityTestRule.getPackageName()));
mDefaultUrl = mServer.setResponse(sRequestPath, sResponseString, embedderAncestorHeader);
Tab activeTab = navigate();

ListenableFuture<String> future =
runOnUiThreadBlocking(() -> activeTab.executeScript("1+1", true));
Assert.assertEquals(future.get(), "2");
}

@Test
@SmallTest
public void executeScriptSucceedsWithoutHeader() throws Exception {
mDefaultUrl = mServer.setResponse(sRequestPath, sResponseString, null);
Tab activeTab = navigate();

ListenableFuture<String> future =
runOnUiThreadBlocking(() -> activeTab.executeScript("1+1", true));
Assert.assertEquals(future.get(), "2");
}

@Test
@SmallTest
public void useSeparateIsolate() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,8 @@ public void onNavigationFailed(Navigation navigation) {
public Context getContext() {
return getActivity();
}

public String getPackageName() {
return getActivity().getPackageName();
}
}
6 changes: 5 additions & 1 deletion weblayer/browser/browser_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -419,15 +419,18 @@ void BrowserImpl::UpdateFragmentResumedState(bool state) {
// friended, as it requires browser_impl.h to include BrowserImpl_jni.h, which
// is problematic (meaning not really supported and generates compile errors).
BrowserImpl* CreateBrowserForAndroid(ProfileImpl* profile,
const std::string& package_name,
const JavaParamRef<jobject>& java_impl) {
BrowserImpl* browser = new BrowserImpl(profile);
browser->java_impl_ = java_impl;
browser->package_name_ = package_name;
return browser;
}

static jlong JNI_BrowserImpl_CreateBrowser(
JNIEnv* env,
jlong profile,
const base::android::JavaParamRef<jstring>& package_name,
const JavaParamRef<jobject>& java_impl) {
// The android side does not trigger restore from the constructor as at the
// time this is called not enough of WebLayer has been wired up. Specifically,
Expand All @@ -436,7 +439,8 @@ static jlong JNI_BrowserImpl_CreateBrowser(
// fully created, leading to all sort of assertions if Tabs are created
// and/or navigations start (which restore may trigger).
return reinterpret_cast<intptr_t>(CreateBrowserForAndroid(
reinterpret_cast<ProfileImpl*>(profile), java_impl));
reinterpret_cast<ProfileImpl*>(profile),
base::android::ConvertJavaStringToUTF8(env, package_name), java_impl));
}

static void JNI_BrowserImpl_DeleteBrowser(JNIEnv* env, jlong browser) {
Expand Down
4 changes: 4 additions & 0 deletions weblayer/browser/browser_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class BrowserImpl : public Browser {
// Called from BrowserPersister when restore has completed.
void OnRestoreCompleted();

const std::string& GetPackageName() { return package_name_; }

#if BUILDFLAG(IS_ANDROID)
bool CompositorHasSurface();

Expand Down Expand Up @@ -138,6 +140,7 @@ class BrowserImpl : public Browser {
#if BUILDFLAG(IS_ANDROID)
friend BrowserImpl* CreateBrowserForAndroid(
ProfileImpl*,
const std::string&,
const base::android::JavaParamRef<jobject>&);
#endif

Expand Down Expand Up @@ -168,6 +171,7 @@ class BrowserImpl : public Browser {
std::unique_ptr<BrowserPersister> browser_persister_;
base::OnceClosure visible_security_state_changed_callback_for_tests_;
PrefChangeRegistrar profile_pref_change_registrar_;
std::string package_name_;
};

} // namespace weblayer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ public BrowserImpl(Context serviceContext, ProfileManager profileManager, Bundle
mFullPersistenceInfo.mPersistenceId = persistenceId;
}

mNativeBrowser = BrowserImplJni.get().createBrowser(mProfile.getNativeProfile(), this);
mNativeBrowser = BrowserImplJni.get().createBrowser(
mProfile.getNativeProfile(), serviceContext.getPackageName(), this);
mPasswordEchoEnabled = null;

mBrowserFragmentImpl = new BrowserFragmentImpl(this, serviceContext);
Expand Down Expand Up @@ -406,7 +407,7 @@ public BrowserFragmentImpl getBrowserFragment() {

@NativeMethods
interface Natives {
long createBrowser(long profile, BrowserImpl caller);
long createBrowser(long profile, String packageName, BrowserImpl caller);
void deleteBrowser(long browser);
void addTab(long nativeBrowserImpl, long nativeTab);
TabImpl[] getTabs(long nativeBrowserImpl);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.components.embedder_support.util.WebResourceResponseInfo;
import org.chromium.weblayer_private.TabImpl.HeaderVerificationStatus;
import org.chromium.weblayer_private.interfaces.APICallException;
import org.chromium.weblayer_private.interfaces.IClientPage;
import org.chromium.weblayer_private.interfaces.INavigateParams;
Expand Down Expand Up @@ -47,6 +48,7 @@ public NavigationControllerImpl(TabImpl tab, INavigationControllerClient client)
@Override
public void navigate(String uri, NavigateParams params) {
StrictModeWorkaround.apply();
mTab.setHeaderVerification(HeaderVerificationStatus.PENDING);
if (WebLayerFactoryImpl.getClientMajorVersion() < 83) {
assert params == null;
}
Expand Down Expand Up @@ -211,6 +213,7 @@ private NavigationImpl createNavigation(long nativeNavigationImpl) {

@CalledByNative
private void navigationStarted(NavigationImpl navigation) throws RemoteException {
mTab.setHeaderVerification(HeaderVerificationStatus.PENDING);
mNavigationControllerClient.navigationStarted(navigation.getClientNavigation());
}

Expand All @@ -231,6 +234,11 @@ private void getOrCreatePageForNavigation(NavigationImpl navigation) throws Remo

@CalledByNative
private void navigationCompleted(NavigationImpl navigation) throws RemoteException {
if (navigation.getIsConsentingContent()) {
mTab.setHeaderVerification(HeaderVerificationStatus.VALIDATED);
} else {
mTab.setHeaderVerification(HeaderVerificationStatus.NOT_VALIDATED);
}
mNavigationControllerClient.navigationCompleted(navigation.getClientNavigation());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ public List<String> getResponseHeaders() {
return Arrays.asList(NavigationImplJni.get().getResponseHeaders(mNativeNavigationImpl));
}

public boolean getIsConsentingContent() {
return NavigationImplJni.get().getIsConsentingContent(mNativeNavigationImpl);
}

@Override
public boolean isSameDocument() {
StrictModeWorkaround.apply();
Expand Down Expand Up @@ -320,6 +324,7 @@ interface Natives {
String[] getRedirectChain(long nativeNavigationImpl);
int getHttpStatusCode(long nativeNavigationImpl);
String[] getResponseHeaders(long nativeNavigationImpl);
boolean getIsConsentingContent(long nativeNavigationImpl);
boolean isSameDocument(long nativeNavigationImpl);
boolean isErrorPage(long nativeNavigationImpl);
boolean isDownload(long nativeNavigationImpl);
Expand Down

0 comments on commit d9efaf8

Please sign in to comment.