Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-18 Thread Michael StJohns

On 1/17/2021 9:29 PM, Valerie Peng wrote:

On Fri, 15 Jan 2021 01:45:07 GMT, Valerie Peng  wrote:


Marked as reviewed by weijun (Reviewer).

_Mailing list message from [Michael StJohns](mailto:mstjo...@comcast.net) on 
[security-dev](mailto:security-dev@openjdk.java.net):_

Sorry - I'm coming to this a bit late.

Any chance of adding the logic for generatePublic() from a PKCS8 RSA
private key??? RFC3477 has the PKCS1 RSAPrivateKey ASN1 which includes
the modulus and publicExponent - so it should be a pretty straight
forward add to generate a public key.

PKCS11 2.40 started requiring that the publicExponent be stored with the
private key to allow for the public key to be regenerated from a private
key object.?? Going forward,? it might be a good idea to modify the
RSAPrivate(Crt)KeyImpl class to store the publicExponent if provided.

Mike

You are correct that for RSA private CRT keys the necessary values are there 
for figuring out its corresponding public keys.

This change is about adding support for PKCS#1 encoded RSA keys and be able to 
translate them into PKCS#8 encoded keys and/or extract various key specs out of 
them. If you already have PKCS#8 RSAPrivateCrtKey obj from SunRsaSign provider, 
you can call its getPublicExponent() method and use that to create a 
RSAPublicKeySpec and generate RSA public key with it. If you are using 3rd 
party impl which does not return the public exponent value somehow, then you 
can translate it using the RSA key factory impl from SunRsaSign provider and 
then repeat the fore-mentioned step. Will this address your need? If not, could 
you elaborate the usage that you have in mind? Not sure if you are suggesting a 
new KeyFactory.generatePublic() method which take a PrivateKey or else.

Mike,
We can continue your feedback with a separate RFE since this RFE is just about 
adding support for PKCS#1 encoding.
I need to wrap this up before my upcoming trip this Wed, hope that's ok with 
you.

Thanks! Valerie

-

PR: https://git.openjdk.java.net/jdk/pull/1787


No worries - I got busy with other things for a few days.

To answer your other question, I've had a few cases where the public key 
for a private key has been misplaced and where we needed it back.  
Ideally,  it should be possible to take a PrivateKey object (in whatever 
form including just PrivateKey) and run it through the KeyFactory to 
extract a PublicKey.  I've used the technique you suggest above before 
(and the related multiply the private scalar against G for EC keys) to 
retrieve the data needed to build a public key, but each of these is a 
bit ad hoc. I'd really prefer to have a standard pattern for all key types.


About 4 years or so ago (e.g. when 2.40 was released), the PKCS11 group 
started requiring that the private keys include the attributes necessary 
to retrieve the public key - for RSA it was the public exponent, and for 
EC it was the public point (which could be stored or regenerated upon 
demand).  It may be time to think about doing something similar here and 
going forward for any given asymmetric key type.


That's a more general RFE than just updating the current implementing 
classes, but as far as I can tell, doesn't change any of the APIs, but 
may change the factory class implementation guidance.


An interesting addition would be to have the Impl classes implement both 
the appropriate public key and private key interfaces.





Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-17 Thread Valerie Peng
On Fri, 15 Jan 2021 01:45:07 GMT, Valerie Peng  wrote:

>> Marked as reviewed by weijun (Reviewer).
>
>> 
>> 
>> _Mailing list message from [Michael StJohns](mailto:mstjo...@comcast.net) on 
>> [security-dev](mailto:security-dev@openjdk.java.net):_
>> 
>> Sorry - I'm coming to this a bit late.
>> 
>> Any chance of adding the logic for generatePublic() from a PKCS8 RSA
>> private key??? RFC3477 has the PKCS1 RSAPrivateKey ASN1 which includes
>> the modulus and publicExponent - so it should be a pretty straight
>> forward add to generate a public key.
>> 
>> PKCS11 2.40 started requiring that the publicExponent be stored with the
>> private key to allow for the public key to be regenerated from a private
>> key object.?? Going forward,? it might be a good idea to modify the
>> RSAPrivate(Crt)KeyImpl class to store the publicExponent if provided.
>> 
>> Mike
> 
> You are correct that for RSA private CRT keys the necessary values are there 
> for figuring out its corresponding public keys.
> 
> This change is about adding support for PKCS#1 encoded RSA keys and be able 
> to translate them into PKCS#8 encoded keys and/or extract various key specs 
> out of them. If you already have PKCS#8 RSAPrivateCrtKey obj from SunRsaSign 
> provider, you can call its getPublicExponent() method and use that to create 
> a RSAPublicKeySpec and generate RSA public key with it. If you are using 3rd 
> party impl which does not return the public exponent value somehow, then you 
> can translate it using the RSA key factory impl from SunRsaSign provider and 
> then repeat the fore-mentioned step. Will this address your need? If not, 
> could you elaborate the usage that you have in mind? Not sure if you are 
> suggesting a new KeyFactory.generatePublic() method which take a PrivateKey 
> or else.

Mike,
We can continue your feedback with a separate RFE since this RFE is just about 
adding support for PKCS#1 encoding.
I need to wrap this up before my upcoming trip this Wed, hope that's ok with 
you.

Thanks! Valerie

-

PR: https://git.openjdk.java.net/jdk/pull/1787


Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-15 Thread Valerie Peng
On Thu, 14 Jan 2021 20:04:33 GMT, Valerie Peng  wrote:

>> src/java.base/share/classes/sun/security/rsa/RSAKeyFactory.java line 344:
>> 
>>> 342: if (keySpec instanceof PKCS8EncodedKeySpec) {
>>> 343: return RSAPrivateCrtKeyImpl.newKey(type, "PKCS#8",
>>> 344: ((PKCS8EncodedKeySpec)keySpec).getEncoded());
>> 
>> Will you clean up the `getEncoded()` output or shall I?
>
> Maybe it's better that you do it this time? Just so that the backport won't 
> miss it.

Or if you integrated before me, I will manually merge the changes and clean up 
the getEncoded() also.

-

PR: https://git.openjdk.java.net/jdk/pull/1787


Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-14 Thread Valerie Peng
On Wed, 13 Jan 2021 17:07:20 GMT, Weijun Wang  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update copyright year from 2020 to 2021.
>
> Marked as reviewed by weijun (Reviewer).

> 
> 
> _Mailing list message from [Michael StJohns](mailto:mstjo...@comcast.net) on 
> [security-dev](mailto:security-dev@openjdk.java.net):_
> 
> Sorry - I'm coming to this a bit late.
> 
> Any chance of adding the logic for generatePublic() from a PKCS8 RSA
> private key??? RFC3477 has the PKCS1 RSAPrivateKey ASN1 which includes
> the modulus and publicExponent - so it should be a pretty straight
> forward add to generate a public key.
> 
> PKCS11 2.40 started requiring that the publicExponent be stored with the
> private key to allow for the public key to be regenerated from a private
> key object.?? Going forward,? it might be a good idea to modify the
> RSAPrivate(Crt)KeyImpl class to store the publicExponent if provided.
> 
> Mike

You are correct that for RSA private CRT keys the necessary values are there 
for figuring out its corresponding public keys.

This change is about adding support for PKCS#1 encoded RSA keys and be able to 
translate them into PKCS#8 encoded keys and/or extract various key specs out of 
them. If you already have PKCS#8 RSAPrivateCrtKey obj from SunRsaSign provider, 
you can call its getPublicExponent() method and use that to create a 
RSAPublicKeySpec and generate RSA public key with it. If you are using 3rd 
party impl which does not return the public exponent value somehow, then you 
can translate it using the RSA key factory impl from SunRsaSign provider and 
then repeat the fore-mentioned step. Will this address your need? If not, could 
you elaborate the usage that you have in mind? Not sure if you are suggesting a 
new KeyFactory.generatePublic() method which take a PrivateKey or else.

-

PR: https://git.openjdk.java.net/jdk/pull/1787


Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-14 Thread Michael StJohns

Sorry - I'm coming to this a bit late.

Any chance of adding the logic for generatePublic() from a PKCS8 RSA 
private key?   RFC3477 has the PKCS1 RSAPrivateKey ASN1 which includes 
the modulus and publicExponent - so it should be a pretty straight 
forward add to generate a public key.


PKCS11 2.40 started requiring that the publicExponent be stored with the 
private key to allow for the public key to be regenerated from a private 
key object.   Going forward,  it might be a good idea to modify the 
RSAPrivate(Crt)KeyImpl class to store the publicExponent if provided.



Mike


On 1/14/2021 4:06 PM, Valerie Peng wrote:

On Wed, 13 Jan 2021 17:00:36 GMT, Weijun Wang  wrote:


Valerie Peng has updated the pull request incrementally with one additional 
commit since the last revision:

   Update copyright year from 2020 to 2021.

test/jdk/sun/security/rsa/TestKeyFactory.java line 159:


157: throw new Exception("Encodings not equal");
158: }
159: }

Can we combine the 2 blocks above into one? That is to say, when key1 and key2 
have the same format, we check the equality of both getEncoded() and 
themselves. Same for the P11 test.

Sure, will do.

-

PR: https://git.openjdk.java.net/jdk/pull/1787





Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-14 Thread Valerie Peng
On Wed, 13 Jan 2021 17:00:36 GMT, Weijun Wang  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update copyright year from 2020 to 2021.
>
> test/jdk/sun/security/rsa/TestKeyFactory.java line 159:
> 
>> 157: throw new Exception("Encodings not equal");
>> 158: }
>> 159: }
> 
> Can we combine the 2 blocks above into one? That is to say, when key1 and 
> key2 have the same format, we check the equality of both getEncoded() and 
> themselves. Same for the P11 test.

Sure, will do.

-

