Write CSP web platform test for worker-src, child-src, script-src fallback

RESOLVED FIXED in Firefox 60

Status

()

P2
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: ckerschb, Assigned: vinoth)

Tracking

(Blocks: 1 bug)

unspecified
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 attachment, 1 obsolete attachment)

Within Bug 1302667, we are implementing worker-src. Within Bug 1302667 we added a bunch of worker tests including tests for fallback from:
worker-src -> child-src -> script-src -> default-src

Within this bug we should write some wpt tests which we can then upstream and share with other vendors.
(Reporter)

Updated

a year ago
Blocks: 1231788
Depends on: 1302667
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
(Reporter)

Updated

a year ago
Assignee: nobody → cegvinoth
Status: NEW → ASSIGNED
Priority: P3 → P2
Whiteboard: [domsecurity-backlog1] → [domsecurity-active]
(In reply to Vinothkumar Nagasayanan [:vinoth] from comment #1)
> Created attachment 8924886 [details]
> Bug 1409706 - Files added for CSP WPT for
> worker-src,child-src,script-src,default fallback behaviour
> 
> Review commit: https://reviewboard.mozilla.org/r/196164/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/196164/

I added the initial files for dedicated worker. Same should be created for Shared and Service workers right?
(Reporter)

Comment 3

a year ago
mozreview-review
Comment on attachment 8924886 [details]
Bug 1409706 - Files added for CSP WPT for worker-src,child-src,script-src,default fallback behaviour

https://reviewboard.mozilla.org/r/196164/#review201398

That looks good and you are right we need the same for dedicated, service and shared workers.
Question: Can we use the following CSPs or some equivalend in case we need to whitelist some other script, but basically we need:
* worker-src 'self'; child-src 'none'; script-src 'none'; default-src 'none';
* child-src 'self'; script-src 'none'; default-src 'none';
* script-src 'self'; default-src 'none';
* default-src 'self'
to make sure the worker is not whitelisted by any other directive.

::: testing/web-platform/meta/content-security-policy/worker-src/worker-src-child-fallback.sub.html.ini:4
(Diff revision 1)
> +[worker-src-child-fallback.sub.html]
> +  type: testharness
> +  [Same-origin dedicated worker allowed by 'self'.]
> +    expected: PASS

You shouldn't need any of those *.ini files. We only use them if tests are disabled.

::: testing/web-platform/tests/content-security-policy/worker-src/worker-src-child-fallback.sub.html:10
(Diff revision 1)
> +<script src=/resources/testharnessreport.js></script>
> +<script src="../support/testharness-helper.js"></script>
> +<meta http-equiv="Content-Security-Policy" content="child-src 'self';">
> +<script>
> +	var url = new URL("../support/ping.js", document.baseURI).toString();
> + 	assert_worker_is_loaded(url, "Same-origin dedicated worker allowed by 'self'.");

nit: tab instead of space here and elsewhere

::: testing/web-platform/tests/content-security-policy/worker-src/worker-src-default-fallback.sub.html:10
(Diff revision 1)
> +<script src=/resources/testharnessreport.js></script>
> +<script src="../support/testharness-helper.js"></script>
> +<meta http-equiv="Content-Security-Policy" content="script-src 'nonce-foo'; default-src 'self'">
> +<script nonce='foo'>
> +	var url = new URL("../support/ping.js", document.baseURI).toString();
> + 	assert_worker_is_loaded(url, "Same-origin dedicated worker allowed by 'self'.");

please expand the string to also indicate it as allowed by which directive. e.g. in that case
"Same-origin dedicated worker allowed by default-src 'self'
Attachment #8924886 - Flags: review?(ckerschb)
(Assignee)

Updated

a year ago
Attachment #8924886 - Attachment is obsolete: true
(Reporter)

Comment 5

a year ago
mozreview-review
Comment on attachment 8925483 [details]
Bug 1409706 - Files updated for CSP WPT for worker-src,child-src,script-src,default fallback behaviour

https://reviewboard.mozilla.org/r/196612/#review201918

That looks good to me. thanks!
Attachment #8925483 - Flags: review?(ckerschb) → review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed
Mozreview says at least one patch still needs to be reviewed. Please take a look.
Flags: needinfo?(ckerschb)
Keywords: checkin-needed
(Reporter)

Comment 7

a year ago
mozreview-review
Comment on attachment 8925483 [details]
Bug 1409706 - Files updated for CSP WPT for worker-src,child-src,script-src,default fallback behaviour

https://reviewboard.mozilla.org/r/196612/#review220482
Is that working now? Don't know why that happenend. thanks
Flags: needinfo?(ckerschb)
(Reporter)

Comment 9

a year ago
mozreview-review
Comment on attachment 8925483 [details]
Bug 1409706 - Files updated for CSP WPT for worker-src,child-src,script-src,default fallback behaviour

https://reviewboard.mozilla.org/r/196612/#review220492
(Assignee)

Updated

a year ago
Keywords: checkin-needed
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8925483 - Attachment is obsolete: true
(Reporter)

Comment 11

a year ago
mozreview-review
Comment on attachment 8924886 [details]
Bug 1409706 - Files added for CSP WPT for worker-src,child-src,script-src,default fallback behaviour

https://reviewboard.mozilla.org/r/196164/#review221982

This already had my r+, but due to some squashing problems it still showed some conflicts - hope that works now. r=me
Attachment #8924886 - Flags: review?(ckerschb) → review+
Comment on attachment 8924886 [details]
Bug 1409706 - Files added for CSP WPT for worker-src,child-src,script-src,default fallback behaviour

https://reviewboard.mozilla.org/r/196164/#review221990
Attachment #8924886 - Flags: review+
Byron, https://reviewboard.mozilla.org/r/196164/diff/2#index_header says Franziskus tried to land it (hours ago) and there is no message about conflicts or it having landed successfully here in the bug. Can you check what's up with it, please?
Flags: needinfo?(glob)
thanks aryx – autoland's logs tell me there was a merge conflict in MANIFEST.json, and the notification back to reviewboard of this failure also failed (bug 1434166).  i'll do some more investigation and unblock the queue once i've gathered enough data to fix the issue.
Flags: needinfo?(glob)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 6b8be671a3f581d2227e3adbc697b5d0aaa61de8 -d c42ad5edc883: rebasing 444498:6b8be671a3f5 "Bug 1409706 - Files added for CSP WPT for worker-src,child-src,script-src,default fallback behaviour r=ckerschb,fkiefer" (tip)
merging testing/web-platform/meta/MANIFEST.json
warning: conflicts while merging testing/web-platform/meta/MANIFEST.json! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Please update the patch so that it applies cleanly. Thank you.
Flags: needinfo?(cegvinoth)
Keywords: checkin-needed
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #16)
> Please update the patch so that it applies cleanly. Thank you.

Patch updated.
Flags: needinfo?(cegvinoth)

Comment 19

a year ago
Pushed by franziskuskiefer@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/588fec4c25f5
Files added for CSP WPT for worker-src,child-src,script-src,default fallback behaviour, r=ckerschb

Comment 20

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/588fec4c25f5
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.