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:48:27 UTC

[httpcomponents-client] branch master updated (8f31e63 -> 81abd55)

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

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


    from 8f31e63  HTTPCLIENT-2147: fixed broken preemptive auth in HC Fluent
     new ef4215b  HTTPCLIENT-2149: When no dNSName, match against CN
     new 1bee47d  Don't match CN for IP addresses
     new 81abd55  Avoid getSubjectAltNames duplication

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../client5/http/ssl/DefaultHostnameVerifier.java  | 55 ++++++++++++----------
 .../client5/http/ssl/CertificatesToPlayWith.java   | 14 ++++++
 .../http/ssl/TestDefaultHostnameVerifier.java      | 28 ++++++++++-
 3 files changed, 70 insertions(+), 27 deletions(-)

[httpcomponents-client] 03/03: Avoid getSubjectAltNames duplication

Posted by ol...@apache.org.
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 81abd55310feee63cfe8f562d0a66840a1869f99
Author: Peter Dettman <pe...@bouncycastle.org>
AuthorDate: Sat Apr 10 14:36:54 2021 +0700

    Avoid getSubjectAltNames duplication
---
 .../client5/http/ssl/DefaultHostnameVerifier.java  | 26 ++--------------------
 1 file changed, 2 insertions(+), 24 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 ec640c9..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
@@ -287,29 +287,7 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
     }
 
     static List<SubjectName> getSubjectAltNames(final X509Certificate cert) {
-        try {
-            final Collection<List<?>> entries = cert.getSubjectAlternativeNames();
-            if (entries == null) {
-                return Collections.emptyList();
-            }
-            final List<SubjectName> result = new ArrayList<>();
-            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) {
-                        final Object o = entry.get(1);
-                        if (o instanceof String) {
-                            result.add(new SubjectName((String) o, type));
-                        } else if (o instanceof byte[]) {
-                            // TODO ASN.1 DER encoded form
-                        }
-                    }
-                }
-            }
-            return result;
-        } catch (final CertificateParsingException ignore) {
-            return Collections.emptyList();
-        }
+        return getSubjectAltNames(cert, -1);
     }
 
     static List<SubjectName> getSubjectAltNames(final X509Certificate cert, final int subjectName) {
@@ -322,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) {
+                    if (type == subjectName || -1 == subjectName) {
                         final Object o = entry.get(1);
                         if (o instanceof String) {
                             result.add(new SubjectName((String) o, type));

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

Posted by ol...@apache.org.
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 ef4215b528030e031b907c657d428c44c83fb441
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  | 39 +++++++++++------
 .../client5/http/ssl/CertificatesToPlayWith.java   | 14 ++++++
 .../http/ssl/TestDefaultHostnameVerifier.java      | 51 +++++++++++++++++-----
 3 files changed, 81 insertions(+), 23 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..73ca314 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
@@ -114,18 +114,15 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
                     matchIPv6Address(host, subjectAlts);
                     break;
                 default:
-                    matchDNSName(host, subjectAlts, this.publicSuffixMatcher);
+                    // In case there are no SubjectName.DNS entries, fallback to CN matching
+                    if (!matchDNSName(host, subjectAlts, this.publicSuffixMatcher)) {
+                        matchCN(host, cert, 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);
+            // as fallback only when no subjectAlts of type SubjectName.DNS are available
+            matchCN(host, cert, this.publicSuffixMatcher);
         }
     }
 
@@ -157,24 +154,40 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
                 "of the subject alternative names: " + subjectAlts);
     }
 
-    static void matchDNSName(final String host, final List<SubjectName> subjectAlts,
-                             final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
+    static boolean matchDNSName(final String host, final List<SubjectName> subjectAlts,
+                                final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
         final String normalizedHost = DnsUtils.normalize(host);
+        boolean foundAnyDNS = false;
         for (int i = 0; i < subjectAlts.size(); i++) {
             final SubjectName subjectAlt = subjectAlts.get(i);
             if (subjectAlt.getType() == SubjectName.DNS) {
+                foundAnyDNS = true;
                 final String normalizedSubjectAlt = DnsUtils.normalize(subjectAlt.getValue());
                 if (matchIdentityStrict(normalizedHost, normalizedSubjectAlt, publicSuffixMatcher)) {
-                    return;
+                    return true;
                 }
             }
         }
+        if (!foundAnyDNS) {
+            return false;
+        }
         throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any " +
                 "of the subject alternative names: " + subjectAlts);
     }
 
