Commit 8e12bd32 authored by Shivani Sharma's avatar Shivani Sharma Committed by Commit Bot

Compute network isolation key for custom tabs detached resource requests

This CL populates the network isolation key for custom tabs detached
resource requests. It is computed using the |site_for_cookies_| since
that represents the origin of the app. The origin is verified using
digital asset links verification and so should be fine to use here.
The existing tests impacted by this change are:
org.chromium.chrome.browser.customtabs.DetachedResourceRequestTest
#testSafeBrowsingMainResource
and
org.chromium.chrome.browser.customtabs.DetachedResourceRequestTest
#testSafeBrowsingMainResourceBeforeNative

Change-Id: Ia05916f1da39ec4628c51dd4de5eeb42da78d820
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854685
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: default avatarBenoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710786}
parent 63fa05e8
...@@ -367,6 +367,8 @@ public abstract class ChromeFeatureList { ...@@ -367,6 +367,8 @@ public abstract class ChromeFeatureList {
public static final String WEB_PAYMENTS_SINGLE_APP_UI_SKIP = "WebPaymentsSingleAppUiSkip"; public static final String WEB_PAYMENTS_SINGLE_APP_UI_SKIP = "WebPaymentsSingleAppUiSkip";
public static final String SERVICE_MANAGER_FOR_BACKGROUND_PREFETCH = public static final String SERVICE_MANAGER_FOR_BACKGROUND_PREFETCH =
"ServiceManagerForBackgroundPrefetch"; "ServiceManagerForBackgroundPrefetch";
public static final String SPLIT_CACHE_BY_NETWORK_ISOLATION_KEY =
"SplitCacheByNetworkIsolationKey";
@NativeMethods @NativeMethods
interface Natives { interface Natives {
......
...@@ -39,6 +39,7 @@ import org.chromium.chrome.browser.preferences.PrefServiceBridge; ...@@ -39,6 +39,7 @@ import org.chromium.chrome.browser.preferences.PrefServiceBridge;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures; import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.components.safe_browsing.SafeBrowsingApiBridge; import org.chromium.components.safe_browsing.SafeBrowsingApiBridge;
import org.chromium.content_public.browser.UiThreadTaskTraits; import org.chromium.content_public.browser.UiThreadTaskTraits;
...@@ -313,8 +314,20 @@ public class DetachedResourceRequestTest { ...@@ -313,8 +314,20 @@ public class DetachedResourceRequestTest {
*/ */
@Test @Test
@SmallTest @SmallTest
@DisableFeatures(ChromeFeatureList.SPLIT_CACHE_BY_NETWORK_ISOLATION_KEY)
public void testSafeBrowsingMainResource() throws Exception { public void testSafeBrowsingMainResource() throws Exception {
testSafeBrowsingMainResource(true); testSafeBrowsingMainResource(true /* afterNative */, false /* splitCacheEnabled */);
}
/**
* Tests that non-cached detached resource requests that are forbidden by SafeBrowsing don't end
* up in the content area, for a main resource.
*/
@Test
@SmallTest
@EnableFeatures(ChromeFeatureList.SPLIT_CACHE_BY_NETWORK_ISOLATION_KEY)
public void testSafeBrowsingMainResourceWithSplitCache() throws Exception {
testSafeBrowsingMainResource(true /* afterNative */, true /* splitCacheEnabled */);
} }
/** /**
...@@ -333,8 +346,9 @@ public class DetachedResourceRequestTest { ...@@ -333,8 +346,9 @@ public class DetachedResourceRequestTest {
*/ */
@Test @Test
@SmallTest @SmallTest
@DisableFeatures(ChromeFeatureList.SPLIT_CACHE_BY_NETWORK_ISOLATION_KEY)
public void testSafeBrowsingMainResourceBeforeNative() throws Exception { public void testSafeBrowsingMainResourceBeforeNative() throws Exception {
testSafeBrowsingMainResource(false); testSafeBrowsingMainResource(false /* afterNative */, false /* splitCacheEnabled */);
} }
/** /**
...@@ -531,7 +545,8 @@ public class DetachedResourceRequestTest { ...@@ -531,7 +545,8 @@ public class DetachedResourceRequestTest {
Assert.assertEquals("\"acookie\"", content); Assert.assertEquals("\"acookie\"", content);
} }
private void testSafeBrowsingMainResource(boolean afterNative) throws Exception { private void testSafeBrowsingMainResource(boolean afterNative, boolean splitCacheEnabled)
throws Exception {
SafeBrowsingApiBridge.setSafeBrowsingHandlerType( SafeBrowsingApiBridge.setSafeBrowsingHandlerType(
new MockSafeBrowsingApiHandler().getClass()); new MockSafeBrowsingApiHandler().getClass());
CustomTabsSessionToken session = prepareSession(); CustomTabsSessionToken session = prepareSession();
...@@ -558,9 +573,18 @@ public class DetachedResourceRequestTest { ...@@ -558,9 +573,18 @@ public class DetachedResourceRequestTest {
CriteriaHelper.pollUiThread( CriteriaHelper.pollUiThread(
() -> tab.getWebContents().getTitle().equals("Security error")); () -> tab.getWebContents().getTitle().equals("Security error"));
// 1 read from the detached request, and 0 from the page load, as if (splitCacheEnabled) {
// the response comes from the cache, and SafeBrowsing blocks it. // Note that since the SplitCacheByNetworkIsolationKey feature is
Assert.assertEquals(1, readFromSocketCallback.getCallCount()); // enabled, the detached request and the original request both
// would be read from the socket as they both would have different
// top frame origins in the cache partitioning key: |ORIGIN| and
// mServer's base url, respectively.
Assert.assertEquals(2, readFromSocketCallback.getCallCount());
} else {
// 1 read from the detached request, and 0 from the page load, as
// the response comes from the cache, and SafeBrowsing blocks it.
Assert.assertEquals(1, readFromSocketCallback.getCallCount());
}
} finally { } finally {
MockSafeBrowsingApiHandler.clearMockResponses(); MockSafeBrowsingApiHandler.clearMockResponses();
} }
......
...@@ -83,7 +83,18 @@ DetachedResourceRequest::DetachedResourceRequest( ...@@ -83,7 +83,18 @@ DetachedResourceRequest::DetachedResourceRequest(
referrer_policy, site_for_cookies_, url_); referrer_policy, site_for_cookies_, url_);
resource_request->referrer_policy = referrer_policy; resource_request->referrer_policy = referrer_policy;
resource_request->site_for_cookies = site_for_cookies_; resource_request->site_for_cookies = site_for_cookies_;
resource_request->request_initiator = url::Origin::Create(site_for_cookies_);
url::Origin site_for_cookies_origin = url::Origin::Create(site_for_cookies_);
resource_request->request_initiator = site_for_cookies_origin;
// Since |site_for_cookies_| has gone through digital asset links
// verification, it should be ok to use it to compute the network isolation
// key.
resource_request->trusted_params = network::ResourceRequest::TrustedParams();
resource_request->trusted_params->network_isolation_key =
net::NetworkIsolationKey(site_for_cookies_origin,
site_for_cookies_origin);
resource_request->resource_type = resource_request->resource_type =
static_cast<int>(content::ResourceType::kSubResource); static_cast<int>(content::ResourceType::kSubResource);
resource_request->do_not_prompt_for_login = true; resource_request->do_not_prompt_for_login = true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment