1500061 - TIghten CSP for activity stream

Status: RESOLVED FIXED

(Firefox :: Messaging System, enhancement, P1)

Description

[Christoph Kerschbaumer [:ckerschb]]

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

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Eventually we wanna turn the assertion within AssertAboutPageHasCSP() to also cover that a CSP does not include 'unsafe-inline'.

Comment 2

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4778

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

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 4

[Frederik Braun [:freddy]]

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 :)

Comment 5

[Ed Lee :Mardak]

(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.

Comment 6

[Ed Lee :Mardak]

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?

Comment 7

[Ed Lee :Mardak]

Reverted with https://github.com/mozilla/activity-stream/commit/a3fe11f0f32415c98fa7a31b996735b4d580bfb1

Comment 8

[Frederik Braun [:freddy]]

(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).

Comment 9

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4846

Comment 10

[Kate Hudson :k88hudson]

Attachment: Bug 1500061 - Tighten CSP for Activity Stream

Comment 11

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4847

Comment 12

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4916

Comment 13

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/01c3e1ae707d