Code Review Request 8144566, Custom HostnameVerifier disables SNI extension

[Xuelei Fan]

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
>>