Add CSP support for Trusted Types for Windows (not Workers) excluding reporting violations
Categories
(Core :: DOM: Security, task, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox129 | --- | fixed |
People
(Reporter: mbrodesser-Igalia, Assigned: mbrodesser-Igalia)
References
(Blocks 1 open bug)
Details
(Whiteboard: [domsecurity-active], [wptsync upstream])
Attachments
(4 files)
|
Assignee | |
Updated•3 months ago
|
|
|
Assignee | |
Comment 1•3 months ago
|
||
Tests for the new functionality are activated in
https://phabricator.services.mozilla.com/D207549.
|
|
Assignee | |
Comment 2•3 months ago
|
||
"c*" is invalid according to
https://w3c.github.io/trusted-types/dist/spec/#trusted-types-csp-directive.
Depends on D208909
|
|
Assignee | |
Comment 3•3 months ago
|
||
Parsing the trusted-types directive's values is not added to
nsCSPParser::sourceList because the latter supports keywords not
supported by the trusted-types directive. E.g. 'report-sample'.
Additionally, the trusted-types directive supports keywords not
supported by any other directive, e.g. 'allow-duplicates'.
Reporting violations will be implemented in a separate part.
|
|
Assignee | |
Comment 4•3 months ago
|
||
|
||
Updated•3 months ago
|
Pushed by mbrodesser@igalia.com: https://hg.mozilla.org/integration/autoland/rev/3e7f5c13f96b part 1) Add `CSP_ALLOW_DUPLICATES` to the `CSPKeyword` enum and `nsCSPPolicy::ShouldCreateViolationForNewTrustedTypesPolicy`. r=tschuster https://hg.mozilla.org/integration/autoland/rev/dd03de7652a1 part 2) Change allowed policy "c*" in <trusted-types-duplicate-names-list.html> to "c". r=tschuster https://hg.mozilla.org/integration/autoland/rev/4e3e9ade5089 part 3) Implement `ShouldTrustedTypePolicyCreationBeBlockedByCSP` for Windows (not Workers) including reporting exceptions, excluding reporting violations. r=tschuster,webidl,saschanaz https://hg.mozilla.org/integration/autoland/rev/f2d1bac2b78e part 4) Add more CSPParser tests for the `trusted-types` CSP directive. r=tschuster
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46829 for changes under testing/web-platform/tests
Backed out for causing build bustages on TrustedTypePolicyFactory.cpp
Upstream PR was closed without merging
|
||
Comment 10•3 months ago
|
||
Pushed by mbrodesser@igalia.com: https://hg.mozilla.org/integration/autoland/rev/e87f647b5080 part 1) Add `CSP_ALLOW_DUPLICATES` to the `CSPKeyword` enum and `nsCSPPolicy::ShouldCreateViolationForNewTrustedTypesPolicy`. r=tschuster https://hg.mozilla.org/integration/autoland/rev/8c535232b6c1 part 2) Change allowed policy "c*" in <trusted-types-duplicate-names-list.html> to "c". r=tschuster https://hg.mozilla.org/integration/autoland/rev/1ad85e47d90c part 3) Implement `ShouldTrustedTypePolicyCreationBeBlockedByCSP` for Windows (not Workers) including reporting exceptions, excluding reporting violations. r=tschuster,webidl,saschanaz
|
|
||
Comment 11•3 months ago
|
||
Pushed by mbrodesser@igalia.com: https://hg.mozilla.org/integration/autoland/rev/a3216d38c5e7 part 4) Add more CSPParser tests for the `trusted-types` CSP directive. r=tschuster
|
||
Comment 12•3 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/e87f647b5080
https://hg.mozilla.org/mozilla-central/rev/8c535232b6c1
https://hg.mozilla.org/mozilla-central/rev/1ad85e47d90c
https://hg.mozilla.org/mozilla-central/rev/a3216d38c5e7
Upstream PR merged by moz-wptsync-bot
|
|
||
Comment 14•2 months ago
|
||
Mirko, just to clarify: Are you intending to tackle the Worker implementation as a follow-up bug? My understanding is that the some sort of "enforcement hint" (we have used boolean flags for eval restrictions, as an example) needs to be copied into the WorkerPrivate class.
|
|
Assignee | |
Comment 15•2 months ago
|
||
(In reply to Frederik Braun [:freddy] from comment #14)
Mirko, just to clarify: Are you intending to tackle the
Workerimplementation as a follow-up bug?
Yes. Currently, https://bugzilla.mozilla.org/show_bug.cgi?id=1901492 is a placeholder for that.
My understanding is that the some sort of "enforcement hint" (we have used boolean flags for eval restrictions, as an example) needs to be copied into the
WorkerPrivateclass.
That, I don't understand.
Worker-support will be implemented in a separate ticket, because WorkerPrivate::GetCsp() (https://searchfox.org/mozilla-central/rev/3d173a6ad865eb778eb7a85de900e92774559ed6/dom/workers/WorkerPrivate.h#897-898) may only be called from the main-thread. Not from a Worker-thread. Work (no pun intended) for that isn't started yet.
|
||
Comment 16•2 months ago
|
||
For MDN docs ... it looks to me like this is the first part of of the CSP for Trusted Types API - support for capturing the CSP directives but not yet reporting against them, and only in Window, not worker. However the API side of this done in https://bugzilla.mozilla.org/show_bug.cgi?id=1882498 is still just a stub to right?
My inclination is to just update compatibility data for the whole API to add support as a partial implementation for the API parts behind dom.security.trusted_types.enabled from FF125, and for the CSP parts behind FF129 and in both cases have a note that the behaviour is mostly stubs. That is useful because it gives us something to start with once the implementation progresses.
Is that a reasonable step to take?
|
|
||
Comment 17•2 months ago
|
||
I don't have a good mental model of the currently implemented state and would caution people to start testing. We know there are various bugs that make the feature unreliable. While we could point people to testing against an unstable implementation, I would mostly expect reports of known bugs.
That is to say, we are not putting this behind a pref for polishing the API but rather for the purpose of building a stable implementation over a longer course of time.
I would much rather prefer we let people wait until we think it's complete and then report unknown bug to finalize and polish our implementation with real-world scenarios.
That being said, I'd also want to hear what Mirko knows.. Maybe there's a reasonable subset in a specific version between 125 and 128 that he considers worth testing against and what kind of caveats there are. :)
|
|
Assignee | |
Comment 18•2 months ago
|
||
(In reply to Frederik Braun [:freddy] from comment #17)
I don't have a good mental model of the currently implemented state and would caution people to start testing. We know there are various bugs that make the feature unreliable. While we could point people to testing against an unstable implementation, I would mostly expect reports of known bugs.
Agreed.
That is to say, we are not putting this behind a pref for polishing the API but rather for the purpose of building a stable implementation over a longer course of time.
Exactly. To avoid merge-conflicts and to make code reviews easily digestible.
I would much rather prefer we let people wait until we think it's complete and then report unknown bug to finalize and polish our implementation with real-world scenarios.
Agreed.
That being said, I'd also want to hear what Mirko knows.. Maybe there's a reasonable subset in a specific version between 125 and 128 that he considers worth testing against and what kind of caveats there are. :)
Such a subset is not clear to me yet. The spec also still contains open in-progress semantic issues. Will continually re-evaluate the situation.
|
|
||
Comment 19•2 months ago
•
|
||
Thanks very much - we will do nothing on this for now.
Note though that I can't guess when you are actually ready for docs, so at that point please start adding dev-doc-needed to the parts that need documentation (and we'll probably then ask for a broad overview of the completed state).
Description
•