PR: https://git.openjdk.java.net/jdk/pull/1787


Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-14 Thread Valerie Peng
On Wed, 13 Jan 2021 17:02:50 GMT, Weijun Wang  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update copyright year from 2020 to 2021.
>
> test/jdk/sun/security/rsa/TestKeyFactory.java line 89:
> 
>> 87: static {
>> 88: byte[] encodedPriv = Base64.getDecoder().decode(PKCS1_PRIV_STR);
>> 89: CUSTOM_PRIV = new PrivateKey() {
> 
> How about just P1_PRIV?

Either one is fine. I can change to P1_PRIV.

-

PR: https://git.openjdk.java.net/jdk/pull/1787


Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-14 Thread Valerie Peng
On Wed, 13 Jan 2021 16:23:17 GMT, Weijun Wang  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update copyright year from 2020 to 2021.
>
> src/java.base/share/classes/sun/security/rsa/RSAKeyFactory.java line 344:
> 
>> 342: if (keySpec instanceof PKCS8EncodedKeySpec) {
>> 343: return RSAPrivateCrtKeyImpl.newKey(type, "PKCS#8",
>> 344: ((PKCS8EncodedKeySpec)keySpec).getEncoded());
> 
> Will you clean up the `getEncoded()` output or shall I?

Maybe it's better that you do it this time? Just so that the backport won't 
miss it.

> src/java.base/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java line 
> 101:
> 
>> 99: return new RSAPrivateKeyImpl(key.type, key.keyParams,
>> 100: key.getModulus(), key.getPrivateExponent());
>> 101: } else return key;
> 
> Create a one-line block for else inside "{" and "}".

Sure.

-

PR: https://git.openjdk.java.net/jdk/pull/1787


Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-14 Thread Valerie Peng
On Wed, 13 Jan 2021 16:11:02 GMT, Weijun Wang  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update copyright year from 2020 to 2021.
>
> src/java.base/share/classes/sun/security/rsa/RSAKeyFactory.java line 47:
> 
>> 45:  * For public keys:
>> 46:  *  . PublicKey with an X.509 encoding
>> 47:  *  . RSA PublicKey with an PKCS#1 encoding
> 
> The line above has not mentioned "RSA". Maybe better to be consistent. Same 
> as for private key.

Sure~

-

PR: https://git.openjdk.java.net/jdk/pull/1787


Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-13 Thread Weijun Wang
On Wed, 13 Jan 2021 00:54:16 GMT, Valerie Peng  wrote:

>> Can someone help review this?
>> 
>> This change enhances RSA KeyFactory impl of SunRsaSign and SunPKCS11 
>> providers to accept RSA keys in PKCS#1 format and encoding and translate 
>> them to provider-specific RSA keys. Updated the relevant tests with a sample 
>> PKCS#1 encoded key pair.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update copyright year from 2020 to 2021.

Marked as reviewed by weijun (Reviewer).

src/java.base/share/classes/sun/security/rsa/RSAKeyFactory.java line 47:

> 45:  * For public keys:
> 46:  *  . PublicKey with an X.509 encoding
> 47:  *  . RSA PublicKey with an PKCS#1 encoding

The line above has not mentioned "RSA". Maybe better to be consistent. Same as 
for private key.

src/java.base/share/classes/sun/security/rsa/RSAKeyFactory.java line 344:

> 342: if (keySpec instanceof PKCS8EncodedKeySpec) {
> 343: return RSAPrivateCrtKeyImpl.newKey(type, "PKCS#8",
> 344: ((PKCS8EncodedKeySpec)keySpec).getEncoded());

Will you clean up the `getEncoded()` output or shall I?

src/java.base/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java line 101:

> 99: return new RSAPrivateKeyImpl(key.type, key.keyParams,
> 100: key.getModulus(), key.getPrivateExponent());
> 101: } else return key;

Create a one-line block for else inside "{" and "}".

test/jdk/sun/security/rsa/TestKeyFactory.java line 159:

> 157: throw new Exception("Encodings not equal");
> 158: }
> 159: }

Can we combine the 2 blocks above into one? That is to say, when key1 and key2 
have the same format, we check the equality of both getEncoded() and 
themselves. Same for the P11 test.

test/jdk/sun/security/rsa/TestKeyFactory.java line 89:

> 87: static {
> 88: byte[] encodedPriv = Base64.getDecoder().decode(PKCS1_PRIV_STR);
> 89: CUSTOM_PRIV = new PrivateKey() {

How about just P1_PRIV?

-

PR: https://git.openjdk.java.net/jdk/pull/1787


Re: RFR: 8023980: JCE doesn't provide any class to handle RSA private key in PKCS#1 [v3]

2021-01-12 Thread Valerie Peng
> Can someone help review this?
> 
> This change enhances RSA KeyFactory impl of SunRsaSign and SunPKCS11 
> providers to accept RSA keys in PKCS#1 format and encoding and translate them 
> to provider-specific RSA keys. Updated the relevant tests with a sample 
> PKCS#1 encoded key pair.
> 
> Thanks,
> Valerie

Valerie Peng has updated the pull request incrementally with one additional 
commit since the last revision:

  Update copyright year from 2020 to 2021.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1787/files
  - new: https://git.openjdk.java.net/jdk/pull/1787/files/a4cddf47..b48afddd

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1787=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1787=01-02

  Stats: 6 lines in 6 files changed: 0 ins; 0 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1787.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1787/head:pull/1787

PR: https://git.openjdk.java.net/jdk/pull/1787