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 custom server name resolution #269

Merged
merged 6 commits into from
Apr 10, 2024
Merged

Conversation

sfackler
Copy link
Contributor

@sfackler sfackler commented Apr 7, 2024

I don't love the duplication of the ipv6 handling. We could have resolve just return a String and do the conversion in the connector maybe?

Closes #195

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mostly looking good, here's a bunch of suggestions.

What's your use case for this?

src/connector.rs Outdated Show resolved Hide resolved
src/connector/builder.rs Outdated Show resolved Hide resolved
src/server_name_resolver.rs Outdated Show resolved Hide resolved
src/server_name_resolver.rs Outdated Show resolved Hide resolved
@sfackler
Copy link
Contributor Author

sfackler commented Apr 9, 2024

My specific use case is performing client-side load-balancing against a headless service in Kubernetes. In a headless service, the cluster publishes a DNS record mapping e.g. my-svc.my-namespace.svc.cluster-domain.example to the IP addresses for each replica of the service. A client that wants to talk to the service resolves the DNS record to get the set of replica IPs, and selects the one it wants to send the request to. However, we can't just send the request to e.g. https://10.0.1.123/my-endpoint since the TLS cert is only issued for my-svc.my-namespace.svc.cluster-domain.example and not the individual pod IPs.

To make this work, cleanest way seems to be to rewrite the URI to include both the hostname and IP. I can then create a custom DNS resolver for the Hyper client which pull the IP out of the URI, and a custom servername resolver in hyper-rustls that pulls the hostname out.

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

src/connector.rs Outdated Show resolved Hide resolved
@djc djc requested a review from cpu April 9, 2024 15:13
@cpu
Copy link
Member

cpu commented Apr 9, 2024

djc requested a review from cpu 3 hours ago

Will take a look soon.

Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good to me. I had a couple of non-blocking suggestions to consider.

src/connector/builder.rs Show resolved Hide resolved
src/connector/builder.rs Outdated Show resolved Hide resolved
Co-authored-by: Daniel McCarney <[email protected]>
@djc djc merged commit 7d31dc5 into rustls:main Apr 10, 2024
11 checks passed
@cpu
Copy link
Member

cpu commented Apr 10, 2024

@djc should we cut a release for this?

@djc
Copy link
Member

djc commented Apr 10, 2024

I was defaulting to not doing so (proactively) unless the OP asks for it.

@sfackler
Copy link
Contributor Author

A release would be appreciated :)

@sfackler sfackler deleted the custom-servername branch April 10, 2024 16:00
@cpu
Copy link
Member

cpu commented Apr 10, 2024

A release would be appreciated :)

In progress: #270

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 this pull request may close these issues.

More elaborate custom server name
3 participants