Impossible to build a JWK with alg: HS512 and a k that is larger than 64 bytes

[louis-jaris]

First of all, thank you for your work! 🙌
I've loved the recent refactoring that took place, as well as the phenomal work done in your README.md to explains various JW* concept, I'm so grateful for all of your work!

Describe the bug
When trying to load JWK Set (also applicable to single JWK loading), it is impossible to load a key with alg: HS512 with a k that is larger than 64 bytes.

While it makes complete sense that keys that are shorter than the requirement should be rejected, it is less evident that secrets that are larger should be rejected as well.

Additionally, this behaviour is inconsistant: when we are creating SecretKey with Keys.hmacShaKeyFor(...) with more than 64 bytes, it works, while we cannot load HS512 JWK with more than 64 bytes.

As you are citing in the DefaultMacAlgorithm, the RFC 7518 is indicating that secrets that are larger than the required minimal size should work...

This is the exact line that is producing this behaviour. A (simple) fix could be to use bitLen < requiredBitLen instead of bitLen != requiredBitLen.

The stack trace is:

Exception in thread "main" io.jsonwebtoken.security.MalformedKeySetException: JWK Set keys[0]: Secret JWK 'alg' (Algorithm) value is 'HS512', but the 'k' (Key Value) length does not equal the 'HS512' length requirement of 512 bits (64 bytes). This discrepancy could be the result of an algorithm substitution attack or simply an erroneously constructed JWK. In either case, it is likely to result in unexpected or undesired security consequences.
	at io.jsonwebtoken.impl.security.JwkSetConverter.applyFrom(JwkSetConverter.java:135)
	at io.jsonwebtoken.impl.security.JwkSetConverter.applyFrom(JwkSetConverter.java:36)
	at io.jsonwebtoken.impl.io.ConvertingParser.parse(ConvertingParser.java:39)
	at io.jsonwebtoken.impl.io.AbstractParser.parse(AbstractParser.java:36)
	at io.jsonwebtoken.impl.io.AbstractParser.parse(AbstractParser.java:29)
	at Scratch.loadJwkSet(scratch_4.java:35)
	at Scratch.main(scratch_4.java:12)
Caused by: io.jsonwebtoken.security.MalformedKeyException: Secret JWK 'alg' (Algorithm) value is 'HS512', but the 'k' (Key Value) length does not equal the 'HS512' length requirement of 512 bits (64 bytes). This discrepancy could be the result of an algorithm substitution attack or simply an erroneously constructed JWK. In either case, it is likely to result in unexpected or undesired security consequences.
	at io.jsonwebtoken.impl.security.SecretJwkFactory.assertKeyBitLength(SecretJwkFactory.java:73)
	at io.jsonwebtoken.impl.security.SecretJwkFactory.createJwkFromValues(SecretJwkFactory.java:89)
	at io.jsonwebtoken.impl.security.SecretJwkFactory.createJwkFromValues(SecretJwkFactory.java:37)
	at io.jsonwebtoken.impl.security.AbstractFamilyJwkFactory.createJwk(AbstractFamilyJwkFactory.java:112)
	at io.jsonwebtoken.impl.security.DispatchingJwkFactory.createJwk(DispatchingJwkFactory.java:96)
	at io.jsonwebtoken.impl.security.AbstractJwkBuilder.build(AbstractJwkBuilder.java:155)
	at io.jsonwebtoken.impl.security.DefaultDynamicJwkBuilder.build(DefaultDynamicJwkBuilder.java:210)
	at io.jsonwebtoken.impl.security.DefaultDynamicJwkBuilder.build(DefaultDynamicJwkBuilder.java:45)
	at io.jsonwebtoken.impl.security.JwkConverter.applyFrom(JwkConverter.java:172)
	at io.jsonwebtoken.impl.security.JwkConverter.applyFrom(JwkConverter.java:42)
	at io.jsonwebtoken.impl.security.JwkSetConverter.applyFrom(JwkSetConverter.java:125)
	... 6 more

To Reproduce

Minimal code to reproduce (and that highlight different scenarios):

import io.jsonwebtoken.security.JwkSet;
import io.jsonwebtoken.security.Jwks;

