(2nd round) Proposed API Changes for JEP 114: TLS Server Name Indication (SNI) Extension

[Xuelei Fan]

On 8/15/2012 2:10 PM, Bradford Wetmore wrote:
> 
> 
> On 8/14/2012 7:45 PM, Xuelei Fan wrote:
> 
>>>>> 197:  You're not planning to process (e.g.
>>>>> ServerHandshaker/ClientHandshaker.process_message) the consumed
>>>>> handshaking bytes immediately during the createSocket call, are
>>>>> you? You
>>>>> still need to allow the user time to set the socket
>>>>> options/SSLParameters/etc.  I was expecting in this method you'd just
>>>>> suck in the consumed bytes into temporary storage, then create/return
>>>>> the socket, and then when the handshaking is started, you then read
>>>>> out
>>>>> from the temporary storage until you run out, then you switch to the
>>>>> Socket's InputStream.
>>>>>
>>>> You're right. It is allowed to set more options in the returned socket
>>>> before kick off handshake.
>>>>
>>>>> 197:  This needs some wordsmithing here.  This method will produce the
>>>>> SSLSocket that will be consuming this data, so of course it has to be
>>>>> called first.
>>>>>
>>>> I'm not sure I understand your point. Please comment it again with the
>>>> revised APIs if you still have concerns.
>>>
>>> I just didn't understand much of this paragraph.
>>>
>>> 1. You have to call this method, then set up your parameters, then start
>>> your handshaking.  So the first half of this sentence doesn't apply.
>>>
>> Oh, I know your concerns. What I want to express is that before the
>> calling to method, the caller should not do real handshaking. The logic
>> I concerned looks like:
>>      // 1. accept a socket
>>      // 2. read ClientHello and reply ServerHello to output stream.
>>      // 3. call this method
>>      SSLSocket sslSocket = (SSLSocket)sslSocketFactory.createSocket(
>>                      socke, inputStream, true);
>>
>> because the handshaking has started in step 2, then in step 3, we cannot
>> get a proper SSLSocket.
> 
> How would the ServerHello be generated until you call this method and
> then start the handshaking on the returned SSLSocket?
One can use SSLEngine.wrap()/unwrap() to generate handshaking messages
from socket I/O stream.
    // accept a socket
    // read/write the I/O with SSLEngine
    // call this method

Xuelei

