JwkSet unexpectedly "secret" / Jackson cannot serialize JwkSet

[gsprdev]

Describe the bug

The default (and only) JWK set builder considers all "keys" members as secret, even if the set contains only public keys.
In turn, this results in a failure to serialize the set using Jackson redacted data cannot be serialized.

To Reproduce

To repro

1. Add Jackson to the classpath
2. Create a JWK for a public key
3. Add that JWK to a JWKSet using the builder
4. Attempt to serialize that set using Jackson's ObjectMapper

A complete example follows.

dependencies {
    implementation("io.jsonwebtoken:jjwt-api:0.12.6")
    runtimeOnly("io.jsonwebtoken:jjwt-impl:0.12.6")
    runtimeOnly("io.jsonwebtoken:jjwt-jackson:0.12.6")
}

static void jwkSetUnexpectedlyPrivate(RSAPublicKey pubKey) throws Exception {
    final ObjectMapper jacksonMapper = new ObjectMapper();

    final RsaPublicJwk pubJwk = Jwks.builder()
            .key(pubKey)
            .build();

    // PASS; unredacted human-readable form
    pubJwk.toString();

    // PASS; unredacted
    jacksonMapper.writeValueAsString(pubJwk);

    // When in set, now redacted
    final JwkSet jwkSet = Jwks.set()
            .add(pubJwk)
            .build();

    // FAIL? Redacted; Possibly a safe default, not terribly problematic for debug statements
    jwkSet.toString();

    // FAIL; Throws an exception. Prevents exposure of the set for external consumption of public keys.
    jacksonMapper.writeValueAsString(jwkSet);
    // com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No serializer found for class
    // io.jsonwebtoken.impl.lang.RedactedSupplier and no properties discovered to create BeanSerializer
    // (to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS)
    // (through reference chain: io.jsonwebtoken.impl.security.DefaultJwkSet["keys"])
}

Expected behavior

1. It is expected that sets are either conditionally redacted based on whether their content is redacted,
   OR this behavior can be configured in some way to allow proper serialization.
2. It should be possible to serialize a JwkSet using Jackson

Screenshots

See code sample above.

[bdemers]

IIRC, I ran into a similar issue, but from a different angle.

I was creating a .well-known/jwks.json, eventually i settled on:

        return Jwks.set()
                .add(Jwks.builder()
                    .id(keyId)
                    .rsaKeyPair(keyPair)
                    .build()
                    .toPublicJwk())
                .build();

But this is specific to RSA keys, I remember seeing a similar exception to yours though, which IIRC, was related to me not having registering the JJWT Jackson Module:

jjwt/extensions/jackson/src/main/java/io/jsonwebtoken/jackson/io/JacksonSerializer.java

Lines 40 to 44 in fd619e0

	static {
	SimpleModule module = new SimpleModule(MODULE_ID);
	module.addSerializer(JacksonSupplierSerializer.INSTANCE);
	MODULE = module;
	}

It's not directly exposed, but you could call the JacksonSerializer constructor to work around it.

@gsprdev, I'm not sure if this helps or not, but let us know how you were using the serialized JwkSet that might help us better expose support for your use case.

[gsprdev]

@bdemers Thank you. Interesting. Forcing inclusion of that module does work in a forced (debug-only or unsafe reflection) scenario, but does not appear to be achievable under normal circumstances. A few issues:

1. Documentation instructs import of Jackson module as runtimeOnly
2. JacksonSerializer.MODULE is package-private
3. JacksonSupplierSerializer is package-private
4. JacksonSerializer constructor IS public and therefore can be theoretically be manually constructed and registered, but would need to be repeated for each relevant type individually (including hierarchically).

It's particularly curious that JWTs and JWKs serialize just fine, with no intervention. All defaults, just with the Jackson classpath dependency. JwkSet is the only exception so far.

[lhazlewood]

@gsprdev welcome!

And thank you for creating such a well-thought and detailed issue, it's a pleasure to help when receiving quality submissions :)

I think this issue elucidates a gap in our documentation, and perhaps a feature that could be added to make people's lives easier.

