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

[Brad Wetmore]

On 8/15/2012 3:26 AM, Xuelei Fan wrote:
> More comments about whether we are able to override the default value.
>
> On 8/15/2012 10:45 AM, Xuelei Fan wrote:
>>>>>> 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.
> Compatibility is my concerns here.
>
> When the SSLParameters class was was introduced in JDK 6,
> set/getCipherSuites() and set/getProtocols were new, so there was no
> compatibility issue for these two pairs methods at that time, as they
> were not used in old applications.
>
> In JDK 7, we introduced two new methods, set/getAlgorithmConstraints().
> We were luck that we cannot allow the override of default constraints
> because of security consideration,

I didn't quite follow this sentence.  We do allow the override, right? 
Looking at SSLSocketImpl.setSSLParameters():

         algorithmConstraints = params.getAlgorithmConstraints();

> and the concept of algorithm
> constraints was new in JDK 7. So we needed not to consider the
> compatibility issue too much between JDK 6 and 7 for this pair of methods.
>
> However, things get changed for the Server Name Inidication (SNI),
> because in JDK 7 we have already support default SNI extension implicit.
> We have to consider the compatibility issues more between JDK 7 and JDK
> 8, we need to make sure the behaviors are consistent whenever we call
> the set/getServerName(). If we allow override of default value, the
> application may run into corner trap as the description in my previous mail.

Not sure this is a problem, possibly I didn't explain my thought fully.

> That's the background of my thoughts. Hope it helps you understand the
> current design.

Here's what I was trying to propose:

SSLParameters:

     // Local storage like the other variables
     private Map<String, String> sniNames = null;

     ...deleted...

     /**
      * Set the SNI Names.
      *
      * A null parameter means don't set anything.  Nothing is set.
      *
      * disallow invalid String->null entries.
      *
      * valid String->String entries will be added to SNI extension
      * if there is a recognized Key type.
      */
     public void setServerNames(Map<String, String> map)

     /**
      * Returns a copy of the array of servernames or null if
      * none have been set.
      */
     public Map<String, String> getServerNames()

SSLSocketImpl:

     synchronized public void setSSLParameters(SSLParameters params) {
         super.setSSLParameters();
         ...deleted...
         Map<String,String> sniNames = params.getServerNames();
         if (sniNames != null) {
             sniNameStorage = sniNames.clone();
         }

     synchronized public SSLParameters getSSLParameters() {
         SSLParameters params = super.getSSLParameters();
         ...deleted...
         // sniNameStorage currently has one entry:
         // "host_name", "www.example.com"
         params.setServerNames = sniNameStorage.clone();

So looking at the two use cases you pointed out:

1.   SSLParameters sslp = new SSLParameters()

getServernames() would return null.  When SSLSocketImpl.setSSLParameters 
is called, the null indicates there is nothing to set, and the default 
value remains.  If app wants to set something here, it can, which would 
override the existing default.

2.  SSLParameters sslp = SSLSocket.getSSLParameters()

The SSLParameters will be populated with the SNI map value stored in 
SSLSocketImpl/SSLEngineImpl, and apps can do whatever they choose with 
the Map.  When setSSLParameters is called, the potentially new values 
are just pushed back to the SNI.  In this case, the application has full 
access to the default Map and can see what will be sent by default.

In both cases, the default remains unless the application takes steps to 
change it.  For applications, this is the same call model as 
setCipherSuites()/setProtocols(), but the magic happens at different levels.

Am I missing something?

 > because in JDK 7 we have already support default SNI extension
 > implicit.

We are just providing API access to that functionality.

Brad



> Thanks,
> Xuelei
>
>>>>   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?
>>
>> 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
>