> You said in a
> previous internal conversation that SSLExplore would not generate any
> ServerHello, so this can't be what you mean.  Are you talking about
> layering another SSLSocket over a already layered SSLSocket?
> 
>>> 2. "consumed network data is resumable" wasn't clear either.  To me this
>>> should mean that you can obtain the data which has already been read
>>> from "s".
>>>
>> Yes, need wordsmithing here.
>>
>>> 3. "Otherwise, the behavior of the return socket is not defined" lost
>>> me.  Does this mean that that SSLParameters and assorted settings are
>>> not otherwise defined?
>>>
>> See above example.
> 
> I think I need the above answered before I can comment further here.
> 
>>> I think you could delete this paragraph.
>>>
>>>  From your second email:
>>>
>>>> Thought more about the design, I would have to say that we cannot
>>>> return
>>>> the default value in sslParameters.getServerNames().  Otherwise, the
>>>> following two block of codes look very weird to me:
>>>>       // case one:
>>>> 1   SSLparameters sslParameters = sslSocket.getSSLParameters();
>>>> 2   sslParameters.clearServerName("host_name");
>>>> 3   Map<String, String> names = sslParameters.getServerNames();
>>>> 4   sslSocket.setSSLParameters(sslParameters);
>>>> 5   sslParameters = sslSocket.getSSLParameters();
>>>> 6   names = sslParameters.getServerNames();
>>>>
>>>> In line 3, the returned map does not contain "host_name" entry. But in
>>>> line 6, it may be expected that no "host_name" in the returned map. But
>>>> if we want to return default values, line 6 do need to return a map
>>>> containing "host_name".  The behavior is pretty confusing. We may want
>>>> to try avoid the confusion.
>>>
>>> I'm not following your confusion, it seemed pretty straightforward to
>>> me, it works much like CipherSuites.  We have a set of ciphersuites
>>> which are enabled by default.  We can turn some off by using
>>> SSLParameters.  Expanding a bit on your example here, I'll describe what
>>> I think would happen internally/externally:
>>>
>>> 1    SSLSocket sslSocket = mySSLSocketFactory.createSocket(
>>>          "www.example.com", 443);
>>>
>>> mySSLSocketFactory sets any initial parameters as usual.  SSLSocketImpl
>>> knows it's connecting to www.example.com and automatically stores
>>> "host_name" -> "www.example.com" in its local host data (map or separate
>>> variables).
>>>
>>> 2   SSLparameters sslParameters = sslSocket.getSSLParameters();
>>>
>>> SSLSocketImpl.getSSLParameters() creates a SSLParameters, and sets the
>>> hostmap to the one value "host_name" -> "www.example.com"
>>>
>>> If the application want to get the "default values", they just pull them
>>> out of the SSLParameters here
>>>
>>> 3   sslParameters.clearServerName("host_name");
>>>
>>> Or sslParameters.setServerName("host_name", null)?
>>>
>>> User just decided to clear it.  Ok, that's what we do.  It becomes an
>>> empty map in SSLParameters.
>>>
>>> 4   Map<String, String> names = sslParameters.getServerNames();
>>>
>>> Returns empty Map.
>>>
>> As far as good.
>>
>>> 5   sslSocket.setSSLParameters(sslParameters);
>>>
>>> SSLSocketImpl.setSSLParameters is empty, so SSLSocketImpl takes this
>>> SSLParameters and as a result, clears it's internal "host_name" map to
>>> null, and thus won't send anything out since it's empty.
>>>
>> We have problems here.  We need to support that if an application does
>> not specified host_name value, we should use default values.
>>    I.   SSLParameters sslParameters = new SSLParameters();
>>    II.  sslParameters.setCipherSuites(...);
>>    III. SSLSocket sslSocket =
>>            sslSocketFactory.createSocket("www.example.com", 443)
>>    IV.  sslSocket.setSSLParameters(sslParameters);
>>
>> Before line IV and after line II, the sslParameters.getServerNames() are
>> empty. In line IV, we need to make sure the internal "host_name",
>> "www.example.com" is used as default value, and send it to server in
>> SNI.  That's the default behaviors in JDK 7.  We cannot break it without
>> strong desires.
>>
>> I think it means that we cannot clear the internal "host_name" when the
>> sslParameters.getServerNames() return empty.
>>
>> Does it make sense to you?
> 
> Ok, that's an issue, I was assuming people would generally get a
> SSLParameters from the SSLSocket/SSLEngine, which would have
> prepopulated values such as CipherSuites/Protocols and SNI values. Now I
> hear what you're saying.
> 
> So in SSLSocket/SSLEngine, we have a very similar situation with the
> CipherSuites/Protocols.  The way we did it there was if
> SSLParameters.getCipherSuites() is null (that is, has not been set in
> this instance), we let the default values stand (that is we did not call
> SSLSocket.setEnabledCipherSuites()).  You could do something like that
> with the SNI info.  I've always felt the setServerName/getServerName
> should take/return the same Map<String,String> value, but you felt
> strongly about a setServerName(String, String) so I didn't push it.  If
> we didn't set this value, then the original value could stand.
> 
> Maybe your current design addresses this in a better way, I'll have to
> look at this in the morning, I still have a couple hours of home stuff
> to do before I can go to bed.
> 
> Brad
> 
>> Thanks,
>> Xuelei
>>
>>> 6   sslParameters = sslSocket.getSSLParameters();
>>>
>>> SSLSocketImpl.getSSLParameters() creates a SSLParameters, which sees
>>> that there's no name indication, so it creates an empty name map and
>>> stores in SSLParameters.
>>>
>>> 7   names = sslParameters.getServerNames();
>>>
>>> returns empty.
>>>
>>> It's no longer the default value, because they have specifically set the
>>> value.
>>>
>>> HTH,
>>>
>>> Brad
>>
>