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