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

Mon Apr 9 14:13:45 UTC 2018

Hi Sean/Adam,

Here is the new patch addressing all comments.
Webrev: http://cr.openjdk.java.net/~ssahoo/8184359/webrev.01/

Changes includes:
1)
KeyAgreementTest.java 
KeySizeTest.java
          I have ensured no duplicate Test cases exist for the functionality provided in these 2 files. Due to that I have remove 3 files, DHGenSecretKey.java, SupportedDHKeys.java, UnsupportedDHKeys.java from "open/test/jdk/com/sun/crypto/provider/KeyAgreement" folder. Statements which affected due to merging the missing code from these deleted files are KeyAgreementTest.java[29, 141], KeySizeTest.java[91-100, 112-162]. Also I have observed that KeySizeTest.java Test takes around 2 minute to complete in SOME PLATFORM to complete all 14 @run and approximately 20 seconds for higher keys > 4096 for "DiffieHellman" only after merge. Please suggest if removing the higher Keys from the Test cases will help or 20-30 seconds for these higher keys is accepted here?

2) I have updated the comment in KeyAgreementTest.java[129] to indicate provider search order.

As per Adam's comments the following changes,
3) KeySizeTest.java & NegativeTest.java, which need utility methods from Convert.java are using the library instead of defining it's own.
4) MultiThreadTest.java generates two key pairs and do two key agreement operations.

Thanks,
Siba

-----Original Message-----
From: Sean Mullan 
Sent: Thursday, April 05, 2018 8:46 PM
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

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
>>