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