As you've already seen, JWKs (and by extension, JWK Sets that contain them), can contain secure data values, so we need to be careful about exposing them; JJWT is designed to hide these values by default by wrapping them in a generic Supplier. This implies that those Supplier wrappers aren't designed to be serialized via an 'unaware' ObjectMapper or printed via toString directly. They are instead designed to be serialized to JSON by passing them through a Serializer implementation that 'knows' how to properly extract the raw value from the Supplier wrapper.

As such, you'll need to use a JJWT Serializer implementation to render the JSON instead of using the ObjectMapper directly. For example:

JacksonSerializer<JwkSet> serializer = new JacksonSerializer<>(); // or new JacksonSerializer(yourObjectMapper);
JwkSet set = Jwks.set().add(jwk).build();
byte[] serialized = serializer.serialize(set);

The assumption then would be next to simply convert the serialized byte array into a String, e.g. new String(serialized, StandardCharsets.UTF_8);, BUT this can be very risky:

In your particular case, because your JWK Set will only contain public key data, which means there are no secret or private values, you'll be ok doing this, and your JSON string will be safe.

However, for anyone else reading this thread or this reply, converting that to a String may not be at all safe if it reflects secret or private key material: the String could easily be added to a log or printed to System.out, etc, plus the String's underlying resulting char[] array will remain in memory.

When possible, it's best to render potentially unsafe JSON bytes directly to an OutputStream (e.g. a ServletOutputStream in a webapp) without converting to Strings.

I hope that helps! Please let us know.

[lhazlewood]

P.S. as shown in the code above, just using new JacksonSerializer(yourObjectMapper) will automatically register the Jackson-specific jjwt-jackson module on that ObjectMapper instance to ensure this behavior works as expected - no need to use the internal/impl-private fields directly.

You're right that our documentation defaults to a runtime-only dependency on jjwt-jackson, unless you want to specify the ObjectMapper itself. That's shown with a compile-time dependency with associated usage here:

https://github.com/jwtk/jjwt?tab=readme-ov-file#jackson-json-processor

I hope that helps!

[bdemers]

@gsprdev Very good points, I was thinking about these when I wrote my previous message, but didn't call them out, sorry about that!

1. Documentation instructs import of Jackson module as runtimeOnly
2. JacksonSerializer.MODULE is package-private
3. JacksonSupplierSerializer is package-private
4. JacksonSerializer constructor IS public and therefore can be theoretically be manually constructed and registered, but would need to be repeated for each relevant type individually (including hierarchically).

Related to this is how you are using the serialized data, for example, if you are creating a REST endpoint, there might be different usage expectations than reading/writing application configuration.

For example, a Spring REST Endpoint you might expect something like this to just work ™️ (or with minimal effort)

@GetMapping(path = "/.well-known/jwks.json")
JwkSet keySet() {
    // ...
}

Regardless, I think we could make small tweaks in area to improve simplify the usage!

[gsprdev]

@lhazlewood Thank you for your detailed response. I certainly understand the risks of accidentally exposing secret information in logs or otherwise, and respect the effort you put into ensuring some "safe by default" behaviors in the library design. It is with that acknowledgement, however, that I filed this report after investigation. Perhaps I should have reported the Jackson serialization issue and the "private" issues separately, but after browsing your code base it seemed to me that the former was a symptom of the latter. Perhaps not.

Some items are always redacted. Some are never. As demonstrated, RsaPublicJwk (for example) contains no redacted elements during serialization because it is understood to contain only non-sensitive parameters. PublicJwk<?> comes from different provider configurations than other Jwk<?>s for good reason. These work exactly as expected and desired when serialized independently (not part of a JwkSet).

I am somewhat surprised by the suggestion of overriding/avoiding both then classpath-implicit autoconfiguration and the serialization protection so carefully added in this library. Please forgive me if I misunderstand your design, but it seemed to me from a code reading was that the one-and-only JwkSet implementation DefaultJwkSet, hardcodes param(JwkConverter.ANY) + ParameterBuilder.setSecret(true), and no alternative (e.g. PublicJwkSet) exists to use param(JwkConverter.PUBLIC_JWK) + ParameterBuilder.setSecret(false) instead. Would you agree?

