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

CORS requests are failed if Basic Auth is enabled

Open stokito opened this issue 3 years ago • 9 comments

I enabled CORS headers and basic auth but a browser still can't fetch information from Prometheus. Before sending a GET request an OPTIONS request is performed first and it doesn't contains the Authorization header. So Prometheus must allow the OPTIONS request without the Authorization header. I can do that with some Nginx configuration but I don't want to use just for this and also I want to keep configuration clear.

stokito avatar Oct 27 '21 07:10 stokito

You are correct, we should allow OPTIONS without basic auth, I will move this to the exporter-toolkit repository. We probably do not want this by default, but as an option in web.yml.

roidelapluie avatar Nov 05 '21 16:11 roidelapluie

I guess we will also need to understand how Prometheus itself handles OPTIONS requests (without basic auth).

roidelapluie avatar Nov 05 '21 16:11 roidelapluie

I created a PR. Honestly I don't see benefits of having this configurable. In theory, that may lead to problem when someone tested an API endpoint with the OPTIONS covered by a basic auth and then they may be wondered that someone disabled basic auth and now sensitive data is exposed. Th OPTIONS was created for as a replacement for Flash crossdomain.xml but originally the method was introduced in WebDAV. For a security reason it must not be a resource specific but rather show features of the server itself. As a possible risk that I see it may be in some API endpoints be a check if r.Method == GET {} else { POST } i.e. that it always assume that there is only two possible http methods and the 405 error is not returned properly. There is no that many api endpoints to check

stokito avatar Jan 28 '22 13:01 stokito

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. Enabling this would then disable basic auth for metrics endpoints if OPTIONS is used as method.

How could we move this forward? What would be an alternative?

roidelapluie avatar Feb 02 '22 09:02 roidelapluie

The only valid way is to fix api and check for api method

stokito avatar Feb 02 '22 09:02 stokito

Yes, we should probably fix client_golang promhttp to allow enforcing the HTTP method.

SuperQ avatar Feb 02 '22 09:02 SuperQ

Can we do something to force the HTTP method checking?

stokito avatar Jul 02 '22 09:07 stokito

Hi again, could you please add the task to a some roadmap. Or maybe we should just close it

stokito avatar Dec 24 '22 10:12 stokito

There is also another related problem that needs to be fixed in scope of the task. Since I using a basic authorization I need to ether pass the Authorization header myself or add withCredentials option and then my browser will ask me for a password. The raw Authorization works but withCredentials requires and additional security and the Access-Control-Allow-Origin: * won't work and it should be specified explicit to origin e.g. Access-Control-Allow-Origin: example.com. See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#requests_with_credentials

But Prometheus doesn't do that because the --web.cors.origin has default value .*. The regexp has a special meaning and will just return wildcard. https://github.com/prometheus/prometheus/blob/8f3b2fc9aeaf1bd3f839e820e7f9ffc571006e89/util/httputil/cors.go#L30

I've set the regexp to https.* and only then it started to return an Origin instead of wildcard. I'm not sure if this is a sane default or maybe it would be totally fine to set the Origin by default instead of wildcard. But this was not trivial for me to understand and configure. Anyway, maybe some tutorials will explain this.

Another problem is that the withCredentials mode also requires the Access-Control-Allow-Credentials: true to be set. So just for one this header I need to keep the Nginx proxy behind. It would be great to set the header by default. It basically should be safe. Or maybe to add an option to web config http_server_config.headers

Here is a workaround Nginx conf that I made https://gist.github.com/stokito/b1e7189ed85d970f20e3e5dd4bfad998

stokito avatar Feb 27 '23 23:02 stokito