<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hello all, updated webrev:</p>
<ul>
<li>Copyright and comment fixes</li>
<li>Leaving NoDesRC4CiphSuite.java in othervm mode per Xuelei's
concerns</li>
<li>Changed output to use System.err so it outputs on the same
stream as SSLLogger.<br>
</li>
</ul>
<p><a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejnimeh/reviews/8208350/webrev.02">http://cr.openjdk.java.net/~jnimeh/reviews/8208350/webrev.02</a></p>
<p>Thanks,</p>
--Jamil<br>
<br>
<div class="moz-cite-prefix">On 08/20/2018 09:01 AM, Xue-Lei Fan
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:c124a54b-193b-14d1-7b36-f919492843d0@oracle.com">NoDesRC4CiphSuite.java
<br>
----------------------
<br>
Please move line 30-31 out of the test comment block. The two
lines will be parsed as part of the run parameters.
<br>
<br>
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.
<br>
<br>
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.
<br>
<br>
Thanks,
<br>
Xuelei
<br>
<br>
<br>
On 8/20/2018 7:33 AM, Jamil Nimeh wrote:
<br>
<blockquote type="cite">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.
<br>
<br>
Thanks,
<br>
--Jamil
<br>
<br>
On 8/20/2018 7:19 AM, Sean Mullan wrote:
<br>
<blockquote type="cite">Looks good, just a few minor comments:
<br>
<br>
CustomizedCipherSuites.java
<br>
<br>
- should have both years (2016, 2018) on copyright
<br>
<br>
NoDesRC4CiphSuite.java
<br>
<br>
- 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.
<br>
<br>
- add comments describing what the testEngAddDisabled method
does (similar to the testEngOnlyDisabled method)
<br>
<br>
--Sean
<br>
<br>
On 8/19/18 9:06 PM, Jamil Nimeh wrote:
<br>
<blockquote type="cite">Hello all,
<br>
<br>
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.
<br>
<br>
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.
<br>
<br>
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).
<br>
<br>
Webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jnimeh/reviews/8208350/webrev.01/">http://cr.openjdk.java.net/~jnimeh/reviews/8208350/webrev.01/</a>
<br>
JBS: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8208350">https://bugs.openjdk.java.net/browse/JDK-8208350</a>
<br>
CSR: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8209318">https://bugs.openjdk.java.net/browse/JDK-8209318</a>
<br>
<br>
Thanks,
<br>
--Jamil
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</body>
</html>