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

[Weijun Wang]

While SignatureSpi says engineGetParameters() would throw an UOE when it's not overridden, it is not specified in Signature::getParameters:

     * <p> If this signature has been previously initialized with parameters
     * (by calling the {@code setParameter} method), this method returns
     * the same parameters. If this signature has not been initialized with
     * parameters, this method may return a combination of default and
     * randomly generated parameter values if the underlying
     * signature implementation supports it and can successfully generate
     * them. Otherwise, {@code null} is returned.

I'm afraid we'll have to return something here, maybe a mixture of a curve name and fields inside EdDSAParameterSpec.

--Max

> On Mar 31, 2020, at 12:25 PM, Anthony Scarpino <anthony.scarpino at oracle.com> wrote:
> 
> On 3/30/20 8:52 PM, Anthony Scarpino wrote:
>> 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
> 
> Ok.. That's frustrating that engineSetParameters takes AlgorithmParameterSpec, but engineGetParameters returns AlgorithmParameter.  That confused me.
> 
> So I would say the reason EdDSASignature.engineGetParameters() is UOE is because the parameters are not exposed publicly.  The NamedParameterSpec tells the internals of EdDSA what parameters to use.  I know this to be a choice by Adam as he found it unnecessary to expose APIs that were unnecessary at this time with predefined EdDSA curves and with a implementation that did not allow arbitrary curves.
> 
> 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