OkHttpChannelBuilder#overrideAuthority not compatible with IPv6 hostname

[OnlyInAmerica]

What version of gRPC are you using?

1.10.0

What did you expect to see?

Background: I'm using mutual TLS with a LAN client that presents an IPv6 link-local address as an IP SAN in its X509 certificate.

I expected to call OkHttpChannelBuilder.overrideAuthority("FE80::FABB:BFFF:FE01:3A52") to allow hostname verification to pass.

Instead, this call yields an IllegalArgumentException("No host in authority"). If I instead represent the authority as "[FE80::FABB:BFFF:FE01:3A52]", OkHostnameVerifier's VERIFY_AS_IP_ADDRESS regex does not match the address (correctly, I believe, as the square brackets are just a URL disambiguation detail and not part of the hostname), and so the verify mechanism ignores the IP SAN entries.

If you agree this behavior should be changed I'm happy to submit a PR. Perhaps we'd modify GrpcUtil#checkAuthority to strip square brackets from a validated hostname?

Message Sequence Charts (Click to expand)

With OkHttpChannelBuilder.overrideAuthority("FE80::FABB:BFFF:FE01:3A52"):

With OkHttpChannelBuilder.overrideAuthority("[FE80::FABB:BFFF:FE01:3A52]"):

[dapengzhang0]

Are you using JDK version earlier than jdk_8u60 or on Mac OS?

[OnlyInAmerica]

I am on Mac OS, using Android Studio 3.1's bundled JDK which appears to be 1.8.0_152 (equivalent to jdk_8u152?)

[ericgribkoff]

Thanks for reporting this. Just discussed with @dapengzhang0. RFC 2732 section 3 seems to indicate that the brackets are indeed part of the host for ipv6 addresses. I'll need to read through the specifications a little more to make sure we fix this in the right place, but the observed behavior is definitely a bug.

I think we'll want to fix this either directly in OkHostnameVerifier (we have our own "fork" of most of OkHttp 2.7, so this is io.grpc.okhttp.internal.OkHostnameVerifier in the grpc-java repo) or somewhere in the calling arguments to this. GrpcUtil.checkAuthority seems to be doing the right thing (and it's basically just calling JDK functions) so I don't think we want to change the behavior there, but rather change how this is consumed in the OkHttp verifier side.

Feel free to submit a PR for this - just let me know if you plan to work on it, as otherwise I'll try to get a fix out later this week or early next week. Thanks!

[OnlyInAmerica]

I'm currently working around this for work so happy to submit a PR.

My reading of RFC 2732 is that the square brackets are not part of the hostname. Excuse my pedant mode: I see that 2732 is an update to RFC 3986, which defines URIs but does not define hostnames. The specification for hostnames, RFC 952, seems to say that [ and ] are not valid hostname characters:

RFC 952. Assumptions, Part 1

A "name" (Net, Host, Gateway, or Domain name) is a text string up
to 24 characters drawn from the alphabet (A-Z), digits (0-9), minus
sign (-), and period (.)

Quickly scanning the updates to 952, I don't see any changes to allow additional characters.

Do let me know if you find info indicating otherwise!

[ericgribkoff]

Correct, the square brackets are not part of the hostname, but they are part of the host for IPv6 addresses. The terminology gets confusing (and probably this isn't helped by OkHostnameVerifier actually verifying hosts, which include hostnames and IPv6 addresses).

The starting point should be RFC 2818, which defines how subject alternative names must be verified and is the spec that OkHostnameVerifier implements. From section 3.2:

In some cases, the URI is specified as an IP address rather than a
hostname. In this case, the iPAddress subjectAltName must be present
in the certificate and must exactly match the IP in the URI.

URI syntax is defined in RFC 2396 and updated for IPv6 in RFC 2732:

authority = server | reg_name
server = [ [ userinfo "@" ] hostport ]
hostport = host [ ":" port ]
host = hostname | IPv4address | IPv6reference
ipv6reference = "[" IPv6address "]"

The clash occurs when gRPC passes the host to OkHostnameVerifier. The host (whether an overriden authority or the "real" authority) will include brackets around the IPv6address. But OkHostnameVerifier needs to match the IPv6address directly in the cert (without brackets), so somewhere the brackets need to be dropped.

This should not be done in GrpcUtil#checkAuthority, because by definition the authority includes the brackets. The pragmatic fix is to strip the brackets in OkHttpTlsUpgrader#upgrade before we call OkHostnameVerifier#verify. This would keep our hostname verifier consistent with the OkHttp library itself and the Android OS's default hostname verifier.

Note: This error can appear when connecting to an IPv6 address without overriding the authority on the channel. If you create a channel with the real (not overriden) authority in the format of [ipv6address], this authority will still go into OkHostnameVerifier and fail its regex.

Note #2: If consistency with upstream OkHttp wasn't a concern, it seems like there is an argument for making the change in OkHostnameVerifier itself. I checked, and JDK APIs such as Socket accept host with the brackets (although they also work with raw IPv6 addresses). JDK's URI API exposes host, with the brackets for IPv6, and AFAICT there is equivalent way to get the raw IPv6 address. Nor is there a need, except to pass it to this hostname verifier. (Also, the comment for OkHostnameVerify.VERIFY_AS_IP_ADDRESS regex mentions it is an approximation of Android's InetAddress#isNumeric private API, but isNumeric does detect (and discard) brackets around an IPv6 address)

[ericgribkoff]

Here is where OkHttp canonicalizes their URLs, which includes defining that host does not include brackets. And here is a discussion of their canonical format and why Java's built-in URL/URI classes were not suitable. It doesn't say specifically why they dropped the brackets, but since Android is using OkHttp under the hood and it's OkHttp code that (typically) invokes HostnameVerifier#verify for non-gRPC HTTPs connections, that seems to be the way things are and should be :)

So the fix here seems like we should strip out the brackets before we invoke #verify, plus test(s) to confirm this case is working. @OnlyInAmerica let me know if that still sounds good to you.

[OnlyInAmerica]

Thanks for the thorough investigation!

The pragmatic fix is to strip the brackets in OkHttpTlsUpgrader#upgrade before we call OkHostnameVerifier#verify

Sounds good, will shoot to submit this today.

[ejona86]

It sounds like @ericgribkoff has confirmed this is a bug. Marking as such.

[ericgribkoff]

Fixed by #4292