1841363 - WPT failures for workers/modules/[dedicated|shared]-worker-import-csp.html

I check again and agree that the spec is correct.
So the issue https://github.com/w3c/webappsec-csp/issues/336, was filed back in Sep 2018.
At that time CSP spec defined that worker should inherit the CSP from its parent context.

See https://web.archive.org/web/20180826085420/https://w3c.github.io/webappsec-csp/#initialize-global-object-csp

CSP spec, Sep 2018

4.2.2. Initialize a global object’s CSP list

Given a global object (global), and a response (response), the user agent performs the following steps in order to initialize global’s CSP list:

    1. If response’s url’s scheme is a local scheme, or if global is a DedicatedWorkerGlobalScope:

        1. Let owners be an empty list.

        2. Add each of the items in global’s owner set to owners.

        3. For each owner in owners:

            1. For each policy in owner’s CSP list:

               1.  Insert a copy of policy into global’s CSP list.

The corresponding HTML spec can be found in
http://web.archive.org/web/20180927040126/https://html.spec.whatwg.org/multipage/workers.html#run-a-worker

Step 12
  ...
perform the fetch:
6. Execute the Initialize a global object's CSP list algorithm on worker global scope and response. [CSP]

So the HTTP headers (including CSP header) were passed to initialize the CSP list, but actually the CSP headers weren't used at all for DedicatedWorkers.

The part of spec was introduced back in 2016,
https://github.com/w3c/webappsec-csp/commit/1af72ed19bf952402c514b7e7a966fb234d63217

Because at the time, worker was treated as a sub-resource hence shouldn't have its own CSP list.

But the spec wasn't fixed until 2021,
CSP: https://github.com/w3c/webappsec-csp/commit/58691e0cd258191d3c5097329747d0f706ea1daf
HTML: https://github.com/whatwg/html/pull/6504
WPT: https://github.com/web-platform-tests/wpt/commit/0a8c303a671

In short, specifications add "Policy container", and removed the inheritance part of the DedicatedWorker.

Although the specs are fixed in 2021, but the original issue, https://github.com/w3c/webappsec-csp/issues/336
was still kept open,
It was closed as soon as the patch author noticed the issue was still open, and the author had answered Yulia's question.
https://github.com/w3c/webappsec-csp/issues/336#event-7553060104

================== Current specification ====================
So for the top-level worker module script, and its dependent scripts,
they all use the environment settings object to do the fetch, which is from the node document

https://html.spec.whatwg.org/multipage/workers.html#run-a-worker
When a user agent is to run a worker for a script with Worker or SharedWorker object worker, URL url, environment settings object outside settings, MessagePort outside port, and a WorkerOptions dictionary options,

The environment settings object is passed all the way to Fetch API.

https://fetch.spec.whatwg.org/#concept-fetch

If request’s policy container is "client", then:

    If request’s client is non-null, then set request’s policy container to a clone of request’s client’s policy container. [HTML]

    Otherwise, set request’s policy container to a new policy container.

(Note, request's client is the "fetch client settings object", which is the "environment settings object" in run-a-worker)

And the original environment settings object,
it comes from the node document of the script element
https://html.spec.whatwg.org/multipage/scripting.html#prepare-the-script-element

30. Let settings object be el's node document's relevant settings object.

======== Conclusion ==============
Static Import => Use document's CSP
Dynamic Import => Use WorkerGlobalScope's CSP, which will get updated when parsing the top-level Worker module script,
however, dynamic-import should be restricted by script-src CSP, not by worker-src.

Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]

Comment 2

•

1 year ago

Hi Smaug
Do you think https://github.com/w3c/webappsec-csp/issues/336 is still an issue?

You left some comments in https://github.com/w3c/webappsec-csp/issues/336#issuecomment-1274594043
and https://github.com/w3c/webappsec-csp/issues/336#issuecomment-1274684557

Both Anne and Yulia have replied your comment.

In short, when fetching a top-level worker module script and its dependent static import scripts, it will use the CSP from the document, as the fetch is initialized by the document.
See comment 1 for the "Current specification" part.

But for importScripts or import(), they will use worker's CSP, which will be got when parsing the top-level worker script.

importScripts
https://html.spec.whatwg.org/#import-scripts-into-worker-global-scope

2. Let settings object be the current settings object.

import()
https://html.spec.whatwg.org/multipage/webappapis.html#hostloadimportedmodule

1. Let settingsObject be the current settings object.

Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]

Updated

•

1 year ago

Flags: needinfo?(smaug)

Jon Coppeard (:jonco)

Comment 3

•

1 year ago

(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] from comment #2)
As I (now) understand it the issue is whether to use the document CSP or the worker CSP for static imports of module workers. The spec requires the former while the current implementation does the latter (by choice).

Olli Pettay [:smaug][bugs@pettay.fi]

Comment 4

•

1 year ago

Isn't this related to bug 1831583?

I think the main issues are that which origin should be used for loading and what about csp.
In Gecko we map principal (==origin) and csp, but I'm not sure that is correct per spec.
Currently the tests are really hard to understand - some tests were copied to mozilla/*, but I don't understand why.

Flags: needinfo?(smaug)

Andrew Sutherland [:asuth] (he/him)

Comment 5

•

1 year ago

(In reply to Olli Pettay [:smaug][bugs@pettay.fi] from comment #4)

Currently the tests are really hard to understand - some tests were copied to mozilla/*, but I don't understand why.

The tests were not just copied; they were modified to match our intentional changes in behavior so we would pass those.

Olli Pettay [:smaug][bugs@pettay.fi]

Comment 6

•

1 year ago

But we don't pass them. The patch in bug 1831583 causes many unexpected passes on them.

Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]

Comment 7

•

1 year ago

•

Edited

(In reply to Olli Pettay [:smaug][bugs@pettay.fi] from comment #4)

Currently the tests are really hard to understand - some tests were copied to mozilla/*, but I don't understand why.

I have done the analysis of the tests added by Yulia

The commit https://hg.mozilla.org/mozilla-central/rev/060c0649c2f5 updated the test files
and the commit https://hg.mozilla.org/mozilla-central/rev/cb4aa6f21ff7 updated the ini files.

testing/web-platform/mozilla/tests/content-security-policy/generic/test-case.sub.js
Identical with testing/web-platform/tests/content-security-policy/generic/test-case.sub.js
But perhaps to be updated to fix the failures on Moz forked WPTs.
testing/web-platform/tests/common/security-features/resources/common.sub.js
Append CSP values to static-import.py
testing/web-platform/tests/common/security-features/subresource/static-import.py
Parse string values to HTTP response CSP headers.
/content-security-policy/gen/top.http-rp/script-src-self/worker-import.http.html
- Moz fork
  testing/web-platform/mozilla/tests/content-security-policy/gen/top.http-rp/script-src-self/worker-import.http.html
  testing/web-platform/mozilla/meta/content-security-policy/gen/top.http-rp/script-src-self/worker-import.http.html.ini
- WPT original
  testing/web-platform/tests/content-security-policy/gen/top.http-rp/script-src-self/worker-import.http.html.headers
  testing/web-platform/tests/content-security-policy/gen/top.http-rp/script-src-self/worker-import.http.html
  testing/web-platform/meta/content-security-policy/gen/top.http-rp/script-src-self/worker-import.http.html.ini
- Test description
  WPT Origin:
  Send Document's CSP through worker-import.http.html.headers, and use document's CSP to restrict static-import.
  The tests failed because currently Gecko uses Worker's CSP for static import, they will be passed once we use
  document's CSP for static-import.
  
  Moz Fork:
  Inject CSP header into worker's CSP to restrict static-import.
  However, the test failed because the test expects the 'securitypolicyviolation' event to be fired on the document,
  but the event is fired on WorkerGlobalScope.
The rest of {script-src-self | worker-src-self} / worker-import-http(s).html are modified for the same reason, only the scheme is changed to http/https, and the CSP header is changed to script-src-self or worker-src-self.
/workers/modules/dedicated-worker-import-csp.html
/workers/modules/resources/dynamic-import-remote-origin-script-worker.sub.js (copy from WPT original)
/workers/modules/resources/new-worker-window.html (copy from WPT original)
/workers/modules/resources/static-import-remote-origin-script-worker.sub.js (copy from WPT original)
- Moz fork
  testing/web-platform/mozilla/tests/workers/modules/dedicated-worker-import-csp.html
  testing/web-platform/mozilla/meta/workers/modules/dedicated-worker-import-csp.html.ini
- WPT original
  testing/web-platform/meta/workers/modules/dedicated-worker-import-csp.html.ini
  testing/web-platform/tests/workers/modules/dedicated-worker-import-csp.html
- Test description
  The original WPT will pass CSP headers to the window if it's for static-import,
  and will pass CSP headers to the worker script if it's for dynamic-import.
  Moz-forked changes to pass the CSP headers to the worker script for both
  static-import and dynamic-import.
  Moz-forked test only fails in one test which is related to dynamic-import (I have a fix already).
testing/web-platform/meta/mixed-content/gen/worker-module-data.http-rp/opt-in/websocket.https.html.ini
testing/web-platform/meta/mixed-content/gen/worker-module-data.meta/opt-in/websocket.https.html.ini
testing/web-platform/meta/mixed-content/gen/worker-module-data.meta/unset/websocket.https.html.ini
- Test description
  The tests failed because they are loading the worker as data: URL. (perhaps related to Bug 1831583 or a new bug is needed)

(In reply to Olli Pettay [:smaug][bugs@pettay.fi] from comment #6)

But we don't pass them. The patch in bug 1831583 causes many unexpected passes on them.

The patch in Bug 1831583 is using CSP from the document (by using the loading principal) so it should fix the failures on WPT original part

Andrew Sutherland [:asuth] (he/him)

Updated

•

1 year ago

Severity: -- → S3

Priority: -- → P3

Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]

Updated

•

1 year ago

Depends on: 1847548

Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]

Comment 8

•

1 year ago

filed the spec issue in https://github.com/whatwg/html/issues/9571

Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]

Comment 9

•

1 year ago

The following tests are also affected by the spec issue

dedicated-worker-import-data-url.any.html
dedicated-worker-import-data-url.any.worker.html
shared-worker-import-data-url.window.html
dedicated-worker-import-data-url-cross-origin.html
shared-worker-import-data-url-cross-origin.html

See: https://hg.mozilla.org/mozilla-central/rev/6e401be8c586,
https://hg.mozilla.org/mozilla-central/rev/460e36a5e5aa

Bugzilla

WPT failures for workers/modules/[dedicated|shared]-worker-import-csp.html

Categories

(Core :: DOM: Workers, defect, P3)

Tracking

()

People

(Reporter: twisniewski, Unassigned)

References

(Blocks 1 open bug)

Details

Crash Data

Security

(public)

User Story

Description

Updated

Updated

Comment 1

Comment 2

Updated

Comment 3

Comment 4

Comment 5

Comment 6

Comment 7

Updated

Updated

Comment 8

Comment 9