HTTPCLIENT-2030: Fix PublicSuffixMatcher::getDomainRoot on invalid hostnames

httpclient/src/main/java/org/apache/http/conn/ssl/DefaultHostnameVerifier.java

@@ -198,9 +198,10 @@ public final class DefaultHostnameVerifier implements HostnameVerifier {
 
     private static boolean matchIdentity(final String host, final String identity,
                                          final PublicSuffixMatcher publicSuffixMatcher,
+                                         final DomainType domainType,
                                          final boolean strict) {
         if (publicSuffixMatcher != null && host.contains(".")) {
-            if (!matchDomainRoot(host, publicSuffixMatcher.getDomainRoot(identity, DomainType.ICANN))) {
+            if (!matchDomainRoot(host, publicSuffixMatcher.getDomainRoot(identity, domainType))) {
                 return false;
             }
         }
@@ -235,20 +236,32 @@ public final class DefaultHostnameVerifier implements HostnameVerifier {
 
     static boolean matchIdentity(final String host, final String identity,
                                  final PublicSuffixMatcher publicSuffixMatcher) {
-        return matchIdentity(host, identity, publicSuffixMatcher, false);
+        return matchIdentity(host, identity, publicSuffixMatcher, null, false);
     }
 
     static boolean matchIdentity(final String host, final String identity) {
-        return matchIdentity(host, identity, null, false);
+        return matchIdentity(host, identity, null, null, false);
     }
 
     static boolean matchIdentityStrict(final String host, final String identity,
                                        final PublicSuffixMatcher publicSuffixMatcher) {
-        return matchIdentity(host, identity, publicSuffixMatcher, true);
+        return matchIdentity(host, identity, publicSuffixMatcher, null, true);
     }
 
     static boolean matchIdentityStrict(final String host, final String identity) {
-        return matchIdentity(host, identity, null, true);
+        return matchIdentity(host, identity, null, null, true);
+    }
+
+    static boolean matchIdentity(final String host, final String identity,
+                                 final PublicSuffixMatcher publicSuffixMatcher,
+                                 final DomainType domainType) {
+        return matchIdentity(host, identity, publicSuffixMatcher, domainType, false);
+    }
+
+    static boolean matchIdentityStrict(final String host, final String identity,
+                                       final PublicSuffixMatcher publicSuffixMatcher,
+                                       final DomainType domainType) {
+        return matchIdentity(host, identity, publicSuffixMatcher, domainType, true);
     }
 
     static String extractCN(final String subjectPrincipal) throws SSLException {

httpclient/src/main/java/org/apache/http/conn/util/PublicSuffixMatcher.java

@@ -166,9 +166,15 @@ public final class PublicSuffixMatcher {
             result = segment;
             segment = nextSegment;
         }
-        return result;
-    }
 
+        // If no expectations then this result is good.
+        if (expectedType == null || expectedType == DomainType.UNKNOWN) {
+            return result;
+        }
+
+        // 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.
      */

httpclient/src/test/java/org/apache/http/conn/ssl/TestDefaultHostnameVerifier.java

@@ -28,14 +28,20 @@
 package org.apache.http.conn.ssl;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.nio.charset.Charset;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
+import java.util.List;
 
 import javax.net.ssl.SSLException;
 
 import org.apache.http.conn.util.DomainType;
+import org.apache.http.conn.util.PublicSuffixList;
+import org.apache.http.conn.util.PublicSuffixListParser;
 import org.apache.http.conn.util.PublicSuffixMatcher;
 import org.junit.Assert;
 import org.junit.Before;
@@ -50,10 +56,22 @@ public class TestDefaultHostnameVerifier {
     private PublicSuffixMatcher publicSuffixMatcher;
     private DefaultHostnameVerifier implWithPublicSuffixCheck;
 
+    private static final String PUBLIC_SUFFIX_MATCHER_SOURCE_FILE = "suffixlistmatcher.txt";
+
+    private static final Charset UTF_8 = Charset.forName("UTF-8");
+
     @Before
-    public void setup() {
+    public void setup() throws IOException {
         impl = new DefaultHostnameVerifier();
-        publicSuffixMatcher = new PublicSuffixMatcher(DomainType.ICANN, Arrays.asList("com", "co.jp", "gov.uk"), null);
+
+        // Load the test PublicSuffixMatcher
+        final ClassLoader classLoader = getClass().getClassLoader();
+        final InputStream in = classLoader.getResourceAsStream(PUBLIC_SUFFIX_MATCHER_SOURCE_FILE);
+        Assert.assertNotNull(in);
+        final List<PublicSuffixList> lists = new PublicSuffixListParser().parseByType(
+                new InputStreamReader(in, UTF_8));
+        publicSuffixMatcher = new PublicSuffixMatcher(lists);
+
         implWithPublicSuffixCheck = new DefaultHostnameVerifier(publicSuffixMatcher);
     }
 
@@ -275,18 +293,88 @@ public class TestDefaultHostnameVerifier {
         Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict("mail.a.b.c.com", "m*.a.b.c.com"));
     }
 
+
     @Test
-    public void testHTTPCLIENT_1997() {
+    public void testHTTPCLIENT_1997_ANY() { // Only True on all domains
-        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(
+        String domain;
-                "service.apps.dev.b.cloud.a", "*.apps.dev.b.cloud.a"));
+        // Unknown
-        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(
+        domain = "dev.b.cloud.a";
-                "service.apps.dev.b.cloud.a", "*.apps.dev.b.cloud.a"));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain));
-        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain));
-                "service.apps.dev.b.cloud.a", "*.apps.dev.b.cloud.a", publicSuffixMatcher));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher));
-        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher));
-                "service.apps.dev.b.cloud.a", "*.apps.dev.b.cloud.a", publicSuffixMatcher));
+
+        // ICANN
+        domain = "dev.b.cloud.com";
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher));
+
+        // PRIVATE
+        domain = "dev.b.cloud.lan";
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher));
     }
 
