exporter-toolkit icon indicating copy to clipboard operation
exporter-toolkit copied to clipboard

#71 Skip basic auth for OPTIONS http method

Open stokito opened this issue 3 years ago • 8 comments

The OPTIONS needed for CORS requests

stokito avatar Jan 28 '22 13:01 stokito

While I understand the need, I need to assess the security impact, as said in the issue. Not only for Prometheus but for generic exporters as well.

roidelapluie avatar Feb 02 '22 06:02 roidelapluie

curl -s -X OPTIONS https://node.demo.do.prometheus.io/metrics |head 
# HELP go_gc_duration_seconds A summary of the pause duration of garbage collection cycles.
# TYPE go_gc_duration_seconds summary
go_gc_duration_seconds{quantile="0"} 4.141e-05
go_gc_duration_seconds{quantile="0.25"} 5.7685e-05
go_gc_duration_seconds{quantile="0.5"} 6.1472e-05
go_gc_duration_seconds{quantile="0.75"} 6.9851e-05
go_gc_duration_seconds{quantile="1"} 0.000358368
go_gc_duration_seconds_sum 297.390302862
go_gc_duration_seconds_count 4.113281e+06
# HELP go_goroutines Number of goroutines that currently exist.
curl -s -X WHATEVER https://node.demo.do.prometheus.io/metrics |head 
# HELP go_gc_duration_seconds A summary of the pause duration of garbage collection cycles.
# TYPE go_gc_duration_seconds summary
go_gc_duration_seconds{quantile="0"} 4.141e-05
go_gc_duration_seconds{quantile="0.25"} 5.7685e-05
go_gc_duration_seconds{quantile="0.5"} 6.1472e-05
go_gc_duration_seconds{quantile="0.75"} 6.9851e-05
go_gc_duration_seconds{quantile="1"} 0.000358368
go_gc_duration_seconds_sum 297.390426157
go_gc_duration_seconds_count 4.113283e+06
# HELP go_goroutines Number of goroutines that currently exist.

It looks like the Prometheus golang API does not check the method. This fix would then disable basic auth for metrics endpoints if OPTIONS is used as method.

roidelapluie avatar Feb 02 '22 09:02 roidelapluie

I recently switched to Go and when I first time saw the HTTP handler API my first thought was "hey, here is not forced to check a method like in Java servlet api". Also all code examples even from go tour didn't handled this :(

stokito avatar Feb 02 '22 09:02 stokito

@stokito I agree, we should be much more careful about request method handling here and in other parts of the http handler(s).

SuperQ avatar Feb 02 '22 09:02 SuperQ

Hi, if you need my assistance I may try to contribute a little bit and fix parts. I can spent about 4-5 hours, just let me know where to change

stokito avatar Jun 06 '22 19:06 stokito

This is not safe. Instead we should try to reply ourselves when there is an OPTIONS query.

It would mean moving CORS config into the toolkit.

roidelapluie avatar Apr 06 '23 12:04 roidelapluie

I will try to detail what I have in mind if it's not clear.

roidelapluie avatar Apr 06 '23 12:04 roidelapluie

Hello,

I will try to detail what I have in mind if it's not clear.

Don't you think that checking for OPTIONS should be the responsibility of the exporter that's being wrapped by the toolkit? The fact that prometheus has its own logic to set CORS headers makes me think so.

When you say that you want to move CORS config in the toolkit, that would mean moving all this code, and the CORS origin regexp config value, into the toolkit? If so, that'd be a breaking change I assume, do you have a timeline for this change, or could some help make such a change come faster and available in prometheus API?

gagbo avatar Aug 16 '23 10:08 gagbo