isAssignableFrom checks in KeyFactorySpi.engineGetKeySpec appear to be backwards

on behalf of Greg Rubin (rubin@amazon.com)

Hi,

I would like to report a minor bug in Java's KeyFactorySpi.engineGetKeySpec implementations. This bug appears to impact all KeyFactories from all providers bundled with the OpenJDK. Specifically, all of the "isAssignableFrom" checks in "engineGetKeySpec" appear to be backwards. The intent of these checks is to ensure that the KeySpec implementation understood by the KeyFactorySpi implements or extends the requested KeySpec and thus can be safely cast to it. Instead, it incorrectly checks to see if the requested KeySpec implements or extends the concrete implementation. The vast majority of the time the requested KeySpec is equal to the concrete implementation so the inversion does not matter. (This is why I believe that this bug dates back to at least JDK 1.6).

There are two users/developer facing implications of these bugs.

* If a caller requests a subclass of the known KeySpec implementation, then the OpenJDK engineGetKeySpec implementations will incorrectly throw a "ClassCastException" rather than an "InvalidKeySpec".
* If a caller requests a RSAPrivateKeySpec for an RSAPrivateCrtKey, then sun.security.rsa.RSAKeyFactory.engineGetKeySpec will return an instance of RSAPrivateKeySpec rather than opportunistically returning the subclass RSAPrivateCrtKeySpec (as appears to be the intention). This means that the CRT parameters will be lost and any keys created from the returned KeySpec will lack the CRT parameters and be less efficient.

Both of these behaviors can be seen for the RSAKeyFactory in the related "BadSpecChecks.java" file.

JDK-tip links:
* https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/com/sun/crypto/provider/DESedeKeyFactory.java#L105
* https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/com/sun/crypto/provider/PBEKeyFactory.java#L246
* https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2Core.java#L94
* https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11KeyFactory.java#L76-L77
* https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11SecretKeyFactory.java#L344-L356
* https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11DHKeyFactory.java#L217-L244
* https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11ECKeyFactory.java#L287-L312
* https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11DSAKeyFactory.java#L213-L242
* https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11RSAKeyFactory.java#L260-L315
* https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2HmacSHA1Factory.java#L94
* https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.ec/share/classes/sun/security/ec/ECKeyFactory.java#L259-L274
* https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/com/sun/crypto/provider/DHKeyFactory.java#L148-L174
* https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/provider/DSAKeyFactory.java#L151-L185
* https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/com/sun/crypto/provider/DESKeyFactory.java#L109
* https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/rsa/RSAKeyFactory.java#L393-L455

Please let me know if you have any additional questions about this issue.

Thank you,
Greg Rubin
Sr. Security Engineer
AWS Cryptography