+    @Test
+    public void testHTTPCLIENT_1997_ICANN() { // Only True on ICANN domains
+        String domain;
+        // Unknown
+        domain = "dev.b.cloud.a";
+        Assert.assertFalse(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.ICANN));
+        Assert.assertFalse(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.ICANN));
+
+        // ICANN
+        domain = "dev.b.cloud.com";
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.ICANN));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.ICANN));
+
+        // PRIVATE
+        domain = "dev.b.cloud.lan";
+        Assert.assertFalse(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.ICANN));
+        Assert.assertFalse(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.ICANN));
+    }
+
+    @Test
+    public void testHTTPCLIENT_1997_PRIVATE() { // Only True on PRIVATE domains
+        String domain;
+        // Unknown
+        domain = "dev.b.cloud.a";
+        Assert.assertFalse(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.PRIVATE));
+        Assert.assertFalse(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.PRIVATE));
+
+        // ICANN
+        domain = "dev.b.cloud.com";
+        Assert.assertFalse(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.PRIVATE));
+        Assert.assertFalse(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.PRIVATE));
+
+        // PRIVATE
+        domain = "dev.b.cloud.lan";
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.PRIVATE));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.PRIVATE));
+    }
+
+    @Test
+    public void testHTTPCLIENT_1997_UNKNOWN() { // Only True on all domains (same as ANY)
+        String domain;
+        // Unknown
+        domain = "dev.b.cloud.a";
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.UNKNOWN));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.UNKNOWN));
+
+        // ICANN
+        domain = "dev.b.cloud.com";
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(        "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.UNKNOWN));
+        Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(  "service.apps." + domain, "*.apps." + domain, publicSuffixMatcher, DomainType.UNKNOWN));
+
+        // PRIVATE
+        domain = "dev.b.cloud.lan";
+        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";

httpclient/src/test/java/org/apache/http/conn/util/TestPublicSuffixMatcher.java

@@ -29,45 +29,47 @@ package org.apache.http.conn.util;
 
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.nio.charset.Charset;
+import java.util.List;
 
-import org.apache.http.Consts;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
 public class TestPublicSuffixMatcher {
 
-    private static final String SOURCE_FILE = "suffixlist.txt";
+    private static final String SOURCE_FILE = "suffixlistmatcher.txt";
 
     private PublicSuffixMatcher matcher;
 
+    private static final Charset UTF_8 = Charset.forName("UTF-8");
+
     @Before
     public void setUp() throws Exception {
         final ClassLoader classLoader = getClass().getClassLoader();
         final InputStream in = classLoader.getResourceAsStream(SOURCE_FILE);
         Assert.assertNotNull(in);
-        final PublicSuffixList suffixList;
+        final List<PublicSuffixList> lists = new PublicSuffixListParser().parseByType(
-        try {
+                new InputStreamReader(in, UTF_8));
-            final PublicSuffixListParser parser = new PublicSuffixListParser();
+        matcher = new PublicSuffixMatcher(lists);
-            suffixList = parser.parse(new InputStreamReader(in, Consts.UTF_8));
-        } finally {
-            in.close();
-        }
-        matcher = new PublicSuffixMatcher(suffixList.getRules(), suffixList.getExceptions());
     }
 
     @Test
-    public void testGetDomainRoot() throws Exception {
+    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"));
+        // 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"));
+        // ICANN
         Assert.assertEquals("metro.tokyo.jp", matcher.getDomainRoot("metro.tokyo.jp"));
         Assert.assertEquals("blah.blah.tokyo.jp", matcher.getDomainRoot("blah.blah.tokyo.jp"));
         Assert.assertEquals("blah.ac.jp", matcher.getDomainRoot("blah.blah.ac.jp"));
+        // Unknown
         Assert.assertEquals("garbage", matcher.getDomainRoot("garbage"));
         Assert.assertEquals("garbage", matcher.getDomainRoot("garbage.garbage"));
         Assert.assertEquals("garbage", matcher.getDomainRoot("*.garbage.garbage"));
@@ -75,7 +77,52 @@ public class TestPublicSuffixMatcher {
     }
 
     @Test
-    public void testMatch() throws Exception {
+    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));
+        // 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));
+        // ICANN
+        Assert.assertEquals(null, matcher.getDomainRoot("metro.tokyo.jp", DomainType.PRIVATE));
+        Assert.assertEquals(null, matcher.getDomainRoot("blah.blah.tokyo.jp", DomainType.PRIVATE));
+        Assert.assertEquals(null, matcher.getDomainRoot("blah.blah.ac.jp", DomainType.PRIVATE));
+        // Unknown
+        Assert.assertEquals(null, matcher.getDomainRoot("garbage", DomainType.PRIVATE));
+        Assert.assertEquals(null, matcher.getDomainRoot("garbage.garbage", DomainType.PRIVATE));
+        Assert.assertEquals(null, matcher.getDomainRoot("*.garbage.garbage", DomainType.PRIVATE));
+        Assert.assertEquals(null, matcher.getDomainRoot("*.garbage.garbage.garbage", DomainType.PRIVATE));
+    }
+
+    @Test
+    public void testGetDomainRootOnlyICANN() {
+        // Private
+        Assert.assertEquals(null, matcher.getDomainRoot("example.XX", DomainType.ICANN));
+        Assert.assertEquals(null, matcher.getDomainRoot("www.example.XX", DomainType.ICANN));
+        Assert.assertEquals(null, matcher.getDomainRoot("www.blah.blah.example.XX", DomainType.ICANN));
+        // Too short
+        Assert.assertEquals(null, matcher.getDomainRoot("xx", DomainType.ICANN));
+        Assert.assertEquals(null, matcher.getDomainRoot("jp", DomainType.ICANN));
+        Assert.assertEquals(null, matcher.getDomainRoot("ac.jp", DomainType.ICANN));
+        Assert.assertEquals(null, matcher.getDomainRoot("any.tokyo.jp", DomainType.ICANN));
+        // ICANN
+        Assert.assertEquals("metro.tokyo.jp", matcher.getDomainRoot("metro.tokyo.jp", DomainType.ICANN));
+        Assert.assertEquals("blah.blah.tokyo.jp", matcher.getDomainRoot("blah.blah.tokyo.jp", DomainType.ICANN));
+        Assert.assertEquals("blah.ac.jp", matcher.getDomainRoot("blah.blah.ac.jp", DomainType.ICANN));
+        // Unknown
+        Assert.assertEquals(null, matcher.getDomainRoot("garbage", DomainType.ICANN));
+        Assert.assertEquals(null, matcher.getDomainRoot("garbage.garbage", DomainType.ICANN));
+        Assert.assertEquals(null, matcher.getDomainRoot("*.garbage.garbage", DomainType.ICANN));
+        Assert.assertEquals(null, matcher.getDomainRoot("*.garbage.garbage.garbage", DomainType.ICANN));
+    }
+
+
+    @Test
+    public void testMatch() {
         Assert.assertTrue(matcher.matches(".jp"));
         Assert.assertTrue(matcher.matches(".ac.jp"));
         Assert.assertTrue(matcher.matches(".any.tokyo.jp"));
@@ -84,7 +131,7 @@ public class TestPublicSuffixMatcher {
     }
 
     @Test
-    public void testMatchUnicode() throws Exception {
+    public void testMatchUnicode() {
         Assert.assertTrue(matcher.matches(".h\u00E5.no")); // \u00E5 is <aring>
         Assert.assertTrue(matcher.matches(".xn--h-2fa.no"));
         Assert.assertTrue(matcher.matches(".h\u00E5.no"));

httpclient/src/test/resources/suffixlistmatcher.txt

@@ -0,0 +1,46 @@
+// ====================================================================
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+// ====================================================================
+//
+// This software consists of voluntary contributions made by many
+// individuals on behalf of the Apache Software Foundation.  For more
+// information on the Apache Software Foundation, please see
+// <http://www.apache.org/>.
+//
+
+// ===BEGIN PRIVATE DOMAINS===
+xx
+lan
+// ===END PRIVATE DOMAINS===
+
+// ===BEGIN ICANN DOMAINS===
+
+jp
+ac.jp
+*.tokyo.jp
+!metro.tokyo.jp
+
+com
+co.jp
+gov.uk
+
+// unicode
+no
+hå.no
+
+// ===END ICANN DOMAINS===