CSP 1.1: Implement CSP for Workers

RESOLVED FIXED in Firefox 50

Status

()

defect
P2
normal
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: deian, Assigned: tnguyen)

Tracking

(Blocks 2 bugs, {dev-doc-complete})

Trunk
mozilla50
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [CSP 1.1], [domsecurity-active], )

Attachments

(3 attachments, 6 obsolete attachments)

Reporter

Description

5 years ago
New CSP 1.1 processing model was updated to allow workers to be
governed by the CSP policy accompanying the script.

See https://github.com/w3c/webappsec/commit/63534a54245df586bf02d44ceab07e6611450d15
Reporter

Updated

5 years ago
Assignee: nobody → deian
Reporter

Comment 1

5 years ago
Posted patch initial approach (obsolete) — Splinter Review
Reporter

Updated

5 years ago
Depends on: 881509
Reporter

Updated

5 years ago
Attachment #8402262 - Flags: feedback?(grobinson)
Comment on attachment 8402262 [details] [diff] [review]
initial approach

Review of attachment 8402262 [details] [diff] [review]:
-----------------------------------------------------------------

This is a great start, Deian! A good next step would be to write a test (perhaps you could rewrite and extend the very broken dom/workers/test/test_csp.html).

About AppendCSPFromHeader: I'd prefer if you didn't move it to nsContentUtils.cpp in order to re-use it (it is kind of a grab-bag of stuff like that, but we're trying to avoid that). FWIW, the new implementation of CSP (see bug 925004 for progress on landing it) has a handy file, nsCSPUtils.cpp, that would be a good location for such a function. Until it lands, I think the best thing you can do is just duplicate the function in ScriptLoader.cpp, with a comment explaining why, and with the intention to remove it when you can.

::: dom/workers/ScriptLoader.cpp
@@ +437,5 @@
> +    // Check if the CSP 1.1 preference is enabled, in which case the
> +    // worker will no longer inherit CSP from the parent and will
> +    // instead use any set CSP headers
> +    bool csp11Enabled =
> +      mozilla::Preferences::GetBool("security.csp.experimentalEnabled");

Would it be better to use a "cached" static var, like we do with CSPService::sCSPEnabled?

@@ +438,5 @@
> +    // worker will no longer inherit CSP from the parent and will
> +    // instead use any set CSP headers
> +    bool csp11Enabled =
> +      mozilla::Preferences::GetBool("security.csp.experimentalEnabled");
> +

I would group this with the next hunk. Otherwise it obfuscates the "return early if the HTTP request didn't succeed" flow.

@@ +581,5 @@
> +
> +          mWorkerPrivate->SetCSP(parent->GetCSP());
> +          mWorkerPrivate->SetEvalAllowed(parent->IsEvalAllowed());
> +        }
> +      }

Nit: add a newline for readability.
Attachment #8402262 - Flags: feedback?(grobinson) → feedback+
Garrett: You were gonna check on prerequired tests for this, right?
Flags: needinfo?(grobinson)
Priority: -- → P3
Whiteboard: [CSP 1.1]
Component: Security → DOM: Security
Deian: do you still want to work on this?
Flags: needinfo?(deian)
Reporter

Comment 5

4 years ago
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #4)
> Deian: do you still want to work on this?

Unfortunately I don't have the bandwidth, sorry. Happy to review any patches though.
Assignee: deian → nobody
Flags: needinfo?(deian)
Chris: is this something you can pick up on the side?

Unflagging Garrett, since it looks like he's not going to respond.
Flags: needinfo?(garrett.f.robinson+mozilla) → needinfo?(mozilla)
Assigning this one to myself, but potentially not getting to implement it in the near future. In case someone else wants to take on the work - please do.
Assignee: nobody → mozilla
Flags: needinfo?(mozilla)
As discussed with Chris offline, I'll take this bug.
Assignee: mozilla → ettseng
Status: NEW → ASSIGNED
Worker and/or CSP docs will need to mention this as appropriate.
Keywords: dev-doc-needed
Depends on: 929292
Summary: CSP 1.1: Workers may have their own CSP policies → CSP 1.1: Workers have their own CSP policies, should not inherit from parent document
Duplicate of this bug: 1186275
Blocks: 929292
No longer depends on: 929292
Priority: P3 → P2
Comment on attachment 8402262 [details] [diff] [review]
initial approach

Review of attachment 8402262 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/ScriptLoader.cpp
@@ +504,5 @@
> +    if (!inheritCSP) { finalURI->SchemeIs("about", &inheritCSP); }
> +    if (!inheritCSP) { finalURI->SchemeIs("file", &inheritCSP); }
> +    if (!inheritCSP) { finalURI->SchemeIs("javascript", &inheritCSP); }
> +    if (!inheritCSP) { finalURI->SchemeIs("chrome", &inheritCSP); }
> +    if (!inheritCSP) { finalURI->SchemeIs("resource", &inheritCSP); }

Over in bug 1223647 I already made it so that workers don't inherit from parent window's for http-urls, but we do inherit for schemes like blob:

Also note that data: workers are always "sandboxed" (at least when started by webpages), and not allowed to make any network requests. So CSP would mainly prevent eval() and Function() from being used.

I don't think we allow javascript URLs in workers at all, do we?

I also don't think that you want to inherit CSP for chrome: and resource:, that sounds scary.
Whiteboard: [CSP 1.1] → [CSP 1.1], [domsecurity-backlog]
See Also: → 1270152
Bug 1223647 fixed the inheritance problem, but we should still implement the part of the spec which allows shipping workers with their own CSP.
Depends on: 1223647
Summary: CSP 1.1: Workers have their own CSP policies, should not inherit from parent document → CSP 1.1: Implement CSP for Workers
Assignee

Updated

3 years ago
Blocks: 1251378
Assignee

Updated

3 years ago
Duplicate of this bug: 1270152
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #12)
> Bug 1223647 fixed the inheritance problem, but we should still implement the
> part of the spec which allows shipping workers with their own CSP.

Chris, thanks for clarifying the relations of these related bugs.
Assignee: ettseng → tnguyen
Assign this bug to Thomas since he has worked on the patch for a while (in bug 1251378).
Assignee

Comment 16

3 years ago
Review commit: https://reviewboard.mozilla.org/r/53120/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53120/
Attachment #8753235 - Flags: review?(ckerschb)
Attachment #8753236 - Flags: review?(ckerschb)
Assignee

Updated

3 years ago
Duplicate of this bug: 1259648
Comment on attachment 8753235 [details]
Bug 959388 - Update web platform test meta file.

https://reviewboard.mozilla.org/r/53120/#review50042

Looks good to me, but we need a /worker peer to accept those changes. Potentially khuey or bkelly can help us out.

::: dom/workers/ScriptLoader.cpp:1177
(Diff revision 1)
> +      // deliver CSP from HTTP header.
> +      if (!mWorkerPrivate->GetCSP() && CSPService::sCSPEnabled) {
> +        NS_ConvertASCIItoUTF16 cspHeaderValue(tCspHeaderValue);
> +        NS_ConvertASCIItoUTF16 cspROHeaderValue(tCspROHeaderValue);
> +
> +        nsCOMPtr<nsIContentSecurityPolicy> csp;

Nit: move the definition of csp right before EnsureCSP()

::: dom/workers/ScriptLoader.cpp:1198
(Diff revision 1)
> +            NS_ENSURE_SUCCESS(rv, rv);
> +          }
> +
> +          // Set evalAllowed, default value is set in GetAllowsEval
> +          bool evalAllowed;
> +          bool reportEvalViolations;

Please initialize both, evalAllowed and reportEvalViolations to false
Attachment #8753235 - Flags: review?(ckerschb) → review+
Attachment #8753241 - Flags: review?(ckerschb) → review+
Whiteboard: [CSP 1.1], [domsecurity-backlog] → [CSP 1.1], [domsecurity-active]
Comment on attachment 8753236 [details]
Bug 959388 - Add csp worker test cases.

https://reviewboard.mozilla.org/r/53122/#review50056

Since we are blocked on a worker-peer review anyway, I would like Kate to review those tests.
Attachment #8753236 - Flags: review?(ckerschb)
Comment on attachment 8753236 [details]
Bug 959388 - Add csp worker test cases.

Kate, can you review those tests please?
Attachment #8753236 - Flags: review?(kmckinley)
Assignee

Comment 24

3 years ago
Comment on attachment 8753235 [details]
Bug 959388 - Update web platform test meta file.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53120/diff/1-2/
Attachment #8753235 - Attachment description: MozReview Request: Bug 959388 - Deliver CSP from HTTP header. r?ckerschb → MozReview Request: Bug 959388 - Deliver CSP from HTTP header. r=ckerschb
Attachment #8753236 - Flags: review?(kmckinley) → review?(ckerschb)
Assignee

