On Wed, 13 Jan 2021 00:54:16 GMT, Valerie Peng <valer...@openjdk.org> 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
