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

[Anthony Scarpino]

On 3/30/20 7:54 PM, Weijun Wang wrote:
> 
> 
>> On Mar 31, 2020, at 10:50 AM, Anthony Scarpino <anthony.scarpino at oracle.com> wrote:
>>
>> On 3/30/20 11:52 AM, Anthony Scarpino wrote:
>>> On 3/30/20 6:21 AM, Weijun Wang wrote:
>>>> I was playing with keytool with your patch and noticed
>>>> sun.security.util.KeyUtil.getKeySize(Key) does not support an
>>>> EdECKey. While we use curve name instead of key size in EC to
>>>> describe the parameters, the size is still useful in determining the
>>>> strength.
>>> I think I should be able to access the parameter with the key's NamedParameterSpec to return the size.
>>
>> I was wrong about this.  The parameters are part of jdk.crypto.ec while KeyUtil is part of java.base, so I cannot access the internal EdDSAParameters class.
>>
>> The easiest way would be to look at the NamedParameterSpec and return a hardcoded length based on the curve used. It's not ideal, but all these curves don't change lengths unlike for RSA, AES, etc.
> 
> Yes.
> 
>>
>> Tony
>>
>>>>
>>>> There is also a KeyUtil.getKeySize(AlgorithmParameters) nearby. I'm
>>>> not involved with previous discussions on EdDSA design, but why does
>>>> EdDSASignature.engineGetParameters() throw an UOE?
>>> Because the public API for engineGetParameter(String param) is deprecated would be my suspicion.
> 
> engineGetParameter() is deprecated, engineGetParameters() is not.

Oh sorry.  I'm not sure why, but I have to ask the question what is the 
point of this method?  If the user needs to set the parameters to do the 
a signature of verify, what is the need for a method that gets the 
parameter from Signature that should have just set?  Are the parameters 
returned changed from the initial setting?  In the EdDSA case they are not.

I don't have an immediate problem in having EdDSA return the same 
parameters back, I'm just not sure why it's necessary and I haven't see 
anything in the JEP as to why Adam decided against this.

Tony

> 
>>>> Another small problem:
>>>>
>>>> You reverted the copyright year from 2020 to an earlier year in
>>>> module-info.java, keytool/Main.java.
>>> The copyright has not been reverted, the jdk repo was updated to 2020 from another changeset.
> 
> Ah yes, I applied your patch to my old repo.
> 
> --Max
> 
>>>>
>>>> Thanks, Max
>>>>
>>>>> On Mar 24, 2020, at 2:53 AM, Anthony Scarpino
>>>>> <anthony.scarpino at oracle.com> wrote:
>>>>>
>>>>> On 2/25/20 12: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
>>>>>
>>>>>
>>>>> I updated the webrev with some minor updates that were commented
>>>>> previously.
>>>>>
>>>>> https://cr.openjdk.java.net/~ascarpino/8166597/webrev.01/
>>>>>
>>>>> Tony
>>>>
>>
>