RFR: 8346705: SNI not sent with Java 22+ using java.net.http.HttpClient.Builder#sslParameters

[Nicolas Henneaux]

On Mon, 6 Jan 2025 12:56:42 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this change which fixes a regression that was introduced in Java 22 release in the implementation of `java.net.http.HttpClient`? This addresses https://bugs.openjdk.org/browse/JDK-8346705.
>> 
>> A client intiating a TLS handshake (through the `ClientHello`) can send a "Server Name Indication" to tell the server, the name of the server it is communicating with (https://www.rfc-editor.org/rfc/rfc6066#section-3). When a `java.net.http.HttpClient` is establishing a (TLS) connection to serve a HTTP request, it determines the SNI server name to send in the handshake.
>> 
>> In Java 22, through https://bugs.openjdk.org/browse/JDK-8309910, we did an (internal implementation detail) change to allow for the internal implementation classes to keep track of what SNI server name was used for the connection, so that that detail can be used for other purposes after the connection has been established. That change wasn't meant to affect the logic that determined the SNI server name to be sent across in the TLS handshake. However, due to an oversight in the refactoring, a regression got introduced which caused the HttpClient to no longer send a SNI server name for a certain scenario (described below).
>> 
>> The implementation of the HttpClient first looks at the request URI to determine the SNI. If the request URI host is a IP address literal, then the HttpClient implementation looks at the `HttpClient.sslParameters()` to see if the application has configured any ServerName(s) through `SSLParameters.setServerNames(...)`. If any are set, then the HttpClient uses those ServerName(s) and sends them in the SNI, else it doesn't set the SNI. On the other hand, if the URI host is not a IP address literal, then the HttpClient implementation doesn't use any application configured ServerName(s) and instead uses the request URI host as the SNI name and sends it in the TLS handshake.
>> 
>> The oversight in JDK-8309910 caused a regression where if the request URI host is a IP address literal, then we no longer looked at the application configured ServerName(s), thus causing the HttpClient to no longer set the SNI server name in the TLS handshake. This can cause (and indeed does cause) the TLS handshakes to fail if the server rejects the handshake due to the missing SNI.
>> 
>> The commit in this PR addresses this regression by making sure that the HttpClient continues to behave the way it used to before JDK-8309910. A new jtreg test case has been introduced to v...
>
> Hello Nicolas @nhenneaux, would you be able to give this change a try against your tests that exposed this regression?

Hi @jaikiran 
I'm not sure how I could validate those? Should I build fully the openjdk?

Is it expected the configured sni is ignored when detected in the hostname? I suspect other use cases when configured sni can be useful on top of the given hostname.

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

PR Comment: https://git.openjdk.org/jdk/pull/22927#issuecomment-2573075786