google / cadvisor

Analyzes resource usage and performance characteristics of running containers.
Other
17.23k stars 2.33k forks source link

[security] "/metrics" endpoint ignores `--http_auth_file` flag #3401

Open rc5hack opened 1 year ago

rc5hack commented 1 year ago

I use cadvisor to get metrics into prometheus via /metrics endpoint accessible from internet. I don't want anyone except prometheus to read cadvisor output.

If use --http_auth_file flag, web-interface of cadvisor becomes protected with http_basic_auth, but /metrics endpoint is still accessible without password.

/metrics endpoint should require password when --http_auth_file flag is used.

rc5hack commented 1 year ago
$ curl -sS -D - -o /dev/null http://example.com/containers/
HTTP/1.1 401 Unauthorized
Content-Type: text/plain
Www-Authenticate: Basic realm="localhost"
Date: Sat, 23 Sep 2023 06:31:21 GMT
Content-Length: 17

$ curl -sS -D - -o /dev/null http://example.com/metrics
HTTP/1.1 200 OK
Content-Type: text/plain; version=0.0.4; charset=utf-8
Date: Sat, 23 Sep 2023 06:31:27 GMT
Transfer-Encoding: chunked
rc5hack commented 1 year ago

ping @bobbypage @iwankgb @creatone @dims @mrunalp , this looks like security breach

alvarofernandezavalos commented 9 months ago

same here !

rc5hack commented 8 months ago

still reproducible on v0.49.1

rc5hack commented 8 months ago

Possible temporary workaround: by using --prometheus_endpoint=/some-long-random-string runtime option, move unprotected metrics from well-known "/metrics" to hard-to-guess "/some-long-random-string" endpoint.

This is rude and does not eliminate the vulnerability in general, but hides it and makes it more difficult for attackers to find. Kinda security through obscurity.

rc5hack commented 7 months ago

related: https://github.com/google/cadvisor/pull/3463/commits/2c1026e946e22a2966f3870a903b42ebbb46dde4

Pexers commented 4 months ago

@rc5hack The documentation explicitly says that the /metrics endpoint can't be protected with BasicAuth, and recommends to not expose it publicly. I don't think we can consider it a vulnerability.

However, I do agree that this endpoint should support optional BasicAuth, just like many other Prometheus Exporters do using the --web.config.file flag. A few examples: Node Exporter, NGINX Exporter.