Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content Security Policy #332

Closed
cgiffard opened this issue Aug 4, 2013 · 9 comments
Closed

Content Security Policy #332

cgiffard opened this issue Aug 4, 2013 · 9 comments

Comments

@cgiffard
Copy link
Contributor

cgiffard commented Aug 4, 2013

I know this probably isn't a huge priority for 0.3, but we should think about incorporating content-security policy headers, at least for the admin panel, and possibly for the user theme too (although we'll probably need to give them the ability to turn it off for the theme — can't protect users from themselves!)

Rationale: CSP is widely supported by modern browsers and is totally ignored by those which don't — meaning it can't damage the experience for substandard UAs. While CSP won't stop us from falling victim to XSS vulnerabilities, it will dramatically limit their scope and severity.

Implementation, once we've agreed on what policy we want to establish, is trivial —only requiring the creation of an express middleware function which simply sets the relevant headers headers (possibly read out of config.)

While 0.3 is probably out the window for CSP, we should think about incorporating it before too much development on Ghost (which might not play so well with CSP if not planned for) is undertaken.

@cgiffard
Copy link
Contributor Author

cgiffard commented Aug 4, 2013

I'm happy to look into CSP if deemed suitable, and when directed.

@ErisDS
Copy link
Member

ErisDS commented Aug 4, 2013

Sounds like a very good idea for the admin panel. Do you have recommendations?

@cgiffard
Copy link
Contributor Author

cgiffard commented Aug 4, 2013

I would suggest blocking all inline and cross-site-originated scripts, and making as much effort as possible never to use inline scripts (since they're the primary XSS vector of attack!) Also if we permit ourselves to use inline scripts, we need to tweak CSP to allow them, meaning we loose nearly all of the benefits of using CSP in the first place.

Then, if we've got domains we want to draw external code from, such as using jQuery off the Google CDN or something, we whitelist those specifically.

What I am not sure about yet, and haven't researched, is scoping. Can we scope the policy to a resource or URI path? Or is it universal to the domain? That'll affect our decision — I'll look into that tomorrow.

Also, do we want to just roll out CSP site-wide, but perhaps allow it to be turned off for the non-admin areas via the config? I can't think of too many reasons to include inline scripts in a theme, with the possible exception of analytics and tracking code, which is a bit of a spanner in the works.

@cgiffard
Copy link
Contributor Author

cgiffard commented Aug 4, 2013

Also, here's some food for thought:

Anytime a requested resource or script execution violates the policy, the browser will fire a POST request to the value specified in report-uri containing details[14] of the violation.

cgiffard referenced this issue in cgiffard/Ghost Sep 26, 2013
Fixes #332

- Added middleware for delivery of CSP headers.
- Set basic policy which blocks remote scripts and styles from /ghost/
- CSP blocks inline scripts by default. Therefore the Ghost.init()
  had to be removed from the HBS and shifted into the init script.
@ghost ghost assigned cgiffard Oct 27, 2013
@ErisDS
Copy link
Member

ErisDS commented Oct 29, 2013

This issue will sit in wait for the results from #1267

@ErisDS ErisDS modified the milestones: 0.5 Multi-user, Future May 25, 2014
@ErisDS ErisDS assigned ErisDS and unassigned cgiffard May 25, 2014
@ErisDS
Copy link
Member

ErisDS commented May 25, 2014

I'm putting together a whitelist based on data gathered from Ghost(Pro). Once this is completed, we need to setup the policy along with a new config option so that it is possible to add to the whitelist.

@ErisDS ErisDS mentioned this issue Jun 17, 2014
26 tasks
ErisDS added a commit to ErisDS/Ghost that referenced this issue Jul 24, 2014
@ErisDS
Copy link
Member

ErisDS commented Jul 24, 2014

I made a PR for CSP. The original PR attempted to use a nonce to allow the inline script we required to boot the Ember admin, but the nonce system doesn't yet work properly across browsers.

Therefore @sebgie took this over and looked into removing the inline script which he did with #3351. I then rebased my PR #3323. This now has a CSP which is operating on the Ghost client.

However, the CSP, although it blocked the inline script which booted Ember, does not block inline scripts which exist in the editor or content screen previews. This kind of renders the exercise mute :/

@ErisDS ErisDS modified the milestones: Future, 0.5 Multi-user Jul 24, 2014
@halfdan
Copy link
Contributor

halfdan commented Sep 28, 2014

@ErisDS ErisDS removed their assignment Oct 21, 2014
@ErisDS ErisDS removed this from the Future Backlog milestone Oct 9, 2015
@ErisDS ErisDS added the later [triage] Things we intend to work but are not immediate priority label Oct 9, 2015
@ErisDS
Copy link
Member

ErisDS commented Oct 9, 2015

Going to close this for now with the label later to reopen when CSP becomes more widely supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants