[JDK 8] Code review request 7188657, There should be a way to reorder the JSSE ciphers
Xuelei Fan
xuelei.fan at oracle.com
Fri Sep 6 01:45:12 UTC 2013
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
>>>
>
More information about the security-dev
mailing list