RFR: 8200219: Develop new tests for using new elliptic curves: curve25519 and curve448

[Sean Mullan]

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.sahoo at oracle.com>; Adam Petcher <adam.petcher at oracle.com>; security-dev at 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
>>