kube-rs / kube

Rust Kubernetes client and controller runtime
https://kube.rs
Apache License 2.0
2.88k stars 303 forks source link

Support `tls-server-name` field of the kubeconfig cluster configuration #991

Open AntonAM opened 2 years ago

AntonAM commented 2 years ago

Would you like to work on this feature?

maybe

What problem are you trying to solve?

Currently kube-rs doesn't support tls-server-name for the cluster definition in the kubeconfig and just ignores it. That doesn't allow to specify correct host for the SNI. We felt it when using kubectl-view-allocation k8s plugin with Teleport (issue there: https://github.com/gravitational/teleport/issues/15106 ), where correct server name in the SNI is required to reach right endpoint, since multiple services are multiplexed on one port.

Describe the solution you'd like

Support tls-server-name field as per k8s docs - it should be transferred as part of SNI during TSL handshake.

Describe alternatives you've considered

-

Documentation, Adoption, Migration Strategy

https://kubernetes.io/docs/reference/config-api/client-authentication.v1/

Target crate for feature

kube-client

clux commented 2 years ago

This is (afaikt) a feature for authentication for RefreshableToken::Exec bubbling down through our Config from the Kubeconfig's Cluster (where it is supposed to be).

If I understand it right, it involves extending our ExecCredential (in particular the spec), and then do something with it in the auth_exec fn? I'm not really sure what.

I can see some light uses of it in client-go but it doesn't really explain. Help would be appreciated with this one.

MikailBag commented 2 years ago

IIUC, this setting should be somehow propagated to the TLS library in use (rustls / openssl).

Maybe, these links will help. rustls: https://docs.rs/rustls/latest/rustls/client/trait.ServerCertVerifier.html, https://docs.rs/rustls/latest/rustls/client/struct.DangerousClientConfig.html#method.set_certificate_verifier openssl: https://docs.rs/openssl/latest/openssl/ssl/struct.SslConnectorBuilder.html#method.set_servername_callback

MikailBag commented 2 years ago

As I see, client-go passes tls-server-name to the underlying TLS layer here: https://github.com/kubernetes/client-go/blob/037b5fd01db4432f8f6bb6139ff2b6dd00008b99/transport/transport.go#L94

Documentation for crypto/tls.Config is https://pkg.go.dev/crypto/tls#Config

This specific field is documented as follows:

ServerName is used to verify the hostname on the returned certificates unless InsecureSkipVerify is given. It is also included in the client's handshake to support virtual hosting unless it is an IP address.

AntonAM commented 2 years ago

@MikailBag yep, you are right, it's and server name to provide to TLS layer, when it connects to the server. And so it can perform necessary checks and SNI can be used correctly 👍

clux commented 1 year ago

This is now supported for rustls via https://github.com/kube-rs/kube/pull/1104 thanks to @MikailBag 's initial PR and it's released in 0.77.0.

Unfortunately, it is not done for openssl yet (where the upstream pr in hyper-openssl is not responded to) and I couldn't find a way to do it directly .

goenning commented 1 year ago

That crate doesn't seem to be very active, is forking hyper-openssl an option until that PR is merged? 🤔

clux commented 1 year ago

Someone would have to step up to do that type of slog though.

As a potential alternative; we just closed all the rustls issues on main after the last rustls/hyper-rustls bump. So switching to rustls-tls feature should be more viable for apps now.

goenning commented 1 year ago

I'll give it a go, I recall seeing some caveats on README which made me pick openssl instead.

yann-soubeyrand commented 11 months ago

Hello, is there some news on this front?

clux commented 1 week ago

Context on bug since this is old. This is supported on rustls for us which is our default stack since 0.86.0 / sep 2023.

The openssl support is missing and help is still welcome on this front from anyone actually using this stack.