RFR: Disable all DES cipher suites

Xue-Lei Fan xuelei.fan at oracle.com
Mon Aug 20 17:43:50 UTC 2018


Looks fine to me.

Thanks,
Xuelei

On 8/20/2018 10:42 AM, Jamil Nimeh wrote:
> Hello all, updated webrev:
> 
>   * Copyright and comment fixes
>   * Leaving NoDesRC4CiphSuite.java in othervm mode per Xuelei's concerns
>   * Changed output to use System.err so it outputs on the same stream as
>     SSLLogger.
> 
> http://cr.openjdk.java.net/~jnimeh/reviews/8208350/webrev.02
> 
> Thanks,
> 
> --Jamil
> 
> On 08/20/2018 09:01 AM, Xue-Lei Fan wrote:
>> NoDesRC4CiphSuite.java
>> ----------------------
>> Please move line 30-31 out of the test comment block.  The two lines 
>> will be parsed as part of the run parameters.
>>
>> I would prefer to use othervm mode.  Otherwise, once there is a test 
>> case does not run with othervm and changes the context, this test may 
>> be not reliable any more.  If this test failed, it is hard to evaluate 
>> the root cause if it is impacted by other test case.
>>
>> I may prefer to use System.err for new test case as the SunJSSE 
>> default debug log now is dumped to System.err.   Dumps on System.out 
>> and System.err are not synchronized.  Using the same stream may make 
>> the debug log easier to read.
>>
>> Thanks,
>> Xuelei
>>
>>
>> On 8/20/2018 7:33 AM, Jamil Nimeh wrote:
>>> I can fix the copyright, no problem.  Good catch on the othervm - the 
>>> original form of the test did set properties but it seemed better to 
>>> not set them explicitly and just use the new defaults. One would not 
>>> expect to ever remove DES and RC4 from the disabledAlgorithms 
>>> identifier set, at least in our delivered code. It doesn't need to be 
>>> run in othervm mode.  And I can comment those other two tests.
>>>
>>> Thanks,
>>> --Jamil
>>>
>>> On 8/20/2018 7:19 AM, Sean Mullan wrote:
>>>> Looks good, just a few minor comments:
>>>>
>>>> CustomizedCipherSuites.java
>>>>
>>>> - should have both years (2016, 2018) on copyright
>>>>
>>>> NoDesRC4CiphSuite.java
>>>>
>>>> - does this need to be run in othervm mode? It doesn't look like you 
>>>> are setting any properties dynamically. Lines 30-31 should also be 
>>>> removed, if so.
>>>>
>>>> - add comments  describing what the testEngAddDisabled method does 
>>>> (similar to the testEngOnlyDisabled method)
>>>>
>>>> --Sean
>>>>
>>>> On 8/19/18 9:06 PM, Jamil Nimeh wrote:
>>>>> Hello all,
>>>>>
>>>>> This change adds all DES cipher suites to the 
>>>>> jdk.tls.disabledAlgorithms Security property.  This will have the 
>>>>> effect of making all DES-based suites unavailable to SunJSSE 
>>>>> SSLSocket and SSLEngine instances, even if explicitly enabled using 
>>>>> calls like SSLEngine.setEnabledCipherSuites() or 
>>>>> SSLSocket.setEnabledCipherSuites().  Users wishing to re-enable 
>>>>> these suites for legacy purposes must first alter the 
>>>>> jdk.tls.disabledAlgorithms property in the java.security file.
>>>>>
>>>>> Please note that prior to this change, DES-based suites were 
>>>>> available, but not enabled by default on SSLSocket and SSLEngine 
>>>>> objects.  This change just makes these suites no longer available 
>>>>> without further intervention.
>>>>>
>>>>> This change also removes RC4_40 from this Security property as it 
>>>>> is already superseded by the RC4 identifier.  It also cleans up a 
>>>>> cut-and-paste bug in a couple of the RC4_40 export suites (those 
>>>>> suites are disabled already).
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8208350/webrev.01/
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8208350
>>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8209318
>>>>>
>>>>> Thanks,
>>>>> --Jamil
>>>
> 



More information about the security-dev mailing list