Comment 25

3 years ago
Comment on attachment 8753236 [details]
Bug 959388 - Add csp worker test cases.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53122/diff/1-2/
Assignee

Comment 26

3 years ago
Comment on attachment 8753241 [details]
Bug 959388 - Deliver CSP from HTTP header.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53138/diff/1-2/
Assignee

Updated

3 years ago
Attachment #8753236 - Flags: review?(ckerschb) → review?(kmckinley)
Assignee

Updated

3 years ago
Attachment #8753235 - Flags: review?(khuey)
Assignee

Comment 27

3 years ago
Thanks Christoph for your review.
Hi Kyle. Could you please take a look into this?
Attachment #8753236 - Flags: review?(kmckinley)
Comment on attachment 8753235 [details]
Bug 959388 - Update web platform test meta file.

https://reviewboard.mozilla.org/r/53120/#review50504

::: dom/workers/ScriptLoader.cpp:1058
(Diff revision 2)
>        NS_ENSURE_SUCCESS(rv, rv);
>  
>        if (!requestSucceeded) {
>          return NS_ERROR_NOT_AVAILABLE;
>        }
> +      httpChannel->GetResponseHeader(

nit: \n after the }

::: dom/workers/ScriptLoader.cpp:1172
(Diff revision 2)
>        MOZ_ASSERT(NS_LoadGroupMatchesPrincipal(channelLoadGroup, channelPrincipal));
>  
>        mWorkerPrivate->SetPrincipal(channelPrincipal, channelLoadGroup);
> +
> +      // We did inherit CSP in bug 1223647. There's no csp here means we have to
> +      // deliver CSP from HTTP header.

I don't really follow what this comment is trying to say.  Maybe "If we do not already have a CSP, we should get it from the HTTP headers on the worker script?"

Note that I'm not actually sure that's what this is trying to say!

::: dom/workers/ScriptLoader.cpp
(Diff revision 2)
> +        // XHR Params Allowed
> +        mWorkerPrivate->SetXHRParamsAllowed(parent->XHRParamsAllowed());
> +      }
>      }
>  
> -    DataReceived();

Did the propagation of CSP from parent worker to child worker go away entirely?
Assignee

Comment 30

3 years ago
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #29)
> Did the propagation of CSP from parent worker to child worker go away
> entirely?
Yep. Basically, CSP is propagated from parent to child conditionally (we do inherit for schemes like blob:, fixed in bug 1223647) in SetPrincipal(...). If worker does not have CSP after this call, we have to deliver CSP from HTTP header. Should not call DataReceived which propagates CSP from parent to child unconditionally.

Yay, thanks Kyle for your review
Assignee

Comment 31

3 years ago
Comment on attachment 8753235 [details]
Bug 959388 - Update web platform test meta file.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53120/diff/2-3/
Attachment #8753235 - Attachment description: MozReview Request: Bug 959388 - Deliver CSP from HTTP header. r=ckerschb → MozReview Request: Bug 959388 - Deliver CSP from HTTP header. r=ckerschb,khuey
Attachment #8753236 - Attachment description: MozReview Request: Bug 959388 - Add csp worker test cases. r?ckerschb → MozReview Request: Bug 959388 - Add csp worker test cases. r=kmckinley
Attachment #8753241 - Attachment description: MozReview Request: Bug 959388 - Update web platform test meta file. r?ckerschb → MozReview Request: Bug 959388 - Update web platform test meta file. r=ckerschb
Attachment #8753236 - Flags: review?(kmckinley)
Assignee

Comment 32

3 years ago
Comment on attachment 8753236 [details]
Bug 959388 - Add csp worker test cases.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53122/diff/2-3/
Assignee

Comment 33

3 years ago
Comment on attachment 8753241 [details]
Bug 959388 - Deliver CSP from HTTP header.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53138/diff/2-3/
Assignee

Updated

3 years ago
Attachment #8753236 - Flags: review?(kmckinley) → review+
Assignee

Updated

3 years ago
Keywords: checkin-needed
Assignee

Comment 35

3 years ago
there're many oranges in Windows 7 test machine, but were ok right after that. I have no idea what's happened with windows 7, and all try runnings on window 7 have the same problem.
If they're clipboard related that's a known problem with tests running on AWS.
Assignee

Comment 37

3 years ago
Ah, I see they're related to clipboard, thanks for your clarification.
sorry had to back this out, seems this caused https://treeherder.mozilla.org/logviewer.html#?job_id=28422708&repo=mozilla-inbound
Flags: needinfo?(tnguyen)
Assignee

Comment 41

3 years ago
I don't see in the log there're traces related to workers and changes in this bug.
I see lots failures on try right now, which aborts at AddonManager, it seems there's no relation here. But if after backing out this could fix, please do it, I will check it immediately.
Flags: needinfo?(tnguyen)
Assignee

Comment 42

3 years ago
Could you please confront with my try result
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1354e2eb8ee
when this landed, we had a nice win on linux64 installer size:
https://treeherder.mozilla.org/perf.html#/alerts?id=1301
Assignee

Comment 44

3 years ago
Hmm, first time seeing this alert, I have to take a closer look.
Thanks for your clarification.
(In reply to Joel Maher (:jmaher) from comment #43)
> when this landed, we had a nice win on linux64 installer size:
> https://treeherder.mozilla.org/perf.html#/alerts?id=1301

Seems unlikely to be related ...
Assignee

Comment 47

3 years ago
Thanks for your looking.
Running on try today [1], and it's likely ok, even the failure you mentioned
I see that this bug and bug 890908 are merged and pushed to in-bound [2]
[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=e780b2c44871&filter-tier=1&filter-tier=2&filter-tier=3

[2] https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=89c46a67642d9c006fa086331db7082a12bcd426
I suspect that bug 890908 could cause the issue.
There are a number of error logs containing messages like:
Error: NetworkError: Failed to load worker script at resource://gre/modules/PageThumbsWorker.js (nsresult = 0x80004005)

PageThumbsWorker.js is the worker script loaded via PromiseWorker by:
https://dxr.mozilla.org/mozilla-central/source/toolkit/components/thumbnails/PageThumbs.jsm

While I think we'd normally expect that page to be loaded in a chrome context, newtab, which seems to have its own custom channel implementation, for example, does import that file:
https://dxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/newTab.js#12

The channel seems wacky; it seems to use this map from https://dxr.mozilla.org/mozilla-central/source/browser/components/newtab/NewTabRemoteResources.jsm

const NewTabRemoteResources = {
  MODE_CHANNEL_MAP: {
    production: {origin: "https://content.cdn.mozilla.net"},
    staging: {origin: "https://s3_proxy_tiles.stage.mozaws.net"},
    test: {origin: "https://example.com"},
    test2: {origin: "http://mochi.test:8888"},
    dev: {origin: "http://localhost:8888"}
  }
};

in the channel implementation which notably passes through the origin of what it's using:
https://dxr.mozilla.org/mozilla-central/source/browser/components/newtab/NewTabWebChannel.jsm#113

I'm assuming the try builds are using a different channel from mozilla-inbound and this changes the applied CSP.
Uh, which is to say, consistent with mozilla-inbound now having determined this patch-set to be the problem (the re-landing mentioned in comment 47 with bug 890908 failed again, but bug 890908 stuck again), I think we can probably conclude:
- It's this patch-set
- It probably involves newtab or some other magic channel varying its behavior when built on try versus mozilla-inbound.

I'll look into this a little more since I think it's the middle of the night for :tnguyen.
I pinged :oyiptong about this on IRC based on the authorship of many of the newtab commits, but meetings/etc. intervened.  Any thoughts on this, :oyiptong?

In particular, from the log https://public-artifacts.taskcluster.net/TLmAQHC8R5CNPQ_wEdHx6w/0/public/logs/live_backing.log we have the excerpt in this attachment.

This happens in many different logs from the attempted re-landing on Monday: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=0e9913cfd157075621ce869da2f73454164eb70f&selectedJob=28465444

It also happens in the cases where treeherder's error summaries choose to mention "FATAL ERROR: AsyncShutdown timeout in AddonManager: Waiting for providers to shut down."  If you look at the attached (long) stack, AddonManagerInternal is in there, so that likely also explains the shutdown timeout.

Note that the error does not happen in the logs from the same jobs from the backout of the patches:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=d3d23c5640717bfb9c72db8e951c462685991854


In terms of my own research, especially as it relates to the "browser.newtabpage.remote.mode" preference and channel choice, initial digging didn't show an immediate smoking gun.  It seems like both the try server and mozilla-inbound should end up with the value set to "production" and failing over to "production" if somehow missing.  I'm not sure MOZ_UPDATE_CHANNEL figures in at all, although it does seem to be "default" in all cases.

I'm not planning to do any more investigation at this time.
Flags: needinfo?(oyiptong)
Assignee

Comment 51

3 years ago
Wow, thank you so much for digging it :). I am looking at this, firstly if I could reproduce the failures.
There shouldn't be any discrimination between try and inbound/release repos for remote newtab.
RNT should be disabled by default anyway.

I'll try and help by taking a look and see what I can find.
Flags: needinfo?(oyiptong)
Assignee

Comment 53

3 years ago
Hmm, oddly I cloned inbound at the failed rev and the tests seems to be fine locally (although I did not run all)
I did modify something but still don't know how to reproduce the failure on inbound.
Does anybody have idea to reproduce the failure?
Assignee

Updated

3 years ago
Flags: needinfo?(cbook)
(In reply to Thomas Nguyen[:tnguyen] from comment #53)
> Hmm, oddly I cloned inbound at the failed rev and the tests seems to be fine
> locally (although I did not run all)
> I did modify something but still don't know how to reproduce the failure on
> inbound.
> Does anybody have idea to reproduce the failure?

i don't know how to reproduce the failure, but there is always the option to loan a slave from releng, so you would have what inbound is setup https://wiki.mozilla.org/ReleaseEngineering/How_To/Request_a_slave
Flags: needinfo?(cbook)
Assignee

Comment 55

3 years ago
Comment on attachment 8753235 [details]
Bug 959388 - Update web platform test meta file.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53120/diff/3-4/
Attachment #8753235 - Attachment description: MozReview Request: Bug 959388 - Deliver CSP from HTTP header. r=ckerschb,khuey → Bug 959388 - Update web platform test meta file.
Attachment #8753236 - Attachment description: MozReview Request: Bug 959388 - Add csp worker test cases. r=kmckinley → Bug 959388 - Add csp worker test cases.
Attachment #8753241 - Attachment description: MozReview Request: Bug 959388 - Update web platform test meta file. r=ckerschb → Bug 959388 - Deliver CSP from HTTP header.
Attachment #8753235 - Flags: review+
Attachment #8753236 - Flags: review+ → review?(kmckinley)
Attachment #8753241 - Flags: review?(khuey)
Assignee

Comment 56

3 years ago
Comment on attachment 8753236 [details]
Bug 959388 - Add csp worker test cases.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53122/diff/3-4/
Assignee

Comment 57

3 years ago
Comment on attachment 8753241 [details]
Bug 959388 - Deliver CSP from HTTP header.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53138/diff/3-4/
Assignee

Comment 58

3 years ago
Thanks everybody for helping me to look at this bug.

Hmm, I have no idea when I push to reviewboard, it only posts here link to diff version 3-4.
The diff should be https://reviewboard.mozilla.org/r/53118/diff/4-5/

The only change of new patch is remove the line NS_ENSURE_SUCCESS(rv,rv) 
of block
        nsCOMPtr<nsIContentSecurityPolicy> csp;
        rv = principal->EnsureCSP(nullptr, getter_AddRefs(csp));
        NS_ENSURE_SUCCESS(rv, rv);

because Bug 1273364 changes getting CSP's systemPrincipal behavior. I could reproduce on try now after rebasing the fix of Bug 1273364 (try result on comment 42 and 47 succeeded because Bug 1273364 was pushed to inbound but had not pushed to central)
Assignee

Comment 59

3 years ago
Comment on attachment 8753236 [details]
Bug 959388 - Add csp worker test cases.

Carry r+ for test case
Attachment #8753236 - Flags: review?(kmckinley) → review+
Comment hidden (typo)
Assignee

Comment 62

3 years ago
Hi Kyle, could you please review the new patch at comment 58?
Thanks
Can you explain what changed ...?
Flags: needinfo?(tnguyen)
Assignee

Comment 64

3 years ago
Oh, sorry I did not explain enough clearly the changes.
The old patch (reviewed)

In ScriptLoader.cpp

  nsresult
  OnStreamCompleteInternal(nsIStreamLoader* aLoader, nsresult aStatus,
                           uint32_t aStringLen, const uint8_t* aString,
                           ScriptLoadInfo& aLoadInfo)
.....
+        nsCOMPtr<nsIContentSecurityPolicy> csp;
+        rv = principal->EnsureCSP(nullptr, getter_AddRefs(csp));
+        NS_ENSURE_SUCCESS(rv, rv);
.....

 return NS_OK;
}

Bug 1273364 was landed and changed the result of principal->EnsureCSP() on a systemPrincipal (from NS_OK to NS_ERROR_FAILURE), hence we would bail out of OnStreamCompleteInternal function early with NS_ERROR_FAILURE return value.
As the result, some workers (add-on) which are triggered by system principal are refused to load. That's exactly the reason why we got the failures above.
The only change of new patch compared to the reviewed one is removing 
+        NS_ENSURE_SUCCESS(rv, rv);
to prevent OnStreamCompleteInternal from returning error in system principal case. We did ignore the CSP headers in this case (see if(csp) condition) and that's enough.

Thanks
Flags: needinfo?(tnguyen)
Comment on attachment 8753241 [details]
Bug 959388 - Deliver CSP from HTTP header.

https://reviewboard.mozilla.org/r/53138/#review58276

Ok, r=me based on that comment.
Attachment #8753241 - Flags: review?(khuey) → review+
Assignee

Updated

3 years ago
Keywords: checkin-needed
needs rebasing :

applying 8ede55b11e57
patching file dom/workers/test/mochitest.ini
Hunk #1 succeeded at 14 with fuzz 1 (offset 0 lines).
8ede55b11e57 transplanted to adbe4695de65
applying cf9bacc3dfd1
patching file dom/base/nsDocument.cpp
Hunk #3 FAILED at 2903
1 out of 3 hunks FAILED -- saving rejects to file dom/base/nsDocument.cpp.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue
Flags: needinfo?(tnguyen)
Keywords: checkin-needed
Assignee

Comment 68

3 years ago
I have no idea about ReviewBoard. Could you please try the pure patches?
Thanks
Flags: needinfo?(tnguyen)
Assignee

Comment 69

3 years ago
Attachment #8402262 - Attachment is obsolete: true
Attachment #8753235 - Attachment is obsolete: true
Attachment #8753236 - Attachment is obsolete: true
Attachment #8753241 - Attachment is obsolete: true
Attachment #8756123 - Attachment is obsolete: true
Attachment #8766654 - Flags: review+
Assignee

Comment 70

3 years ago
Attachment #8766655 - Flags: review+
Assignee

Comment 71

3 years ago
Posted patch Deliver CSP from HTTP header. (obsolete) — Splinter Review
Attachment #8766656 - Flags: review+
Assignee

Comment 72

3 years ago
Reviewboard became oddly when I updated my patches (rev 5 but it only points to rev 3, 4). Please try attached patches. Thanks
Assignee

Comment 73

3 years ago
Hmm, I see, there're differences between inbound and mozilla-central. Updated patch for inbound branch
Assignee

Comment 74

3 years ago
Attachment #8766656 - Attachment is obsolete: true
Attachment #8766657 - Flags: review+
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 75

3 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/61bc9a864653
Update web platform test meta file. r=ckerschb
https://hg.mozilla.org/integration/mozilla-inbound/rev/da7b881d5d9d
Add csp worker test cases. r=kmckinley
https://hg.mozilla.org/integration/mozilla-inbound/rev/35f26ccf79d0
Deliver CSP from HTTP header. r=ckerschb, r=khuey
Keywords: checkin-needed

Comment 76

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/61bc9a864653
https://hg.mozilla.org/mozilla-central/rev/da7b881d5d9d
https://hg.mozilla.org/mozilla-central/rev/35f26ccf79d0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Awesome!  Thanks for making this bug to completion, Thomas!

Updated

3 years ago
Depends on: 1286024
I've added a note on this in the worker docs:

https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Using_web_workers#Content_security_policy

and in the docs for the Content-Security-Policy header:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy

Please let me know if this covers things.
Flags: needinfo?(tnguyen)
Assignee

Comment 79

3 years ago
"To specify a content security policy for the worker, you can set a Content-Security-Policy header for the worker's own source".

This is not the Content-Security-Policy header for the worker's own source, I think it should be : The content security policy for the worker is specified by the Content-Security-Policy response header of request which delivered the worker script.
Flags: needinfo?(tnguyen)
Thanks, that is what I was trying to say, but yours is much clearer.
You need to log in before you can comment on or make changes to this bug.