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

[Sean Mullan]

I have finished my review. I have no further comments.

Thanks,
Sean

On 3/12/20 11:10 AM, Sean Mullan wrote:
> 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