You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2021/04/10 09:51:33 UTC

[httpcomponents-client] 01/01: HTTPCLIENT-2149: When no dNSName, match against CN

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit 58a17cc549a60b54d78edf6b4a25e92ad948c0da
Author: Peter Dettman <pe...@bouncycastle.org>
AuthorDate: Fri Apr 9 19:16:18 2021 +0700

    HTTPCLIENT-2149: When no dNSName, match against CN
---
 .../client5/http/ssl/DefaultHostnameVerifier.java  | 55 ++++++++++++----------
 .../client5/http/ssl/CertificatesToPlayWith.java   | 14 ++++++
 .../http/ssl/TestDefaultHostnameVerifier.java      | 28 ++++++++++-
 3 files changed, 70 insertions(+), 27 deletions(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultHostnameVerifier.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultHostnameVerifier.java
index 2bbfc6c..2c030ac 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultHostnameVerifier.java
+++ b/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(
-            final String host, final X509Certificate cert) throws SSLException {
+    public void verify(final String host, final X509Certificate cert) throws SSLException {
         final HostNameType hostType = determineHostFormat(host);
-        final List<SubjectName> subjectAlts = getSubjectAltNames(cert);
-        if (subjectAlts != null && !subjectAlts.isEmpty()) {
-            switch (hostType) {
-                case IPv4:
-                    matchIPAddress(host, subjectAlts);
-                    break;
-                case IPv6:
-                    matchIPv6Address(host, subjectAlts);
-                    break;
-                default:
-                    matchDNSName(host, subjectAlts, this.publicSuffixMatcher);
+        switch (hostType) {
+        case IPv4:
+            matchIPAddress(host, getSubjectAltNames(cert, SubjectName.IP));
+            break;
+        case IPv6:
+            matchIPv6Address(host, getSubjectAltNames(cert, SubjectName.IP));
+            break;
+        default:
+            final List<SubjectName> subjectAlts = getSubjectAltNames(cert, SubjectName.DNS);
+            if (subjectAlts.isEmpty()) {
+                // 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,
-                 final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
+    static void matchCN(final String host, final X509Certificate cert,
+                        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));
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/CertificatesToPlayWith.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/CertificatesToPlayWith.java
index 27fc974..3e81ea9 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/CertificatesToPlayWith.java
+++ b/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();
 }
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestDefaultHostnameVerifier.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestDefaultHostnameVerifier.java
index 929bad9..88487ff 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestDefaultHostnameVerifier.java
+++ b/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);
@@ -393,6 +394,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"));
         Assert.assertEquals("blah", DefaultHostnameVerifier.extractCN("cn=blah, cn=yada, cn=booh"));
@@ -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
+        }
     }
 
 }