Code Review Request 8144566, Custom HostnameVerifier disables SNI extension

[Xuelei Fan]

Ping ...

On 12/8/2015 8:12 PM, Xuelei Fan wrote:
> Good catch!
> 
> I copied the comment here:
> 
>   ----------
>     SocketFactory sslsf = SSLSocketFactory.getDefault();
>     SSLSocket ssls = (SSLSocket) sslsf.createSocket();
>     ssls.connect(new InetSocketAddress(
>             "bugs.openjdk.java.net", 443), 0);
>     ssls.startHandshake();
> 
>     No SNI is sent in this case.
>   ----------
> 
> Although this is not a regression, but this is a very good catch.
> 
> However, I don't think the code path is a best practice, as the actual
> behavior depends on providers behaviors and platform behaviors too much.
>  I would recommend to use SSLParameters.setServerNames() to specify the
> server names explicitly.
> 
> But, best effort should be made for the default server names for such
> code paths.  Here is the webrev that also fixes this code path issue:
> 
>     http://cr.openjdk.java.net/~xuelei/8144566/webrev.01/
> 
> The fix gets a little bit complicated because of the need to consider
> the SSLParameters.setServerNames() impact in the code path:
> 
>    SSLSocket ssls = (SSLSocket) sslsf.createSocket();
> 
>    // before the connection, need to consider the impact:
>    // Get the SSLParameters from the socket, or create on the fly?
>    // Call ssls.setSSLParameters(params) or not?
>    ssls.connect(socketAddress);
> 
>    // after the connection, need to consider the impact:
>    // Call ssls.setSSLParameters(params) or not?
> 
> Basically, the implementation honors the server name set by
> SSLParameters.setServerNames().
> 
> Bernd's comment:
>> Isnt this codepath used as a workaround for dynamically disabling
>> SNI? Using connect(host..) instead of SSLSocket(host) was at
>> least a common, well known workaround.
> If the code path is really used in practice, the
> SSLParameters.setServerNames() would better be called actually to
> disable SNI explicitly.
> 
>     SocketFactory sslsf = SSLSocketFactory.getDefault();
>     SSLSocket ssls = (SSLSocket) sslsf.createSocket();
>     ssls.connect(new InetSocketAddress(
>         "bugs.openjdk.java.net", 443), 0);
>     sslParameters.setServerNames(emptyList);
>     ssls.setSSLParameters(sslParameters);
>     ssls.startHandshake();
> 
> Thank you, Bernd and Brad, for the feedback.
> 
> Thanks,
> Xuelei
> 
> On 12/8/2015 8:21 AM, Bradford Wetmore wrote:
>>
>> Please see my comment in the bug.  I haven't verified this, but it seems
>> the problem might be generic to the codepath through SSLSocket, not just
>> Https.
>>
>> Brad
>>
>>
>>
>>
>>
>> On 12/6/2015 4:32 AM, Xuelei Fan wrote:
>>> Hi,
>>>
>>> Please review the update for JDK-8144566:
>>>
>>>     http://cr.openjdk.java.net/~xuelei/8144566/webrev.00/
>>>
>>> For HttpsURLConnection, the server name may be set after the TLS
>>> connection and handshake has been initialized.  As may result in that
>>> the server name does not present at TLS ClientHello messages.
>>>
>>> This fix resets the server name for the initialized handshake for above
>>> cases.
>>>
>>> Thanks,
>>> Xuelei
>>>
>