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

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

Open
AntonAM opened this issue Aug 24, 2022 · 10 comments
Open

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

AntonAM opened this issue Aug 24, 2022 · 10 comments
Labels
config Kube config related help wanted Not immediately prioritised, please help! openssl openssl-tls related

Comments

@AntonAM
Copy link

AntonAM commented Aug 24, 2022

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: gravitational/teleport#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
Copy link
Member

clux commented Aug 30, 2022

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.

@clux clux added config Kube config related help wanted Not immediately prioritised, please help! labels Aug 30, 2022
@MikailBag
Copy link
Contributor

@MikailBag
Copy link
Contributor

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
Copy link
Author

AntonAM commented Sep 18, 2022

@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
Copy link
Member

clux commented Dec 15, 2022

This is now supported for rustls via #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
Copy link
Contributor

goenning commented Apr 5, 2023

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

@clux
Copy link
Member

clux commented Apr 5, 2023

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
Copy link
Contributor

goenning commented Apr 5, 2023

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

@yann-soubeyrand
Copy link

Hello, is there some news on this front?

@clux
Copy link
Member

clux commented Sep 10, 2024

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.

bors-openebs-mayastor bot pushed a commit to openebs/mayastor-control-plane that referenced this issue Oct 7, 2024
854: build: bump kube from v0.85.0 to v0.87.2 and k8s-openapi from v0.19.0 to v0.20.0 r=michaelbeaumont a=michaelbeaumont

This switches to the `rustls` backend, it's the default for the first time in this version, and thus fixes openebs/mayastor#1729 by way of the rustls-only fix of kube-rs/kube#991

This is the minimum change necessary to _just_ fix the above issue. Probably this and other dependencies should be updated in addition to this PR.

Closes openebs/mayastor#1729

This also bumps the minimum k8s version to v1.22

Co-authored-by: Mike Beaumont <mjboamail@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Kube config related help wanted Not immediately prioritised, please help! openssl openssl-tls related
Projects
None yet
Development

No branches or pull requests

5 participants