[DO NOT MERGE] Reject non-ASCII hostnames and SANs, Ignore DNS names in certificate CN.android-security-8.1.0_r93android-security-8.1.0_r92android-security-8.1.0_r91android-security-8.1.0_r90android-security-8.1.0_r89android-security-8.1.0_r88android-security-8.1.0_r87android-security-8.1.0_r86android-security-8.1.0_r85oreo-mr1-security-release

Removes the possibility of certificates spoofing DNS names by
exploiting name collisions when lowercasing Unicode characters.
Note that the relevant RFCs mandate that domain names in
certificates should be stored using IDNA 2008 rules, i.e. as
ASCII punycode.

Ignoring DNS names in the certificate CN is a backport of a
change made in Android P as this behaviour has been deprecated
for many years.  The P change was not gated on targetSdkVersion,
so all apps on P or later ignore the CN field when verifiying
hostnames, this change bring O MR1 into line with best practices
without introducing significant fragmentation.  The change makes
hostname verification consistent across all releases with security
patch support.

Bug: 171980069
Test: cts-tradefed cts run -m CtsLibcoreTestCases
Test: cts-tradefed cts run -m CtsLibcoreOkHttpTestCases
Change-Id: I6be13c94b2db22229093d2f0403337798c3ad9ad
Merged-In: I96d52609ce4966ff11f649ca940de3b02a43b0b2
(cherry picked from commit ef3cc00da34dcef024e39bcabcca098e49499b3e)

2 files changed, 92 insertions, 43 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 d7f1c78..d2b4a83 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
@@ -24,7 +24,6 @@ import java.security.cert.X509Certificate;
 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 static org.junit.Assert.assertEquals;
@@ -36,7 +35,7 @@ import static org.junit.Assert.assertTrue;
  * itself includes tests from the Apache HTTP Client test suite.
  */
 public final class HostnameVerifierTest {
-  private HostnameVerifier verifier = OkHostnameVerifier.INSTANCE;
+  private final HostnameVerifier verifier = OkHostnameVerifier.INSTANCE;
 
   @Test public void verify() throws Exception {
     FakeSSLSession session = new FakeSSLSession();
@@ -71,7 +70,9 @@ public final class HostnameVerifierTest {
         + "HwlNrAu8jlZ2UqSgskSWlhYdMTAP9CPHiUv9N7FcT58Itv/I4fKREINQYjDpvQcx\n"
         + "SaTYb9dr5sB4WLNglk7zxDtM80H518VvihTcP7FHL+Gn6g4j5fkI98+S\n"
         + "-----END CERTIFICATE-----\n");
-    assertTrue(verifier.verify("foo.com", session));
+    // Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("foo.com", session));
+    assertFalse(verifier.verify("foo.com", session));
     assertFalse(verifier.verify("a.foo.com", session));
     assertFalse(verifier.verify("bar.com", session));
   }
@@ -104,7 +105,9 @@ public final class HostnameVerifierTest {
         + "9BsO7qe46hidgn39hKh1WjKK2VcL/3YRsC4wUi0PBtFW6ScMCuMhgIRXSPU55Rae\n"
         + "UIlOdPjjr1SUNWGId1rD7W16Scpwnknn310FNxFMHVI0GTGFkNdkilNCFJcIoRA=\n"
         + "-----END CERTIFICATE-----\n");
-    assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session));
+    // Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session));
+    assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session));
     assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session));
   }
 
@@ -143,12 +146,7 @@ public final class HostnameVerifierTest {
     assertFalse(verifier.verify("a.bar.com", session));
   }
 
-  /**
-   * Ignored due to incompatibilities between Android and Java on how non-ASCII
-   * subject alt names are parsed. Android fails to parse these, which means we
-   * fall back to the CN. The RI does parse them, so the CN is unused.
-   */
-  @Test @Ignore public void verifyNonAsciiSubjectAlt() throws Exception {
+  @Test public void verifyNonAsciiSubjectAlt() throws Exception {
     // CN=foo.com, subjectAlt=bar.com, subjectAlt=花子.co.jp
     // (hanako.co.jp in kanji)
     SSLSession session = session(""
@@ -178,16 +176,15 @@ public final class HostnameVerifierTest {
         + "sWIKHYrmhCIRshUNohGXv50m2o+1w9oWmQ6Dkq7lCjfXfUB4wIbggJjpyEtbNqBt\n"
         + "j4MC2x5rfsLKKqToKmNE7pFEgqwe8//Aar1b+Qj+\n"
         + "-----END CERTIFICATE-----\n");
-    assertTrue(verifier.verify("foo.com", session));
+    // Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("foo.com", session));
+    assertFalse(verifier.verify("foo.com", session));
     assertFalse(verifier.verify("a.foo.com", session));
-    // these checks test alternative subjects. The test data contains an
-    // alternative subject starting with a japanese kanji character. This is
-    // not supported by Android because the underlying implementation from
-    // harmony follows the definition from rfc 1034 page 10 for alternative
-    // subject names. This causes the code to drop all alternative subjects.
-    // assertTrue(verifier.verify("bar.com", session));
-    // assertFalse(verifier.verify("a.bar.com", session));
-    // assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session));
+    assertTrue(verifier.verify("bar.com", session));
+    assertFalse(verifier.verify("a.bar.com", session));
+    assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session));
+    // Android-added: Reject non-ASCII hostnames and SANs. http://b/171980069
+    assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session));
   }
 
   @Test public void verifySubjectAltOnly() throws Exception {
@@ -257,7 +254,9 @@ public final class HostnameVerifierTest {
     assertFalse(verifier.verify("a.foo.com", session));
     assertFalse(verifier.verify("bar.com", session));
     assertFalse(verifier.verify("a.bar.com", session));
-    assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session));
+    // Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session));
+    assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session));
     assertFalse(verifier.verify("a.\u82b1\u5b50.co.jp", session));
   }
 
