Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ok2c merged 1 commit into from
Apr 5, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions httpclient5/src/main/java/org/apache/hc/client5/http/psl/PublicSuffixMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

import org.apache.hc.client5.http.utils.DnsUtils;
import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.annotation.ThreadingBehavior;
import org.apache.hc.core5.util.Args;

Expand Down Expand Up @@ -137,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 {
Expand All @@ -155,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);
Expand All @@ -173,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);
Expand Down Expand Up @@ -239,20 +249,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.
*
* @since 5.5
*/
public boolean verifyStrict(final String domain) {
Args.notNull(domain, "Domain");
if (domain.startsWith(".")) {
return false;
}
@Internal
public boolean verifyInternal(final String domain) {
final DomainRootInfo domainRootInfo = resolveDomainRoot(domain, null);
if (domainRootInfo == null) {
return false;
Expand Down
53 changes: 12 additions & 41 deletions httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultHostnameVerifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand All @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions httpclient5/src/main/java/org/apache/hc/client5/http/utils/DnsUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@

package org.apache.hc.client5.http.utils;

import java.net.IDN;

/**
* A collection of utilities relating to Domain Name System.
*
Expand Down Expand Up @@ -72,4 +74,21 @@ public static String normalize(final String s) {
return s;
}

/**
* Decodes to Unicode and normalizes the given DNS name.
* @since 5.5
*/
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);
}

}
Loading