[JDK 8] Code review request 7188657, There should be a way to reorder the JSSE ciphers

[Xuelei Fan]

If no objections, I will push the changeset.

Xuelei

On 9/4/2013 11:34 AM, Xuelei Fan wrote:
> new webrev: http://cr.openjdk.java.net/~xuelei/7188657/webrev.01/
> 
> On 9/4/2013 10:35 AM, Weijun Wang wrote:
>> Mostly good, only some word/style issues.
>>
>> SSLParameters.java:
>>
>>   83      * server name matchers are set to <code>null</code>, cipher
>> suites
>>   84      * preference, wantClientAuth and needClientAuth are set to
>>   85      * <code>false</code>.
>>
>> Why not just use "preferLocalCipherSuites" instead of "cipher suites
>> preference"? Yes it looks ugly to refer to a variable name, but you've
>> already used "wantClientAuth". Or, at least use "useCipherSuitesOrder"
>> because that's used in the public method names.
>>
> Good catch. I will use "useCipherSuitesOrder".
> 
>> Handshaker.java:
>>
>>  148     // Whether local cipher suites preference in server side should be
>>  149     // honored during handshaking?
>>  150     boolean preferLocalCipherSuites = false;
>>
>> Since you apply the flag to both server and client, how about adding
>> something like "(it's always honored in client side)".
>>
> Better to have such words.
> 
> // Whether local cipher suites preference should be honored during
> // handshaking?
> //
> // Note that in this provider, this option only applies to server side.
> // Local cipher suites preference is always honored in client side in
> // this provider.
> 
> 
>>  550     boolean isNegotiable(CipherSuite s) {
>>
>> You might need to update the doc for this method saying "within the
>> current active cipher suites". You can even let it call the new
>> isNegotiable(*,*) method.
>>
> Good suggestion.
> 
>> UseCipherSuitesOrder.java:
>>
>>   2  * Copyright (c) 2001, 2011, Oracle and/or its affiliates. All
>> rights reserved.
>>
>> Should be "2001, 2013".
>>
> Should be 2013.
> 
>>  157     // client enabled cipher suites
>>  158     private static String[] CliEnabledCipherSuites;
>>  159
>>  160     // server enabled cipher suites
>>  161     private static String[] SrvEnabledCipherSuites;
>>
>> It looks weird for a variable to starts with a capital letter.
>>
> It looks weird to me, too.  Not sure what I was thinking at that time.
> 
> Thanks,
> Xuelei
> 
>> Thanks
>> Max
>>
>> On 8/28/13 5:02 PM, Xuelei Fan wrote:
>>> Hi,
>>>
>>> Please review this update to support cipher suites reorder:
>>>
>>> webrev: http://cr.openjdk.java.net/~xuelei/7188657/webrev.00/
>>>
>>> Two new methods are added to SSLParameters:
>>>     public final void setUseCipherSuitesOrder(boolean honorOrder);
>>>     public final boolean getUseCipherSuitesOrder();
>>>
>>> If SSLParameters.getUseCipherSuitesOrder() return true, the local cipher
>>> suites order returned in SSLParameters.getCipherSuites() should be
>>> honored during SSL/TLS handshaking.
>>>
>>> Considering the potential compatibility issues of third party's
>>> implementation, I won't define the behaviors if
>>> SSLParameters.getUseCipherSuitesOrder() return false.  For Oracle
>>> provider, SunJSSE, if getUseCipherSuitesOrder() returns false, the order
>>> of SSLParameters.getCipherSuites() is honored in client side, and the
>>> order of the requested cipher suites in client handshake message is
>>> honored in server side.
>>>
>>> Thanks,
>>> Xuelei
>>>
>