Skip to content

GitLab

T
tangled
Commit 611548d6 authored by Andy Paicu's avatar Andy Paicu Committed by Commit Bot
Browse files
Options

Un-deprecated 'child-src' 

'child-src' is now part of the fallback chain for 'worker-src'.
This means that 'child-src' always takes precedence over 'script-src'
when checking worker requests.
Added extra tests to ensure that a worker request blocked by
'child-src' and allowed by 'script-src' is blocked.

Removed previous logic that considered 'script-src' to be the
fallback for 'worker-src' and amended tests. Removed "temporary"
logic put in place to not break sites using "child-src".

Refactored the OperativeDirective logic to ensure that the caller
does not need to be aware of precise fallback chain of the directive,
otherwise the way to get the 'worker-src' operative directive would be:

OperativeDirective(worker_src_.Get(),
      OperativeDirective(child_src_.Get(),
          OperativeDirective(script_src_.Get())));

To be submitted with the spec PR as it includes tests.
Spec: https://github.com/w3c/webappsec-csp/pull/313

Bug: 669496
Change-Id: I7ca9552df1d0ce203a604b0e469a268f6b112e49
Reviewed-on: https://chromium-review.googlesource.com/1128087
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573278}
parent 8ffc5838
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 309 additions and 177 deletions
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/dedicated-worker-helper.js
View file @ 611548d6
var url = new URL("../support/ping.js", document.baseURI).toString();
assert_worker_is_loaded(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
\ No newline at end of file
if (document.getElementById("foo").hasAttribute("blocked-worker"))
assert_worker_is_blocked(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
else
assert_worker_is_loaded(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
\ No newline at end of file
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/service-worker-helper.js
View file @ 611548d6
var url = new URL("../support/ping.js", document.baseURI).toString();
assert_service_worker_is_loaded(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
\ No newline at end of file
if (document.getElementById("foo").hasAttribute("blocked-worker"))
assert_service_worker_is_blocked(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
else
assert_service_worker_is_loaded(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
\ No newline at end of file
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/support/shared-worker-helper.js
View file @ 611548d6
var url = new URL("../support/ping.js", document.baseURI).toString();
assert_shared_worker_is_loaded(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
\ No newline at end of file
if (document.getElementById("foo").hasAttribute("blocked-worker"))
assert_shared_worker_is_blocked(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
else
assert_shared_worker_is_loaded(url, document.getElementById("foo").getAttribute("data-desc-fallback"));
\ No newline at end of file
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/worker-src/dedicated-worker-src-child-fallback-blocked.sub.html 0 → 100644
View file @ 611548d6
<!doctype html>
<meta charset=utf-8>
<title>Web platform test for dedicated worker allowed by worker-src self</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src="../support/testharness-helper.js"></script>
<meta http-equiv="Content-Security-Policy" content="child-src 'none'; script-src 'self'; default-src 'none'; ">
<script src="../support/dedicated-worker-helper.js" blocked-worker id="foo" data-desc-fallback="Same-origin dedicated worker allowed by worker-src 'self'."></script>
\ No newline at end of file
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/worker-src/service-worker-src-child-fallback-blocked.https.sub.html 0 → 100644
View file @ 611548d6
<!doctype html>
<meta charset=utf-8>
<title>Web platform test for service worker allowed by child-src self</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src="../support/testharness-helper.js"></script>
<meta http-equiv="Content-Security-Policy" content="child-src 'none'; script-src 'self'; default-src 'none'; ">
<script src="../support/service-worker-helper.js" blocked-worker id="foo" data-desc-fallback="Same-origin service worker allowed by child-src 'self'."></script>
\ No newline at end of file
third_party/WebKit/LayoutTests/external/wpt/content-security-policy/worker-src/shared-worker-src-child-fallback-blocked.sub.html 0 → 100644
View file @ 611548d6
<!doctype html>
<meta charset=utf-8>
<title>Web platform test for shared worker allowed by child-src self</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src="../support/testharness-helper.js"></script>
<meta http-equiv="Content-Security-Policy" content="child-src 'none'; script-src 'self'; default-src 'none'; ">
<script src="../support/shared-worker-helper.js" blocked-worker id="foo" data-desc-fallback="Same-origin shared worker allowed by child-src 'self'."></script>
\ No newline at end of file
third_party/blink/renderer/core/frame/csp/csp_directive_list.h
View file @ 611548d6
......@@ -231,21 +231,21 @@ class CORE_EXPORT CSPDirectiveList
Member<CSPDirectiveType>&,
bool should_parse_wasm_eval = false);
SourceListDirective* OperativeDirective(SourceListDirective*) const;
SourceListDirective* OperativeDirective(SourceListDirective*,
SourceListDirective* override) const;
ContentSecurityPolicy::DirectiveType FallbackDirective(
const ContentSecurityPolicy::DirectiveType current_directive,
const ContentSecurityPolicy::DirectiveType original_directive) const;
void ReportViolation(const String& directive_text,
const ContentSecurityPolicy::DirectiveType&,
const ContentSecurityPolicy::DirectiveType,
const String& console_message,
const KURL& blocked_url,
ResourceRequest::RedirectStatus) const;
void ReportViolationWithFrame(const String& directive_text,
const ContentSecurityPolicy::DirectiveType&,
const ContentSecurityPolicy::DirectiveType,
const String& console_message,
const KURL& blocked_url,
LocalFrame*) const;
void ReportViolationWithLocation(const String& directive_text,
const ContentSecurityPolicy::DirectiveType&,
const ContentSecurityPolicy::DirectiveType,
const String& console_message,
const KURL& blocked_url,
const String& context_url,
......@@ -253,7 +253,7 @@ class CORE_EXPORT CSPDirectiveList
Element*,
const String& source) const;
void ReportEvalViolation(const String& directive_text,
const ContentSecurityPolicy::DirectiveType&,
const ContentSecurityPolicy::DirectiveType,
const String& message,
const KURL& blocked_url,
ScriptState*,
......@@ -300,11 +300,10 @@ class CORE_EXPORT CSPDirectiveList
bool is_script,
const String& hash_value) const;
bool CheckSourceAndReportViolation(
SourceListDirective*,
const KURL&,
const ContentSecurityPolicy::DirectiveType&,
ResourceRequest::RedirectStatus) const;
bool CheckSourceAndReportViolation(SourceListDirective*,
const KURL&,
const ContentSecurityPolicy::DirectiveType,
ResourceRequest::RedirectStatus) const;
bool CheckMediaTypeAndReportViolation(MediaListDirective*,
const String& type,
const String& type_attribute,
......@@ -320,19 +319,22 @@ class CORE_EXPORT CSPDirectiveList
bool DenyIfEnforcingPolicy() const { return IsReportOnly(); }
// This function returns a SourceListDirective of a given type
// or if it is not defined, the default SourceListDirective for that type.
// or if it is not defined, the fallback SourceListDirective for that type.
SourceListDirective* OperativeDirective(
const ContentSecurityPolicy::DirectiveType&) const;
const ContentSecurityPolicy::DirectiveType type,
ContentSecurityPolicy::DirectiveType original_type =
ContentSecurityPolicy::DirectiveType::kUndefined) const;
// This function aggregates from a vector of policies all operative
// SourceListDirectives of a given type into a vector.
static SourceListDirectiveVector GetSourceVector(
const ContentSecurityPolicy::DirectiveType&,
const ContentSecurityPolicy::DirectiveType,
const CSPDirectiveListVector& policies);
bool AllowHash(const CSPHashValue&,
ContentSecurityPolicy::InlineType,
SourceListDirective* directive) const;
bool AllowHash(
const CSPHashValue& hash_value,
const ContentSecurityPolicy::InlineType type,
const ContentSecurityPolicy::DirectiveType directive_type) const;
Member<ContentSecurityPolicy> policy_;
......
third_party/blink/renderer/core/frame/csp/csp_directive_list_test.cc
View file @ 611548d6
......@@ -88,8 +88,8 @@ TEST_F(CSPDirectiveListTest, IsMatchingNoncePresent) {
// Report-only
Member<CSPDirectiveList> directive_list =
CreateList(test.list, kContentSecurityPolicyHeaderTypeReport);
Member<SourceListDirective> script_src =
directive_list->OperativeDirective(directive_list->script_src_.Get());
Member<SourceListDirective> script_src = directive_list->OperativeDirective(
ContentSecurityPolicy::DirectiveType::kScriptSrc);
EXPECT_EQ(test.expected,
directive_list->IsMatchingNoncePresent(script_src, test.nonce));
// Empty/null strings are always not present, regardless of the policy.
......@@ -99,8 +99,8 @@ TEST_F(CSPDirectiveListTest, IsMatchingNoncePresent) {
// Enforce
directive_list =
CreateList(test.list, kContentSecurityPolicyHeaderTypeEnforce);
script_src =
directive_list->OperativeDirective(directive_list->script_src_.Get());
script_src = directive_list->OperativeDirective(
ContentSecurityPolicy::DirectiveType::kScriptSrc);
EXPECT_EQ(test.expected,
directive_list->IsMatchingNoncePresent(script_src, test.nonce));
// Empty/null strings are always not present, regardless of the policy.
......@@ -564,14 +564,14 @@ TEST_F(CSPDirectiveListTest, WorkerSrcChildSrcFallback) {
// When 'worker-src' is not present, 'child-src' can allow a worker when
// present.
{"child-src https://example.test", true},
{"child-src https://not-example.test", true},
{"child-src https://not-example.test", false},
{"script-src https://example.test", true},
{"script-src https://not-example.test", false},
{"child-src https://example.test; script-src https://example.test", true},
{"child-src https://example.test; script-src https://not-example.test",
true},
{"child-src https://not-example.test; script-src https://example.test",
true},
false},
{"child-src https://not-example.test; script-src "
"https://not-example.test",
false},
......@@ -869,7 +869,7 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
kDefault,
kNoDefault,
kChildAndDefault,
kScriptAndDefault
kChildAndScriptAndDefault
};
struct TestCase {
......@@ -893,7 +893,8 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
{ContentSecurityPolicy::DirectiveType::kFormAction, kNoDefault},
// Directive with multiple default directives.
{ContentSecurityPolicy::DirectiveType::kFrameSrc, kChildAndDefault},
{ContentSecurityPolicy::DirectiveType::kWorkerSrc, kScriptAndDefault},
{ContentSecurityPolicy::DirectiveType::kWorkerSrc,
kChildAndScriptAndDefault},
};
// Initial set-up.
......@@ -922,7 +923,7 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
std::stringstream all_except_this;
std::stringstream all_except_child_src_and_this;
std::stringstream all_except_script_src_and_this;
std::stringstream all_except_child_src_and_script_src_and_this;
for (const auto& subtest : cases) {
if (subtest.directive == test.directive)
continue;
......@@ -936,9 +937,11 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
<< directive_name << ".com; ";
}
if (subtest.directive !=
ContentSecurityPolicy::DirectiveType::kScriptSrc) {
all_except_script_src_and_this << directive_name << " http://"
<< directive_name << ".com; ";
ContentSecurityPolicy::DirectiveType::kChildSrc &&
subtest.directive !=
ContentSecurityPolicy::DirectiveType::kScriptSrc) {
all_except_child_src_and_script_src_and_this
<< directive_name << " http://" << directive_name << ".com; ";
}
}
CSPDirectiveList* all_except_this_list = CreateList(
......@@ -946,8 +949,8 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
CSPDirectiveList* all_except_child_src_and_this_list =
CreateList(all_except_child_src_and_this.str().c_str(),
kContentSecurityPolicyHeaderTypeEnforce);
CSPDirectiveList* all_except_script_src_and_this_list =
CreateList(all_except_script_src_and_this.str().c_str(),
CSPDirectiveList* all_except_child_src_and_script_src_and_this_list =
CreateList(all_except_child_src_and_script_src_and_this.str().c_str(),
kContentSecurityPolicyHeaderTypeEnforce);
switch (test.type) {
......@@ -971,12 +974,17 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
EXPECT_EQ(sources.size(), 1u);
EXPECT_EQ(sources[0]->host_, "default-src.com");
break;
case kScriptAndDefault:
case kChildAndScriptAndDefault:
sources =
all_except_this_list->OperativeDirective(test.directive)->list_;
EXPECT_EQ(sources.size(), 1u);
EXPECT_EQ(sources[0]->host_, "child-src.com");
sources = all_except_child_src_and_this_list
->OperativeDirective(test.directive)
->list_;
EXPECT_EQ(sources.size(), 1u);
EXPECT_EQ(sources[0]->host_, "script-src.com");
sources = all_except_script_src_and_this_list
sources = all_except_child_src_and_script_src_and_this_list
->OperativeDirective(test.directive)
->list_;
EXPECT_EQ(sources.size(), 1u);
......
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