jjwt icon indicating copy to clipboard operation
jjwt copied to clipboard

Issues in upgrading to 0.12.6

Open tiwarishubham635 opened this issue 1 year ago • 1 comments

Hello! I am tring to upgrade twilio-java to 0.12.6. This because of some known vulnerabilities like CVE-2024-31033 in jjwt 0.11.2. I saw there are some breaking changes in 0.12 version. Since this vulnerability is a blocker for our customers, we have to upgrade. However, I would appreciate if there is some other way we can get rid of this vulnerability.

Anyways, the main issue is that while upgrading to 0.12.6 I am facing a lot of errors. Here is the PR that I have raised. For example, setSigningKey method (though it is deprecated, it is showing errors.

Can someone help in upgrading the version? Maybe if there is some upgrade guide, that would be helpful.

tiwarishubham635 avatar Jul 23 '24 14:07 tiwarishubham635

Hi @tiwarishubham635!

A couple of things, CVE-2024-31033 is contested, there is a bit more discussion around it here: https://github.com/jwtk/jjwt/issues/930 TL;DR we are waiting for that to get resolved. (but I get it... nobody wants vuln reports, even false reports in their builds)

Related to 0.12.6, the error you are seeing is related to your cty header value of: twilio-fpa;v=1 In 0.12, JJWT became more strict in how it parses JWTs, twilio-fpa;v=1 is an unknown content type, and JJWT, doesn't know how to parse that, same would be true if this were image/png.

You can read more about how the cty header is processed in the JWS RFC

In order to support non JWT content types, you can use the method: parseSignedContent(<token>) That [indirectly] gives you a byte array (call .accept(JwtVisitor) to convert it to your desired object.

There is a similar discussion on this here: https://github.com/jwtk/jjwt/issues/897 (where there is also some discussion on wrapping this logic in a something like a ContentTypeHandler, e.g. Jwts.parser().cty(handler)...

Your feedback on the ideas in that thread would be helpful!

bdemers avatar Jul 23 '24 16:07 bdemers

I, too, am having an issue with migrating to version 0.12.6.

An example from my original code looks like this:

  jwtBuilder.signWith(signingKey, SignatureAlgorithm.PS384);

It is flagged with warnings regarding deprecation.

I thought that the way to update it was to do this:

  jwtBuilder.signWith(signingKey, Jwts.SIG.PS384);

But that fails because the members of the Jwts.SIG class have types SignatureAlgorithm and MacAlgorithm which are not compatible with the SecureDigestAlgorithm<? extends Key, ?> type of the non-deprecated version of the signWith() method: <K extends Key> JwtBuilder signWith(K key, SecureDigestAlgorithm<? super K, ?> alg).

How should one go about migrating usages of the deprecated signWith() methods which allow the caller to specify the desired signing algorithm?

tonedef71 avatar Nov 26 '24 21:11 tonedef71

@tonedef71 it's hard to tell without seeing the type of Key instance, but the newer (non-deprecated) method signature guarantees compile-time safety that the type of key being used matches the type of key required by the SecureDigestAlgorithm.

For example, if the alg instance requires a PrivateKey, ensure that your key instance is that type, and not just a generic Key. Or if a MAC algorithm, the key is a SecretKey and not just a Key.

Does that help? Hopefully that makes sense, but if not, let me know and we'll figure it out :)

lhazlewood avatar Nov 26 '24 21:11 lhazlewood

@lhazlewood Thank you for the reply.

I have a utility POJO that dynamically derives signing keys and such based on configuration property String values:

  static PublicKey readPublicKey(...) throws Exception {
  }

  static PrivateKey deriveRsaSigningKey(...) throws Exception {
  }

  static SecretKey deriveHmacSigningKey(...) {
  }

  static Key deriveConsumerSigningKey(...) throws Exception {
    Key signingKey = null;
    if (...) {
      signingKey = readPublicKey(...); 
    }
    else if (...) {
      signingKey = deriveHmacSigningKey(...);
    }

    return signingKey;
  }

  static Key deriveProducerSigningKey(...) throws Exception {
    Key signingKey = null;
    if (...) {
      signingKey = deriveRsaSigningKey(...);
    }
    else if (...) {
      signingKey = deriveHmacSigningKey(...);
    }

    return signingKey;
  }

  static String generate(...) throws Exception {
    final Key signingKey = deriveProducerSigningKey(...);
    if (null != signingKey) {
      jwtBuilder.signWith(signingKey, signatureAlgorithm); // e.g. SignatureAlgorithm.RS384, SignatureAlgorithm.HS384, SignatureAlgorithm.ES384, SignatureAlgorithm.PS384
    }
    ...
    
    return jwtBuilder.compact();
  }

I conveniently refer to the various key interface types by their base interface type Key.

tonedef71 avatar Nov 26 '24 21:11 tonedef71

@lhazlewood When the audience of the JWT is a single recipient, how does one coax JwtBuilder.compact() to generate a JWT with an aud claim assigned a single string value instead of a single value array?

UPDATE: Actually, it appears that the JwtBuilder.compact() method handles the single recipient aud just fine when the deprecated JwtBuilder.audience().single() method is used to add the single String value. Why is the method deprecated?

tonedef71 avatar Dec 02 '24 19:12 tonedef71

@tonedef71 it's deprecated to discourage its use, the RFC updated to prefer/recommend a String array, so people ideally shouldn't be using single string values anymore. It's there for backwards compatibility for legacy systems only. I hope that helps!

Also, if additional context helps, there's this as well: https://github.com/jwtk/jjwt/issues/944#issuecomment-2116068549

lhazlewood avatar Dec 02 '24 20:12 lhazlewood

also if it matters, the JavaDoc for the .single method states:

image

lhazlewood avatar Dec 02 '24 20:12 lhazlewood

@tonedef71 it's deprecated to discourage its use, the RFC updated to prefer/recommend a String array, so people ideally shouldn't be using single string values anymore. It's there for backwards compatibility for legacy systems only. I hope that helps!

Also, if additional context helps, there's this as well: #944 (comment)

@lhazlewood SIGH I dislike deprecation warnings. Oracle NetSuite seems to reject JWTs with single value arrays.

Do you plan on keeping all of the deprecated stuff around in future releases of the library indefinitely?

tonedef71 avatar Dec 02 '24 20:12 tonedef71

@tonedef71 when we release v 1.0, we will remove all deprecated methods except this one and any others mandated as a requirement by the RFCs. For the very few (one? two? can't remember) that must remain per RFC requirements, marking them @Deprecated is the only way to indicate to developers that they ideally shouldn't be used, and the associated JavaDoc is very clear as to why - I'm afraid that's the best we can do as there's no other way that I know of other than "you should have read the project documentation" (which very few people do unfortunately).

In this particular case, you can't see it without viewing the source code, but we have an internal developer note for that specific API method to ensure its later retention:

https://github.com/jwtk/jjwt/blob/2ad964a3f1d77ffa1a915640d94e9238216a52d6/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java#L266

lhazlewood avatar Dec 02 '24 20:12 lhazlewood

Oracle NetSuite seems to reject JWTs with single value arrays.

They are in clear violation of the RFCs then. If you have the ability to submit a ticket to them, it could help others. 🙏

lhazlewood avatar Dec 02 '24 20:12 lhazlewood

@tonedef71 when we release v 1.0, we will remove all deprecated methods except this one and any others mandated as a requirement by the RFCs. In this particular case, you can't see it without viewing the source code, but we have an internal developer note for that specific API method to ensure its later retention:

https://github.com/jwtk/jjwt/blob/2ad964a3f1d77ffa1a915640d94e9238216a52d6/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java#L266

Will JwtBuilder.signWith() also be spared from the great purge of deprecated stuff?

tonedef71 avatar Dec 02 '24 20:12 tonedef71

Will JwtBuilder.signWith() also be spared from the great purge of deprecated stuff?

There are two overloaded variants of that method that are not marked as deprecated and will remain, but the other 4 currently marked as deprecated will be removed because they:

  1. Confuse enough people that don't understand that Strings are not themselves keys, which could open them up to security risks, and
  2. Use the old io.jsonwebtoken.SignatureAlgorithm enum which was replaced by the newer io.jsonwebtoken.security.*Algorithm interfaces for custom algorithm support in 0.12.0 and later (0.11.x and earlier could not support custom algorithms, nor overriding the default ones if one necessary).

lhazlewood avatar Dec 02 '24 20:12 lhazlewood

@lhazlewood Thank you for elaborating further. What does one need to do to get JwtBuilder.signWith() to sign with the PS384 algorithm when provided with an RSA private key?

tonedef71 avatar Dec 02 '24 20:12 tonedef71

@tonedef71 .signWith(rsaPrivateKey, Jwts.SIG.PS384)... should do what you're looking for:

Per that method's JavaDoc (which references Jwts.SIG):

image

I hope that helps! 😄

lhazlewood avatar Dec 02 '24 21:12 lhazlewood

@lhazlewood Thank you, but I should have been more specific. How does one coax the simple method JwtBuilder.signWith(Key) to sign with the PS384 algorithm when provided with an RSA private key? What qualifications does JwtBuilder.signWith(Key) look for in order to autonomously select PS384 as the signing algorithm?

tonedef71 avatar Dec 02 '24 21:12 tonedef71

Use the old io.jsonwebtoken.SignatureAlgorithm enum which was replaced

By the way, the old io.jsonwebtoken.SignatureAlgorithm enum is tagged as deprecated. Will it be exempt from future purges of deprecated material?

tonedef71 avatar Dec 02 '24 21:12 tonedef71

@tonedef71 those qualifications are documented in the .signWith(Key) method. The PS* algorithms are not auto-selected in any case, because the standard RS* algorithms take precedence for all RSA key sizes:

Pasted_Image_12_2_24__1_14 PM

This precedence was defined long ago in JJWT's history when JDK 8 was the dominant/most-adopted JDK because the PS* algorithms weren't introduced until JDK 11, implying .signWith(rsaPrivateKey) would have thrown an exception for most applications at the time.

This will likely change in future JJWT releases however because RSA PS* algorithms are always better than the RS* ones due to the additional secure-random elements in every PS* signature. So I definitely recommend using the overloaded .signWith(rsaPrivateKey, Jwts.SIG.PS***) variant for now if you're on >= JDK 11 and/or using BouncyCastle.

lhazlewood avatar Dec 02 '24 21:12 lhazlewood

Use the old io.jsonwebtoken.SignatureAlgorithm enum which was replaced

By the way, the old io.jsonwebtoken.SignatureAlgorithm enum is tagged as deprecated. Will it be exempt from future purges of deprecated material?

No, it will be removed because the newer io.jsonwebtoken.security.*Algorithm variants have replaced the enum concept. They offer better type-safety, they're more flexible, can be overridden in a pinch, and also allow applications to use custom algorithms if desired - the enum didn't allow any of that, so it will be removed fully when releasing 1.0 since there's no benefit to them at all compared to those in the Jwts.SIG.* registry.

lhazlewood avatar Dec 02 '24 21:12 lhazlewood

Is there any chance of getting .audience() variants that take a String or Set<String> as parameters and still return T? Converting from .setAudience() is messing up my otherwise nice builder chain flow because it returns AudienceCollection<T> and breaks the chain.

jac-bah avatar Feb 06 '25 22:02 jac-bah

Is there any chance of getting .audience() variants that take a String or Set as parameters and still return T? Converting from .setAudience() is messing up my otherwise nice builder chain flow because it returns AudienceCollection and breaks the chain.

@jac-bah AudienceCollection is a NestedCollection which has an .and() method to return back up to the parent builder.

For example:

.issuer("me")
.audience().add("you").and() // <--- .and() returns the parent JwtBuilder to resume the chain flow
.subject("Bob")
// etc...

lhazlewood avatar Feb 07 '25 02:02 lhazlewood

Just closing this out as a matter of housekeeping - it doesn't represent actionable work for the JJWT codebase, but we're happy to help with similar/future questions like this as a discussion.

lhazlewood avatar Apr 23 '25 19:04 lhazlewood