Hi Sean,
My comments In-lined..
Thanks,
Siba
-Original Message-
From: Sean Mullan
Sent: Saturday, March 31, 2018 12:13 AM
To: Sibabrata Sahoo ; Adam Petcher
; security-dev@openjdk.java.net
Subject: Re: RFR: 8200219: Develop new tests for using new elliptic curves:
curve25519 and curve448
A few comments so far; have not finished my review yet.
General comment:
Many of these tests test more than XDH. That is fine and good for increasing
coverage, but have you looked through existing tests to see if you are
duplicating anything we are already testing and maybe those tests could be
removed or you could share the same code. One of the things we should be
looking at is to figure out how to reduce the overall time the security tests
take.
There are few Lines of code related to " DiffieHellman " are duplicate in
KeyAgreementTest.java and KeySizeTest.java which are available in 2 existing
Test folders. i.e. "open\test\jdk\sun\security\pkcs11\KeyAgreement" and
"open\test\jdk\com\sun\crypto\provider\KeyAgreement". While there is no
equivalent Tests for the same for "ECDH" and "XDH". The remaining files
available in the webrev are mostly new. Our initial thinking was to have Test
files covering all similar algorithms in one place. In that case I may remove
2-3 existing Test files inside these folders with next patch.
* KeyAgreementTest.java
128 // Uses platform supported provider to test interoperability.
What do you mean by "platform supported provider"? Isn't this based on the
provider search order? So in some cases, you might be testing against the same
provider and not really doing interop testing?
Yes my thinking here is Provider search order. I may need to change the comment
appropriately. Here the intention is not really interop based Test unless a
different provider found other than SunJCE.
* KeySizeTest.java
You are generating some large keys - any issues with test timeouts? Do we need
to test the generation of the keypairs? Could we use cached keypairs instead?
Yes here my intention is to test generation of keypairs with different key
sizes too. I have ran these Test files several times. I have not seen these
test files taking more times than few couple of seconds.
--Sean
On 3/26/18 12:38 PM, Sibabrata Sahoo wrote:
> Hi,
>
> Please review the patch for,
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8184359
>
> Webrev: http://cr.openjdk.java.net/~ssahoo/8184359/webrev.00/
>
> All the Test files uses KeyAgreement, KeyPairGenerator, Several
> KeySpecs from SunJCE library to Test DiffieHellman, ECDH and XDH with
> curve25519 and curve448 algorithms. Each Test files try to address
> several cases and the purpose of each has been commented in their own files.
>
> Thanks,
>
> Siba
>