@@ -290,8 +289,12 @@ public final class HostnameVerifierTest {
         + "l3Q/RK95bnA6cuRClGusLad0e6bjkBzx/VQ3VarDEpAkTLUGVAa0CLXtnyc=\n"
         + "-----END CERTIFICATE-----\n");
     assertFalse(verifier.verify("foo.com", session));
-    assertTrue(verifier.verify("www.foo.com", session));
-    assertTrue(verifier.verify("\u82b1\u5b50.foo.com", session));
+    // Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("www.foo.com", session));
+    assertFalse(verifier.verify("www.foo.com", session));
+    // Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("\u82b1\u5b50.foo.com", session));
+    assertFalse(verifier.verify("\u82b1\u5b50.foo.com", session));
     assertFalse(verifier.verify("a.b.foo.com", session));
   }
 
@@ -324,16 +327,15 @@ public final class HostnameVerifierTest {
         + "UGPLEUDzRHMPHLnSqT1n5UU5UDRytbjJPXzF+l/+WZIsanefWLsxnkgAuZe/oMMF\n"
         + "EJMryEzOjg4Tfuc5qM0EXoPcQ/JlheaxZ40p2IyHqbsWV4MRYuFH4bkM\n"
         + "-----END CERTIFICATE-----\n");
-    assertTrue(verifier.verify("foo.co.jp", session));
-    assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session));
+    // Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("foo.co.jp", session));
+    assertFalse(verifier.verify("foo.co.jp", session));
+    // Android-changed: Reject non-ASCII hostnames and SANs. http://b/171980069
+    // assertTrue(verifier.verify("\u82b1\u5b50.co.jp", session));
+    assertFalse(verifier.verify("\u82b1\u5b50.co.jp", session));
   }
 
