RFR: 8263404: RsaPrivateKeySpec is always recognized as RSAPrivateCrtKeySpec in RSAKeyFactory.engineGetKeySpec [v2]

[Valerie Peng]

On Fri, 12 Mar 2021 22:00:18 GMT, Ziyi Luo <luoziyi at openjdk.org> wrote:

>> This is a P2 regression introduced by JDK-8254717.
>> 
>> In `RSAKeyFactory.engineGetKeySpec`, when the key is a RSA key and the KeySpec is RSAPrivateKeySpec or RSAPrivateCrtKeySpec. The method behavior is described as follow:
>> 
>> X-axis: type of `keySpec`
>> Y-axis: type of `key`
>> 
>> Before JDK-8254717:
>> 
>> |  | RSAPrivateKeySpec.class | RSAPrivateCrtKeySpec.class |
>> |--|--|--|
>> | RSAPrivateKey | Return RSAPrivateKeySpec  | Throw `InvalidKeySpecException` |
>> | RSAPrivateCrtKey | Return RSAPrivateKeySpec | Return RSAPrivateKeyCrtSpec |
>> 
>> After JDK-8254717 (Green check is what we want to fix, red cross is the regression):
>> 
>> |  | RSAPrivateKeySpec.class | RSAPrivateCrtKeySpec.class |
>> |--|--|--|
>> | RSAPrivateKey | Throw `InvalidKeySpecException` ❌  | Throw `InvalidKeySpecException` |
>> | RSAPrivateCrtKey | Return RSAPrivateKeyCrtSpec ✅ | Return RSAPrivateKeyCrtSpec |
>> 
>> This commit fixes the regression.
>> 
>> 
>> ### Tests
>> 
>> * Jtreg: All tests under `java/security`, `sun/security`, `javax/crypto` passed
>> * JCK: All JCK-16 (I do not have jCK-17)tests under `api/java_security` passed
>
> Ziyi Luo has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add one test case for the regression fixed by 8263404

test/jdk/java/security/KeyFactory/KeyFactoryGetKeySpecForInvalidSpec.java line 80:

> 78:         // === Case 1: private key is RSAPrivateCrtKey, expected spec is RSAPrivateKeySpec
> 79:         // === Expected: return RSAPrivateCrtKeySpec
> 80:         // Since RSAPrivateCrtKeySpec inherits from RSAPrivateKeySpec, we'd expect this next line to return an instance of RSAPrivateKeySpec

Typo? I think you mean RSAPrivateCrtKeySpec?

test/jdk/java/security/KeyFactory/KeyFactoryGetKeySpecForInvalidSpec.java line 83:

> 81:         // (because the private key has CRT parts).
> 82:         KeySpec spec = factory.getKeySpec(pair.getPrivate(), RSAPrivateKeySpec.class);
> 83:         if (!(spec instanceof RSAPrivateCrtKeySpec)) {

The generated key is implementation specific, you should check the key type before checking the returned key spec?

test/jdk/java/security/KeyFactory/KeyFactoryGetKeySpecForInvalidSpec.java line 99:

> 97:         // InvalidKeySpecException should not be thrown
> 98:         KeySpec notCrtSpec = factory.getKeySpec(notCrtKey, RSAPrivateKeySpec.class);
> 99:         if (notCrtSpec instanceof RSAPrivateCrtKeySpec) {

Just to be safe, check the returned keyspec is RSAPrivateKeySpec?

-------------

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