Verify IPv6 hostnames #5889
Verify IPv6 hostnames #5889
Conversation
| @@ -51,8 +52,10 @@ object OkHostnameVerifier : HostnameVerifier { | |||
|
|
|||
| /** Returns true if [certificate] matches [ipAddress]. */ | |||
| private fun verifyIpAddress(ipAddress: String, certificate: X509Certificate): Boolean { | |||
| val canonicalIpAddress = ipAddress.toCanonicalHost() | |||
There was a problem hiding this comment.
FWIW I didn't consider IP Addresses a host path to optimise too much. But that's certainly debatable.
| // [distinguished_name] | ||
| // [req_extensions] | ||
| // [x509_extensions] | ||
| // subjectAltName=IP:0:0:0:0:0:0:0:1,IP:2a03:2880:f003:c07:face:b00c::2,IP:0::5,IP:192.168.1.1 |
There was a problem hiding this comment.
Wanna do an IPv4 mapped address also?
#4451
There was a problem hiding this comment.
What do you want the behaviour to be for ::ffff:192.168.1.1?
There was a problem hiding this comment.
Will land that as a follow up.
There was a problem hiding this comment.
I think it’s the IPv4 address.
There was a problem hiding this comment.
What's the reasoning? Are you self signing certificates but don't want to specify both forms of IPv4 and IPV6 (mapped IPv4) and you expect it to still work for either?
I'm nervous about following Postel's law here to be nice. Maybe keeping it explicit is the better option for security?
There was a problem hiding this comment.
The canonical form of certain IPv6 addresses (with colons!) is an IPv4 address (no colons!). It’s weird, but it’s how it’s specified and I’ve come to terms with it.
If we’re canonicalizing IP addresses, we should canonicalize all of ’em and test that it works. I expect the test already passes but I’d like coverage of that case.
Fixes #5885