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

Allow weak key lengths #521

Closed
TobiasReich opened this issue Oct 24, 2019 · 19 comments
Closed

Allow weak key lengths #521

TobiasReich opened this issue Oct 24, 2019 · 19 comments
Milestone

Comments

@TobiasReich
Copy link

I get a WeakKeyException stating that

The verification key's size is 1024 bits which is not secure enough for the RS512 algorithm.

I understand that this should not happen and new certificates should have a larger size.
However, we still have some legacy keys in use and need them to be validated - even though we know they are weak.

Is there an option / configuration that allows us to check their claims, even with smaller sized keys? The warning is nice but it seems like after that exception we can't parse any claims of it anyway.

@lhazlewood
Copy link
Contributor

lhazlewood commented Oct 24, 2019

Hi there,

There isn't currently support for weak keys - JJWT aims to be a specification-compliant library, and the RFC spec mandates (not suggests) that weak keys cannot be used for JWSs. Our docs cover this, specifically:

What's really important about these algorithms - other than their security properties - is that the JWT specification RFC 7518, Sections 3.2 through 3.5 requires (mandates) that you MUST use keys that are sufficiently strong for a chosen algorithm.

Also see https://github.com/jwtk/jjwt#rsa

Because using weak keys compromises security, and the RFC mandates key lengths, we don't have a strong reason to change the library to support such use cases.

However

These assertions were added in the 0.10.release (changelog here), so you could theoretically use a JJWT release 0.9.x or earlier. Not great, I know - but perhaps it will get you out of a time-sensitive bind.

Also, I think that once #493 is done, you would be able to plug in your own algorithm (maybe wrapping the 'real one' and ignoring WeakKeyExceptions for known/expected conditions). We hope to have that in the 1.0 release in the next couple of months. I know that doesn't help you now, but at least gives you an idea of how you might be able to address this at that time.

Finally #474 could allow you to override the key validation logic a different way if we support that for key strength, but that's still undecided.

Anyway, I hope that helps give you context for what we have to deal with in the coming months.

@lhazlewood lhazlewood changed the title Allow smaller key size Allow weak key lengths Oct 24, 2019
@lhazlewood
Copy link
Contributor

P.S. I made a comment on #474 referencing this issue, so I would consider this issue a duplicate of #474. We'll track any actionable work associated with this ticket in that one, so I'm closing this one. Feel free to comment and/or ask to re-open if you disagree - happy to discuss further if so.

@lhazlewood lhazlewood added this to the 0.12.0 milestone Aug 11, 2023
@lhazlewood
Copy link
Contributor

lhazlewood commented Aug 11, 2023

Functionality was introduced via #279 which will be in the 0.12.0 release that can support this request.

This would be possible in 0.12.0 or later by implementing a custom SignatureAlgorithm and performing key validation yourself. All of the JJWT default implementations are RFC-compliant, but you could overwrite the default RS512 algorithm with your custom one, e.g.

JwtParserBuilder.addSignatureAlgorithms(collectionContainingYourCustomRS512).

If the JWT has an alg header value that matches your SignatureAlgorithm.getId() value, your custom implementation will be used instead, where you can ignore RFC key length mandates.

@johan-roets
Copy link

Do we know when this is available? We have an issue where we have old keys with clients, so can't easily upgrade the keys. Additionally, we cannot upgrade spring/hibernate since that requires a newer version of jakarta.xml.bind:jakarta.xml.bind-api. This causes class loading issues with the version required by jwt 0.9.1

@lhazlewood
Copy link
Contributor

My hope was to have the release out this week, but with last week's brutal PKCS11 testing challenges (that took all week to get through), my hope is next week.

@eelcodevlieger
Copy link

Is there any updated schedule on the 0.12.0 release?

@lhazlewood
Copy link
Contributor

@eelcodevlieger #813

@dalenmar
Copy link

dalenmar commented Oct 5, 2023

The 0.12.0 version is finally shipped, thank you guys for your hard work!

But do I understand correctly: there's still no way to work with weak keys except using custom algorithm implementation?
Unfortunately we cannot change JWT headers (as 'alg' header you mentioned with custom algo), nor can we use stronger keys.
And that is such a dilemma, since it's impossible to upgrade our project to new java and spring version to this day.

@lhazlewood
Copy link
Contributor

lhazlewood commented Oct 5, 2023

@dalenmar, that's correct, the JJWT default implementations prevent keys that violate the RFC specification requirements, but with the 0.12.0 release, it's much easier to implement your own SignatureAlgorithm or MacAlgorithm, so you can indeed upgrade JJWT.

For example:

Jwts.builder()... signWith(key, myCustomAlgorithm)...compact()

and parsing:

Jwts.parser()...sig().add(myCustomAlgorithm)...parse(jwt)

If myCustomAlgorithm has the exact same ID as a RFC standard algorithm (e.g. HS256, etc), your custom algorithm will be used when encountering those types of JWTs instead of the RFC default. Then your algorithm implementation can relax key strength requirements.

From the JwtParserBuilder.sig() JavaDoc:

