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 2020/01/29 08:36:28 UTC

[httpcomponents-client] branch 4.5.x updated (ca0f849 -> 736c00d)

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

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


    from ca0f849  Upgraded HttpClient version to 4.5.12-SNAPSHOT
     new dbc3342  Bug fix: fixed handling of private domains by PublicSuffixMatcher
     new 736c00d  HTTPCLIENT-2047: fixed regression in DefaultHostnameVerifier causing rejection of certs with non-standard domains.

The 2 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:
 .../http/conn/ssl/DefaultHostnameVerifier.java     |  4 +--
 .../apache/http/conn/util/PublicSuffixMatcher.java | 31 +++++++++++++---------
 .../http/conn/ssl/TestDefaultHostnameVerifier.java | 26 ++++++++++++++++++
 .../http/conn/util/TestPublicSuffixMatcher.java    | 18 +++++++------
 .../src/test/resources/suffixlistmatcher.txt       |  2 ++
 5 files changed, 58 insertions(+), 23 deletions(-)


[httpcomponents-client] 02/02: HTTPCLIENT-2047: fixed regression in DefaultHostnameVerifier causing rejection of certs with non-standard domains.

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 736c00da6dfe4c91210d80d06cf7b28f857c035b
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sat Jan 25 15:49:44 2020 +0100

    HTTPCLIENT-2047: fixed regression in DefaultHostnameVerifier causing rejection of certs with non-standard domains.
    
    This reverts commit e0416f07
---
 .../http/conn/ssl/DefaultHostnameVerifier.java     |  4 ++--
 .../http/conn/ssl/TestDefaultHostnameVerifier.java | 26 ++++++++++++++++++++++
 .../src/test/resources/suffixlistmatcher.txt       |  1 +
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java b/httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java
index 4a0ae1f..18dd5dc 100644
--- a/httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java
+++ b/httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java
@@ -169,7 +169,7 @@ public final class DefaultHostnameVerifier implements HostnameVerifier {
             final SubjectName subjectAlt = subjectAlts.get(i);
             if (subjectAlt.getType() == SubjectName.DNS) {
                 final String normalizedSubjectAlt = DnsUtils.normalize(subjectAlt.getValue());
-                if (matchIdentityStrict(normalizedHost, normalizedSubjectAlt, publicSuffixMatcher, DomainType.ICANN)) {
+                if (matchIdentityStrict(normalizedHost, normalizedSubjectAlt, publicSuffixMatcher)) {
                     return;
                 }
             }
@@ -182,7 +182,7 @@ public final class DefaultHostnameVerifier implements HostnameVerifier {
                  final PublicSuffixMatcher publicSuffixMatcher) throws SSLException {
         final String normalizedHost = DnsUtils.normalize(host);
         final String normalizedCn = DnsUtils.normalize(cn);
-        if (!matchIdentityStrict(normalizedHost, normalizedCn, publicSuffixMatcher, DomainType.ICANN)) {
+        if (!matchIdentityStrict(normalizedHost, normalizedCn, publicSuffixMatcher)) {
             throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match " +
                     "common name of the certificate subject: " + cn);
         }
diff --git a/httpclient/src/test/java/org/apache/http/conn/ssl/TestDefaultHostnameVerifier.java b/httpclient/src/test/java/org/apache/http/conn/ssl/TestDefaultHostnameVerifier.java
index ec6f2a9..71bf7e0 100644
--- a/httpclient/src/test/java/org/apache/http/conn/ssl/TestDefaultHostnameVerifier.java
+++ b/httpclient/src/test/java/org/apache/http/conn/ssl/TestDefaultHostnameVerifier.java
@@ -35,6 +35,7 @@ import java.nio.charset.Charset;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import javax.net.ssl.SSLException;
@@ -375,6 +376,7 @@ public class TestDefaultHostnameVerifier {
         Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.UNKNOWN));
         Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.UNKNOWN));
     }
+
     @Test // Check compressed IPv6 hostname matching
     public void testHTTPCLIENT_1316() throws Exception{
         final String host1 = "2001:0db8:aaaa:bbbb:cccc:0:0:0001";
@@ -417,4 +419,28 @@ public class TestDefaultHostnameVerifier {
         }
     }
 
+    @Test
+    public void testMatchDNSName() throws Exception {
+        DefaultHostnameVerifier.matchDNSName(
+                "host.domain.com",
+                Collections.singletonList(SubjectName.DNS("*.domain.com")),
+                publicSuffixMatcher);
+        DefaultHostnameVerifier.matchDNSName(
+                "host.xx",
+                Collections.singletonList(SubjectName.DNS("*.xx")),
+                publicSuffixMatcher);
+        DefaultHostnameVerifier.matchDNSName(
+                "host.appspot.com",
+                Collections.singletonList(SubjectName.DNS("*.appspot.com")),
+                publicSuffixMatcher);
+        DefaultHostnameVerifier.matchDNSName(
+                "demo-s3-bucket.s3.eu-central-1.amazonaws.com",
+                Collections.singletonList(SubjectName.DNS("*.s3.eu-central-1.amazonaws.com")),
+                publicSuffixMatcher);
+        DefaultHostnameVerifier.matchDNSName(
+                "hostname-workspace-1.local",
+                Collections.singletonList(SubjectName.DNS("hostname-workspace-1.local")),
+                publicSuffixMatcher);
+    }
+
 }
diff --git a/httpclient/src/test/resources/suffixlistmatcher.txt b/httpclient/src/test/resources/suffixlistmatcher.txt
index b027fe4..e9377cb 100644
--- a/httpclient/src/test/resources/suffixlistmatcher.txt
+++ b/httpclient/src/test/resources/suffixlistmatcher.txt
@@ -27,6 +27,7 @@
 xx
 lan
 appspot.com
+s3.eu-central-1.amazonaws.com
 // ===END PRIVATE DOMAINS===
 
 // ===BEGIN ICANN DOMAINS===


[httpcomponents-client] 01/02: Bug fix: fixed handling of private domains by PublicSuffixMatcher

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit dbc3342781361acb6907a2e607d1318833b1bc16
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sat Jan 25 15:24:19 2020 +0100

    Bug fix: fixed handling of private domains by PublicSuffixMatcher
---
 .../apache/http/conn/util/PublicSuffixMatcher.java | 31 +++++++++++++---------
 .../http/conn/util/TestPublicSuffixMatcher.java    | 18 +++++++------
 .../src/test/resources/suffixlistmatcher.txt       |  1 +
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/httpclient/src/main/java/org/apache/http/conn/util/PublicSuffixMatcher.java b/httpclient/src/main/java/org/apache/http/conn/util/PublicSuffixMatcher.java
index 2f63c38..4c246e7 100644
--- a/httpclient/src/main/java/org/apache/http/conn/util/PublicSuffixMatcher.java
+++ b/httpclient/src/main/java/org/apache/http/conn/util/PublicSuffixMatcher.java
@@ -97,20 +97,15 @@ public final class PublicSuffixMatcher {
         }
     }
 
