On Mon, 1 Dec 2025 14:12:04 GMT, Sergey Chernyshev <[email protected]> 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.tls.disabledAlgorithms': > ecdsa_sha1 usage HandshakeSignature > Dez. 01, 2025 2... test/jdk/javax/net/ssl/HttpsURLConnection/SubjectAltNameIPv6.java line 28: > 26: * @bug 8369950 > 27: * @library /test/lib > 28: * @summary TLS connection to IPv6 address fails with BCJSSE due to > IllegalArgumentException Nit - as per the jtreg tag order recommendation, the `@summary` should come before the `@library` https://openjdk.org/jtreg/tag-spec.html#ORDER test/jdk/javax/net/ssl/HttpsURLConnection/SubjectAltNameIPv6.java line 71: > 69: * currently 3 minutes by default, but you might try to be > 70: * smart about it.... > 71: */ I am not too familiar with these style of tests, but it looks like there are several similar ones in the `javax/net/ssl` area. Would you know if these comments are still accurate and up to date? In other words, do we need these comments in this test? test/jdk/javax/net/ssl/HttpsURLConnection/SubjectAltNameIPv6.java line 97: > 95: bw.write("HTTP/1.1 200 OK\r\n\r\n\r\n"); > 96: bw.flush(); > 97: Thread.sleep(5000); Is the sleep() necessary for this test? test/jdk/javax/net/ssl/HttpsURLConnection/SubjectAltNameIPv6.java line 165: > 163: */ > 164: SubjectAltNameIPv6() throws Exception { > 165: if (separateServerThread) { Are these if/else blocks needed or could we simplify the test to just expect the server and client to run as separate threads? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580413566 PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580386092 PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580387059 PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580393762
