[11] RFR: JDK-8205111: Develop new Test to verify different key types for supported TLS protocols.

[sha.jiang at oracle.com]

Hi Siba,
  165         System.setProperty("jdk.tls13.version", "7F1C");
This property may be unnecessary.
JSSE server and client just use the same TLS 1.3 version number, 
regardless of what the number is.

  322         KeyStore ts = KeyStore.getInstance("JKS");
  323         KeyStore ks = KeyStore.getInstance("JKS");
Would it be better to use PKCS12?

  424             // EC private key related to cert endEntityCertStrs[0].
  451             // EC private key related to cert endEntityCertStrs[0].
  489             // RSA private key related to cert endEntityCertStrs[0].
  581             // RSA private key related to cert endEntityCertStrs[0].
  ...
What's the mean of these comments?

  220         void doServerSide() throws Exception
  288         void doClientSide() throws Exception
Could you apply try-with-resources to sslServerSocket and sslSocket in 
the above two methods?

  157     private volatile static boolean clientRenegoReady = false;
  ...
  246                     while (!clientRenegoReady) {
  247                         System.out.println("Waiting for ClientHello");
  248                         Thread.sleep(50);
  249                     }
Why does clientRenegoReady exist?
I don't see the value of clientRenegoReady is changed.
Because clientRenegoReady always is false, so "while 
(!clientRenegoReady)" should be as the same as "while(true)".
And, what's the purpose of this while block?
I assume you want to make sure that server doesn't exit before the 
second handshaking finishes.
If true, in practice, do you get the case: Before client side socket I/O 
closes, server already exits?

In addition, why not building case combinations in the codes?
It would not be easy to maintain so many hard-coded combinations in @run 
lines.
And generally, this style may increase gross test execution time.

Best regards,
John Jiang

On 21/06/2018 14:58, Sibabrata Sahoo wrote:
>
> Hi Xuelei,
>
> Please review the patch for,
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8205111
>
> Webrev: http://cr.openjdk.java.net/~ssahoo/8205111/webrev.00/ 
> <http://cr.openjdk.java.net/%7Essahoo/8205111/webrev.00/>
>
> Change:
>
> This Test file verifies all TLS protocols with the supported keytypes.
>
> Thanks,
>
> Siba
>

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