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

Handle Content-Security Policy (CSP) violation reports #729

Closed
benvinegar opened this Issue Jan 16, 2013 · 39 comments

Comments

Projects
None yet
@benvinegar
Copy link
Member

benvinegar commented Jan 16, 2013

Content-Security Policy (CSP) is a new browser feature that makes it possible to restrict JavaScript files and other assets to a trusted set of origins:

http://www.html5rocks.com/en/tutorials/security/content-security-policy/
http://www.w3.org/TR/CSP/

CSP has a reporting mechanism whereby violations can be automatically POSTed by the browser to a reporting endpoint. It would be awesome if Sentry supported CSP violation reports, making it super easy for Sentry users to identify and track CSP violations (e.g. possible XSS attacks).

More on CSP's reporting mechanism:

http://www.html5rocks.com/en/tutorials/security/content-security-policy/#reporting
http://www.w3.org/TR/CSP/#sample-violation-report

@dcramer

This comment has been minimized.

Copy link
Member

dcramer commented Jan 16, 2013

One note: need to figure out how security is handled here (is there an Origin? Referrer?)

@christianvuerings

This comment has been minimized.

Copy link

christianvuerings commented Apr 3, 2013

+1

1 similar comment
@xncoder

This comment has been minimized.

Copy link

xncoder commented May 9, 2013

+1

@oreoshake

This comment has been minimized.

Copy link

oreoshake commented Jun 17, 2013

@dcramer there is a referrer, origin is null, cookies will be stripped unless allowed by CORS.

http://www.w3.org/TR/2013/WD-CSP11-20130604/#sample-violation-report

@joostdevries

This comment has been minimized.

Copy link

joostdevries commented Oct 7, 2014

+1 a lot.

@hd-deman

This comment has been minimized.

Copy link

hd-deman commented Oct 24, 2014

+1

1 similar comment
@36degrees

This comment has been minimized.

Copy link

36degrees commented Nov 14, 2014

+1

@joostdevries

This comment has been minimized.

Copy link

joostdevries commented Nov 14, 2014

anyone working on a pr for this?

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Nov 14, 2014

@joostdevries I was, then realized that this is a mess.

  • A browser can make tons of requests to this, so we'd just be getting DDoS'd 24/7.
    • A separate HTTP request is made for every single violation. In my initial testing in my normal browser without touching anything, I put a strict CSP rule on a page and it made like, 12 HTTP requests to report the problems. One for every single browser plugin I had that tried to modify the page or something.
  • How do we aggregate and report on this?
  • How does this look in the UI?

Imagine every visitor to your site making 12+ error reports to Sentry on every page load. getsentry.com wouldn't be able to handle that and I doubt anyone trying to do this at any scale with lots of page views would die as well. I think there needs to be a lighter more specific mechanism for logging these. Treating them as a normal exception might not be the right approach here.

@alex

This comment has been minimized.

Copy link
Contributor

alex commented Dec 30, 2014

@mattrobenolt it seems like they should be aggregated by {url, asset}, or maybe just {asset}; URLs might need to be turned into patterns somehow?

@alex

This comment has been minimized.

Copy link
Contributor

alex commented Dec 30, 2014

(Also, anything that we come up with here probably makes sense for HTTP Public-key-pinning as well)

@dcramer

This comment has been minimized.

Copy link
Member

dcramer commented Dec 30, 2014

@alex I think the main concern was just the volume of data. There doesn't seem to be a way to sample/restrict it, and JS events are already pretty high volume (I imagine this being higher).

This is more of a concern for getsentry.com, but until we have a really solid "you're going to pay us for the CPU you cost us" solution it's a bit scary to offer something like this. I see it in a similar vein as HTTP 404 logging.

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Dec 30, 2014

Yeah, I think there's just a lack of information here so far. I should probably start tracking it somewhere just to collect data and see where we're at and what sense I can make of it. It's definitely a thing we should figure out though, I agree.

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Dec 30, 2014

And yes, volume of data can be ridiculous, but I think can be solved.

@alex

This comment has been minimized.

Copy link
Contributor

alex commented Dec 30, 2014

So, the primary use case I'm interested in is helping site maintainers detect resources that are inadvertantly accessed over HTTP, when they have a theoretically HTTPS-only site, to help them avoid mixed content warnings.

It seems like that use case can probably be acheived with less data. Are there other important use cases?

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Dec 30, 2014

I think most of the use cases of CSP are valid. :)

The noisy/gross things are from browser plugins. Those are things I'd want to ignore. But since they don't go through a client, we don't have an opportunity to filter them out.

One of my ideas was to always relay through a raven-python or something similar to do filter on your server first, and raven-python could expose a url to their Django app. But that's obviously very limited to raven-python and Django.

@alex

This comment has been minimized.

Copy link
Contributor

alex commented Dec 30, 2014

it seems like expecting someone to have a server component beyond sentry
itself is a mistake

On Tue Dec 30 2014 at 10:17:48 AM Matt Robenolt notifications@github.com
wrote:

I think most of the use cases of CSP are valid. :)

The noisy/gross things are from browser plugins. Those are things I'd want
to ignore. But since they don't go through a client, we don't have an
opportunity to filter them out.

One of my ideas was to always relay through a raven-python or something
similar to do filter on your server first, and raven-python could
expose a url to their Django app. But that's obviously very limited to
raven-python and Django.


Reply to this email directly or view it on GitHub
#729 (comment).

@dcramer

This comment has been minimized.

Copy link
Member

dcramer commented Dec 30, 2014

I think at some point we will have to build a proxy in front of Sentry (rather than relying on nginx hacks/etc) which can help solve this. That said, I agree with just pushing straight to the server. It's fairly fast to accept events already, but at some point we'll want to allow "block these kinds of events" which isn't cheap enough to do today.

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Dec 30, 2014

it seems like expecting someone to have a server component beyond sentry itself is a mistake

I agree, which is why this is still open. :)

@dcramer

This comment has been minimized.

Copy link
Member

dcramer commented Dec 30, 2014

FWIW I'm willing to support this immediately with the caveat that we dont suggest to the world they use it yet.

I dont have time to build anything yet, but I believe this is almost entirely "handle CSP reports explicitly at the store endpoint". That code is fairly old so it's not the most editable part of Sentry, but I dont think it would be extremely difficult.

The big question is "Do we need any custom interfaces for this data?" If we do then we should first spec those out, as I'm not a big fan of changing that side of Sentry so I want to do it with a solid grasp of whats required, and if it can be filled by other means.

@alex

This comment has been minimized.

Copy link
Contributor

alex commented Dec 30, 2014

Do you mean interfaces as in user interfaces, or programmatic ones?

On Tue Dec 30 2014 at 10:27:12 AM David Cramer notifications@github.com
wrote:

FWIW I'm willing to support this immediately with the caveat that we dont
suggest to the world they use it yet.

I dont have time to build anything yet, but I believe this is almost
entirely "handle CSP reports explicitly at the store endpoint". That code
is fairly old so it's not the most editable part of Sentry, but I dont
think it would be extremely difficult.

The big question is "Do we need any custom interfaces for this data?" If
we do then we should first spec those out, as I'm not a big fan of changing
that side of Sentry so I want to do it with a solid grasp of whats
required, and if it can be filled by other means.


Reply to this email directly or view it on GitHub
#729 (comment).

@dcramer

This comment has been minimized.

Copy link
Member

dcramer commented Dec 30, 2014

@alex I mean the ones where we store the structured data (sentry.interfaces).

Mostly "if we capture this, how can we represent data in the most reusable fashion, ideally reusing existing stuff (but unlikely)"

@dcramer

This comment has been minimized.

@jvehent

This comment has been minimized.

Copy link

jvehent commented Oct 8, 2015

Did this feature ever get implemented?

@dcramer

This comment has been minimized.

Copy link
Member

dcramer commented Oct 8, 2015

@jvehent it hasn't. We're still planning to explore this but historically we've heard lots of negatives on CSP from people using it (noisy and limited).

@oreoshake

This comment has been minimized.

Copy link

oreoshake commented Oct 8, 2015

It's gotten much, much better over time.

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Oct 8, 2015

I'm going to take a peek at this stuff over the weekend and see what this looks like in Sentry now as a proof of concept. Not sure if we'll commit to using it, but I haven't looked at CSP reporting since 2013. :)

@oreoshake

This comment has been minimized.

Copy link

oreoshake commented Oct 8, 2015

🤘 well a lot has changed since 2013. Except for safari. Safari's implementation is awful and is probably closer to what you experienced globally in 2013. I highly recommend discarding safari CSP reports by default.

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Oct 8, 2015

@oreoshake

This comment has been minimized.

Copy link

oreoshake commented Oct 8, 2015

That isn't up to date, namely it doesn't reference the effective-directive that Chrome/Opera send and is part of the Level 2 spec sample violation report.

This list includes everything you should expect to see in the various reports. Additionally, Firefox sends an incredibly useful but non-standard value for script-sample. This can be used to detect plugins like lastpass (which will account for a large number of violations).

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Oct 8, 2015

Awesome, thanks for the information!

mattrobenolt added a commit that referenced this issue Oct 11, 2015

CSP reporting
Fixes GH-729

mattrobenolt added a commit that referenced this issue Oct 11, 2015

CSP reporting
Fixes GH-729

mattrobenolt added a commit that referenced this issue Oct 11, 2015

CSP reporting
Fixes GH-729

@mattrobenolt mattrobenolt referenced this issue Oct 11, 2015

Merged

CSP reporting #2154

2 of 2 tasks complete

mattrobenolt added a commit that referenced this issue Oct 11, 2015

CSP reporting
Fixes GH-729

mattrobenolt added a commit that referenced this issue Oct 11, 2015

CSP reporting
Fixes GH-729
@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Oct 15, 2015

For those that are interested in testing this, can you reach out to me? matt @ getsentry.com.

We're looking to get some community feedback before we unleash this into the wild. :)

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Oct 15, 2015

Also, btw, thanks everyone for nudging this along!

@jquacinella

This comment has been minimized.

Copy link

jquacinella commented Jul 11, 2016

Any thoughts on when this might be released? Could be willing to test

@dcramer

This comment has been minimized.

Copy link
Member

dcramer commented Jul 11, 2016

@jquacinella its live for early adopters -- go to organization settings, and tick the 'Early Adopters' box. From there head to [Project] > Settings > CSP Reports (under Data)

@brianmhunt

This comment has been minimized.

Copy link

brianmhunt commented Jul 30, 2016

😎 Will Public Key Pin reports be handled appropriately by the CSP handler?

@mattrobenolt

This comment has been minimized.

Copy link
Member

mattrobenolt commented Jul 30, 2016

We do not support Public Key Pin violations yet. The demand is very very low. It'll likely happen at some point. See #2165

@rohitpaulk

This comment has been minimized.

Copy link

rohitpaulk commented Jul 14, 2017

So we're trying this out at Gratipay. Bit confused - where do we get to see reports in the Sentry UI? Are they supposed to be shown in the 'Issues' page?

@rohitpaulk

This comment has been minimized.

Copy link

rohitpaulk commented Jul 14, 2017

^ False alarm, we're seeing a 403 forbidden when submitting CSP reports. Will update here if it doesn't work once we resolve that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment