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 2019/11/26 10:53:59 UTC
[httpcomponents-client] branch master updated: HTTPCLIENT-2030: Fix
PublicSuffixMatcher::getDomainRoot on invalid hostnames
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
The following commit(s) were added to refs/heads/master by this push:
new 9552d5d HTTPCLIENT-2030: Fix PublicSuffixMatcher::getDomainRoot on invalid hostnames
9552d5d is described below
commit 9552d5dd1d21407be98427444c27d678006dc9c9
Author: Niels Basjes <ni...@basjes.nl>
AuthorDate: Tue Nov 26 11:22:14 2019 +0100
HTTPCLIENT-2030: Fix PublicSuffixMatcher::getDomainRoot on invalid hostnames
---
.../hc/client5/http/psl/PublicSuffixMatcher.java | 9 +-
.../client5/http/ssl/DefaultHostnameVerifier.java | 23 +++-
.../client5/http/psl/TestPublicSuffixMatcher.java | 69 ++++++++++--
.../http/ssl/TestDefaultHostnameVerifier.java | 123 +++++++++++++++++----
.../src/test/resources/suffixlistmatcher.txt | 46 ++++++++
5 files changed, 233 insertions(+), 37 deletions(-)
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/psl/PublicSuffixMatcher.java b/httpclient5/src/main/java/org/apache/hc/client5/http/psl/PublicSuffixMatcher.java
index 29c50dc..bb1fdcd 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/psl/PublicSuffixMatcher.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/psl/PublicSuffixMatcher.java
@@ -171,7 +171,14 @@ 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;
}
/**
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 cb88375..a163946 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
@@ -193,9 +193,10 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
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;
}
}
@@ -230,20 +231,32 @@ public final class DefaultHostnameVerifier implements HttpClientHostnameVerifier
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 {
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/psl/TestPublicSuffixMatcher.java b/httpclient5/src/test/java/org/apache/hc/client5/http/psl/TestPublicSuffixMatcher.java
index 071d4cc..6218a54 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/psl/TestPublicSuffixMatcher.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/psl/TestPublicSuffixMatcher.java
@@ -30,6 +30,7 @@ package org.apache.hc.client5.http.psl;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
+import java.util.List;
import org.junit.Assert;
import org.junit.Before;
@@ -37,7 +38,7 @@ 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;
@@ -46,28 +47,27 @@ public class TestPublicSuffixMatcher {
final ClassLoader classLoader = getClass().getClassLoader();
final InputStream in = classLoader.getResourceAsStream(SOURCE_FILE);
Assert.assertNotNull(in);
- final PublicSuffixList suffixList;
- try {
- final PublicSuffixListParser parser = new PublicSuffixListParser();
- suffixList = parser.parse(new InputStreamReader(in, StandardCharsets.UTF_8));
- } finally {
- in.close();
- }
- matcher = new PublicSuffixMatcher(suffixList.getRules(), suffixList.getExceptions());
+ final List<PublicSuffixList> lists = new PublicSuffixListParser().parseByType(
+ new InputStreamReader(in, StandardCharsets.UTF_8));
+ matcher = new PublicSuffixMatcher(lists);
}
@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 +75,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 +129,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"));
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 bd961d3..db84c86 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
@@ -27,20 +27,25 @@
package org.apache.hc.client5.http.ssl;
-import java.io.ByteArrayInputStream;
-import java.io.InputStream;
-import java.security.cert.CertificateFactory;
-import java.security.cert.X509Certificate;
-import java.util.Arrays;
-
-import javax.net.ssl.SSLException;
-
import org.apache.hc.client5.http.psl.DomainType;
+import org.apache.hc.client5.http.psl.PublicSuffixList;
+import org.apache.hc.client5.http.psl.PublicSuffixListParser;
import org.apache.hc.client5.http.psl.PublicSuffixMatcher;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
+import javax.net.ssl.SSLException;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.security.cert.CertificateFactory;
+import java.security.cert.X509Certificate;
+import java.util.Arrays;
+import java.util.List;
+
/**
* Unit tests for {@link org.apache.hc.client5.http.ssl.DefaultHostnameVerifier}.
*/
@@ -50,10 +55,20 @@ public class TestDefaultHostnameVerifier {
private PublicSuffixMatcher publicSuffixMatcher;
private DefaultHostnameVerifier implWithPublicSuffixCheck;
+ private static final String PUBLIC_SUFFIX_MATCHER_SOURCE_FILE = "suffixlistmatcher.txt";
+
@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, StandardCharsets.UTF_8));
+ publicSuffixMatcher = new PublicSuffixMatcher(lists);
+
implWithPublicSuffixCheck = new DefaultHostnameVerifier(publicSuffixMatcher);
}
@@ -276,15 +291,85 @@ public class TestDefaultHostnameVerifier {
}
@Test
- public void testHTTPCLIENT_1997() {
- Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(
- "service.apps.dev.b.cloud.a", "*.apps.dev.b.cloud.a"));
- Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(
- "service.apps.dev.b.cloud.a", "*.apps.dev.b.cloud.a"));
- Assert.assertTrue(DefaultHostnameVerifier.matchIdentity(
- "service.apps.dev.b.cloud.a", "*.apps.dev.b.cloud.a", publicSuffixMatcher));
- Assert.assertTrue(DefaultHostnameVerifier.matchIdentityStrict(
- "service.apps.dev.b.cloud.a", "*.apps.dev.b.cloud.a", publicSuffixMatcher));
+ public void testHTTPCLIENT_1997_ANY() { // Only True on all domains
+ String domain;
+ // Unknown
+ domain = "dev.b.cloud.a";
+ 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));
+
+ // 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
diff --git a/httpclient5/src/test/resources/suffixlistmatcher.txt b/httpclient5/src/test/resources/suffixlistmatcher.txt
new file mode 100644
index 0000000..463c8b9
--- /dev/null
+++ b/httpclient5/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===