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

Handle Public-Key-Pinning (HPKP) violation reports #2165

Closed
reedloden opened this issue Oct 13, 2015 · 18 comments
Closed

Handle Public-Key-Pinning (HPKP) violation reports #2165

reedloden opened this issue Oct 13, 2015 · 18 comments

Comments

@reedloden
Copy link
Contributor

To expand upon the CSP reporting support added in #729 and #2154, would love to have HPKP reporting support as well. This would really make Sentry very useful for handling these reports coming from browsers.

https://developer.mozilla.org/en-US/docs/Web/Security/Public_Key_Pinning
https://developers.google.com/web/updates/2015/09/HPKP-reporting-with-chrome-46
https://tools.ietf.org/html/rfc7469#section-3

@reedloden
Copy link
Contributor Author

(cc @mattrobenolt since he just added the CSP support)

@mattrobenolt
Copy link
Contributor

You are on top of our shit. 👍

I'll look into this. We're technically just testing the CSP reporting ourselves internally before launching publicly to get a feel for these things.

I'll add this to my list and see if we can just do some generic "security reporting" solution.

@reedloden
Copy link
Contributor Author

Cool, and thanks again for getting this implemented. So excited to start using this, even just for CSP reports.

You might can also get some ideas from @ScottHelme's https://report-uri.io service (see https://scotthelme.co.uk/csp-and-hpkp-violation-reporting-with-report-uri-io/), though that's just one guy's take on this problem. Everywhere else I've seen this done has just written their own custom collector (such as https://github.com/skbkontur/csp_reporter) and parser, so would be good to have something more widely available that people could use rather than reinventing the wheel every time (especially if it comes with the power of Sentry's existing functionality).

@mattrobenolt
Copy link
Contributor

Everywhere else I've seen this done has just written their own custom collector (such as https://github.com/skbkontur/csp_reporter) and parser

Yeah, so our needs (if you look at the code for our support) is probably much different than what others need. I'm not sure it's too valuable to have a generic parser for a CSP report since frankly, it's so simple.

The challenge we have is trying to group the same violation from different browsers to try and reduce the noise. Firefox likes to report the same thing a bit differently than Chrome, for example, so we try to do some normalizing of the values to make them group together as expected.

But ideally, having this inside Sentry is a good framework since we already do notifications and alerting around issues, so bolting on features like this just makes sense. :)

@mattrobenolt
Copy link
Contributor

Hey @reedloden, do you have a way to actually test sending a report? I haven't seemed to be able to get Chrome or Firefox to actually send one to the report-uri.

I've added a Public-Key-Pins header with a bad pin-sha265 value (assuming this will cause it to trigger), with a report-uri, and I don't get anything. :(

@ScottHelme
Copy link

If there is no valid pin then the policy won't be cached or enforced. This stops you from easily DoSing yourself.

I use another subdomain to demonstrate hpkp violations. You can read more here, https://scotthel.me/pkpf

Please excuse my brevity, I'm on mobile.

@reedloden
Copy link
Contributor Author

Firefox doesn't yet support report-uri for HPKP (https://bugzilla.mozilla.org/show_bug.cgi?id=1091176), but Chrome definitely does as of Chrome 46.

@ScottHelme's blog post helps with other issues.

@reedloden
Copy link
Contributor Author

@mattrobenolt, any update on this and #2190? The CSP reports have been very useful (even with all the noise), so would be nice to have support for these other types of reports as well. :)

@reedloden
Copy link
Contributor Author

@mattrobenolt, anything I can do to help here? Right now, I'm splitting my reporting between Sentry and between report-uri.io, and I would like to move it all to Sentry (so I can host it internally eventually).

Happy to provide any support you need.

@dcramer
Copy link
Member

dcramer commented Dec 10, 2015

@reedloden tbh the feature isnt super high priority for us. It's not overly difficult to build, but its more of a "scratch your own itch" kind of thing. We're pretty much all internally prioritizing strong infrastructure and workflow improvements at the moment.

@jesseendahl
Copy link

FWIW I'd also love to see support for this in Sentry, for the same reason (don't like having to use multiple services for reporting).

@dcramer
Copy link
Member

dcramer commented Oct 10, 2017

For easy ref, here's the specification for what the report looks like:

https://tools.ietf.org/html/rfc7469#section-3

@reedloden
Copy link
Contributor Author

HPKP is getting deprecated by Chrome (and wouldn't be surprised if other browsers follow), so my recommendation would be to WONTFIX this for now.

https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/he9tr7p3rZ8
https://www.reddit.com/r/netsec/comments/796exs/public_key_pinning_being_removed_from_chrome/

However, plenty of other things that need additional support added (X-XSS-Protection, Expect-CT, Expect-Staple) or the generic Reporting API.

@alex-hofsteede
Copy link
Contributor

Yep, had just implemented this in #6417 when I saw it was getting deprecated :( But it will be a good starting point for adding other types of security reports

@reedloden
Copy link
Contributor Author

Closing this out, as HPKP is deprecated.

@ScottHelme
Copy link

ScottHelme commented Jan 9, 2018

*HPKP isn't deprecated yet. Chrome have announced they intend to deprecate it. It's still supported in Firefox and a defined RFC.

@reedloden
Copy link
Contributor Author

@ScottHelme Fair enough. Do you think it's worth supporting in Sentry at this time, though? That's basically what I was meaning...

@ScottHelme
Copy link

Only Sentry could answer that one but my guess would probably be no :)

@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants