On 3/30/20 7:54 PM, Weijun Wang wrote:


On Mar 31, 2020, at 10:50 AM, Anthony Scarpino <anthony.scarp...@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.scarp...@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




Reply via email to