Code review request, 7093640, Enable TLS 1.2 for client-side default contexts

[Xuelei Fan]

Thank you for the code review.

On 12/18/2013 12:27 PM, Bradford Wetmore wrote:
> Hi Xuelei,
> 
> I looked as several of the new tests but not all, and looked at the
> existing tests plus new code.
> 
> SSLContextImpl.java
> ===================
> 
> 227:  As I mentioned in Instant Message tonight, I'm not sure this code
> is needed.  I think these values are being set as part of the
> super.engineGetDefaultSSLParameters, so unless your change somehow
> tweaks them, this can probably go.  Thanks for checking this out.
> 
The code is not needed.  Removed.

> 631:  Minor nit, you could tighten up the exception message a little:
> 
> PROPERTY_NAME + ": " + protocols[i] +
>     " is not a standard TLS protocol name";
> 
OK.

> IllegalProtocolProperty.java and others
> ============================
> Minor nit, you won't be able to run this from the command line in case
> someone wants to do so, I generally do a System.getProperty() on the
> first line instead of a @run option.
> 
It is easier to update the parameter, without need to looking the code.
 Let's use this style for now.

Thanks,
Xuelei

> Brad
> 
> 
> 
> On 12/17/2013 2:08 AM, Xuelei Fan wrote:
>> Hi,
>>
>> This is a request to enabled TLS 1.2 for client-side default contexts.
>> Please review this update.
>>
>> webrev: http://cr.openjdk.java.net/~xuelei/7093640/webrev.00/
>>
>> We are still concern about the version intolerance issue with some older
>> SSL/TLS server implementation.  As a workaround, a new system property,
>> "jdk.tls.client.protocols", is defined to configure the protocols in
>> default contexts.
>>
>> By default, TLS 1.1 and TLS 1.2 (plus other supported and safe
>> protocols) are enabled unless the system property is explicit configured
>> and does not contain "TLSv1.1" or "TLSv1.2".
>>
>> The property string is a list of comma separated standard SSL protocol
>> names. The syntax of the property string can be described as this Java
>> BNF-style:
>>       ClientProtocols:
>>              ('"' SSLProtocolNames '"') | SSLProtocolNames
>>       SSLProtocolNames:
>>              SSLProtocolName { , SSLProtocolName }
>>       SSLProtocolName:
>>          (see below)
>>
>> The "SSLProtocolName" is the standard SSL protocol name as described in
>> the "Java Cryptography Architecture Standard Algorithm Name
>> Documentation". If the property value does not comply to the above
>> syntax, or the specified value of SSLProtocolName is not a supported SSL
>> protocol name, the instantiation of the SSLContext provider service (via
>> SSLContext.getInstance() methods) may generate a
>> java.security.NoSuchAlgorithmException. Please note that the protocol
>> name is case-sensitive.
>>
>> If the system property is not set or is empty, the default enabled
>> protocol setting in both client and server looks like:
>>
>> Protocol         Enabled           Enabled
>>                   for Client        for Server
>> --------         ----------        ----------
>> SSLv3            Yes               Yes
>> TLSv1            Yes               Yes
>> TLSv1.1          Yes               Yes
>> TLSv1.2          Yes               Yes
>> SSLv2Hello       No                Yes
>>
>>
>> If the system property is set to "TLSv1,TLSv1.1", the default enabled
>> protocol setting in both client and server looks like:
>>
>> Protocol         Enabled           Enabled
>>                   for Client        for Server
>> --------         ----------        ----------
>> SSLv3            No                Yes
>> TLSv1            Yes               Yes
>> TLSv1.1          Yes               Yes
>> TLSv1.2          No                Yes
>> SSLv2Hello       No                Yes
>>
>> This update does not impact the API specification of JSSE, JSSE server
>> side and third party's provider.
>>
>> Thanks,
>> Xuelei
>>