Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8254717

isAssignableFrom checks in KeyFactorySpi.engineGetKeySpec appear to be backwards

    XMLWordPrintable

    Details

      Backports

        Description

        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

          Attachments

            Issue Links

              Activity

                People

                Assignee:
                luoziyi Ziyi Luo (Inactive)
                Reporter:
                luoziyi Ziyi Luo (Inactive)
                Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                  Dates

                  Created:
                  Updated:
                  Resolved: