Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-06-04 Thread Valerie Peng
Sure, I have no more comments. Thanks, Valerie On 6/3/2020 7:48 PM, Weijun Wang wrote: RSAPrivateKeyImpl and RSAPrivateCrtKeyImpl - throws InvalidKeyException when RSAUtil.createAlgorithmId(type, keyParams) fails. I'll keep it. EdDSAPrivateKeyImpl, XDHPrivateKeyImpl and ECPrivateKeyImpl -

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-06-03 Thread Weijun Wang
RSAPrivateKeyImpl and RSAPrivateCrtKeyImpl - throws InvalidKeyException when RSAUtil.createAlgorithmId(type, keyParams) fails. I'll keep it. EdDSAPrivateKeyImpl, XDHPrivateKeyImpl and ECPrivateKeyImpl - check the input ECParameterSpec params and might throw an InvalidKeyException. I'll keep

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-06-03 Thread Valerie Peng
Hi, Max, Overall looks very good. Just one more thing: Different key classes seems to differ in their handling of IOException in their constructor which produces the DER bytes. DSAPrivateKey changed to use AssertionError, XDHPrivateKeyImpl and EdDSAPrivateKeyImpl throws ProviderException.

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-06-01 Thread Weijun Wang
Updated webrev at https://cr.openjdk.java.net/~weijun/8244565/webrev.03. I've inlined more methods that is only called once and inside the same method. Thanks, Max > On May 28, 2020, at 9:16 AM, Weijun Wang wrote: > > > >> On May 28, 2020, at 8:46 AM, Valerie Peng wrote: >> >> Hi

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-27 Thread Weijun Wang
> On May 28, 2020, at 8:46 AM, Valerie Peng wrote: > > Hi Max, > > I like this new structure better. Much easier to understand. Most of the > changes are technical debt that's accumulated inside PKCS8Key class. > > A few notable differences which I am not so sure about are > > - the

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-27 Thread Valerie Peng
Hi Max, I like this new structure better. Much easier to understand. Most of the changes are technical debt that's accumulated inside PKCS8Key class. A few notable differences which I am not so sure about are - the encodedKey is returned by getEncoded() directly w/o cloning, and - the

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-26 Thread Weijun Wang
Can you please take a look (not a review) at an updated webrev at http://cr.openjdk.java.net/~weijun/8244565/webrev.02/? It contains the code to inspect the extra fields. I haven't deal with the "..." here yet. However, when I tried to consolidate the DER parsing into one place, I've made more

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-26 Thread Valerie Peng
I suppose we can allow trailing data to conform to "..." then. But the "[[2: publicKey [1] PublicKey OPTIONAL ]]," line mean that the publicKey should not be present for V1? This should be checked? Valerie On 5/25/2020 9:02 PM, Weijun Wang wrote: The new definition at

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-25 Thread Weijun Wang
The new definition at https://tools.ietf.org/html/rfc5958 shows, OneAsymmetricKey ::= SEQUENCE { version Version, privateKeyAlgorithm PrivateKeyAlgorithmIdentifier, privateKeyPrivateKey, attributes[0] Attributes

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-25 Thread Weijun Wang
I find the duplication of parsing code annoyed. I'll see if I can consolidate them into one. --Max > On May 20, 2020, at 1:37 PM, Valerie Peng wrote: > > > True, the current handling of the extra bytes in parseKey() and decode() are > kind of opposite, one does not allow any extra bytes but

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-19 Thread Valerie Peng
True, the current handling of the extra bytes in parseKey() and decode() are kind of opposite, one does not allow any extra bytes but the other ignores all. The trailing bytes may look harmless but simply ignores it may open up something unexpected. Given that this is key related class, I

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-18 Thread Weijun Wang
> On May 19, 2020, at 1:41 AM, Valerie Peng wrote: > > Hi Max, > > I saw in the bug description that handling and parsing of the public key will > be resolved in a separate enhancement which is fine with me. > > In addition to relaxing the PKCS8 version check to accept 1, the current >

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-18 Thread Valerie Peng
Hi Max, I saw in the bug description that handling and parsing of the public key will be resolved in a separate enhancement which is fine with me. In addition to relaxing the PKCS8 version check to accept 1, the current webrev does not check for the trailing bytes and its validity. Perhaps

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-13 Thread Valerie Peng
I will take a look. Valerie On 5/8/2020 3:39 AM, Weijun Wang wrote: Found an existing test I can update. Webrev updated at http://cr.openjdk.java.net/~weijun/8244565/webrev.01/ Thanks, Max On May 8, 2020, at 5:41 PM, Weijun Wang wrote: Please take a review at

Re: RFR 8244565: Accept PKCS #8 with version number 1

2020-05-08 Thread Weijun Wang
Found an existing test I can update. Webrev updated at http://cr.openjdk.java.net/~weijun/8244565/webrev.01/ Thanks, Max > On May 8, 2020, at 5:41 PM, Weijun Wang wrote: > > Please take a review at > > http://cr.openjdk.java.net/~weijun/8244565/webrev.00/ > > Now we accepts a PKCS8

RFR 8244565: Accept PKCS #8 with version number 1

2020-05-08 Thread Weijun Wang
Please take a review at http://cr.openjdk.java.net/~weijun/8244565/webrev.00/ Now we accepts a PKCS8 file with version being 0 or 1. The publicKey and attributes are still not parsed. I also take this chance to make some format change. There is no regression test and I'll add one. I can