On Mon, 5 Jun 2023 18:22:13 GMT, Matthew Donovan <mdono...@openjdk.org> wrote:
> This PR implements a test to verify that Java will not negotiate TLS
> connections when one side of the connection requests disabled cipher suites.
test/jdk/javax/net/ssl/TLS/TLSWontNegotiateDisabledCipherAlgos.java line 35:
> 33: * @test id=Server
> 34: * @bug 8301379
> 35: * @summary Verify that Java will not negotiate disabled cipher suites
> when the
This test could be enhanced to test all of the other suites that are disabled.
I think that might be a good idea, to centralize that. Although you could do it
as a follow-on RFE.
test/jdk/javax/net/ssl/TLS/TLSWontNegotiateDisabledCipherAlgos.java line 39:
> 37: *
> 38: * @library /javax/net/ssl/templates
> 39: * @run main/othervm TLSWontNegotiateDisabledCipherAlgos server true
> TLS_ECDH_ECDSA_WITH_AES_256_GCM_SHA384
Consider collapsing all of these run lines into one run line which sets all of
the disabled cipher suites together in one handshake. I think you still are
testing that none of the suites are negotiated.
test/jdk/javax/net/ssl/TLS/TLSWontNegotiateDisabledCipherAlgos.java line 83:
> 81:
> 82: if (args[0].equals("server")) {
> 83: try(TLSServer server = new TLSServer(useDisabledAlgo,
> args[2])) {
Nit, space after `try` (also on line 98 and 152).
test/jdk/javax/net/ssl/TLS/TLSWontNegotiateDisabledCipherAlgos.java line 119:
> 117: public void run() throws IOException {
> 118: try {
> 119: socket.getOutputStream().write("SECRET
> MESSAGE".getBytes(StandardCharsets.UTF_8));
Should this fail if this handshake passes?
test/jdk/javax/net/ssl/TLS/TLSWontNegotiateDisabledCipherAlgos.java line 121:
> 119: socket.getOutputStream().write("SECRET
> MESSAGE".getBytes(StandardCharsets.UTF_8));
> 120: } catch (SSLHandshakeException exc) {
> 121: // handshake failures are expected
Can you inspect the handshake failure message to make sure it is failing for
the right reason as you do for the server case?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14316#discussion_r1224569812
PR Review Comment: https://git.openjdk.org/jdk/pull/14316#discussion_r1224568398
PR Review Comment: https://git.openjdk.org/jdk/pull/14316#discussion_r1224563096
PR Review Comment: https://git.openjdk.org/jdk/pull/14316#discussion_r1224566883
PR Review Comment: https://git.openjdk.org/jdk/pull/14316#discussion_r1224564674
