okhttp: properly verify IPv6 address hosts (#4292)

Address mismatch between IPv6 address hosts derived from URIs and X509 subjectAltName extensions

2 files changed, 32 insertions, 1 deletions

diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpTlsUpgrader.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpTlsUpgrader.java
index 4044ec262..d37b80338 100644
--- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpTlsUpgrader.java
+++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpTlsUpgrader.java
@@ -70,9 +70,28 @@ final class OkHttpTlsUpgrader {
     if (hostnameVerifier == null) {
       hostnameVerifier = OkHostnameVerifier.INSTANCE;
     }
-    if (!hostnameVerifier.verify(host, sslSocket.getSession())) {
+    if (!hostnameVerifier.verify(canonicalizeHost(host), sslSocket.getSession())) {
       throw new SSLPeerUnverifiedException("Cannot verify hostname: " + host);
     }
     return sslSocket;
   }
+
+  /**
+   * Converts a host from URI to X509 format.
+   *
+   * <p>IPv6 host addresses derived from URIs are enclosed in square brackets per RFC2732, but
+   * omit these brackets in X509 certificate subjectAltName extensions per RFC5280.
+   *
+   * @see <a href="https://www.ietf.org/rfc/rfc2732.txt">RFC2732</a>
+   * @see <a href="https://tools.ietf.org/html/rfc5280#section-4.2.1.6">RFC5280</a>
+   *
+   * @return {@param host} in a form consistent with X509 certificates
+   */
+  @VisibleForTesting
+  static String canonicalizeHost(String host) {
+    if (host.startsWith("[") && host.endsWith("]")) {
+      return host.substring(1, host.length() - 1);
+    }
+    return host;
+  }
 }
diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTlsUpgraderTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTlsUpgraderTest.java
index 7e6f1eecf..561b2d219 100644
--- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTlsUpgraderTest.java
+++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTlsUpgraderTest.java
@@ -16,6 +16,8 @@
 
 package io.grpc.okhttp;
 
+import static io.grpc.okhttp.OkHttpTlsUpgrader.canonicalizeHost;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import io.grpc.okhttp.internal.Protocol;
@@ -32,4 +34,14 @@ public class OkHttpTlsUpgraderTest {
             || OkHttpTlsUpgrader.TLS_PROTOCOLS.indexOf(Protocol.GRPC_EXP)
                 < OkHttpTlsUpgrader.TLS_PROTOCOLS.indexOf(Protocol.HTTP_2));
   }
+
+  @Test public void canonicalizeHosts() {
+    assertEquals("::1", canonicalizeHost("::1"));
+    assertEquals("::1", canonicalizeHost("[::1]"));
+    assertEquals("127.0.0.1", canonicalizeHost("127.0.0.1"));
+    assertEquals("some.long.url.com", canonicalizeHost("some.long.url.com"));
+
+    // Extra square brackets in a malformed URI are retained
+    assertEquals("[::1]", canonicalizeHost("[[::1]]"));
+  }
 }