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.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
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.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