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

Ensure the host can be parsed as an IPv6 address. #6

Merged
merged 2 commits into from Jan 20, 2021

Conversation

gabi-250
Copy link
Contributor

@gabi-250 gabi-250 commented Jan 8, 2021

If the host is an IPv6 address, the value returned by Uri::host() will include the surrounding brackets. This causes boring to fail to parse the host as an IP address. Eventually, this causes hostname verification to fail. I assume the hostname verification failure is due to the fact that the host will be compared with the DNSNames from SubjectAlternativeName, instead of the IPAddresses (set_ip vs set_host).

@gabi-250 gabi-250 force-pushed the ipv6-hosts branch 2 times, most recently from 3fcf282 to 280d034 Compare January 8, 2021 11:46
// it (otherwise, boring will fail to parse the host as an IP address, eventually
// causing the handshake to fail due a hostname verification error).
let host_or_ip = match (host.find('['), host.find(']')) {
(Some(0), Some(i)) => &host[1..i],
Copy link
Collaborator

@inikulin inikulin Jan 8, 2021

Choose a reason for hiding this comment

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

We need to ensure here that i == host.len() - 1. Otherwise, we can strip a part of host, doesn't seem to be a safe thing to do. Should be smthg like:

let last = host.len() - 1;

if last > 0 && host[0] == '[' && host[last] == ']' {
    host = &host[1..last];
}

Ideally we should also validate that what's enclosed in [ ] is indeed and IPv6

Copy link
Contributor Author

@gabi-250 gabi-250 Jan 8, 2021

Choose a reason for hiding this comment

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

Good point (host is extracted from a valid Uri, so I assumed it can only contain brackets if is a valid IPv6 address). That being said I'm happy to make this change.

@gabi-250
Copy link
Contributor Author

gabi-250 commented Jan 8, 2021

@inikulin Thank you for the review. I made the changes you've suggested.

@gabi-250 gabi-250 requested a review from inikulin January 8, 2021 17:03
@inikulin inikulin merged commit 3364ecc into cloudflare:master Jan 20, 2021
nox pushed a commit to nox/boring that referenced this pull request May 5, 2021
Merge in ECO/boring-rpk from hyper-rpk to master

* commit '7f2cdcd96b53a58c3e828cf4868f9965604bc107':
  MAL-359 Add RPK support to tokio-boring
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.

None yet

3 participants