RFR: 8369950: TLS connection to IPv6 address fails with BCJSSE due to IllegalArgumentException [v2]

[Volkan Yazici]

On Tue, 2 Dec 2025 14:25:36 GMT, Sergey Chernyshev <schernyshev at openjdk.org> wrote:

>> Hi all,
>> 
>> Let me propose a fix and a test case for JDK-8369950.
>> 
>> The failure reproduces with BCJSSE provider and all implementations of SSLSocket other than SSLSocketImpl.
>> 
>> In the test case an anonymous wrapper is used, over the standard SSLSocketImpl, to simulate an external JSSE provider. The test case shows the same behavior as in BCJSSE (failure due to non-LDH ASCII characters in the SNI host name).
>> 
>> The fix avoids constructing SNIHostName when the URL host name is an IPv4 or IPv6 literal address. Other than that, all other FQDN host names that have invalid characters (non-LDH ASCII characters) still produce that exception.
>> 
>> SNIHostName, as defined in
>> https://github.com/openjdk/jdk/blob/873f8a696fa45c7d94a164be20cf3c797ce7f2a6/src/java.base/share/classes/javax/net/ssl/SNIHostName.java#L44-L66
>> 
>> has the fully qualified DNS hostname of the server. As follows from the section 3, "Server Name Indication", RFC 6066, `Literal IPv4 and IPv6 addresses are not permitted in "HostName"`.
>> 
>> The fix mirrors the behavior of SSLSocketImpl, that avoids constructing the SNIHostName from literal addresses. Please see
>> 
>> https://github.com/openjdk/jdk/blob/873f8a696fa45c7d94a164be20cf3c797ce7f2a6/src/java.base/share/classes/sun/security/ssl/Utilities.java#L110-L116
>> 
>> Testing:
>> - standard jtreg tests goups showed no regressions
>> - the new test passes with the fix and fails otherwise
>> - passes also with BCJSSE in FIPS and standard mode 
>> 
>> <details><summary> BCJSSE standard</summary>
>> 
>> 
>> STDOUT:
>> STDERR:
>> Dez. 01, 2025 2:44:02 PM org.bouncycastle.jsse.provider.PropertyUtils getBooleanSecurityProperty
>> INFORMATION: Found boolean security property [keystore.type.compat]: true
>> Dez. 01, 2025 2:44:02 PM org.bouncycastle.jsse.provider.PropertyUtils getStringSecurityProperty
>> INFORMATION: Found string security property [jdk.tls.disabledAlgorithms]: SSLv3, TLSv1, TLSv1.1, DTLSv1.0, RC4, DES, MD5withRSA, DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL, ECDH, TLS_RSA_*, rsa_pkcs1_sha1 usage HandshakeSignature, ecdsa_sha1 usage HandshakeSignature, dsa_sha1 usage HandshakeSignature
>> Dez. 01, 2025 2:44:02 PM org.bouncycastle.jsse.provider.DisabledAlgorithmConstraints create
>> WARNUNG: Ignoring unsupported entry in 'jdk.tls.disabledAlgorithms': rsa_pkcs1_sha1 usage HandshakeSignature
>> Dez. 01, 2025 2:44:02 PM org.bouncycastle.jsse.provider.DisabledAlgorithmConstraints create
>> WARNUNG: Ignoring unsupported entry in 'jdk.tl...
>
> Sergey Chernyshev has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - addressed more review comments
>  - addressed review comments

src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java line 1:

> 1: /*

Copyright year needs a bump.

src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java line 476:

> 474:                     // host has been set previously for SSLSocketImpl
> 475:                     if (!(s instanceof SSLSocketImpl) &&
> 476:                             !IPAddressUtil.isIPv4LiteralAddress(host) &&

This introduces a new behavior for both IPv4 and IPv6, yet we only verify the IPv6 behavior. Shouldn't we also verify that when the provided host is an IPv4 literal, `setServerNames()` is left untouched?

src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java line 478:

> 476:                             !IPAddressUtil.isIPv4LiteralAddress(host) &&
> 477:                             !(host.charAt(0) == '[' && host.charAt(host.length() - 1) == ']' &&
> 478:                                 IPAddressUtil.isIPv6LiteralAddress(host.substring(1, host.length() - 1))

There is one more place in `sun.net.www`, which is in this very class, that


(host.charAt(0) == '[' && host.charAt(host.length() - 1) == ']') { return host.substring(1, host.length() - 1)


logic is practiced. Would it make sense to refactor this into a `private static Optional<String> ipv6FromHost(String host)` method, preferably with some short explanation in the method's Javadoc on why we do this?

test/jdk/javax/net/ssl/HttpsURLConnection/SubjectAltNameIPv6.java line 1:

> 1: /*

FWIW, _without_ the proposed `HttpsClient` changes, I can reproduce


java.lang.IllegalArgumentException: Contains non-LDH ASCII characters
	at java.base/java.net.IDN.toASCIIInternal(IDN.java:310)
	at java.base/java.net.IDN.toASCII(IDN.java:139)
	at java.base/javax.net.ssl.SNIHostName.<init>(SNIHostName.java:114)
	at java.base/sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:475)


using this test against `master` (i.e., 7278d2e8e58). Put another way, AFAICT, fix and test is legit.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580621834
PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580726453
PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580631079
PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580718016