Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API update in order to support signing through PKCS#11 (and other) #700

Closed
netmackan opened this issue Jan 3, 2022 · 2 comments
Closed
Milestone

Comments

@netmackan
Copy link

Is your feature request related to a problem? Please describe.
This issue brings up the issue that it is cumbersome to use JJWT for signing together with a smartcard/HSM as there are key parameter checks that are done on the PrivateKey instances that does not work in those cases and the user would have to make a lot of overrides and duplicate code in order to get past that.

Describe the solution you'd like
The suggested solution is to change/add to the API methods to get also the PublicKey and to instead use that one to make the key algorithm checks. That way the library would be possible to use for instance through PKCS#11.

Describe alternatives you've considered
Workaround today is to override the parts and remove or change the key parameters checks but that is very clumsy.

Additional context
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. Recently I provided a Pull Request #693 which solved those issues that could be solved without API changes. However, that did not cover the cases where the key paramaters are being checked so some overriding is still needed by the user.

The problem is that today the parameters are checked on the private key classes but when using SunPKCS11 or other providers and the keys are protected in a smartcard or HSM those key parameters are not available in the private key instances. Instead the parameters can be obtained from the public key instance.

Probably for those cases the API would have to also take the PublicKey object (or to take a KeyPair object which contains both PrivateKey and PublicKey).

@ghost ghost mentioned this issue Jan 20, 2022
@stale stale bot added stale Stale issues pending deletion due to inactivity and removed stale Stale issues pending deletion due to inactivity labels Apr 16, 2022
@lhazlewood
Copy link
Contributor

Just a note as I think about this: This could impact the jwe branch. Implementations there currently assert ECKey && PrivateKey or ECKey && PublicKey and similar for RSAKey concepts via generics. Example:

The problem with removing the typesafe RSAKey and ECKey generics parameters, is that the new signing methods like this don't work as well:

<K extends Key> T signWith(K key, io.jsonwebtoken.security.SignatureAlgorithm<K, ?> alg) throws InvalidKeyException;

which are invoked as jwtBuilder.signWith(SignatureAlgorithms.ES256, myECPrivateKey)

The SignatureAlgorithms.ES256 instance's interface (EllipticCurveSignatureAlgorithm) make it impossible to call signWith with anything other than a key that implements both ECKey and PrivateKey, which is totally ideal - it makes it impossible to call that method with an RSA PrivateKey instance (for example). The only arguments that are allowed are those that are compatible for the operation.

Changing SignatureAlgorithms.ES256 to implement an interface which only requires PrivateKey and PublicKey arguments is much less ideal as it allows for illegal arguments that type-safety solves otherwise.

It seems like a significant step backwards to drop this (particularly elegant) generics-enforced support for key implementations that are relatively rarely used.

FWIW, the jwe branch does allow one to define custom SignatureAlgorithm instances - a custom implementation could ignore ECKey and RSAKey requirements entirely for these PKCS#11 keys.

Perhaps there's still something we could do to simplify this...

@lhazlewood lhazlewood added this to the 1.0 milestone Apr 26, 2022
@lhazlewood lhazlewood modified the milestones: 1.0, 0.12.0 May 28, 2022
@jwtk jwtk deleted a comment from stale bot Aug 11, 2023
@lhazlewood
Copy link
Contributor

lhazlewood commented Sep 9, 2023

Closed via #178, #805 and #809. Extensive PKCS11 testing with both signing/verifying and encryption/decryption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants