-
Notifications
You must be signed in to change notification settings - Fork 84
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
HTTP Headers support #41
Conversation
8c74a4c
to
dfd1881
Compare
Should we set any of the defaults to non-empty? |
X-Frame-Options: There are valid usecases for X-Frame-Options, e.g. someone could embed a prometheus console in an iframe. X-Content-Type-Options: Future changes (default to openmetrics?) would make the /metrics page downloadable by browsers. I would not set it as a default. X-XSS-Protection: Could be set to 1 but moderns browsers do not even implement it. Strict-Transport-Security: No default |
A better question to ask is, what if a backend starts specifying these headers itself? It could lead to security issues if e.g. X-Content-Type-Options ended up unset due to this code. I'd also suggest a fat warning on Strict-Transport-Security, as blindly setting that can cause extended outages with impact beyond the process itself with no real way to recover. It's a massive footgun. |
It should be completely safe to set this on any /metrics page, and I don't see any relationship with OpenMetrics or downloads. Downloads are the Content-Disposition header not Content-Type, and the OM spec requires a correct content type. The question is more what other endpoints may break due to depending on this. Offhand within this may cause issues with Prometheus console templates, as Go content type detection has had issues there before, but otherwise we should be okay. Thus it is not safe to set - this is the sort of thing that only the application itself can know if it's safe to set be default. |
Yes, some strong warnings here are worthwhile. |
I'm not following you here. Are you confusing it with HSTS preloading? Unless someone runs their exporter on a naked domain under |
if you set Strict-Transport-Security badly, browsers will no longer try to load HTTP pages, which might be unexpected. They would always try to load the HTTPS version. |
No, I'm not. Someone running it on the naked domain is one of the biggest risks here, and we have seen at least one user who was blindly attempting to do basically that based entirely on what a security scanner told them. |
Without HSTS preloading only developers going to |
Yes, anyone who hits that endpoint will end up with HSTS - whether that be a customer or anyone else. In addition it's possible that there's things like reverse proxies in play, so that the /metrics page is ends up served on a customer facing website somewhere. It is not practical to ask all of your customers to reset their browser cache (and from a quick peek, this is not stored in the cache in Chrome - it's a separate internals thing), especially if you just effectively took down your entire website so would have difficult communicating with them. This would be tricky even for just your employees. As I said, HSTS is a massive footgun. |
This pull request let users configure the HTTP headers as recommended by the 2020 Cure53 security audit on Node Exporter. It is not an arbitrary way to inject any HTTP headers as some backends (e.g. Prometheus) already add headers (CORS), and we do not want to let users change those. Signed-off-by: Julien Pivotto <[email protected]>
086cc68
to
5ee3e9c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request let users configure the HTTP headers as recommended by the 2020 Cure53 security audit on Node Exporter. It is not an arbitrary way to inject any HTTP headers as some backends (e.g. Prometheus) already add headers (CORS), and we do not want to let users change those. Signed-off-by: Julien Pivotto <[email protected]>
This pull request let users configure the HTTP headers as recommended by
the 2020 Cure53 security audit on Node Exporter.
It is not an arbitrary way to inject any HTTP headers as some backends
(e.g. Prometheus) already add headers (CORS), and we do not want to let
users change those.
Signed-off-by: Julien Pivotto [email protected]