[RFR] 8166597: Crypto support for the EdDSA Signature Algorithm (JEP 339)

[Sean Mullan]

Here are some more comments, all minor. I will probably need one more 
round of review.

- src/jdk.crypto.ec/share/classes/sun/security/ec/SunEC.java

   362         /* XDH does not require native implementation */

Looks like a cut-paste error. I think you can remove this line.

- src/java.base/share/classes/java/security/interfaces/EdECPrivateKey.java
- src/java.base/share/classes/java/security/interfaces/EdECPublicKey.java
- src/java.base/share/classes/java/security/spec/EdECPoint.java

In the class description of these 3 classes, you are missing a <p> at 
the start of the 2nd paragraph. Otherwise when you generate javadoc, it 
won't create a new paragraph.

- src/java.base/share/classes/java/security/spec/EdDSAParameterSpec.java

   71      * @param context the context that is bound to the signature

This should probably say that the parameter is copied. Also getContext() 
should say that it returns a copy (if not null).

   92      * Get the context that should be used, if any

Missing period at end of sentence.

- src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdDSAPublicKeyImpl.java

   58         // set the high-order bit of the

.. of the ?

   73         // construct the EdECPoint representation
   74

This comment applies to code after line 74, so I think it would make 
more sense if you removed line 74 and added a blank line before 73.

- src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdECOperations.java

It would be useful to have some additional comments in this code.

--Sean

On 2/25/20 3:49 PM, Anthony Scarpino wrote:
> Hi
> 
> I need a code review for the EdDSA support in JEP 339.  The code builds 
> on the existing java implemented constant time classes used for XDH and 
> the NIST curves.  The change also adds classes to the public API to 
> support EdDSA operations.
> 
> All information about the JEP is located at:
> JEP 339: https://bugs.openjdk.java.net/browse/JDK-8199231
> CSR: https://bugs.openjdk.java.net/browse/JDK-8190219
> 
> webrev: https://cr.openjdk.java.net/~ascarpino/8166597/webrev/
> 
> thanks
> 
> Tony