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

access http health probes without client-cert auth #18477

Open
tjungblu opened this issue Aug 20, 2024 · 9 comments
Open

access http health probes without client-cert auth #18477

tjungblu opened this issue Aug 20, 2024 · 9 comments
Labels
area/security area/tls priority/backlog Higher priority than priority/awaiting-more-evidence. type/feature

Comments

@tjungblu
Copy link
Contributor

What would you like to be added?

With #17039 in 3.5.11 we also wanted to revise our usage of health and readiness probes, which currently relies on a custom sidecar container to facilitate grpc and client cert auth to "proxy" health checks to etcd (imagine a very naive grpc proxy).

We were thinking to replace it with the simple liveness definition of:

livenessProbe:
  httpGet:
    port: 2379
    path: health
    scheme: HTTPS

which fails with:

{"level":"warn",...,"msg":"rejected connection", ... ,"error":"tls: client didn't provide a certificate"}

We currently supply both --client-cert-auth=true and --peer-client-cert-auth=true and would like to keep it that way in terms of authN/Z.

As K8s doesn't have the option to pass certificates into the probes, we would continue to be stuck with the current proxy solution.

TL;DR: I would love to allow the endpoints under etcdhttp.HandleHealth to be served without client auth.


It's certainly not that easy to implement in the codebase, we have this configured in:

cfg.ClientAuth = tls.NoClientCert
if info.TrustedCAFile != "" || info.ClientCertAuth {
cfg.ClientAuth = tls.RequireAndVerifyClientCert
}

One hacky option would be to have a separate server with tls.NoClientCert that just runs a mux with the health probes.

Why is this needed?

Kubernetes doesn't allow to pass certificates for client authentication to liveness probes.

@jmhbnz jmhbnz added area/security area/tls priority/backlog Higher priority than priority/awaiting-more-evidence. labels Aug 23, 2024
@ahrtr
Copy link
Member

ahrtr commented Aug 26, 2024

The solution is to set a HTTP endpoint to serve metrics and health endpoints using --listen-metrics-urls.

@ahrtr
Copy link
Member

ahrtr commented Aug 26, 2024

@tjungblu are you happy to close this ticket?

@tjungblu
Copy link
Contributor Author

Thanks for the suggestion @ahrtr.
I'm just briefly browsing through the code right now, but this still would require a clientTLSInfo:

https://github.com/etcd-io/etcd/blob/main/server/embed/etcd.go#L828-L842

We need all endpoints with https, metrics need to continue with client cert, the probes can go without.

@ahrtr
Copy link
Member

ahrtr commented Aug 27, 2024

Thanks for the suggestion @ahrtr.
I'm just briefly browsing through the code right now, but this still would require a clientTLSInfo:

Please test it out. No matter what endpoints (either http or https) you set for --listen-client-urls. You can always set a HTTP endpoint for for --listen-metrics-urls.

@tjungblu
Copy link
Contributor Author

Sure.

$ etcd --cert-file=etcd-serving-fedora.crt \
            --key-file=etcd-serving-fedora.key \
            --trusted-ca-file=ca.crt \
            --client-cert-auth=true \
            --listen-client-urls=https://localhost:2379 \
            --advertise-client-urls=https://localhost:2379 \
            --listen-metrics-urls=https://localhost:1337

curling just the endpoint yields:

$ curl https://localhost:1337/health
curl: (60) SSL certificate problem: self-signed certificate in certificate chain
More details here: https://curl.se/docs/sslcerts.html

curl failed to verify the legitimacy of the server and therefore could not
establish a secure connection to it. To learn more about this situation and
how to fix it, please visit the web page mentioned above.

ignoring certs in curl yields:

$ curl -k https://localhost:1337/health
curl: (56) OpenSSL SSL_read: OpenSSL/3.2.2: error:0A00045C:SSL routines::tlsv13 alert certificate required, errno 0

server logs:
{"level":"warn","ts":"2024-08-27T11:58:20.300075+0200","caller":"embed/config_logging.go:188","msg":"rejected connection on client endpoint","remote-addr":"127.0.0.1:44448","server-name":"localhost","error":"tls: client didn't provide a certificate"}

curling with certificates yields:

$ curl --key etcd-serving-fedora.key --cert etcd-serving-fedora.crt --cacert ca.crt  https://localhost:1337/health
{"health":"true","reason":""}

You can always set a HTTP endpoint for for --listen-metrics-urls.

We need all endpoints with https

Unfortunately not an option here. The metrics are unfortunately coupled with the probes in one server :/

@serathius
Copy link
Member

-            --listen-metrics-urls=https://localhost:1337
+            --listen-metrics-urls=http://localhost:1337

@tjungblu
Copy link
Contributor Author

tjungblu commented Aug 27, 2024

Yes @serathius I understood what @ahrtr meant, we can't have unprotected (as in plain HTTP) endpoints running.

@ahrtr
Copy link
Member

ahrtr commented Aug 27, 2024

Usually --listen-metrics-urls is only accessible inside the K8s cluster, so HTTP might be acceptable.

If you insist on HTTPS for --listen-metrics-urls (e.g. due to strong compliance requirements), then applying tls.NoClientCert client authentication on the metrics & health endpoint separately is the only choice. But I don't feel comfortable to complicate it before we resolve #18032.

@tjungblu
Copy link
Contributor Author

so HTTP might be acceptable.

I wish, we have many gov and FIPS customers that employ awesome compliance tools that scan for these things.

applying tls.NoClientCert client authentication on the metrics & health endpoint separately is the only choice. But I don't feel comfortable to complicate it before we resolve #18032.

I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security area/tls priority/backlog Higher priority than priority/awaiting-more-evidence. type/feature
Development

No branches or pull requests

4 participants