Fallback to checking algorithm name if key not instanceof RSAKey or EC.

[netmackan]

Thank you all for a great library!

For this pull request I am suggesting a small improvement which would take away the need to override the RsaSigner or EllipticCurveSigner to support PrivateKeys not being instances of RSAKey or ECKey but still being able to check the key type as today but to also use the method recommended in the PKCS#11 guide. The change is basically to fallback to check PrivateKey.getAlgorithm().

Background:

PrivateKey objects protected by hardware such as smart card or HSM and using the SunPKCS11 provider do not implement RSAKey and ECKey as they are unextractable. This makes it cumbersome with a lot of overriding to do in order to use JJWT. This has previously been discussed in #273 and #160 and possibly some other all though from a different angle.

Explanation in the Java PKCS#11 guide is here:

Key objects representing unextractable token keys should only implement the relevant generic interfaces in the java.security and javax.crypto packages (PrivateKey, PublicKey, or SecretKey). Identification of the algorithm of a key should be performed using the Key.getAlgorithm() method. PKCS#11 Reference Guide section Token Keys

For the cases where those interfaces are used to check key parameters it is not so easy to see how this could be improved without API changes. Probably for those cases the API would have to take a KeyPair and not just the PrivateKey as the parameters could then be checked on the PublicKey object instead of the PrivateKey one. This is out of scope for this PR.

[bdemers]

+1, I took a quick shot at testing this out. After a bit of fumbling, I was able to load a private key from a Yubikey's PIV interface. I wasn't actually able to sign anything with said key (I blame my own ignorance, cobbling together various guides). But the resulting key pulled from the device (via a Keystore) was a sun.security.pkcs11.P11Key$P11PrivateKey (and getAlgorithm() returned RSA).

@netmackan as for the follow-up comment about the keypair, can you create a separate issue for that? @lhazlewood has been working on supporting JWE's and has been making changes in that area, (if interested check out the jwe branch)

[netmackan]

I submitted #700 for the possible further improvement but that would require API change. Can we have this one merged in the meantime?

[bdemers]

@netmackan that sounds like a plan!

As for the other API changes Any chance you have a minimal example that would hit all the touchpoints you mentioned in #700?

[lhazlewood]

@netmackan Closing this just as a matter of housekeeping since #700 was resolved via the JWE branch work that has been merged to master. If you feel that something hasn't been addressed, please do let us know and we can discuss before the upcoming release. Thank you so much for the PR and the discussion!