594584 - Implement Content Security Policy (CSP) for Olympia / AMO (addons.mozilla.org)

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Code Quality, enhancement, P5)

Description

[James Socol [:jsocol, :james]]

Django-CSP[1] is a complete, if simplistic, implementation of Content Security Policy middleware for Django. It's ready to go and flip on CSP in production.

Hopefully the instructions in the README are clear. If not, let me know.

I also welcome feedback and patches on the project as a whole.

Be aware of bug 594446 if you plan on using the report-uri feature, though that should be fixed soon enough. Also, my report processor just emails admins. I'd like to do something fancier but this was designed to be minimal and working.

[1] http://github.com/mozilla/django-csp

Comment 1

[Wil Clouser [:clouserw]]

Probably won't hit 5.12, but I'm looking at this w/ some people.

Comment 2

[Wil Clouser [:clouserw]]

This is ready to go at http://github.com/clouserw/zamboni/tree/csp

but I'm not committing it because it breaks all the dev sites.  Bug 594446 would fix that.

Comment 3

[Wil Clouser [:clouserw]]

Future Wil:  You already committed this to vendors/, so you're good to go.

Comment 4

[Wil Clouser [:clouserw]]

This is on and in report-only mode.  Kicking a couple milestones down to consider turning off report-only.

Comment 5

[Wil Clouser [:clouserw]]

*sigh* -> 4.x triaged

Comment 7

[Wil Clouser [:clouserw]]

Current status:

The bugs blocking this one are causing so much traffic that we can't turn CSP on, even in reporting mode.  Until they are fixed this is dead in the water.

Comment 8

[Frederik Braun [:freddy]]

Wil, it seems like a lot of blockers have been resolved. Can we try to move forward on this?

Comment 9

[Wil Clouser [:clouserw]]

(In reply to Frederik Braun [:freddyb] from comment #8)
> Wil, it seems like a lot of blockers have been resolved. Can we try to move
> forward on this?

No, the remaining bug, bug 615708, is a hard blocker.  See comment 9 and comment 15 from that bug.

Comment 10

[Daniel Veditz [:dveditz]]

I'd like to breathe some life back into this--what needs to happen to unblock things? Fixing bug 615708 doesn't seem like the only way forward. For example, you could remove the report-uri and then you won't get swamped with reports.

If we can't make CSP work for AMO (which is far simpler than many commercial sites) then we've had a major failure. I think we can get there through a combination of Firefox client changes (we probably don't have too many Chrome visitors to AMO), AMO policy changes, and maybe as a last resort we'll learn that we have to push for spec changes. Since we're barreling toward approving version 1.1 of the spec now would be a good time to figure out what's not working before it's even more set in stone.

How are the CSP headers generated for AMO? The main site doesn't have them but addons.allizom.org and addons-dev.allizom.org are using the old-style header. Django-CSP mentioned at the top of this bug seems to generate only the new-style header so we're not using that apparently (or are using a very old version?).

1. We should use the standard Content-Security-Policy header. (syntax is slightly different, too, e.g. "default-src" instead of "allow")

2. Both sites are using policy-uri which didn't make it into the spec. If header length is a problem (and with the current complex policy it might be!) we could instead generate a <meta> tag policy in each page's header.

2a. Firefox doesn't support <meta> policies yet, so we may need to block on bug 663570

3. the current allizom policies are complex. It's a noble attempt to define every expected resource, but it exacerbates the addon-triggered reporting problem. A simpler policy could be just as safe in terms of blocking XSS attempts. for example:

   <CSP>: default-src *; object-src 'none'; script-src 'self' <others>; report-uri....

4. the current script-src includes www.google.com. CSP 1.1 supports paths so it would be better to have something more restrictive if possible like www.google.com/js/ because google hosts a lot of stuff.

5. The current allizom policies seem to trigger two reports on each page even without addon-caused headaches. One is a font whose source wasn't added to the list (my proposed header fixes that by not restricting fonts, or we just add the CDN domain), and the other was a Google library trying to call eval(). The latter is trickier. Would hate to have to add 'unsafe-eval' to the policy, but if you can't find an alternative to the Google thing I don't know what else to do. Google's trying to use CSP in more places though, they may have a csp-safe equivalent. On the other hand I can't tell what's broken because of it so maybe it's extraneous anyway.

6. we could always turn off reporting if that's what's swamping us (bug 615708), or inject report-uri only some fraction of the time as a spot check. If we're not paying attention to the reports we might as well not bother with them.

7. Maybe CSP needs a feature to limit reporting. This could be something strictly in our own implementation (heuristic duplicate suppression) or maybe we need to lobby for features in the spec to make things easier.

  7a. could add a fractional term to the report-uri directive. old clients would ignore
      the syntax (I hope, will have to test the difference between syntax in theory and
      fragile implementations) and new clients could use that as the odds of reporting.

  7b. could maybe add a new directive 'noreport-src' that's a list of hosts that you don't
      want to see reports about. Of course that's as much work and space as just adding
      them to the whitelist--maybe not so useful ("I still want to block these just don't
      bother telling me").

Comment 11

[Daniel Veditz [:dveditz]]

In mail Wil also suggested two other possibilities for reducing the reporting deluge

a. implement and respect a back-off signal from report sinks

b. extend the report syntax so that clients can bundle multiple reports into a single POST rather than sending them one-by-one.

Comment 13

[Andy McKay]

Tracked in https://github.com/mozilla/olympia/issues/995.

Comment 14

[April King [:April]]

Renaming for clarity in the tracking bug.