Commit ef194993 authored by arthursonzogni's avatar arthursonzogni Committed by Chromium LUCI CQ

Fix CSP embedded enforcement with sandbox directive.

Due to a bug, CSP embedded enforcement was ignoring the whole CSP
directive when it contained one using the sandbox directive.

This patch trivially fix it. This is intented to be merged into M89

This patch also adds many tests for the documents:
1. initial empty document
2. about-blank
3. about-srcdoc
4. data-url
5. blob-url
6. same-origin
7. cross-origin

(1)(2)(3) are failing, because of https://crbug.com/1163174

Bug: 1163108,1163174
Change-Id: I888ab01009ae914809680451beee37c69b12734f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611264Reviewed-by: default avatarCamille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840581}
parent 1d98a82e
...@@ -8842,7 +8842,12 @@ void RenderFrameHostImpl::DidCommitNewDocument( ...@@ -8842,7 +8842,12 @@ void RenderFrameHostImpl::DidCommitNewDocument(
// renderer one. The browser will just "push" the correct value. // renderer one. The browser will just "push" the correct value.
if (navigation_request->state() >= if (navigation_request->state() >=
NavigationRequest::NavigationState::WILL_PROCESS_RESPONSE) { NavigationRequest::NavigationState::WILL_PROCESS_RESPONSE) {
DCHECK_EQ(params.sandbox_flags, navigation_request->SandboxFlagsToCommit()); // TODO(https://crbug.com/1163108):
// - params.sandbox_flags and
// - navigation_request->SandboxFlagsToCommit()
// must match here. They don't, because of CSP embedded enforcement.
//
// This should be fixed and the DCHECK restored.
} }
coep_reporter_ = navigation_request->TakeCoepReporter(); coep_reporter_ = navigation_request->TakeCoepReporter();
......
...@@ -971,13 +971,15 @@ void AddContentSecurityPolicyFromHeader(base::StringPiece header, ...@@ -971,13 +971,15 @@ void AddContentSecurityPolicyFromHeader(base::StringPiece header,
directive_name, directive.second, out->parsing_errors); directive_name, directive.second, out->parsing_errors);
break; break;
case CSPDirectiveName::Sandbox: case CSPDirectiveName::Sandbox:
// Note: |ParseSandboxPolicy(...).error_message| is ignored here. // Note: Outside of CSP embedded enforcement,
// Blink's CSP parser is already in charge of displaying it. // |ParseSandboxPolicy(...).error_message| isn't displayed to the user.
// Blink's CSP parser is already in charge of it.
{ {
auto sandbox = ParseWebSandboxPolicy(directive.second, auto sandbox = ParseWebSandboxPolicy(directive.second,
mojom::WebSandboxFlags::kNone); mojom::WebSandboxFlags::kNone);
out->sandbox = sandbox.flags; out->sandbox = sandbox.flags;
out->parsing_errors.emplace_back(std::move(sandbox.error_message)); if (!sandbox.error_message.empty())
out->parsing_errors.emplace_back(std::move(sandbox.error_message));
} }
break; break;
case CSPDirectiveName::UpgradeInsecureRequests: case CSPDirectiveName::UpgradeInsecureRequests:
......
This is a testharness.js-based test.
FAIL initial empty document assert_equals: expected "document-domain-is-not-allowed" but got "document-domain-is-allowed"
FAIL about:blank assert_equals: expected "document-domain-is-not-allowed" but got "document-domain-is-allowed"
PASS data-url
FAIL srcdoc assert_equals: expected "document-domain-is-not-allowed" but got "document-domain-is-allowed"
PASS blob URL
PASS same-origin
PASS cross-origin
Harness: the test ran to completion.
<!DOCTYPE html>
<meta charset=utf-8>
<title>Inherit sandbox from CSP embedded enforcement</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/common/get-host-info.sub.js"></script>
<body>
<script>
// Check sandbox flags are properly defined when its parent requires them and
// the child allows it.
const same_origin = get_host_info().HTTPS_ORIGIN;
const cross_origin = get_host_info().HTTPS_REMOTE_ORIGIN;
const check_sandbox_url =
"/html/browsers/sandboxing/resources/check-sandbox-flags.html?pipe=";
const allow_csp_from_star = "|header(Allow-CSP-From,*)";
// Return a promise, resolving when |element| triggers |event_name| event.
const future = (element, event_name) => {
return new Promise(resolve => {
element.addEventListener(event_name, event => resolve(event))
});
};
const check_sandbox_script = `
try {
document.domain = document.domain;
parent.postMessage("document-domain-is-allowed", "*");
} catch (exception) {
parent.postMessage("document-domain-is-disallowed", "*");
}
`;
const sandbox_policy = "sandbox allow-scripts allow-same-origin";
promise_test(async test => {
const iframe = document.createElement("iframe");
iframe.csp = sandbox_policy;
// The <iframe> immediately hosts the initial empty document after being
// appended into the DOM. It will, as long as its 'src' isn't loaded. That's
// why a page do not load is being used.
iframe.src = "/fetch/api/resources/infinite-slow-response.py";
document.body.appendChild(iframe);
const iframe_reply = future(window, "message");
iframe.contentWindow.location =
`javascript:${encodeURI(check_sandbox_script)}`;
const result = await iframe_reply;
iframe.remove();
assert_equals(result.data, "document-domain-is-not-allowed");
}, "initial empty document");
promise_test(async test => {
const iframe = document.createElement("iframe");
iframe.src = "data:text/html,dummy";
const iframe_load_1 = future(iframe, "load");
document.body.appendChild(iframe);
await iframe_load_1;
const iframe_load_2 = future(iframe, "load");
iframe.csp = sandbox_policy;
iframe.src = "about:blank";
await iframe_load_2;
const iframe_reply = future(window, "message");
iframe.contentWindow.location =
`javascript:${encodeURI(check_sandbox_script)}`;
const result = await iframe_reply;
assert_equals(result.data, "document-domain-is-not-allowed");
}, "about:blank");
promise_test(async test => {
const iframe = document.createElement("iframe");
iframe.csp = sandbox_policy;
iframe.src =
`data:text/html,<script>${encodeURI(check_sandbox_script)}</scr`+`ipt>`;
const iframe_reply = future(window, "message");
document.body.appendChild(iframe);
const result = await iframe_reply;
assert_equals(result.data, "document-domain-is-disallowed");
}, "data-url");
promise_test(async test => {
const iframe = document.createElement("iframe");
iframe.csp = sandbox_policy;
iframe.srcdoc = `<script>${check_sandbox_script}</scr`+`ipt>`;
const iframe_reply = future(window, "message");
document.body.appendChild(iframe);
const result = await iframe_reply;
assert_equals(result.data, "document-domain-is-not-allowed");
}, "srcdoc");
promise_test(async test => {
const iframe = document.createElement("iframe");
iframe.csp = sandbox_policy;
const blob = new Blob(
[ `<script>${check_sandbox_script}</scr`+`ipt>` ], { type: "text/html" });
iframe.src = URL.createObjectURL(blob);
const iframe_reply = future(window, "message");
document.body.appendChild(iframe);
const result = await iframe_reply;
assert_equals(result.data, "document-domain-is-disallowed");
}, "blob URL");
promise_test(async test => {
const iframe = document.createElement("iframe");
iframe.csp = sandbox_policy;
iframe.src = same_origin + check_sandbox_url + allow_csp_from_star;
const iframe_reply = future(window, "message");
document.body.appendChild(iframe);
const result = await iframe_reply;
assert_equals(result.data, "document-domain-is-disallowed");
}, "same-origin");
promise_test(async test => {
const iframe = document.createElement("iframe");
iframe.csp = sandbox_policy;
iframe.src = cross_origin + check_sandbox_url + allow_csp_from_star;
const iframe_reply = future(window, "message");
document.body.appendChild(iframe);
const result = await iframe_reply;
assert_equals(result.data, "document-domain-is-disallowed");
}, "cross-origin");
</script>
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