---

Also, @bdemers you guess correctly; that's exactly my intended use case: Producing /.well-known/jwks.json via a simple Spring controller. Although, I encountered and reported this issue while testing in anticipation of that case, not during implementation.

Due to the conventional use of a single ObjectMapper across application scope, I am somewhat uncomfortable with instantiating and immediately discarding a io.jsonwebtoken.jackson.io.JacksonSerializer simply for its immediate, permanent, application-wide side-effect of module registration.

Fortunately, because only JwtSet is affected (and not any other type from this library), an extremely simple workaround was possible and apparent. Speaking in Kotlin for a moment for brevity, it was a simple replacement of

fun getPubJwks() : io.jsonwebtoken.security.JwkSet {
    return Jwks.set().add(listOf(pubJwk1, pubJwk2)).build()
}

with

data class MyJwkSet(val keys: List<RsaPublicJwk>)

fun getPubJwks() : MyJwkSet {
    return MyJwkSet(listOf(pubJwk1, pubJwk2))
}

which is enough to be fully functional with no configuration of Jackson or its ObjectMapper whatsoever.

[bdemers]

@gsprdev thanks for following up (and including the work around)!

We will look into ways to simplify this. My gut feel, is that public key usage (e.g. the jwks.json endpoint should work out of the box and objects containing private keys could require a little more configuration.)

[lhazlewood]

the one-and-only JwkSet implementation DefaultJwkSet, hardcodes param(JwkConverter.ANY) + ParameterBuilder.setSecret(true), and no alternative (e.g. PublicJwkSet) exists to use param(JwkConverter.PUBLIC_JWK) + ParameterBuilder.setSecret(false) instead. Would you agree?

Yes, indeed, and this was intentional at the time it was created. The keys parameter was configured as secret as a 'catch all' to indicate that anything it contains might be confidential, just to be safe and ensure something was not accidentally exposed. In retrospect, looking at this now, that was probably too aggressive a default 😅.

I think we can now change that to setSecret(false) since each JWK within the set should always have its own parameters indicated as confidential or not.

Due to the conventional use of a single ObjectMapper across application scope, I am somewhat uncomfortable with instantiating and immediately discarding a io.jsonwebtoken.jackson.io.JacksonSerializer simply for its immediate, permanent, application-wide side-effect of module registration.

While I agree this is an inelegant solution (to call a constructor and then have that created object unused and immediately available for garbage collection), this does appear a valid workaround in the short term - the jjwt-jackson module only registers a JacksonSupplierSerializer to ensure io.jsonwebtoken.lang.Supplier instances can be serialized, and nothing more. This is intended/expected for an application-wide ObjectMapper so that any encounter of such instances anywhere in the application can be serialized to JSON properly (and not just, say, limited to a single REST endpoint).

Limiting further impact on the application is also why we created our own Supplier interface and wouldn't use the JDK's native one - to ensure that logic only applied to JJWT-specific instances and wouldn't impact anything else in application code.

which is enough to be fully functional with no configuration of Jackson or its ObjectMapper whatsoever.

@gsprdev Glad to hear this worked well for you! This is exactly why all JWT and JWK implementations implement the Map interface - for one, it's an accurate reflection of a JSON Object structure, but also for interoperability like this.

That said, this worked because you have only public key parameters exposed, so none of them are wrapped in the redacting Supplier.

Based on all of the above, to translate this into actionable work, would you mind letting us know your thoughts on these questions?

1. I assume that setting the JwkSet implementation's keys parameter to setSecret(false) would have done what you expected, and you probably wouldn't have ever needed to open a ticket. Is that a sufficient resolution of this issue, or do you think we should address the next question as well?

2. Given that the JacksonSerializer constructor that accepts an ObjectMapper currently purposefully intends to modify an application-wide instance (for universal ability to serialize JJWT Supplier instances wherever they may be encountered in application code), do you feel it is necessary/preferred to make that Module constant public so that developers can manually configure some ObjectMapper instances but not others? It's unclear to me how likely/desirable this is given that my assumption is that most devs want it to 'just work everywhere in my application'.

Thoughts? Thank you for your feedback! It is appreciated :)