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

[Xuelei Fan]

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