-  /**
-   * Ignored due to incompatibilities between Android and Java on how non-ASCII
-   * subject alt names are parsed. Android fails to parse these, which means we
-   * fall back to the CN. The RI does parse them, so the CN is unused.
-   */
-  @Test @Ignore public void testWilcardNonAsciiSubjectAlt() throws Exception {
+  @Test public void testWilcardNonAsciiSubjectAlt() throws Exception {
     // CN=*.foo.com, subjectAlt=*.bar.com, subjectAlt=*.花子.co.jp
     // (*.hanako.co.jp in kanji)
     SSLSession session = session(""
@@ -364,19 +366,22 @@ public final class HostnameVerifierTest {
         + "pgJsDbJtZfHnV1nd3M6zOtQPm1TIQpNmMMMd/DPrGcUQerD3\n"
         + "-----END CERTIFICATE-----\n");
     // try the foo.com variations
-    assertTrue(verifier.verify("foo.com", session));
-    assertTrue(verifier.verify("www.foo.com", session));
-    assertTrue(verifier.verify("\u82b1\u5b50.foo.com", session));
+    // BEGIN Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("foo.com", session));
+    // assertTrue(verifier.verify("www.foo.com", session));
+    // assertTrue(verifier.verify("\u82b1\u5b50.foo.com", session));
+    assertFalse(verifier.verify("foo.com", session));
+    assertFalse(verifier.verify("www.foo.com", session));
+    assertFalse(verifier.verify("\u82b1\u5b50.foo.com", session));
+    // END Android-changed: Ignore common name in hostname verification. http://b/70278814
     assertFalse(verifier.verify("a.b.foo.com", session));
-    // these checks test alternative subjects. The test data contains an
-    // alternative subject starting with a japanese kanji character. This is
-    // not supported by Android because the underlying implementation from
-    // harmony follows the definition from rfc 1034 page 10 for alternative
-    // subject names. This causes the code to drop all alternative subjects.
-    // assertFalse(verifier.verify("bar.com", session));
-    // assertTrue(verifier.verify("www.bar.com", session));
+    // these checks test alternative subjects.
+    assertFalse(verifier.verify("bar.com", session));
+    assertTrue(verifier.verify("www.bar.com", session));
+    // Android-changed: Reject non-ASCII hostnames and SANs. http://b/171980069
     // assertTrue(verifier.verify("\u82b1\u5b50.bar.com", session));
-    // assertTrue(verifier.verify("a.b.bar.com", session));
+    assertFalse(verifier.verify("\u82b1\u5b50.bar.com", session));
+    assertFalse(verifier.verify("a.b.bar.com", session));
   }
 
   @Test public void subjectAltUsesLocalDomainAndIp() throws Exception {
@@ -451,7 +456,9 @@ public final class HostnameVerifierTest {
         + "U6LFxmZr31lFyis2/T68PpjAppc0DpNQuA2m/Y7oTHBDi55Fw6HVHCw3lucuWZ5d\n"
         + "qUYo4ES548JdpQtcLrW2sA==\n"
         + "-----END CERTIFICATE-----");
-    assertTrue(verifier.verify("google.com", session));
+    // Android-changed: Ignore common name in hostname verification. http://b/70278814
+    // assertTrue(verifier.verify("google.com", session));
+    assertFalse(verifier.verify("google.com", session));
   }
 
   @Test public void subjectAltName() throws Exception {
@@ -542,6 +549,14 @@ public final class HostnameVerifierTest {
     assertFalse(OkHostnameVerifier.verifyAsIpAddress("www.nintendo.co.jp"));
   }
 
+  @Test public void isPrintableAscii() {
+    assertTrue(OkHostnameVerifier.isPrintableAscii("foo-bar_baz.com"));
+    assertTrue(OkHostnameVerifier.isPrintableAscii("FoO-bAr_BaZ.cOm"));
+    assertFalse(OkHostnameVerifier.isPrintableAscii("Føø-bAr_BaZ.cøm"));
+    // Char 0xc0 (capital A with grave accent in ISO 8859-1) fits in 8 bits but not 7.
+    assertFalse(OkHostnameVerifier.isPrintableAscii("\u00c0.com"));
+  }
+
   private X509Certificate certificate(String certificate) throws Exception {
     return (X509Certificate) CertificateFactory.getInstance("X.509").generateCertificate(
         new ByteArrayInputStream(certificate.getBytes(Util.UTF_8)));
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 740de1b..399614b 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
@@ -29,7 +29,6 @@ import java.util.regex.Pattern;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLSession;
-import javax.security.auth.x500.X500Principal;
 
 /**
  * A HostnameVerifier consistent with <a
@@ -95,6 +94,11 @@ public final class OkHostnameVerifier implements HostnameVerifier {
    * Returns true if {@code certificate} matches {@code hostName}.
    */
   private boolean verifyHostName(String hostName, X509Certificate certificate) {
+    // BEGIN Android-added: Reject non-ASCII hostnames and SANs. http://b/171980069
+    if (!isPrintableAscii(hostName)) {
+      return false;
+    }
+    // END Android-added: Reject non-ASCII hostnames and SANs. http://b/171980069
     hostName = hostName.toLowerCase(Locale.US);
     boolean hasDns = false;
     List<String> altNames = getSubjectAltNames(certificate, ALT_DNS_NAME);
@@ -105,6 +109,8 @@ public final class OkHostnameVerifier implements HostnameVerifier {
       }
     }
 
+    // BEGIN Android-removed: Ignore common name in hostname verification. http://b/70278814
+    /*
     if (!hasDns) {
       X500Principal principal = certificate.getSubjectX500Principal();
       // RFC 2818 advises using the most specific name for matching.
@@ -113,6 +119,8 @@ public final class OkHostnameVerifier implements HostnameVerifier {
         return verifyHostName(hostName, cn);
       }
     }
+    */
+    // END Android-removed: Ignore common name in hostname verification. http://b/70278814
 
     return false;
   }
@@ -193,6 +201,11 @@ public final class OkHostnameVerifier implements HostnameVerifier {
     }
     // hostName and pattern are now absolute domain names.
 
+    // BEGIN Android-added: Reject non-ASCII hostnames and SANs. http://b/171980069
+    if (!isPrintableAscii(pattern)) {
+      return false;
+    }
+    // END Android-added: Reject non-ASCII hostnames and SANs. http://b/171980069
     pattern = pattern.toLowerCase(Locale.US);
     // hostName and pattern are now in lower case -- domain names are case-insensitive.
 
@@ -249,4 +262,25 @@ public final class OkHostnameVerifier implements HostnameVerifier {
     // hostName matches pattern
     return true;
   }
+
+  // BEGIN Android-added: Reject non-ASCII hostnames and SANs. http://b/171980069
+  /**
+   * Returns true if the  input string contains only printable 7-bit ASCII
+   * characters, otherwise false.
+   */
+  private static final char DEL = 127;
+  static boolean isPrintableAscii(String input) {
+    if (input == null) {
+      return false;
+    }
+    for (char c : input.toCharArray()) {
+      // Space is illegal in a DNS name. DEL and anything less than space is non-printing so
+      // also illegal. Anything greater than DEL is not 7-bit.
+      if (c <= ' ' || c >= DEL) {
+        return false;
+      }
+    }
+    return true;
+  }
+  // END Android-added: Reject non-ASCII hostnames and SANs. http://b/171980069
 }