+    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 SSLException("Certificate subject for <" + host + "> doesn't contain " +
+                    "a common name and does not have alternative names");
+        }
+        matchCN(host, cn, publicSuffixMatcher);
+    }
+
     static void matchCN(final String host, final String cn,
-                 final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
+                        final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
         final String normalizedHost = DnsUtils.normalize(host);
         final String normalizedCn = DnsUtils.normalize(cn);
         if (!matchIdentityStrict(normalizedHost, normalizedCn, publicSuffixMatcher)) {
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..3ec90e8 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"));
@@ -416,26 +432,41 @@ public class TestDefaultHostnameVerifier {
 
     @Test
     public void testMatchDNSName() throws Exception {
-        DefaultHostnameVerifier.matchDNSName(
+        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
                 "host.domain.com",
                 Collections.singletonList(SubjectName.DNS("*.domain.com")),
-                publicSuffixMatcher);
-        DefaultHostnameVerifier.matchDNSName(
+                publicSuffixMatcher));
+        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
                 "host.xx",
                 Collections.singletonList(SubjectName.DNS("*.xx")),
-                publicSuffixMatcher);
-        DefaultHostnameVerifier.matchDNSName(
+                publicSuffixMatcher));
+        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
                 "host.appspot.com",
                 Collections.singletonList(SubjectName.DNS("*.appspot.com")),
-                publicSuffixMatcher);
-        DefaultHostnameVerifier.matchDNSName(
+                publicSuffixMatcher));
+        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
                 "demo-s3-bucket.s3.eu-central-1.amazonaws.com",
                 Collections.singletonList(SubjectName.DNS("*.s3.eu-central-1.amazonaws.com")),
-                publicSuffixMatcher);
-        DefaultHostnameVerifier.matchDNSName(
+                publicSuffixMatcher));
+        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
                 "hostname-workspace-1.local",
                 Collections.singletonList(SubjectName.DNS("hostname-workspace-1.local")),
+                publicSuffixMatcher));
+
+        Assert.assertFalse(DefaultHostnameVerifier.matchDNSName(
+            "host.domain.com",
+            Collections.singletonList(SubjectName.IP("1.1.1.1")),
+            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
+        }
     }
 
 }

[httpcomponents-client] 02/03: Don't match CN for IP addresses

Posted by ol...@apache.org.
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 1bee47d2997c00d1508bd7b425ae2402214d850b
Author: Peter Dettman <pe...@bouncycastle.org>
AuthorDate: Sat Apr 10 11:27:17 2021 +0700

    Don't match CN for IP addresses
---
 .../client5/http/ssl/DefaultHostnameVerifier.java  | 80 +++++++++++++---------
 .../http/ssl/TestDefaultHostnameVerifier.java      | 25 +++----
 2 files changed, 56 insertions(+), 49 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 73ca314..ec640c9 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,28 +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:
-                    // In case there are no SubjectName.DNS entries, fallback to CN matching
-                    if (!matchDNSName(host, subjectAlts, this.publicSuffixMatcher)) {
-                        matchCN(host, cert, 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 of type SubjectName.DNS are available
-            matchCN(host, cert, this.publicSuffixMatcher);
         }
     }
 
@@ -154,23 +150,18 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
                 "of the subject alternative names: " + subjectAlts);
     }
 
