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

[Weijun Wang]

On 08/15/2012 10:45 AM, Xuelei Fan wrote:
> I only reply on the items that I may need more review.
>
> On 8/15/2012 7:54 AM, Brad Wetmore wrote:
>>>> SSLParameters.java
>>>> ==================
>>>> 76:  Not sure why you want/need a LinkedHashMap with only one currently
>>>> defined NameType.  Even if there were multiple types, I don't think that
>>>> SNI requires an ordering.  You also mention this in
>>>> setAccessibleServerName, but not sure what purpose this serves.  I'm not
>>>> strongly against it, just wondering.
>>>>
>>> I am also not sure about the strong desire that the SNI should be
>>> ordered. But Weijun prefers it to be ordered because he think the SNI in
>>> RFC6066 is defined as an ordered sequence.
>>>
>>>         struct {
>>>             ServerName server_name_list<1..2^16-1>
>>>         } ServerNameList;
>>
>> I've gone through RFC6066 pretty carefully, and I'm not seeing any
>> indication that this should be ordered.
>>
>> In RFC 2246, if there is an ordering required, such as
>> cipher_suites/compression/certs/cert_requests, it's specifically called
>> out.  For any other "lists", it is not specified.
>>
>> Weijun, did you see something else in your read of the spec that
>> indicates an ordering?  If not, maybe we should not put in the order
>> wording now.  If it turns out we do need it, we can always add that
>> wording later in a later release, but it will be impossible to remove it
>> if we add it now.

No, I didn't read anything else. However, I cannot think of a reason why 
we will need to remove it later.

>>
> I think you are right. If Weijun has no other concerns, I will remove
> related description.
>
>>> 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?

This is how I understand this problem: There are two ways you get a 
SSLParameters object with an empty server name map.

1. SSLparameters sslParameters = sslSocket.getSSLParameters();
    sslParameters.clearServerName("host_name");

2. SSLParameters sslParameters = new SSLParameters();

Now these 2 objects are identical. If you call 
sslSocket.setSSLParameters(sslParameters) now, what should it do? By 
looking at 1, it seems you cleared something; but by 2, you touched 
nothing and it should go default.


Thanks
Max

>
> Thanks,
> Xuelei