Skip to content

GitLab

T
tangled
Commit c2b99265 authored by Domenic Denicola's avatar Domenic Denicola Committed by Commit Bot
Browse files
Options

Origin policy: update CSP parsing to match the latest spec 

This updates the parsing of the feature policy parts of the origin policy manifest to mostly match the latest spec draft at https://wicg.github.io/origin-policy/, in particular https://wicg.github.io/origin-policy/#parsing. That is, it moves away from "content-security-policy": [{ "policy": "string", "report-only": boolean }] to "content_security": { "policies": ["...CSP strings"], "policies_report_only": ["...CSP strings"] }.

Additionally, it removes the failure on parsing errors, as those are no longer in the spec.

This does not yet properly parse the CSP string as a CSP; instead it still treats it as a header (so, commas are allowed inside). A failing test is added for that case, which will be addressed in a followup CL.

Bug: 751996
Change-Id: I8d14815b486afd4a5622bc4b25874c81418fd38c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1977148
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarDaniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732666}
parent 0d3ebf3b
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 495 additions and 151 deletions
services/network/origin_policy/origin_policy_parser.cc
View file @ c2b99265
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/optional.h"
#include "base/values.h" #include "base/values.h"
#include "services/network/public/cpp/isolation_opt_in_hints.h" #include "services/network/public/cpp/isolation_opt_in_hints.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -19,30 +20,26 @@ namespace network { ...@@ -19,30 +20,26 @@ namespace network {
OriginPolicyContentsPtr OriginPolicyParser::Parse(base::StringPiece text) { OriginPolicyContentsPtr OriginPolicyParser::Parse(base::StringPiece text) {
OriginPolicyParser parser; OriginPolicyParser parser;
if (!parser.DoParse(text)) parser.DoParse(text);
return std::make_unique<OriginPolicyContents>();
return std::move(parser.policy_contents_); return std::move(parser.policy_contents_);
} }
OriginPolicyParser::OriginPolicyParser() {} OriginPolicyParser::OriginPolicyParser() = default;
OriginPolicyParser::~OriginPolicyParser() {} OriginPolicyParser::~OriginPolicyParser() = default;
bool OriginPolicyParser::DoParse(base::StringPiece policy_contents_text) { void OriginPolicyParser::DoParse(base::StringPiece policy_contents_text) {
if (policy_contents_text.empty()) policy_contents_ = std::make_unique<OriginPolicyContents>();
return false;
std::unique_ptr<base::Value> json = base::Optional<base::Value> json =
base::JSONReader::ReadDeprecated(policy_contents_text); base::JSONReader::Read(policy_contents_text);
if (!json || !json->is_dict()) if (!json || !json->is_dict())
return false; return;
policy_contents_ = std::make_unique<OriginPolicyContents>();
base::Value* csp = json->FindListKey("content-security-policy"); if (base::Value* content_security = json->FindDictKey("content_security")) {
bool csp_ok = !csp || ParseContentSecurityPolicies(*csp); ParseContentSecurity(*content_security);
}
base::Value* features = json->FindDictKey("features"); if (base::Value* features = json->FindDictKey("features")) {
if (features) {
ParseFeatures(*features); ParseFeatures(*features);
} }
...@@ -53,33 +50,30 @@ bool OriginPolicyParser::DoParse(base::StringPiece policy_contents_text) { ...@@ -53,33 +50,30 @@ bool OriginPolicyParser::DoParse(base::StringPiece policy_contents_text) {
} else if (base::Value* isolation = json->FindDictKey("isolation")) { } else if (base::Value* isolation = json->FindDictKey("isolation")) {
ParseIsolation(*isolation); ParseIsolation(*isolation);
} }
return csp_ok;
} }
bool OriginPolicyParser::ParseContentSecurityPolicies( void OriginPolicyParser::ParseContentSecurity(
const base::Value& policies) { const base::Value& content_security) {
bool ok = true; const base::Value* policies = content_security.FindListKey("policies");
for (const auto& csp : policies.GetList()) { if (policies) {
ok &= csp.is_dict() && ParseContentSecurityPolicy(csp); for (const auto& policy : policies->GetList()) {
if (policy.is_string()) {
policy_contents_->content_security_policies.push_back(
policy.GetString());
}
}
} }
return ok;
}
bool OriginPolicyParser::ParseContentSecurityPolicy(const base::Value& csp) {
const std::string* policy = csp.FindStringKey("policy");
if (!policy)
return false;
const base::Optional<bool> report_only = csp.FindBoolKey("report-only");
if (report_only.has_value() && report_only.value()) { const base::Value* policies_report_only =
policy_contents_->content_security_policies_report_only.push_back(*policy); content_security.FindListKey("policies_report_only");
} else { if (policies_report_only) {
policy_contents_->content_security_policies.push_back(*policy); for (const auto& policy : policies_report_only->GetList()) {
if (policy.is_string()) {
policy_contents_->content_security_policies_report_only.push_back(
policy.GetString());
}
}
} }
return true;
} }
void OriginPolicyParser::ParseFeatures(const base::Value& features) { void OriginPolicyParser::ParseFeatures(const base::Value& features) {
......
services/network/origin_policy/origin_policy_parser.h
View file @ c2b99265
...@@ -18,31 +18,20 @@ class Value; ...@@ -18,31 +18,20 @@ class Value;
namespace network { namespace network {
// https://wicg.github.io/origin-policy/#parsing
class COMPONENT_EXPORT(NETWORK_SERVICE) OriginPolicyParser { class COMPONENT_EXPORT(NETWORK_SERVICE) OriginPolicyParser {
public: public:
// Parse the given origin policy. Returns an empty policy if parsing is not // Parse the given origin policy. Returns an empty policy if parsing is not
// successful. // successful.
// TODO(vogelheim): Decide how parsing errors should be handled.
static OriginPolicyContentsPtr Parse(base::StringPiece); static OriginPolicyContentsPtr Parse(base::StringPiece);
private: private:
OriginPolicyParser(); OriginPolicyParser();
~OriginPolicyParser(); ~OriginPolicyParser();
// The older spec treated parsing errors as failures that would cause an void DoParse(base::StringPiece);
// interstitial, so we have a boolean return value to represent that. The
// newer spec does not treat parsing errors as failures; see
// https://github.com/WICG/origin-policy/issues/49.
// TODO(domenic): update everything to the newer spec and remove all boolean
// return values.
bool DoParse(base::StringPiece); void ParseContentSecurity(const base::Value&);
// The following methods are implemented according to the older spec:
bool ParseContentSecurityPolicies(const base::Value&);
bool ParseContentSecurityPolicy(const base::Value&);
// The following methods are implemented according to the newer spec:
void ParseFeatures(const base::Value&); void ParseFeatures(const base::Value&);
void ParseIsolation(const base::Value&); void ParseIsolation(const base::Value&);
......
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-content-security-comma-in-policy 0 → 100644
View file @ c2b99265
{
"content_security": {
"policies": ["script-src 'self' 'unsafe-inline', img-src 'none'"]
}
}
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-content-security-double-content-security 0 → 100644
View file @ c2b99265
{
"content_security": {
"policies": ["script-src 'self' 'unsafe-inline'"]
},
"content_security": {
"policies": ["img-src 'none'"]
}
}
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-content-security-double-policies 0 → 100644
View file @ c2b99265
{
"content_security": {
"policies": ["script-src 'self' 'unsafe-inline'"],
"policies": ["img-src 'none'"]
}
}
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-content-security-non-array 0 → 100644
View file @ c2b99265
{
"content_security": {
"policies": "script-src 'self' 'unsafe-inline'"
}
}
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-content-security-non-string 0 → 100644
View file @ c2b99265
{
"content_security": {
"policies": [["script-src 'self' 'unsafe-inline'"]]
}
}
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-content-security-valid 0 → 100644
View file @ c2b99265
{
"content_security": {
"policies": ["script-src 'self' 'unsafe-inline'"]
}
}
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-content-security-valid-with-multi-item-array 0 → 100644
View file @ c2b99265
{
"content_security": {
"policies": ["script-src 'self' 'unsafe-inline'", "img-src 'none'"]
}
}
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-content-security-valid-with-semicolon 0 → 100644
View file @ c2b99265
{
"content_security": {
"policies": ["script-src 'self' 'unsafe-inline'; img-src 'none'"]
}
}
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-csp-1
View file @ c2b99265
{ {
"content-security-policy": [{ "policy": "script-src 'self' 'unsafe-inline'" }] "content_security": {
"policies": ["script-src 'self' 'unsafe-inline'"]
}
} }
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-csp-2
View file @ c2b99265
{ {
"content-security-policy": [{ "content_security": {
"policy": "script-src 'self' 'nonce-test'" "policies": ["script-src 'self' 'nonce-test'"]
}] }
} }
third_party/blink/web_tests/external/wpt/.well-known/origin-policy/policy-noimg deleted 100644 → 0
View file @ 0d3ebf3b
{
"content-security-policy": [{ "policy": "img-src 'none'" }]
}
third_party/blink/web_tests/external/wpt/origin-policy/content-security/comma-in-policy.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>Commas in "content_security/policy" cause parse errors and thus no CSP</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<script>
"use strict";
runCSPTest({ unsafeEval: true });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/double-content-security.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>Of two "content_security" items only the second counts</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<body>
<script>
"use strict";
runCSPTest({ unsafeEval: true, img: false });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/double-policies.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>Of two "content_security/policies" items only the second counts</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<body>
<script>
"use strict";
runCSPTest({ unsafeEval: true, img: false });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/helper.js 0 → 100644
View file @ c2b99265
window.waitForOneSecurityPolicyViolationEvent = expectedBlockedURI => {
return new Promise(resolve => {
let eventCount = 0;
let blockedURI = null;
document.addEventListener("securitypolicyviolation", e => {
++eventCount;
blockedURI = e.blockedURI;
// We want to test that only one event is fired, but we want to do so
// without waiting indefinitely. By waiting for one tick, we at least
// ensure that there's no bug that leads to two securitypolicyviolation
// events being fired at the same time, as a result of the one violation.
step_timeout(() => {
assert_equals(eventCount, 1);
resolve(blockedURI);
});
});
});
};
window.waitForImgFail = imgSrc => {
return new Promise((resolve, reject) => {
const img = document.createElement("img");
img.onload = () => reject(new Error("Must not load the image"));
img.onerror = () => resolve();
img.src = imgSrc;
document.body.append(img);
});
};
window.waitForImgSuccess = imgSrc => {
return new Promise((resolve, reject) => {
const img = document.createElement("img");
img.onload = () => resolve();
img.onerror = () => reject(new Error("Must load the image"));
img.src = imgSrc;
document.body.append(img);
});
};
// Both params are optional; if they are not given as booleans then we will not test that aspect.
window.runCSPTest = ({ unsafeEval, img }) => {
if (unsafeEval === true) {
test(() => {
eval("window.evalAllowed = true;");
assert_equals(window.evalAllowed, true);
}, "eval must be allowed");
} else if (unsafeEval === false) {
test(() => {
try {
eval("window.evalAllowed = true;");
} catch (e) { }
assert_equals(window.evalAllowed, undefined);
}, "eval must be disallowed");
}
if (img === true) {
promise_test(
() => waitForImgSuccess("/common/security-features/subresource/image.py"),
"img loading must be allowed"
);
} else if (img === false) {
promise_test(
() => waitForImgFail("/common/security-features/subresource/image.py"),
"img loading must be disallowed"
);
}
};
third_party/blink/web_tests/external/wpt/origin-policy/content-security/non-array.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>Non-array "content_security/policies" member must be ignored</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<script>
"use strict";
runCSPTest({ unsafeEval: true });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/non-object.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>Non-object "content_security" member must be ignored</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<script>
"use strict";
runCSPTest({ unsafeEval: true });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/non-string.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>Non-string "content_security/policies" array member must be ignored</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<script>
"use strict";
runCSPTest({ unsafeEval: true });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/trigger-violation-report-report-only.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>CSP via origin policy must trigger a securitypolicyviolation event even when the CSP is report-only</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<body>
<script>
"use strict";
promise_test(() => {
const imgURL = (new URL("/common/security-features/subresource/image.py", document.location)).href;
return Promise.all([
waitForOneSecurityPolicyViolationEvent(imgURL).then(blockedURI => {
assert_equals(blockedURI, imgURL);
}),
waitForImgSuccess(imgURL)
]);
});
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/trigger-violation-report.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>CSP via origin policy must trigger a securitypolicyviolation event</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<body>
<script>
"use strict";
promise_test(() => {
const imgURL = (new URL("/common/security-features/subresource/image.py", document.location)).href;
return Promise.all([
waitForOneSecurityPolicyViolationEvent(imgURL).then(blockedURI => {
assert_equals(blockedURI, imgURL);
}),
waitForImgFail(imgURL)
]);
});
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/valid-with-multi-item-array.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>"content_security/policy" can contain multiple array items to enforce multiple CSPs</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<body>
<script>
"use strict";
runCSPTest({ unsafeEval: false, img: false });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/valid-with-semicolon.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>"content_security/policy" array items can contain semicolons to enforce multiple CSP directives</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<body>
<script>
"use strict";
runCSPTest({ unsafeEval: false, img: false });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/content-security/valid.https.html 0 → 100644
View file @ c2b99265
<!DOCTYPE HTML>
<meta charset="utf-8">
<title>Valid "content_security" member disallows eval</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="helper.js"></script>
<script>
"use strict";
runCSPTest({ unsafeEval: false });
</script>
third_party/blink/web_tests/external/wpt/origin-policy/origin-policy-single-report.https.tentative.html deleted 100644 → 0
View file @ 0d3ebf3b
<!DOCTYPE HTML>
<html>
<head>
<script src='/resources/testharness.js'></script>
<script src='/resources/testharnessreport.js'></script>
</head>
<body>
<iframe id=frame></iframe>
<script>
async_test(t => {
let violations = [];
window.addEventListener("message", (e) => {
violations.push(e);
t.step_timeout(() => {
assert_equals(violations.length, 1);
t.done();
});
});
let forbidden_image = "<img src=https://127.0.0.1:1234/bla.jpg>";
let event_bouncer = "<script>document.addEventListener(" +
"'securitypolicyviolation'," +
"(e) => window.parent.postMessage(e.blockedURI, '*'));</sc" +
"ript>";
document.getElementById("frame").src =
"data:text/html;charset=utf-8," + event_bouncer + forbidden_image;
}, "Origin-Policy-based CSP violation should trigger 1 violation event");
</script>
</body>
</html>
third_party/blink/web_tests/virtual/origin-policy/external/wpt/origin-policy/content-security/comma-in-policy.https-expected.txt 0 → 100644
View file @ c2b99265
This is a testharness.js-based test.
FAIL eval must be allowed Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' 'unsafe-inline'".
Harness: the test ran to completion.
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