DNS host / identity normalization should be performed only once per public API call

ok2c · ok2c · commit b5dc3505c40b · 2025-04-05T13:06:51.000+02:00
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
@@ -138,8 +138,17 @@ public String getDomainRoot(final String domain, final DomainType expectedType)
         if (domain.startsWith(".")) {
             return null;
         }
-        final DomainRootInfo match = resolveDomainRoot(domain, expectedType);
-        return match != null ? match.root : null;
+        String normalized = DnsUtils.normalize(domain);
+        final boolean punyCoded = normalized.contains("xn-");
+        if (punyCoded) {
+            normalized = IDN.toUnicode(normalized);
+        }
+        final DomainRootInfo match = resolveDomainRoot(normalized, expectedType);
+        String domainRoot = match != null ? match.root : null;
+        if (domainRoot != null && punyCoded) {
+            domainRoot = IDN.toASCII(domainRoot);
+        }
+        return domainRoot;
     }
 
     static final class DomainRootInfo {
@@ -156,11 +165,11 @@ static final class DomainRootInfo {
     }
 
     DomainRootInfo resolveDomainRoot(final String domain, final DomainType expectedType) {
-        String segment = DnsUtils.normalize(domain);
+        String segment = domain;
         String result = null;
         while (segment != null) {
             // An exception rule takes priority over any other matching rule.
-            final String key = IDN.toUnicode(segment);
+            final String key = segment;
             final DomainType exceptionRule = findEntry(exceptions, key);
             if (match(exceptionRule, expectedType)) {
                 return new DomainRootInfo(segment, key, exceptionRule);
@@ -174,7 +183,7 @@ DomainRootInfo resolveDomainRoot(final String domain, final DomainType expectedT
             final String nextSegment = nextdot != -1 ? segment.substring(nextdot + 1) : null;
 
             // look for wildcard entries
-            final String wildcardKey = (nextSegment == null) ? "*" : "*." + IDN.toUnicode(nextSegment);
+            final String wildcardKey = (nextSegment == null) ? "*" : "*." + nextSegment;
             final DomainType wildcardDomainRule = findEntry(rules, wildcardKey);
             if (match(wildcardDomainRule, expectedType)) {
                 return new DomainRootInfo(result, wildcardKey, wildcardDomainRule);
@@ -239,19 +248,11 @@ public boolean verify(final String domain) {
          if (domain == null) {
              return false;
          }
-         return verifyStrict(domain.startsWith(".") ? domain.substring(1) : domain);
+         return verifyInternal(domain.startsWith(".") ? domain.substring(1) : domain);
      }
 
-    /**
-     * Verifies if the given domain does not represent a public domain root and is
-     * allowed to set cookies, have an identity represented by a certificate, etc.
-     */
     @Internal
-    public boolean verifyStrict(final String domain) {
-        Args.notNull(domain, "Domain");
-        if (domain.startsWith(".")) {
-            return false;
-        }
+    public boolean verifyInternal(final String domain) {
         final DomainRootInfo domainRootInfo = resolveDomainRoot(domain, null);
         if (domainRootInfo == null) {
             return false;
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
@@ -27,7 +27,6 @@
 
 package org.apache.hc.client5.http.ssl;
 
-import java.net.IDN;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.security.cert.Certificate;
@@ -159,11 +158,11 @@ static void matchIPv6Address(final String host, final List<SubjectName> subjectA
 
     static void matchDNSName(final String host, final List<SubjectName> subjectAlts,
                              final PublicSuffixMatcher publicSuffixMatcher) throws SSLPeerUnverifiedException {
-        final String normalizedHost = DnsUtils.normalize(host);
+        final String normalizedHost = DnsUtils.normalizeUnicode(host);
         for (final SubjectName subjectAlt : subjectAlts) {
             if (subjectAlt.getType() == SubjectName.DNS) {
-                final String normalizedSubjectAlt = DnsUtils.normalize(subjectAlt.getValue());
-                if (matchIdentityStrict(normalizedHost, normalizedSubjectAlt, publicSuffixMatcher)) {
+                final String normalizedSubjectAlt = DnsUtils.normalizeUnicode(subjectAlt.getValue());
+                if (matchIdentity(normalizedHost, normalizedSubjectAlt, publicSuffixMatcher, true)) {
                     return;
                 }
             }
@@ -180,9 +179,9 @@ static void matchCN(final String host, final X509Certificate cert,
             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)) {
+        final String normalizedHost = DnsUtils.normalizeUnicode(host);
+        final String normalizedCn = DnsUtils.normalizeUnicode(cn);
+        if (!matchIdentity(normalizedHost, normalizedCn, publicSuffixMatcher, true)) {
             throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match " +
                     "common name of the certificate subject: " + cn);
         }
@@ -224,21 +223,11 @@ static boolean matchDomainRoot(final String host, final String domainRoot) {
         return false;
     }
 
-    private static boolean matchIdentity(final String host, final String identity,
+    static boolean matchIdentity(final String host, final String identity,
                                          final PublicSuffixMatcher publicSuffixMatcher,
                                          final boolean strict) {
-
-        final String normalizedIdentity;
-        try {
-            // Convert only the identity to its Unicode form
-            normalizedIdentity = IDN.toUnicode(identity);
-        } catch (final IllegalArgumentException e) {
-            return false;
-        }
-
-        // Public suffix check on the Unicode identity
         if (publicSuffixMatcher != null && host.contains(".")) {
-            if (!publicSuffixMatcher.verifyStrict(normalizedIdentity)) {
+            if (!publicSuffixMatcher.verifyInternal(identity)) {
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("Public Suffix List verification failed for identity '{}'", identity);
                 }
@@ -251,10 +240,10 @@ private static boolean matchIdentity(final String host, final String identity,
         // character * which is considered to match any single domain name
         // component or component fragment..."
         // Based on this statement presuming only singular wildcard is legal
-        final int asteriskIdx = normalizedIdentity.indexOf('*');
+        final int asteriskIdx = identity.indexOf('*');
         if (asteriskIdx != -1) {
-            final String prefix = normalizedIdentity.substring(0, asteriskIdx);
-            final String suffix = normalizedIdentity.substring(asteriskIdx + 1);
+            final String prefix = identity.substring(0, asteriskIdx);
+            final String suffix = identity.substring(asteriskIdx + 1);
 
             if (!prefix.isEmpty() && !host.startsWith(prefix)) {
                 return false;
@@ -274,25 +263,7 @@ private static boolean matchIdentity(final String host, final String identity,
         }
 
         // Direct Unicode comparison
-        return host.equalsIgnoreCase(normalizedIdentity);
-    }
-
-    static boolean matchIdentity(final String host, final String identity,
-                                 final PublicSuffixMatcher publicSuffixMatcher) {
-        return matchIdentity(host, identity, publicSuffixMatcher, false);
-    }
-
-    static boolean matchIdentity(final String host, final String identity) {
-        return matchIdentity(host, identity, null, false);
-    }
-
-    static boolean matchIdentityStrict(final String host, final String identity,
-                                       final PublicSuffixMatcher publicSuffixMatcher) {
-        return matchIdentity(host, identity, publicSuffixMatcher, true);
-    }
-
-    static boolean matchIdentityStrict(final String host, final String identity) {
-        return matchIdentity(host, identity, null, true);
+        return host.equalsIgnoreCase(identity);
     }
 
     static String extractCN(final String subjectPrincipal) throws SSLException {
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/utils/DnsUtils.java b/httpclient5/src/main/java/org/apache/hc/client5/http/utils/DnsUtils.java
@@ -27,6 +27,10 @@
 
 package org.apache.hc.client5.http.utils;
 
+import java.net.IDN;
+
+import org.apache.hc.core5.annotation.Internal;
+
 /**
  * A collection of utilities relating to Domain Name System.
  *
@@ -72,4 +76,21 @@ public static String normalize(final String s) {
         return s;
     }
 
+    /**
+     * Decodes to Unicode and normalizes the given DNS name.
+     */
+    @Internal
+    public static String normalizeUnicode(final String s) {
+        if (s == null) {
+            return null;
+        }
+        String decoded;
+        try {
+            decoded = IDN.toUnicode(s);
+        } catch (final IllegalArgumentException ignore) {
+            decoded = s;
+        }
+        return normalize(decoded);
+    }
+
 }
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
