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

Unauthenticated health probes and mTLS #590

Closed
sebastianblunt opened this issue Oct 7, 2022 · 3 comments · Fixed by #595
Closed

Unauthenticated health probes and mTLS #590

sebastianblunt opened this issue Oct 7, 2022 · 3 comments · Fixed by #595

Comments

@sebastianblunt
Copy link

sebastianblunt commented Oct 7, 2022

With mTLS enabled, /status requires a valid client certificate for a successful response, preventing unauthenticated use of /status as a health check endpoint. (For example as an http probe in kubernetes.)

Workarounds such as just using a tcp probe, or a command probe that runs curl with a valid client certificate, do work.

Is there a recommended way of doing health checks when mTLS is enabled?

One solution could possibly be to have another endpoint, on another port, that only provides /status and does not require mTLS.

@mostynb
Copy link
Collaborator

mostynb commented Oct 7, 2022

Hmm, this is a corner case that I missed. It's possible to implement, but a little scary since you have to give up some automatic certificate verification provided by the go standard library.

I think the /grpc.health.v1.Health/Check service should work when using mTLS without requiring a client certificate, would that work with your setup?

Alternatively, using --allow_unauthenticated_reads should make this work (but also allow unauthenticated read access to the cache). I think I would recommend the grpc method instead of adding this flag just for the health check.

@sebastianblunt
Copy link
Author

I think the grpc health endpoint has the same problem, though it is possible I messed something up when testing. This is what I tried

mkdir -p /tmp/bazel/data
cd /tmp/bazel
mkcert localhost
cp "$(mkcert -CAROOT)"/rootCA.pem .
docker run -v /tmp/bazel:/bazel \
  -u 1000:1000 \
  -p 8080:8080 \
  -p 9092:9092 \
  buchgr/bazel-remote-cache:v2.3.9 \
  --max_size 1 \
  --dir /bazel/data \
  --tls_cert_file /bazel/localhost.pem \
  --tls_key_file /bazel/localhost-key.pem \
  --tls_ca_file /bazel/rootCA.pem

which then gets

./grpc_health_probe -tls -tls-no-verify -addr=127.0.0.1:9092 
timeout: failed to connect service "127.0.0.1:9092" within 1s

Either removing tls_ca_file or adding allow_unauthenticated_reads does allow it to succeed.

a little scary since you have to give up some automatic certificate verification provided by the go standard library

I can understand this. I also imagine there are people who run this with mTLS exposed to the public internet, who may not want status information publicly available.

mostynb added a commit to mostynb/bazel-remote that referenced this issue Oct 23, 2022
Previously if bazel-remote was using authentication (mtls or basic auth)
then unauthenticated health check requests would fail unless also using
--allow_unauthenticated_reads.

Fixes buchgr#590.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Oct 23, 2022
Previously if bazel-remote was using authentication (mtls or basic auth)
then unauthenticated health check requests would fail unless also using
--allow_unauthenticated_reads.

Fixes buchgr#590.
mostynb added a commit to mostynb/bazel-remote that referenced this issue Oct 30, 2022
Previously if bazel-remote was using authentication (mtls or basic auth)
then unauthenticated health check requests would fail unless also using
--allow_unauthenticated_reads.

Fixes buchgr#590.
@mostynb mostynb closed this as completed in 50adb2c Nov 6, 2022
@mostynb
Copy link
Collaborator

mostynb commented Nov 6, 2022

I pushed a fix so that the grpc health service should always be reachable, even if the server has authentication enabled but the client doesn't provide it. For now I left things so that the /status http URL still requires authentication.

Aside: watch out if you're using a "localhost" cert- you should specify localhost instead of 127.0.0.1 on the client side.

shubhindia pushed a commit to shubhindia/bazel-remote that referenced this issue Dec 7, 2022
Previously if bazel-remote was using authentication (mtls or basic auth)
then unauthenticated health check requests would fail unless also using
--allow_unauthenticated_reads.

Fixes buchgr#590.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants