RFR: Disable all DES cipher suites

[Jamil Nimeh]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180820/70645c5e/attachment.htm>