import java.util.Base64;

class Scratch {
    public static void main(String[] args) {
        for (int secretSize : new int[]{10, 63, 64, 65, 140}) {
            System.out.println("secretSize = " + secretSize);
            String b64UrlSafeSecret = Base64.getUrlEncoder().encodeToString("a".repeat(secretSize).getBytes());
            try {
                loadJwkSet(b64UrlSafeSecret);
                System.out.printf("Successfully loaded JWKS for secretSize=%s\n", secretSize);
            }
            catch (Exception e) {
                System.out.printf("Failed to load JWKS for secretSize=%s, got exception=%s\n", secretSize, e.getMessage());
            }
            System.out.println("-----------------");
        }
    }
    private static JwkSet loadJwkSet (String b64UrlSafeSecret){
        var rawJwkSet = """
                    {
                      "keys": [
                        {
                          "kid": "simple_key_v1",
                          "kty": "oct",
                          "alg": "HS512",
                          "k": "%s"
                        }
                      ]
                    }
                    """.formatted(b64UrlSafeSecret);
        return Jwks.setParser().ignoreUnsupported(false).build().parse(rawJwkSet);
    }
}

Expected behavior
We should be able to load JWK with k that are doing more than 64 bytes.
We should be able to load the following JWK:

{
  "kid": "some UUID",
  "kty": "oct",
  "alg": "HS512",
  "k": "base64Urlblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblah"
}

Screenshots
Screenshot of the result of the code above:

[lhazlewood]

@louis-jaris thank you so much for the kind words, and also thank you for the extremely well-written issue! It really helped me immediately understand what you're experiencing and how to address it.

A (simple) fix could be to use bitLen < requiredBitLen instead of bitLen != requiredBitLen.

Great catch! And thank you so much for reporting this!

I believe this was a 'carryover' from similar logic validating EllipticCurve keys (which have to be exactly a specific length, not >= a specific length).

That said, it's not exactly as simple as that, but it's not much more difficult. That method is called from below in the createJwkFromValues method:

jjwt/impl/src/main/java/io/jsonwebtoken/impl/security/SecretJwkFactory.java

Lines 78 to 95 in f61cfa8

	protected SecretJwk createJwkFromValues(JwkContext<SecretKey> ctx) {
	ParameterReadable reader = new RequiredParameterReader(ctx);
	byte[] bytes = reader.get(DefaultSecretJwk.K);
	String jcaName = null;
	String id = ctx.getAlgorithm();
	if (Strings.hasText(id)) {
	SecureDigestAlgorithm<?, ?> alg = Jwts.SIG.get().get(id);
	if (alg instanceof MacAlgorithm) {
	jcaName = ((CryptoAlgorithm) alg).getJcaName(); // valid for all JJWT alg implementations
	Assert.hasText(jcaName, "Algorithm jcaName cannot be null or empty.");
	assertKeyBitLength(bytes, (MacAlgorithm) alg);
	}
	}
	if (!Strings.hasText(jcaName)) {
	if (ctx.isSigUse()) {
	// The only JWA SecretKey signature algorithms are HS256, HS384, HS512, so choose based on bit length:
	jcaName = "HmacSHA" + Bytes.bitLength(bytes);

And that logic (currently) uses the byte array size (in bits) to set it's JCA algorithm name (, e.g. "HmacSHA${sizeInBits}"), which wouldn't be correct for sizes greater than the standard. We'll just need to delegate to a discovered/associated MacAlgorithm's JCA name instead of trying to re-create it from the discovered byte array length directly (i.e. use macAlgorithm.getKeyBitLength() instead of Bytes.bitLength(kBytes)).

This is a minor fix, but it also elucidates another issue I'd like to clean up: Any length/size validation(i.e. assertKeyBitLength shouldn't exist at all in SecretJwkFactory if we can avoid it. All of that nicer logic is already in DefaultMacAlgorithm, so SecretJwkFactory should just delegate to it - much easier/better than replicating logic.

This should be simple enough - and thanks for the sample test cases! That'll help. This should be a relatively quick fix 😃

[lhazlewood]

Fixed via #909