Comments below ...
On 4/2/18 4:07 AM, Sibabrata Sahoo wrote:
Hi Sean,
My comments In-lined..
Thanks,
Siba
-----Original Message-----
From: Sean Mullan
Sent: Saturday, March 31, 2018 12:13 AM
To: Sibabrata Sahoo <sibabrata.sa...@oracle.com>; Adam Petcher
<adam.petc...@oracle.com>; 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.
Ok.
* 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.
Ok.
* 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.
Ok.
I looked at the other tests and have no further comments.
Thanks,
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
