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

Can't resolve IP address with square brakets #181

Closed
digitwolf opened this issue Dec 5, 2022 · 7 comments
Closed

Can't resolve IP address with square brakets #181

digitwolf opened this issue Dec 5, 2022 · 7 comments

Comments

@digitwolf
Copy link

Problem

Sending to destination like https://[::1]/ fails with error invalid dnsname.

Error is thrown here https://github.com/rustls/hyper-rustls/blob/main/src/connector.rs#L95

Analysis

hyper-rustls resolves the Hostname as hyper::Uri::host
This makes hostname = "[::1]" which is HTTP-specific form of writing IPv6 address.

Because IPv6 addresses contain colons, they cannot be directly used in URLs because the colons would conflict with both the protocol declaration (http:// or https://) and port numbers. Therefore, when a literal IPv6 address is used, it is encased in a bracket, like http://[2001:db8::1]:80
https://en.wikiversity.org/wiki/IPv6

Root-Cause

hyper-rustls is resolving the host name using rustls::ServerName::try_from(hostname) which does not expect square brackets in IP because internally it uses

Proposed solution

Since square brakets are coming from hyper and not part standard IP notation I propose that we remove any square brakets inside of the connector before calling the rustls::ServerName::try_from(hostname)

Will post a PR in bit

@djc
Copy link
Member

djc commented Dec 5, 2022

Are you setting an actual DNS server name separately? Because rustls doesn't currently support IP addresses in certificates it won't otherwise be able to establish a session anyway.

@digitwolf
Copy link
Author

I am not using a DNS, but I am using custom certs and all custom rustls cert verifiers. I have checked locally and this works.

@digitwolf
Copy link
Author

digitwolf commented Dec 5, 2022

This is the only change (in external codebase) that is needed to make requests to IP with rustls work #182

@BiagioFesta
Copy link
Contributor

BiagioFesta commented Dec 6, 2022

Because rustls doesn't currently support IP addresses in certificates

Shouldn't rustls/rustls#1032 mitigate this?

I see this current main branch depends on rustls 0.20.1 (so probably that PR is not in the code)

With that patch on rustls and a custom cert verifiers (thus, dangerous config), it should be feasible if I am not wrong.

@digitwolf
Copy link
Author

Hi BiagioFesta!
I tried your idea locally. I've upgraded hyper-rustls dependency of rustls to 0.20.7 however on it's own that does not address it.
I think that the main issue still exists - square brackets are not accounted for in rustls.
But I do like the idea of upgrading the dependency. I'll update the PR to include the version bump.

@digitwolf
Copy link
Author

Guys, thank you for pulling in the PR.
Does anyone have an idea when I can expect it to be released?

I have to make a call today to decide if I am pulling in this fix from crates.io or if I need to embed a copy of hyper-rustls in my software release.

@djc
Copy link
Member

djc commented Dec 8, 2022

I've submitted #183.

@djc djc closed this as completed Dec 8, 2022
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

No branches or pull requests

3 participants