Add strict mode to OkHostnameVerifier for top-level domain wildcards.

When enabled, Subject Alternative Names in certificates will
be rejected if they have wildcards for top-level domains,
such as "*.com".

Strict mode is off for the instance of OkHostnameVerifier returned
by HttpsURLConnection.getDefaultHostnameVerifier(), so
this CL should have no visible behaviour change.

Bug: 144694112
Test: atest CtsLibcoreTestCases CtsLibcoreOkHttpTestCases
Change-Id: I9fd28995441d462d9106703f06762549591797ad

3 files changed, 100 insertions, 5 deletions

diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/tls/HostnameVerifierTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/tls/HostnameVerifierTest.java
index beb2b6c..d4187e8 100644
--- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/tls/HostnameVerifierTest.java
+++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/tls/HostnameVerifierTest.java
@@ -21,11 +21,17 @@ import com.squareup.okhttp.internal.Util;
 import java.io.ByteArrayInputStream;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.Collection;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
 import javax.security.auth.x500.X500Principal;
 import org.junit.Ignore;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -35,8 +41,23 @@ import static org.junit.Assert.assertTrue;
  * Tests for our hostname verifier. Most of these tests are from AOSP, which
  * itself includes tests from the Apache HTTP Client test suite.
  */
+@RunWith(Parameterized.class)
 public final class HostnameVerifierTest {
-  private HostnameVerifier verifier = OkHostnameVerifier.INSTANCE;
+  // BEGIN Android-changed: Run tests for both default and strict verifiers. http://b/144694112
+  // private HostnameVerifier verifier = OkHostnameVerifier.INSTANCE;
+  @Parameters()
+  public static Collection<Object[]> data() {
+    // Both verifiers should behave the same in all tests except for
+    // subjectAltNameWithToplevelWildcard(), and that test is not parameterized for clarity.
+    return Arrays.asList(new Object[][] {
+        { OkHostnameVerifier.INSTANCE },
+        { OkHostnameVerifier.STRICT_INSTANCE }
+    });
+  }
+
+  @Parameter
+  public HostnameVerifier verifier;
+  // END Android-changed: Run tests for both default and strict verifiers. http://b/144694112
 
   @Test public void verify() throws Exception {
     FakeSSLSession session = new FakeSSLSession();
@@ -532,6 +553,32 @@ public final class HostnameVerifierTest {
     assertFalse(verifier.verify("quux.com", session));
   }
 
+  // BEGIN Android-added: Verify behaviour with top level wildcard SAN. http://b/144694112
+  @Test
+  public void subjectAltNameWithToplevelWildcard() throws Exception {
+    // Default OkHostnameVerifier instance should allow SANs which
+    // have wildcards for top-level domains.  The strict instance should not.
+    //
+    // Certificate generated using:-
+    //     openssl req -x509 -nodes -days 36500 -subj "/CN=Google Inc" \
+    //         -addext "subjectAltName=DNS:*.com" -newkey rsa:512
+    SSLSession session = session(""
+        + "-----BEGIN CERTIFICATE-----\n"
+        + "MIIBlTCCAT+gAwIBAgIUe1RB6C61ZW/SEQpKiywSEJOEOUMwDQYJKoZIhvcNAQEL\n"
+        + "BQAwFTETMBEGA1UEAwwKR29vZ2xlIEluYzAgFw0xOTExMjExMjE1NTBaGA8yMTE5\n"
+        + "MTAyODEyMTU1MFowFTETMBEGA1UEAwwKR29vZ2xlIEluYzBcMA0GCSqGSIb3DQEB\n"
+        + "AQUAA0sAMEgCQQCu24jT8hktpvnmcde4dqC6e7G5F4cNNLUFnTi3Ay9BzPH1r7sN\n"
+        + "v2lHTIQLKSlvjxa48mpeRBlOjDQigv7c+rfRAgMBAAGjZTBjMB0GA1UdDgQWBBQd\n"
+        + "myvYKfluxb0+kNEJoh1ZER2wUTAfBgNVHSMEGDAWgBQdmyvYKfluxb0+kNEJoh1Z\n"
+        + "ER2wUTAPBgNVHRMBAf8EBTADAQH/MBAGA1UdEQQJMAeCBSouY29tMA0GCSqGSIb3\n"
+        + "DQEBCwUAA0EAK710g2hQpXSmpbOQH4dHG61fkVDtM/kR/4/R61vDDqVkgOuyHqXl\n"
+        + "GUZFKHMeOZ8peQLT8b+5ik6pIO7Vu2pF6w==\n"
+        + "-----END CERTIFICATE-----\n");
+    assertTrue(OkHostnameVerifier.INSTANCE.verify("google.com", session));
+    assertFalse(OkHostnameVerifier.STRICT_INSTANCE.verify("google.com", session));
+  }
+  // END Android-added: Verify behaviour with top level wildcard SAN. http://b/144694112
+
   @Test public void verifyAsIpAddress() {
     // IPv4
     assertTrue(OkHostnameVerifier.verifyAsIpAddress("127.0.0.1"));
diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/tls/OkHostnameVerifier.java b/okhttp/src/main/java/com/squareup/okhttp/internal/tls/OkHostnameVerifier.java
index c947d7d..d83c5f4 100644
--- a/okhttp/src/main/java/com/squareup/okhttp/internal/tls/OkHostnameVerifier.java
+++ b/okhttp/src/main/java/com/squareup/okhttp/internal/tls/OkHostnameVerifier.java
@@ -35,7 +35,10 @@ import javax.net.ssl.SSLSession;
  * href="http://www.ietf.org/rfc/rfc2818.txt">RFC 2818</a>.
  */
 public final class OkHostnameVerifier implements HostnameVerifier {
-  public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier();
+  // Android-changed: Add an instance which disallows top-level domain wildcards. b/144694112
+  // public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier();
+  public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier(false);
+  public static final OkHostnameVerifier STRICT_INSTANCE = new OkHostnameVerifier(true);
 
   /**
    * Quick and dirty pattern to differentiate IP addresses from hostnames. This
@@ -54,8 +57,15 @@ public final class OkHostnameVerifier implements HostnameVerifier {
   private static final int ALT_DNS_NAME = 2;
   private static final int ALT_IPA_NAME = 7;
 
-  private OkHostnameVerifier() {
+  // BEGIN Android-changed: Add an instance which disallows top-level domain wildcards. b/144694112
+  // private OkHostnameVerifier() {
+  // }
+  private final boolean strictWildcardMode;
+
+  private OkHostnameVerifier(boolean strictWildcardMode) {
+    this.strictWildcardMode = strictWildcardMode;
   }
+  // END Android-changed: Add an instance which disallows top-level domain wildcards. b/144694112
 
   @Override
   public boolean verify(String host, SSLSession session) {
@@ -214,6 +224,8 @@ public final class OkHostnameVerifier implements HostnameVerifier {
     //    For example, *.example.com matches test.example.com but does not match
     //    sub.test.example.com.
     // 3. Wildcard patterns for single-label domain names are not permitted.
+    // 4. Android-added: if strictWildcardMode is true then wildcards matching top-level domains,
+    //    e.g. *.com, are not permitted.
 
     if ((!pattern.startsWith("*.")) || (pattern.indexOf('*', 1) != -1)) {
       // Asterisk (*) is only permitted in the left-most domain name label and must be the only
@@ -234,6 +246,18 @@ public final class OkHostnameVerifier implements HostnameVerifier {
       return false;
     }
 
+    // BEGIN Android-added: Disallow top-level wildcards in strict mode. http://b/144694112
+    if (strictWildcardMode) {
+      // By this point we know the pattern has been normalised and starts with a wildcard,
+      // i.e. "*.domainpart."
+      String domainPart = pattern.substring(2, pattern.length() - 1);
+      // If the domain part contains no dots then this pattern will match top level domains.
+      if (domainPart.indexOf('.') < 0) {
+        return false;
+      }
+    }
+    // END Android-added: Disallow top-level wildcards in strict mode. http://b/144694112
+
     // hostName must end with the region of pattern following the asterisk.
     String suffix = pattern.substring(1);
     if (!hostName.endsWith(suffix)) {
diff --git a/repackaged/okhttp/src/main/java/com/android/okhttp/internal/tls/OkHostnameVerifier.java b/repackaged/okhttp/src/main/java/com/android/okhttp/internal/tls/OkHostnameVerifier.java
index 3cdd1bf..9985514 100644
--- a/repackaged/okhttp/src/main/java/com/android/okhttp/internal/tls/OkHostnameVerifier.java
+++ b/repackaged/okhttp/src/main/java/com/android/okhttp/internal/tls/OkHostnameVerifier.java
@@ -37,7 +37,10 @@ import javax.net.ssl.SSLSession;
  * @hide This class is not part of the Android public SDK API
  */
 public final class OkHostnameVerifier implements HostnameVerifier {
-  public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier();
+  // Android-changed: Add an instance which disallows top-level domain wildcards. b/144694112
+  // public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier();
+  public static final OkHostnameVerifier INSTANCE = new OkHostnameVerifier(false);
+  public static final OkHostnameVerifier STRICT_INSTANCE = new OkHostnameVerifier(true);
 
   /**
    * Quick and dirty pattern to differentiate IP addresses from hostnames. This
@@ -56,8 +59,15 @@ public final class OkHostnameVerifier implements HostnameVerifier {
   private static final int ALT_DNS_NAME = 2;
   private static final int ALT_IPA_NAME = 7;
 
-  private OkHostnameVerifier() {
+  // BEGIN Android-changed: Add an instance which disallows top-level domain wildcards. b/144694112
+  // private OkHostnameVerifier() {
+  // }
+  private final boolean strictWildcardMode;
+
+  private OkHostnameVerifier(boolean strictWildcardMode) {
+    this.strictWildcardMode = strictWildcardMode;
   }
+  // END Android-changed: Add an instance which disallows top-level domain wildcards. b/144694112
 
   @Override
   public boolean verify(String host, SSLSession session) {
@@ -216,6 +226,8 @@ public final class OkHostnameVerifier implements HostnameVerifier {
     //    For example, *.example.com matches test.example.com but does not match
     //    sub.test.example.com.
     // 3. Wildcard patterns for single-label domain names are not permitted.
+    // 4. Android-added: if strictWildcardMode is true then wildcards matching top-level domains,
+    //    e.g. *.com, are not permitted.
 
     if ((!pattern.startsWith("*.")) || (pattern.indexOf('*', 1) != -1)) {
       // Asterisk (*) is only permitted in the left-most domain name label and must be the only
@@ -236,6 +248,18 @@ public final class OkHostnameVerifier implements HostnameVerifier {
       return false;
     }
 
+    // BEGIN Android-added: Disallow top-level wildcards in strict mode. http://b/144694112
+    if (strictWildcardMode) {
+      // By this point we know the pattern has been normalised and starts with a wildcard,
+      // i.e. "*.domainpart."
+      String domainPart = pattern.substring(2, pattern.length() - 1);
+      // If the domain part contains no dots then this pattern will match top level domains.
+      if (domainPart.indexOf('.') < 0) {
+        return false;
+      }
+    }
+    // END Android-added: Disallow top-level wildcards in strict mode. http://b/144694112
+
     // hostName must end with the region of pattern following the asterisk.
     String suffix = pattern.substring(1);
     if (!hostName.endsWith(suffix)) {