image

@johan-roets
Copy link

I tried, but this is not straightforward. All I need for backwards compatibility is a key length of 1024.

So in theory, I only want to change

private static final int MIN_KEY_BIT_LENGTH = 2048; in RsaSignatureAlgorithm

I cannot extend RsaSignatureAlgorithm or even AbstractSignatureAlgorithm. It goes down a rabbit hole trying to keep most of the functionality of RsaSignatureAlgorithm but only downgrading the min key length.

An example on how to possibly do this would be appreciated

@bdemers
Copy link
Member

bdemers commented Nov 7, 2023

@johan-roets is it possible to start rotating your keys while you are using <0.12 ?
What is the expiration duration on your current tokens?

@johan-roets
Copy link

we will have to implement a rotation strategy. Honestly not sure what effort that would involve. At the moment I'm looking for a path of least resistance

@bdemers
Copy link
Member

bdemers commented Nov 7, 2023

I understand that! And the desire to not couple your key rotation effort with an upgrade.

Regardless of which version of JJWT (or any other crypto library you are using) I'd strongly suggest upgrading your keys and have a key rotation policy in place.

There are better resources to read about key rotation than my quick message here, but a couple quick suggestions:

  • Plan for general rotation (rotate them on a given schedule, monthly, quarterly, bi-yearly, etc)
  • When adding a new key, keep the previous key around to validate signatures of previously minted tokens (how long you keep the previous key around depends on your expiration duration)
  • Plan for leaked keys. If your current key leaks, replace it. (this will invalidate all previous keys, and force your clients to re-authenticate). The impact of this event may be specific to your usage.

For JJWT < 0.11 you can do something like this:
https://github.com/okta/okta-jwt-verifier-java/blob/okta-jwt-verifier-parent-0.5.7/impl/src/main/java/com/okta/jwt/impl/jjwt/RemoteJwkSigningKeyResolver.java#L59-L70

TL;DR, grab the kid from the header, and look up your key id.

@lhazlewood
Copy link
Contributor

I cannot extend RsaSignatureAlgorithm or even AbstractSignatureAlgorithm. It goes down a rabbit hole trying to keep most of the functionality of RsaSignatureAlgorithm but only downgrading the min key length.

There's no easy way to use existing JJWT implementation classes, and this is mostly by design since our internal implementations can change at any time.

My recommendation is to just copy-and-paste the JJWT implementation source code and any required parent class logic into a new class within your project, change what you need, and then delete that class hopefully when you no longer need 1024-bit keys. It's not exactly 'nice', but it's simple enough and allows you the workaround you need.

@tortru
Copy link

tortru commented Jan 26, 2024

The correct way is to change the key so it matches the requirement.

But sometimes the key comes from a different system and you have to deal with it.

The following worked for me with 0.12.3:

//create a custom MacAlgorithm with a custom minKeyBitLength
int minKeyBitLength = 80;
String id = "HS512";
String jcaName = "HmacSHA512";
Class<?> c = Class.forName("io.jsonwebtoken.impl.security.DefaultMacAlgorithm");
Constructor<?> ctor = c.getDeclaredConstructor(String.class, String.class, int.class);
ctor.setAccessible(true);
MacAlgorithm custom = (MacAlgorithm) ctor.newInstance(id, jcaName, minKeyBitLength);
this.custom512 = custom;
			
//use the custom MacAlgorithm
var key = new SecretKeySpec(myKey.getBytes(java.nio.charset.StandardCharsets.UTF_8), "HmacSHA512");
Claims claims = Jwts.parser().verifyWith(key).sig().add(custom512).and().build().parseSignedClaims(token).getPayload();	

@SushSpaceBasic
Copy link

SushSpaceBasic commented Jul 28, 2024

Even I'm facing this issue wherein I cannot change the secret which is of 48 bits. Tried using @tortru's approach by creating a custom MacAlgorithm but I'm still facing the same WeakKeyException. Is there any updates on this issue? Any help will be appreciated.

@lhazlewood
Copy link
Contributor

lhazlewood commented Jul 29, 2024

@SushSpaceBasic Thanks for commenting, it appears you've found a bug, and calling .sig().add(instance)... will not override the existing elements in the collection. So you'll have to remove the existing one first and replace it with your custom implementation:

Jwts.parser().verifyWith(key).sig()
        .remove(Jwts.SIG.HS512) // <-- remove existing
        .add(custom512)         // <-- add custom one
        .and().build().parseSignedClaims(token).getPayload();	

I'll create a new issue to fix this bug so that .add automatically replaces the exiting one (as is currently JavaDoc'd and should be that behavior).

@SushSpaceBasic
Copy link

Thanks for the quick update and work around for this issue @lhazlewood. Really appreciate it

@tortru
Copy link

tortru commented Aug 30, 2024

You are right. There is a this.collection.remove(e) before the add(e) but since the DefaultMacAlgorithm does not implement equals() it will not remove the existing one even if both have the same ID.
I wonder why it worked for me. Maybe because I use it in a native image.

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

8 participants