-    static boolean matchDNSName(final String host, final List<SubjectName> subjectAlts,
-                                final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
+    static void matchDNSName(final String host, final List<SubjectName> subjectAlts,
+                             final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
         final String normalizedHost = DnsUtils.normalize(host);
-        boolean foundAnyDNS = false;
         for (int i = 0; i < subjectAlts.size(); i++) {
             final SubjectName subjectAlt = subjectAlts.get(i);
             if (subjectAlt.getType() == SubjectName.DNS) {
-                foundAnyDNS = true;
                 final String normalizedSubjectAlt = DnsUtils.normalize(subjectAlt.getValue());
                 if (matchIdentityStrict(normalizedHost, normalizedSubjectAlt, publicSuffixMatcher)) {
-                    return true;
+                    return;
                 }
             }
         }
-        if (!foundAnyDNS) {
-            return false;
-        }
         throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any " +
                 "of the subject alternative names: " + subjectAlts);
     }
@@ -180,14 +171,9 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
         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 " +
+            throw new SSLPeerUnverifiedException("Certificate subject for <" + host + "> doesn't contain " +
                     "a common name and does not have alternative names");
         }
-        matchCN(host, cn, publicSuffixMatcher);
-    }
-
-    static void matchCN(final String host, final String cn,
-                        final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
         final String normalizedHost = DnsUtils.normalize(host);
         final String normalizedCn = DnsUtils.normalize(cn);
         if (!matchIdentityStrict(normalizedHost, normalizedCn, publicSuffixMatcher)) {
@@ -326,6 +312,32 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
         }
     }
 
+    static List<SubjectName> getSubjectAltNames(final X509Certificate cert, final int subjectName) {
+        try {
+            final Collection<List<?>> entries = cert.getSubjectAlternativeNames();
+            if (entries == null) {
+                return Collections.emptyList();
+            }
+            final List<SubjectName> result = new ArrayList<>();
+            for (final List<?> entry : entries) {
+                final Integer type = entry.size() >= 2 ? (Integer) entry.get(0) : null;
+                if (type != null) {
+                    if (type == subjectName) {
+                        final Object o = entry.get(1);
+                        if (o instanceof String) {
+                            result.add(new SubjectName((String) o, type));
+                        } else if (o instanceof byte[]) {
+                            // TODO ASN.1 DER encoded form
+                        }
+                    }
+                }
+            }
+            return result;
+        } catch (final CertificateParsingException ignore) {
+            return Collections.emptyList();
+        }
+    }
+
     /*
      * Normalize IPv6 or DNS name.
      */
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 3ec90e8..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
@@ -432,31 +432,26 @@ public class TestDefaultHostnameVerifier {
 
     @Test
     public void testMatchDNSName() throws Exception {
-        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
+        DefaultHostnameVerifier.matchDNSName(
                 "host.domain.com",
                 Collections.singletonList(SubjectName.DNS("*.domain.com")),
-                publicSuffixMatcher));
-        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
+                publicSuffixMatcher);
+        DefaultHostnameVerifier.matchDNSName(
                 "host.xx",
                 Collections.singletonList(SubjectName.DNS("*.xx")),
-                publicSuffixMatcher));
-        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
+                publicSuffixMatcher);
+        DefaultHostnameVerifier.matchDNSName(
                 "host.appspot.com",
                 Collections.singletonList(SubjectName.DNS("*.appspot.com")),
-                publicSuffixMatcher));
-        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
+                publicSuffixMatcher);
+        DefaultHostnameVerifier.matchDNSName(
                 "demo-s3-bucket.s3.eu-central-1.amazonaws.com",
                 Collections.singletonList(SubjectName.DNS("*.s3.eu-central-1.amazonaws.com")),
-                publicSuffixMatcher));
-        Assert.assertTrue(DefaultHostnameVerifier.matchDNSName(
+                publicSuffixMatcher);
+        DefaultHostnameVerifier.matchDNSName(
                 "hostname-workspace-1.local",
                 Collections.singletonList(SubjectName.DNS("hostname-workspace-1.local")),
-                publicSuffixMatcher));
-
-        Assert.assertFalse(DefaultHostnameVerifier.matchDNSName(
-            "host.domain.com",
-            Collections.singletonList(SubjectName.IP("1.1.1.1")),
-            publicSuffixMatcher));
+                publicSuffixMatcher);
 
         try {
             DefaultHostnameVerifier.matchDNSName(