Commit fceff3a2 authored by Karandeep Bhatia's avatar Karandeep Bhatia Committed by Commit Bot

IsolatedWorldCSP: Enforce isolated world CSP for fetches.

Enforce isolated world CSP for fetches from isolated worlds instead of
just bypassing the main world CSP. This also helps fix a bug where a
redirected fetch from a content script doesn't correctly bypass the main
world CSP. Add a regression test for the same.

BUG=934819, 1099975

Change-Id: I3c471464274f1657ef3c08378f06a3963f95a0aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2278626
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786595}
parent 1b6ea0be
...@@ -328,6 +328,77 @@ IN_PROC_BROWSER_TEST_F(ContentScriptApiTest, ...@@ -328,6 +328,77 @@ IN_PROC_BROWSER_TEST_F(ContentScriptApiTest,
ASSERT_TRUE(scripts_injected_twice); ASSERT_TRUE(scripts_injected_twice);
} }
// Tests that fetches made by content scripts are exempt from the page's CSP.
// Regression test for crbug.com/934819.
IN_PROC_BROWSER_TEST_F(ContentScriptApiTest, FetchExemptFromCSP) {
ASSERT_TRUE(StartEmbeddedTestServer());
// Create and load an extension that will inject a content script which does a
// fetch based on the host document's "fetchUrl" url search parameter.
constexpr char kManifest[] =
R"(
{
"name":"Fetch redirect test",
"version":"0.0.1",
"manifest_version": 2,
"content_scripts": [
{
"matches": ["*://bar.com/*"],
"js": ["content_script.js"],
"run_at": "document_start"
}
]
})";
constexpr char kContentScript[] = R"(
let params = (new URL(document.location)).searchParams;
let fetchUrl = params.get('fetchUrl');
fetch(fetchUrl)
.then(response => response.text())
.then(text => chrome.test.sendMessage(text))
.catch(error => chrome.test.sendMessage(error.message));
)";
TestExtensionDir test_dir;
test_dir.WriteManifest(kManifest);
test_dir.WriteFile(FILE_PATH_LITERAL("content_script.js"), kContentScript);
ASSERT_TRUE(LoadExtension(test_dir.UnpackedPath()));
ExtensionTestMessageListener listener(false /* will_reply */);
// The fetch will undergo a redirect. Note that the fetched file sets the
// "Access-Control-Allow-Origin: *" header to allow for cross origin access.
GURL fetch_url =
embedded_test_server()->GetURL("foo.com", "/extensions/xhr.txt");
GURL redirect_url = embedded_test_server()->GetURL(
"bar.com", "/server-redirect?" + fetch_url.spec());
// Navigate to a page with a CSP set that prevents resources from other
// origins to be loaded and wait for a response from the content script.
GURL csp_page_url = embedded_test_server()->GetURL(
"bar.com",
"/extensions/page_with_csp.html?fetchUrl=" + redirect_url.spec());
ui_test_utils::NavigateToURL(browser(), csp_page_url);
// Ensure the fetch is exempt from the page CSP and succeeds.
ASSERT_TRUE(listener.WaitUntilSatisfied());
EXPECT_EQ("File to request via XHR.\n", listener.message());
// Sanity check that fetching a url which doesn't allow cross origin access
// fails.
listener.Reset();
fetch_url =
embedded_test_server()->GetURL("foo.com", "/extensions/test_file.txt");
redirect_url = embedded_test_server()->GetURL(
"bar.com", "/server-redirect?" + fetch_url.spec());
csp_page_url = embedded_test_server()->GetURL(
"bar.com",
"/extensions/page_with_csp.html?fetchUrl=" + redirect_url.spec());
ui_test_utils::NavigateToURL(browser(), csp_page_url);
ASSERT_TRUE(listener.WaitUntilSatisfied());
EXPECT_EQ("Failed to fetch", listener.message());
}
class ContentScriptCssInjectionTest : public ExtensionApiTest { class ContentScriptCssInjectionTest : public ExtensionApiTest {
protected: protected:
// TODO(rdevlin.cronin): Make a testing switch that looks like FeatureSwitch, // TODO(rdevlin.cronin): Make a testing switch that looks like FeatureSwitch,
......
<!DOCTYPE html>
<meta http-equiv="Content-Security-Policy" content="connect-src 'none';">
<title>Page With CSP</title>
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <utility> #include <utility>
#include "base/check.h"
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/pending_remote.h"
...@@ -99,7 +100,7 @@ class FetchManager::Loader final ...@@ -99,7 +100,7 @@ class FetchManager::Loader final
FetchManager*, FetchManager*,
ScriptPromiseResolver*, ScriptPromiseResolver*,
FetchRequestData*, FetchRequestData*,
bool is_isolated_world, scoped_refptr<const DOMWrapperWorld>,
AbortSignal*); AbortSignal*);
~Loader() override; ~Loader() override;
void Trace(Visitor*) const override; void Trace(Visitor*) const override;
...@@ -245,7 +246,7 @@ class FetchManager::Loader final ...@@ -245,7 +246,7 @@ class FetchManager::Loader final
int response_http_status_code_; int response_http_status_code_;
bool response_has_no_store_header_ = false; bool response_has_no_store_header_ = false;
Member<SRIVerifier> integrity_verifier_; Member<SRIVerifier> integrity_verifier_;
bool is_isolated_world_; scoped_refptr<const DOMWrapperWorld> world_;
Member<AbortSignal> signal_; Member<AbortSignal> signal_;
Vector<KURL> url_list_; Vector<KURL> url_list_;
Member<ExecutionContext> execution_context_; Member<ExecutionContext> execution_context_;
...@@ -255,7 +256,7 @@ FetchManager::Loader::Loader(ExecutionContext* execution_context, ...@@ -255,7 +256,7 @@ FetchManager::Loader::Loader(ExecutionContext* execution_context,
FetchManager* fetch_manager, FetchManager* fetch_manager,
ScriptPromiseResolver* resolver, ScriptPromiseResolver* resolver,
FetchRequestData* fetch_request_data, FetchRequestData* fetch_request_data,
bool is_isolated_world, scoped_refptr<const DOMWrapperWorld> world,
AbortSignal* signal) AbortSignal* signal)
: fetch_manager_(fetch_manager), : fetch_manager_(fetch_manager),
resolver_(resolver), resolver_(resolver),
...@@ -264,9 +265,10 @@ FetchManager::Loader::Loader(ExecutionContext* execution_context, ...@@ -264,9 +265,10 @@ FetchManager::Loader::Loader(ExecutionContext* execution_context,
finished_(false), finished_(false),
response_http_status_code_(0), response_http_status_code_(0),
integrity_verifier_(nullptr), integrity_verifier_(nullptr),
is_isolated_world_(is_isolated_world), world_(std::move(world)),
signal_(signal), signal_(signal),
execution_context_(execution_context) { execution_context_(execution_context) {
DCHECK(world_);
url_list_.push_back(fetch_request_data->Url()); url_list_.push_back(fetch_request_data->Url());
} }
...@@ -548,7 +550,7 @@ void FetchManager::Loader::Start(ExceptionState& exception_state) { ...@@ -548,7 +550,7 @@ void FetchManager::Loader::Start(ExceptionState& exception_state) {
// "- should fetching |request| be blocked as content security returns // "- should fetching |request| be blocked as content security returns
// blocked" // blocked"
if (!execution_context_->GetContentSecurityPolicyForCurrentWorld() if (!execution_context_->GetContentSecurityPolicyForWorld(world_.get())
->AllowConnectToSource(fetch_request_data_->Url(), ->AllowConnectToSource(fetch_request_data_->Url(),
fetch_request_data_->Url(), fetch_request_data_->Url(),
RedirectStatus::kNoRedirect)) { RedirectStatus::kNoRedirect)) {
...@@ -764,7 +766,7 @@ void FetchManager::Loader::PerformHTTPFetch(ExceptionState& exception_state) { ...@@ -764,7 +766,7 @@ void FetchManager::Loader::PerformHTTPFetch(ExceptionState& exception_state) {
request.SetReferrerString(fetch_request_data_->ReferrerString()); request.SetReferrerString(fetch_request_data_->ReferrerString());
request.SetReferrerPolicy(fetch_request_data_->GetReferrerPolicy()); request.SetReferrerPolicy(fetch_request_data_->GetReferrerPolicy());
request.SetSkipServiceWorker(is_isolated_world_); request.SetSkipServiceWorker(world_->IsIsolatedWorld());
if (fetch_request_data_->Keepalive()) { if (fetch_request_data_->Keepalive()) {
if (!RuntimeEnabledFeatures::OutOfBlinkCorsEnabled() && if (!RuntimeEnabledFeatures::OutOfBlinkCorsEnabled() &&
...@@ -795,6 +797,7 @@ void FetchManager::Loader::PerformHTTPFetch(ExceptionState& exception_state) { ...@@ -795,6 +797,7 @@ void FetchManager::Loader::PerformHTTPFetch(ExceptionState& exception_state) {
// and the |CORS flag| is unset, and unset otherwise." // and the |CORS flag| is unset, and unset otherwise."
ResourceLoaderOptions resource_loader_options; ResourceLoaderOptions resource_loader_options;
resource_loader_options.world = world_;
resource_loader_options.initiator_info.name = resource_loader_options.initiator_info.name =
fetch_initiator_type_names::kFetch; fetch_initiator_type_names::kFetch;
resource_loader_options.data_buffering_policy = kDoNotBufferData; resource_loader_options.data_buffering_policy = kDoNotBufferData;
...@@ -891,9 +894,9 @@ ScriptPromise FetchManager::Fetch(ScriptState* script_state, ...@@ -891,9 +894,9 @@ ScriptPromise FetchManager::Fetch(ScriptState* script_state,
request->SetContext(mojom::RequestContextType::FETCH); request->SetContext(mojom::RequestContextType::FETCH);
request->SetDestination(network::mojom::RequestDestination::kEmpty); request->SetDestination(network::mojom::RequestDestination::kEmpty);
auto* loader = MakeGarbageCollected<Loader>( auto* loader =
GetExecutionContext(), this, resolver, request, MakeGarbageCollected<Loader>(GetExecutionContext(), this, resolver,
script_state->World().IsIsolatedWorld(), signal); request, &script_state->World(), signal);
loaders_.insert(loader); loaders_.insert(loader);
signal->AddAlgorithm(WTF::Bind(&Loader::Abort, WrapWeakPersistent(loader))); signal->AddAlgorithm(WTF::Bind(&Loader::Abort, WrapWeakPersistent(loader)));
// TODO(ricea): Reject the Response body with AbortError, not TypeError. // TODO(ricea): Reject the Response body with AbortError, not TypeError.
......
...@@ -11,15 +11,11 @@ CONSOLE MESSAGE: PASS: Request blocked by CSP as expected. ...@@ -11,15 +11,11 @@ CONSOLE MESSAGE: PASS: Request blocked by CSP as expected.
CONSOLE MESSAGE: line 65: Testing isolated world with permissive csp. CONSOLE MESSAGE: line 65: Testing isolated world with permissive csp.
CONSOLE MESSAGE: PASS: Request succeeded as expected. CONSOLE MESSAGE: PASS: Request succeeded as expected.
CONSOLE MESSAGE: line 71: Testing fetch redirect in isolated world with permissive csp. CONSOLE MESSAGE: line 71: Testing fetch redirect in isolated world with permissive csp.
CONSOLE ERROR: Refused to connect to 'http://127.0.0.1:8000/security/isolatedWorld/resources/access_control_allow_origin.php' because it violates the following Content Security Policy directive: "connect-src 'none'". CONSOLE MESSAGE: PASS: Request succeeded as expected.
CONSOLE MESSAGE: FAIL: Request failed unexpectedly.
CONSOLE MESSAGE: line 78: Testing isolated world with strict csp. CONSOLE MESSAGE: line 78: Testing isolated world with strict csp.
CONSOLE MESSAGE: line 79: internals.runtimeFlags.isolatedWorldCSPEnabled is false CONSOLE MESSAGE: line 79: internals.runtimeFlags.isolatedWorldCSPEnabled is false
CONSOLE MESSAGE: PASS: Request succeeded as expected. CONSOLE MESSAGE: PASS: Request succeeded as expected.
CONSOLE MESSAGE: line 91: Testing fetch redirect in isolated world with strict csp. CONSOLE MESSAGE: line 91: Testing fetch redirect in isolated world with strict csp.
CONSOLE MESSAGE: line 92: internals.runtimeFlags.isolatedWorldCSPEnabled is false CONSOLE MESSAGE: line 92: internals.runtimeFlags.isolatedWorldCSPEnabled is false
CONSOLE ERROR: Refused to connect to 'http://127.0.0.1:8000/security/isolatedWorld/resources/access_control_allow_origin.php' because it violates the following Content Security Policy directive: "connect-src 'none'". CONSOLE MESSAGE: PASS: Request succeeded as expected.
CONSOLE MESSAGE: FAIL: Request failed unexpectedly.
This tests the interaction of the fetch API run in the isolated world with the isolated world CSP. This tests the interaction of the fetch API run in the isolated world with the isolated world CSP.
...@@ -96,9 +96,6 @@ const tests = [ ...@@ -96,9 +96,6 @@ const tests = [
testRunner.setIsolatedWorldInfo( testRunner.setIsolatedWorldInfo(
isolatedWorldId, isolatedWorldSecurityOrigin, 'connect-src \'self\''); isolatedWorldId, isolatedWorldSecurityOrigin, 'connect-src \'self\'');
testFetchInIsolatedWorld(expectBlocked, true /* redirect */); testFetchInIsolatedWorld(expectBlocked, true /* redirect */);
// Clear the isolated world data.
testRunner.setIsolatedWorldInfo(1, null, null);
}, },
]; ];
......
...@@ -11,9 +11,7 @@ CONSOLE MESSAGE: PASS: Request blocked by CSP as expected. ...@@ -11,9 +11,7 @@ CONSOLE MESSAGE: PASS: Request blocked by CSP as expected.
CONSOLE MESSAGE: line 65: Testing isolated world with permissive csp. CONSOLE MESSAGE: line 65: Testing isolated world with permissive csp.
CONSOLE MESSAGE: PASS: Request succeeded as expected. CONSOLE MESSAGE: PASS: Request succeeded as expected.
CONSOLE MESSAGE: line 71: Testing fetch redirect in isolated world with permissive csp. CONSOLE MESSAGE: line 71: Testing fetch redirect in isolated world with permissive csp.
CONSOLE ERROR: Refused to connect to 'http://127.0.0.1:8000/security/isolatedWorld/resources/access_control_allow_origin.php' because it violates the following Content Security Policy directive: "connect-src 'none'". CONSOLE MESSAGE: PASS: Request succeeded as expected.
CONSOLE MESSAGE: FAIL: Request failed unexpectedly.
CONSOLE MESSAGE: line 78: Testing isolated world with strict csp. CONSOLE MESSAGE: line 78: Testing isolated world with strict csp.
CONSOLE MESSAGE: line 79: internals.runtimeFlags.isolatedWorldCSPEnabled is true CONSOLE MESSAGE: line 79: internals.runtimeFlags.isolatedWorldCSPEnabled is true
CONSOLE ERROR: Refused to connect to 'http://127.0.0.1:8000/security/isolatedWorld/resources/access_control_allow_origin.php' because it violates the following Content Security Policy directive: "connect-src 'self'". CONSOLE ERROR: Refused to connect to 'http://127.0.0.1:8000/security/isolatedWorld/resources/access_control_allow_origin.php' because it violates the following Content Security Policy directive: "connect-src 'self'".
......
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