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

[Bradford Wetmore]

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