HTTPCLIENT-2149: When no dNSName, match against CN

httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultHostnameVerifier.java

@@ -101,31 +101,24 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
     }
 
     @Override
-    public void verify(
+    public void verify(final String host, final X509Certificate cert) throws SSLException {
-            final String host, final X509Certificate cert) throws SSLException {
         final HostNameType hostType = determineHostFormat(host);
-        final List<SubjectName> subjectAlts = getSubjectAltNames(cert);
+        switch (hostType) {
-        if (subjectAlts != null && !subjectAlts.isEmpty()) {
+        case IPv4:
-            switch (hostType) {
+            matchIPAddress(host, getSubjectAltNames(cert, SubjectName.IP));
-                case IPv4:
+            break;
-                    matchIPAddress(host, subjectAlts);
+        case IPv6:
-                    break;
+            matchIPv6Address(host, getSubjectAltNames(cert, SubjectName.IP));
-                case IPv6:
+            break;
-                    matchIPv6Address(host, subjectAlts);
+        default:
-                    break;
+            final List<SubjectName> subjectAlts = getSubjectAltNames(cert, SubjectName.DNS);
-                default:
+            if (subjectAlts.isEmpty()) {
-                    matchDNSName(host, subjectAlts, this.publicSuffixMatcher);
+                // CN matching has been deprecated by rfc2818 and can be used
+                // as fallback only when no subjectAlts of type SubjectName.DNS are available
+                matchCN(host, cert, this.publicSuffixMatcher);
+            } else {
+                matchDNSName(host, subjectAlts, this.publicSuffixMatcher);
             }
-        } else {
-            // CN matching has been deprecated by rfc2818 and can be used
-            // as fallback only when no subjectAlts are available
-            final X500Principal subjectPrincipal = cert.getSubjectX500Principal();
-            final String cn = extractCN(subjectPrincipal.getName(X500Principal.RFC2253));
-            if (cn == null) {
-                throw new SSLException("Certificate subject for <" + host + "> doesn't contain " +
-                        "a common name and does not have alternative names");
-            }
-            matchCN(host, cn, this.publicSuffixMatcher);
         }
     }
 
@@ -173,8 +166,14 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
                 "of the subject alternative names: " + subjectAlts);
     }
 
-    static void matchCN(final String host, final String cn,
+    static void matchCN(final String host, final X509Certificate cert,
-                 final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
+                        final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
+        final X500Principal subjectPrincipal = cert.getSubjectX500Principal();
+        final String cn = extractCN(subjectPrincipal.getName(X500Principal.RFC2253));
+        if (cn == null) {
+            throw new SSLPeerUnverifiedException("Certificate subject for <" + host + "> doesn't contain " +
+                    "a common name and does not have alternative names");
+        }
         final String normalizedHost = DnsUtils.normalize(host);
         final String normalizedCn = DnsUtils.normalize(cn);
         if (!matchIdentityStrict(normalizedHost, normalizedCn, publicSuffixMatcher)) {
@@ -288,6 +287,10 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
     }
 
     static List<SubjectName> getSubjectAltNames(final X509Certificate cert) {
+        return getSubjectAltNames(cert, -1);
+    }
+
+    static List<SubjectName> getSubjectAltNames(final X509Certificate cert, final int subjectName) {
         try {
             final Collection<List<?>> entries = cert.getSubjectAlternativeNames();
             if (entries == null) {
@@ -297,7 +300,7 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
             for (final List<?> entry : entries) {
                 final Integer type = entry.size() >= 2 ? (Integer) entry.get(0) : null;
                 if (type != null) {
-                    if (type == SubjectName.DNS || type == SubjectName.IP) {
+                    if (type == subjectName || -1 == subjectName) {
                         final Object o = entry.get(1);
                         if (o instanceof String) {
                             result.add(new SubjectName((String) o, type));

httpclient5/src/test/java/org/apache/hc/client5/http/ssl/CertificatesToPlayWith.java

@@ -575,4 +575,18 @@ public class CertificatesToPlayWith {
         "-----END CERTIFICATE-----"
         ).getBytes();
 
+    /**
+     * subject CN=www.foo.com, subjectAlt=IP Address:127.0.0.1
+     */
+    public final static byte[] SUBJECT_ALT_IP_ONLY = (
+        "-----BEGIN CERTIFICATE-----\n" +
+        "MIIBQjCB6qADAgECAgYBeLZWSL0wCgYIKoZIzj0EAwIwFjEUMBIGA1UEAwwLd3d3\n" +
+        "LmZvby5jb20wHhcNMjEwNDA5MTExMzI2WhcNMjEwNDA5MTE0MzMxWjAWMRQwEgYD\n" +
+        "VQQDDAt3d3cuZm9vLmNvbTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABPC+8O/v\n" +
+        "IPWSC/iPdPAgpgzpyLKZevNH8ENOb6PaJRDyNHdd1MbJvurKtJ+HP6UYV3keNHUk\n" +
+        "r657s2JjufiTmuSjJDAiMAwGA1UdEwQFMAMBAf8wEgYDVR0RAQH/BAgwBocEfwAA\n" +
+        "ATAKBggqhkjOPQQDAgNHADBEAiA2svKw50Mr5nnF4TXyFcvzhJWkC+7m46JROMiy\n" +
+        "TMt3BQIgK5IHScVH6Cbi106y+BILx4U0Ygt5IFNnMx/K+Jusuls=\n" +
+        "-----END CERTIFICATE-----"
+        ).getBytes();
 }

httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestDefaultHostnameVerifier.java

@@ -176,9 +176,10 @@ public class TestDefaultHostnameVerifier {
         in = new ByteArrayInputStream(CertificatesToPlayWith.IP_1_1_1_1);
         x509 = (X509Certificate) cf.generateCertificate(in);
         impl.verify("1.1.1.1", x509);
+        impl.verify("dummy-value.com", x509);
 
         exceptionPlease(impl, "1.1.1.2", x509);
-        exceptionPlease(impl, "dummy-value.com", x509);
+        exceptionPlease(impl, "not-the-cn.com", x509);
 
         in = new ByteArrayInputStream(CertificatesToPlayWith.EMAIL_ALT_SUBJECT_NAME);
         x509 = (X509Certificate) cf.generateCertificate(in);
@@ -392,6 +393,21 @@ public class TestDefaultHostnameVerifier {
         }
     }
 
+    @Test
+    public void testHTTPCLIENT_2149() throws Exception {
+        final CertificateFactory cf = CertificateFactory.getInstance("X.509");
+        final InputStream in = new ByteArrayInputStream(CertificatesToPlayWith.SUBJECT_ALT_IP_ONLY);
+        final X509Certificate x509 = (X509Certificate) cf.generateCertificate(in);
+
+        Assert.assertEquals("CN=www.foo.com", x509.getSubjectDN().getName());
+
+        impl.verify("127.0.0.1", x509);
+        impl.verify("www.foo.com", x509);
+
+        exceptionPlease(impl, "127.0.0.2", x509);
+        exceptionPlease(impl, "www.bar.com", x509);
+    }
+
     @Test
     public void testExtractCN() throws Exception {
         Assert.assertEquals("blah", DefaultHostnameVerifier.extractCN("cn=blah, ou=blah, o=blah"));
@@ -436,6 +452,16 @@ public class TestDefaultHostnameVerifier {
                 "hostname-workspace-1.local",
                 Collections.singletonList(SubjectName.DNS("hostname-workspace-1.local")),
                 publicSuffixMatcher);
+
+        try {
+            DefaultHostnameVerifier.matchDNSName(
+                "host.domain.com",
+                Collections.singletonList(SubjectName.DNS("some.other.com")),
+                publicSuffixMatcher);
+            Assert.fail("SSLException should have been thrown");
+        } catch (final SSLException ex) {
+            // expected
+        }
     }
 
 }