-    private static boolean hasEntry(final Map<String, DomainType> map, final String rule, final DomainType expectedType) {
+    private static DomainType findEntry(final Map<String, DomainType> map, final String rule) {
         if (map == null) {
-            return false;
+            return null;
         }
-        final DomainType domainType = map.get(rule);
-        return domainType == null ? false : expectedType == null || domainType.equals(expectedType);
-    }
-
-    private boolean hasRule(final String rule, final DomainType expectedType) {
-        return hasEntry(this.rules, rule, expectedType);
+        return map.get(rule);
     }
 
-    private boolean hasException(final String exception, final DomainType expectedType) {
-        return hasEntry(this.exceptions, exception, expectedType);
+    private static boolean match(final DomainType domainType, final DomainType expectedType) {
+        return domainType != null && (expectedType == null || domainType.equals(expectedType));
     }
 
     /**
@@ -147,10 +142,15 @@ public final class PublicSuffixMatcher {
         while (segment != null) {
             // An exception rule takes priority over any other matching rule.
             final String key = IDN.toUnicode(segment);
-            if (hasException(key, expectedType)) {
+            final DomainType exceptionRule = findEntry(exceptions, key);
+            if (match(exceptionRule, expectedType)) {
                 return segment;
             }
-            if (hasRule(key, expectedType)) {
+            final DomainType domainRule = findEntry(rules, key);
+            if (match(domainRule, expectedType)) {
+                if (domainRule == DomainType.PRIVATE) {
+                    return segment;
+                }
                 return result;
             }
 
@@ -158,7 +158,11 @@ public final class PublicSuffixMatcher {
             final String nextSegment = nextdot != -1 ? segment.substring(nextdot + 1) : null;
 
             if (nextSegment != null) {
-                if (hasRule("*." + IDN.toUnicode(nextSegment), expectedType)) {
+                final DomainType wildcardDomainRule = findEntry(rules, "*." + IDN.toUnicode(nextSegment));
+                if (match(wildcardDomainRule, expectedType)) {
+                    if (wildcardDomainRule == DomainType.PRIVATE) {
+                        return segment;
+                    }
                     return result;
                 }
             }
@@ -174,6 +178,7 @@ public final class PublicSuffixMatcher {
         // If we did have expectations apparently there was no match
         return null;
     }
+
     /**
      * Tests whether the given domain matches any of entry from the public suffix list.
      */
diff --git a/httpclient/src/test/java/org/apache/http/conn/util/TestPublicSuffixMatcher.java b/httpclient/src/test/java/org/apache/http/conn/util/TestPublicSuffixMatcher.java
index 6640384..4823164 100644
--- a/httpclient/src/test/java/org/apache/http/conn/util/TestPublicSuffixMatcher.java
+++ b/httpclient/src/test/java/org/apache/http/conn/util/TestPublicSuffixMatcher.java
@@ -57,11 +57,11 @@ public class TestPublicSuffixMatcher {
     @Test
     public void testGetDomainRootAnyType() {
         // Private
-        Assert.assertEquals("example.xx", matcher.getDomainRoot("example.XX"));
-        Assert.assertEquals("example.xx", matcher.getDomainRoot("www.example.XX"));
-        Assert.assertEquals("example.xx", matcher.getDomainRoot("www.blah.blah.example.XX"));
+        Assert.assertEquals("xx", matcher.getDomainRoot("example.XX"));
+        Assert.assertEquals("xx", matcher.getDomainRoot("www.example.XX"));
+        Assert.assertEquals("xx", matcher.getDomainRoot("www.blah.blah.example.XX"));
+        Assert.assertEquals("appspot.com", matcher.getDomainRoot("example.appspot.com"));
         // Too short
-        Assert.assertEquals(null, matcher.getDomainRoot("xx"));
         Assert.assertEquals(null, matcher.getDomainRoot("jp"));
         Assert.assertEquals(null, matcher.getDomainRoot("ac.jp"));
         Assert.assertEquals(null, matcher.getDomainRoot("any.tokyo.jp"));
@@ -79,11 +79,11 @@ public class TestPublicSuffixMatcher {
     @Test
     public void testGetDomainRootOnlyPRIVATE() {
         // Private
-        Assert.assertEquals("example.xx", matcher.getDomainRoot("example.XX", DomainType.PRIVATE));
-        Assert.assertEquals("example.xx", matcher.getDomainRoot("www.example.XX", DomainType.PRIVATE));
-        Assert.assertEquals("example.xx", matcher.getDomainRoot("www.blah.blah.example.XX", DomainType.PRIVATE));
+        Assert.assertEquals("xx", matcher.getDomainRoot("example.XX", DomainType.PRIVATE));
+        Assert.assertEquals("xx", matcher.getDomainRoot("www.example.XX", DomainType.PRIVATE));
+        Assert.assertEquals("xx", matcher.getDomainRoot("www.blah.blah.example.XX", DomainType.PRIVATE));
+        Assert.assertEquals("appspot.com", matcher.getDomainRoot("example.appspot.com"));
         // Too short
-        Assert.assertEquals(null, matcher.getDomainRoot("xx", DomainType.PRIVATE));
         Assert.assertEquals(null, matcher.getDomainRoot("jp", DomainType.PRIVATE));
         Assert.assertEquals(null, matcher.getDomainRoot("ac.jp", DomainType.PRIVATE));
         Assert.assertEquals(null, matcher.getDomainRoot("any.tokyo.jp", DomainType.PRIVATE));
@@ -128,6 +128,8 @@ public class TestPublicSuffixMatcher {
         Assert.assertTrue(matcher.matches(".any.tokyo.jp"));
         // exception
         Assert.assertFalse(matcher.matches(".metro.tokyo.jp"));
+        Assert.assertFalse(matcher.matches(".xx"));
+        Assert.assertFalse(matcher.matches(".appspot.com"));
     }
 
     @Test
diff --git a/httpclient/src/test/resources/suffixlistmatcher.txt b/httpclient/src/test/resources/suffixlistmatcher.txt
index 463c8b9..b027fe4 100644
--- a/httpclient/src/test/resources/suffixlistmatcher.txt
+++ b/httpclient/src/test/resources/suffixlistmatcher.txt
@@ -26,6 +26,7 @@
 // ===BEGIN PRIVATE DOMAINS===
 xx
 lan
+appspot.com
 // ===END PRIVATE DOMAINS===
 
 // ===BEGIN ICANN DOMAINS===