Switch Activity-Stream CSP on, including about:welcome
Categories
(Firefox :: New Tab Page, enhancement, P2)
Tracking
()
People
(Reporter: Mardak, Assigned: k88hudson)
References
Details
(Keywords: sec-want, Whiteboard: [adv-main63-])
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
Details |
From https://github.com/mozilla/activity-stream/issues/3045 We added a CSP via meta tag in activity-stream.html, but right now it's set to Report-Only. It would be nice to get someone from security to review our current CSP It would also be helpful to know if there's a way to prevent us from being iframed from a tag CSP. frame-ancestors didn't appear to work when tried.
Comment 1•7 years ago
|
||
The recently landed bug 1449845 requires new about: pages to have CSP policies, and currently about:newtab and about:home are grandfathered into a whitelist. For about:welcome, therefore, we need to do something. @ewright figured out a smart solution for just doing that, but after some discussion, we think we'd rather just get all three birds knocked out with one stone, rather than a) entraining our (possibly non-trivial) CSP cleanup and fallout handling in an essentially unrelated patch which is already large or b) leaving the code & security story in an arbitrarily inconsistent state that may (or may not) be vulnerable to racing. So, bumping this up to 62.1 and making it block about:welcome.
Updated•7 years ago
|
Updated•7 years ago
|
Updated•6 years ago
|
Comment 2•6 years ago
|
||
Current policy: script-src 'unsafe-inline'; img-src http: https: data: blob:; style-src 'unsafe-inline'; child-src 'none'; object-src 'none'; script-src: - Can the script loading at the bottom of about:welcome be done inside a script of it's own to remove the use of unsafe-inline scripts? - Don't you need chrome: and resource: here? img-src: βοΈ style-src: Don't you need chrome: and resource: here? child-src -> frame-src object-src: βοΈ Other comments: - We should likely add a default-src of 'none' to simplify and secure the CSP - Also is the reporting even working as it's delivered by meta tag? - Did the reporting get a data review? - My other concern is that the CSP is in each translation html file in central. Won't this increase the risk that it gets modified accidentally on a locale? Or are these files generated like the path suggests? I would suggest the following would be possible: <meta http-equiv="Content-Security-Policy" content="default-src 'none'; script-src chrome: resource:; img-src http: https: data: blob:; style-src chrome: resource: 'unsafe-inline';">
Comment 3•6 years ago
|
||
Thanks :jkt; A few more notes: * Ideally we should not make use of 'unsafe-inline' at all, can we change the code so we do not need 'unsafe-inline' in the CSP? * Do we need img-src http: as well? I thought we load everything over https anyway? * default-src 'none' would be great to have. * having object-src 'none' is great! Happy to discuss in a meeting if needed. Thanks for adding a CSP!
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 4•6 years ago
|
||
Comment 5•6 years ago
|
||
Commit pushed to master at https://github.com/mozilla/activity-stream https://github.com/mozilla/activity-stream/commit/8d599e1775483355b7e844e734faff55be33d525 feat(prerender): Enable CSP for Activity Stream (#4211) Fix Bug 1448359 - Switch Activity-Stream CSP on, including about:welcome
Updated•6 years ago
|
Comment 7•6 years ago
|
||
Hey Ed, it's great to see that we are moving from a report-only policy to a policy that is going to enforced. I have two quick questions: 1) Is the CSP shipped here going to be applied to all about:newtab as well as about:welcome pages? If so, that would allow us to remove about:newtab as well as about:welcome (potentially about:home) from the whitelist of the 'every content privileged about page has a CSP' here [1], right? [1] https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#2507 2) I see that the CSP applied contains 'unsafe-inline' within script-src as well as style-src [2]. I do understand that applying a strong CSP is not an easy undertaking. I am mainly curious if we have a plan to tighten that CSP further for upcoming releases? I am more than happy to chat if I/we can be of any help here - thanks! [2] <meta http-equiv="Content-Security-Policy" content="default-src 'none'; script-src 'unsafe-inline' resource: chrome:; connect-src https:; img-src https: data: blob:; style-src 'unsafe-inline';">
Reporter | ||
Comment 8•6 years ago
|
||
k88hudson can provide more details for comment 7 1) Yes, it's applied to all about:newtab about:home and about:welcome view-source:about:newtab view-source:about:home view-source:about:welcome (They all by default load the same file: FirefoxNightly.app/Contents/Resources/browser/features/activity-stream@mozilla.org.xpi!/chrome/content/prerendered/en-US/activity-stream.html) 2) I believe the unsafe-inline is to allow for existing snippets, which we're in the process of replacing with Activity Stream Router and templates
Assignee | ||
Comment 9•6 years ago
|
||
The reason for unsafe in-line in script and style is indeed because of the implementation of snippets; changing this requires a rewrite of the system. We do have a plan to replace this by 63, the metabug for which is here: https://bugzilla.mozilla.org/show_bug.cgi?id=1432588
Comment 10•6 years ago
|
||
Hi, do you plan to request this patch to be uplifted to 62 beta or can I change the tracking flag for Firefox 62 from affected to wontfix? Thanks
Reporter | ||
Updated•6 years ago
|
Assignee | ||
Comment 11•6 years ago
|
||
If this didn't make it into 62, I think it's ok just to have it land with 63 (i.e. wontfix for 62) unless anyone else has any objections.
Reporter | ||
Updated•6 years ago
|
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Comment 12•5 years ago
|
||
Looking at this again, it looks like all of the issues are resolved.
The current policy I am seeing is:
default-src 'none'; object-src 'none'; script-src resource: chrome:; connect-src https:; img-src https: data: blob:; style-src 'unsafe-inline';
The only thing I see that isn't addressed is the unsafe inline styles. Shall we file that as a low priority follow up bug?
Reporter | ||
Comment 13•5 years ago
|
||
What are we revisiting or what's the question about "prerendered CSP"? The nightly CSP is set to
<meta http-equiv="Content-Security-Policy" content="default-src 'none'; object-src 'none'; script-src resource: chrome:; connect-src https:; img-src https: data: blob:; style-src 'unsafe-inline';">
Comment 14•5 years ago
|
||
Sorry I updated my comment.
Shall I file a bug about the unsafe-inline styles and close this off? Everything else seems fine.
Reporter | ||
Comment 15•5 years ago
|
||
The style-src can't be removed now while we have bug 1513311 conditionally adding a dynamically generated stylesheet (where the server can optionally provide selectors and declarations that then get sanitized with CSSOM). This was added for rapid experimentation and dynamic layouts without requiring Firefox code changes for temporary style overrides.
If we want to remove that or change how it's implemented, we'll want a separate bug.
Updated•5 years ago
|
Description
•