search

mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
127 stars 41 forks source link

Turn on CSP #1660

Closed magopian closed 8 years ago

magopian commented 8 years ago

This is something we keep thinking about, and then forgetting. @muffinresearch care to give a few hints and tips on this one? Advices or pointers on how to do that? Any idea how big a project it is?

andymckay commented 8 years ago

See https://bugzilla.mozilla.org/show_bug.cgi?id=594584 for old information.

muffinresearch commented 8 years ago

The config should already be in place - so it just needs updating based on testing.

The biggest impact will be the potential for blocking something that matters. The best way to deal with that is going to be to turn-it on for real in local dev and maybe addons-dev initially. Look for blocked scripts and iterate on the config until everything works.

Once we have that working we can go to stage and have QA do similar checks. It's going to be a similar process to dealing with jquery-migrate. For that reason we shouldn't do both at the same time.

The only other thing is working out where reports should go - we probably want @amuntner to chime in on that.

andymckay commented 8 years ago

Make sure this works in docker: https://bugzilla.mozilla.org/show_bug.cgi?id=1210633

jvehent commented 8 years ago

FYI, we're working on generalizing the reporting and deduplication pipeline for cloud services. I'd suggest using the same reporting method @micheletto put in place for Hello, which uses a report URI in Nginx and feeds into Heka. (cc @jasonthomas)

andymckay commented 8 years ago

We ddos'ed ourselves last time due to https://bugzilla.mozilla.org/show_bug.cgi?id=615708. Just so you know.

jvehent commented 8 years ago

@andymckay Are you suggesting that sending reports to the same origin might induce a ddos risk?

andymckay-limited-access commented 8 years ago

It has in the past, which is why it was never turned on. Would like to avoid that again.

On Thu, Dec 17, 2015 at 10:19 AM, Julien Vehent [:ulfr] < notifications@github.com> wrote:

@andymckay https://github.com/andymckay Are you suggesting that sending reports to the same origin might induce a ddos risk?

— Reply to this email directly or view it on GitHub https://github.com/mozilla/olympia/issues/995#issuecomment-165536899.

andymckay commented 8 years ago

Tracker mozilla/addons#2282

april commented 8 years ago

I'm certainly willing to provide assistance on CSP as well, if anybody has any questions. Just let me know! :)

muffinresearch commented 8 years ago

@marumari thanks - we'd definitely appreciate any time you have in casting your eye over the configuration / headers that we're currently running on -dev and stage.

muffinresearch commented 8 years ago

So an update - the remaining issues are the endpoint being setup and finishing off the update to recaptcha which is proving to be taking time due to issues with the libs and l10n.

Those are:

I've mentioned to QA to look-out for CSP violations.

Once those are done we can turn off report-only mode.

april commented 8 years ago

By and large it looks great to me from a CSP perspective. Really really nice work! The only things I would possibly suggest:

With CSP, AMO will actually have the highest security score of any of our major websites. AMO-Stage already does. :smile: The only other things that it needs to achieve a perfect score are:

I can open up a new bug for those, if you like?

muffinresearch commented 8 years ago

@marumari thanks for the info, it's nice to hear we're on the right track!

Re: upgrade-insecure-requests. That option isn't currently supported by django-csp but I've already filed a bug re: updating to all the CSP 2.0 options. [1]

Re: the X-XSS-Protection header do you know if the bug in IE8 [2] was fixed? I've tried to find some info on it but there doesn't seem to be any clear info as to when it was patched. Some resources say it's best to set X-XSS-Protection to 0 for IE < 9.

We could also potentially use the reflected-xss option in CSP [3] which is equivalent to X-XSS-Protection according to the 1.1 spec [4]. (Though we would need to check the support and the same caveat may apply for IE8). Again that's not yet available in django-csp.

I'll file an ops bug to add X-Content-Type-Options: nosniff.

[1] https://github.com/mozilla/django-csp and https://github.com/mozilla/django-csp/issues/50 [2] https://hackademix.net/2009/11/21/ies-xss-filter-creates-xss-vulnerabilities/ [3] https://www.w3.org/TR/2014/WD-CSP11-20140211/#reflected-xss [4] https://www.w3.org/TR/2014/WD-CSP11-20140211/#relationship-to-x-xss-protection

april commented 8 years ago

I believe that Microsoft fixed those bugs over the course of a couple patches in 2010. Twitter, Google, and GitHub all set it to "1; mode=block" even if you set your UA string to IE8.

I should also note that IE8 is past its EOL date, so I don't think we're generally too concerned about supporting it from a security perspective outside of bedrock as it is.

muffinresearch commented 8 years ago

I should also note that IE8 is past its EOL date, so I don't think we're generally too concerned about supporting it from a security perspective outside of bedrock as it is.

Hah, yes good point!

muffinresearch commented 8 years ago

OK I've fIled the following ops tickets re the headers.

Update: this is now superceded by mozilla/addons#2425 since we are setting other related headers in django already.

muffinresearch commented 8 years ago

Here's an etherpad with some qa notes: https://public.etherpad-mozilla.org/p/csp-qa-addons

muffinresearch commented 8 years ago

Ok so latest update the report-only mode is back for another week so we can clear up the last remaining issues.

ValentinaPC commented 7 years ago

This can be marked as verified fixed because all related bugs were verified or superseded.