Closed Bug 1500061 Opened 1 year ago Closed 1 year ago

TIghten CSP for activity stream

Categories

(Firefox :: Messaging System, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 68
Iteration:
68.3 - Apr 15 - 28
Tracking Status
firefox68 --- fixed

People

(Reporter: ckerschb, Assigned: k88hudson)

References

(Blocks 1 open bug)

Details

(Keywords: github-merged)

Attachments

(3 files, 2 obsolete files)

Within Bug 1448359 [1] we discussed tightening the CSP for activity stream to remove the 'unsafe-inline' bits [2].

Filing this bug to see if we might be able to lock things down gradually.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1448359#c9
[2] https://searchfox.org/mozilla-central/source/browser/components/newtab/bin/render-activity-stream-html.js#100
Eventually we wanna turn the assertion within AssertAboutPageHasCSP() to also cover that a CSP does not include 'unsafe-inline'.
Blocks: 1448359
Iteration: --- → 65.2 (Nov 16)
Priority: -- → P2
Iteration: 65.2 (Nov 16) → 65.3 (Nov 30)
Iteration: 65.3 (Nov 30) → 66.1 - Dec 10-23
Iteration: 66.1 - Dec 10-23 → 67.1 - Jan 28 - Feb 10
Assignee: nobody → khudson
Priority: P2 → P1
Attachment #9042574 - Flags: review?(fbraun)

Drive by note: Removing 'unsafe-inline' from script-src is definitely the way to go and I am really happy to see that happening. Ultimately it would be great if we could also remove unsafe-inline from style-src and load all stylesheets from a file instead of inlining it - but we could do that in a follow up.

For this revision however - would it be possible to add object-src 'none'?

Comment on attachment 9042574 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4778

@Christoph: I think unsafe-inline style is required due to bug 1513311
+1 to adding `object-src 'none'` as I'm sure we do not want to add flash resources :-)

I also commented on GitHub:
> Looks good! +1
> The adjacent `connect-src` surprises me a bit. Do you actually use fetch, XHR, EventSource, sendBeacon, `<a ping>` or Websockets?
> To clarify, I found it just surprising. Not concerning :)
Attachment #9042574 - Flags: review?(fbraun) → feedback+

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #3)

Ultimately it would be great if we could also remove unsafe-inline from style-src and load all stylesheets from a file instead of inlining it - but we could do that in a follow up.

Bug 1513311 uses CSSOM to dynamically add a sheet and rules to sanitize css-ish json from the server, so that approach would probably need to change to work without inline styles.

See Also: → 1513311
Keywords: github-merged
Blocks: 1527504

freddyb, we're running into mochitest failures:

Console message: [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”)." {file: "about:newtab" line: 16}]
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228060887&repo=try&lineNumber=1980-1987

Have you seen those before related to testing? My current guess is ContentTask.spawn ends up stringifying a function to send as a message to then eval in the content side. Is there a way to disable CSP when testing?

Flags: needinfo?(fbraun)
Iteration: 67.1 - Jan 28 - Feb 10 → 67.3 - Feb 25 - Mar 10

(In reply to Ed Lee :Mardak from comment #6)

freddyb, we're running into mochitest failures:

Console message: [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”)." {file: "about:newtab" line: 16}]
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228060887&repo=try&lineNumber=1980-1987

Have you seen those before related to testing? My current guess is ContentTask.spawn ends up stringifying a function to send as a message to then eval in the content side. Is there a way to disable CSP when testing?

This error message complains about inline script, so it must be either a <script>...</script> tag or an on.. attribute event handler.

eval related errors get a different message (An attempt to call JavaScript from a string (by calling a function like eval) has been blocked).

Flags: needinfo?(fbraun)
Iteration: 67.3 - Feb 25 - Mar 10 → 67.4 - Mar 11 - 17
Attachment #9050747 - Attachment is obsolete: true
Iteration: 67.4 - Mar 11 - 17 → 68.1 - Mar 18 - 31
Iteration: 68.1 - Mar 18 - 31 → 68.2 - Apr 1 - 14
Iteration: 68.2 - Apr 1 - 14 → ---
Priority: P1 → P2
Blocks: 1546535
Status: NEW → RESOLVED
Iteration: --- → 68.3 - Apr 15 - 28
Closed: 1 year ago
Keywords: github-merged
Priority: P2 → P1
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Attachment #9050774 - Attachment is obsolete: true
See Also: → 1553375
Component: Activity Streams: Newtab → Messaging System
Blocks: 1567867
See Also: → 1584485
You need to log in before you can comment on or make changes to this bug.