spring-security
spring-security copied to clipboard
RemoteJwkSet is not refreshed when encountering an unknown KID
Describe the bug If you are setting up a NimbusJwtDecoder using Spring Security, we have the possibility to provide a Cache that will be used for storing the JWK Set. This is a good feature, since we can then override the default TTL, instead of using DefaultJWKSetCache, which has a default refresh time of 5 minutes.
Reference to documentation: https://docs.spring.io/spring-security/reference/servlet/oauth2/resource-server/jwt.html#oauth2resourceserver-jwt-timeouts
The issue occurs when you use your own cache implementation, and you try to decode a JWT with an unknown KID. I would expect this to trigger a refresh of the JWK set, but this is not what is happening.
Example code of setup:
var jwkSetCache = new ConcurrentMapCache("jwkSetCache", CacheBuilder.newBuilder()
.expireAfterWrite(Duration.ofMinutes(30))
.build().asMap(), false);
var decoder = NimbusJwtDecoder.withJwkSetUri(jwkSetUri)
.cache(jwkSetCache)
.build();
We end up here with an unknown KID: https://bitbucket.org/connect2id/nimbus-jose-jwt/src/43b5435c548d70ce250afe6f932a6a55b3833bb8/src/main/java/com/nimbusds/jose/jwk/source/RemoteJWKSet.java#lines-486, and we expect the JWK set to be refetched.
The problem is that the NimbusJwtDecoder.java always uses the CachingResourceRetriever, even when it encounters an unknown KID: https://github.com/spring-projects/spring-security/blob/main/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java#L406-L407. This will lead to the JWK set only being fetched from the cache, and not refetched from the remote source.
To Reproduce
- Setup NimbusJwtDecoder with a Cache and remote JWK set.
- Try to decode a JWT signed with a KID not present in the JWK set.
- Expect NimbusJwtDecoder to refetch JWK set from jwksetUri.
If we don't specify a cache to the NimbusJwtDecoder builder, it works as expected. The jwk set is refetched when encountering a signed JWS with an unknown KID.
Expected behavior I expect JWK set to be refetched when encountering a KID not present in the current cached JWK set.
Thanks for reaching out @tinolazreg, and thanks for the detailed explanation!
Reviewing the NimbusJwtDecoder.JwkSetUriJwtDecoderBuilder
and the Nimbus library class RemoteJWKSet
, it looks like this could happen as you outlined, though it would be helpful to verify with a minimal sample.
If you need to use the Spring Cache
abstraction, a workaround in the meantime could be tricky. I have not tested this, and don't work directly with the Cache
abstraction often, but you could try something like the following:
@Bean
public JwtDecoder jwtDecoder() throws Exception {
String jwkSetUri = "...";
URL jwkSetUrl = new URL(jwkSetUri);
return NimbusJwtDecoder.withJwkSetUri(jwkSetUri)
.jwtProcessorCustomizer(jwtProcessor -> {
JWSVerificationKeySelector<SecurityContext> jwsKeySelector =
(JWSVerificationKeySelector<SecurityContext>) jwtProcessor.getJWSKeySelector();
RemoteJWKSet<SecurityContext> remoteJwkSet =
(RemoteJWKSet<SecurityContext>) jwsKeySelector.getJWKSource();
ResourceRetriever restOperationsResourceRetriever =
remoteJwkSet.getResourceRetriever();
JWKSetCache jwkSetCache = new JWKSetCache() {
private final Cache cache = new ConcurrentMapCache(
"jwkSetCache"/*, CacheBuilder.newBuilder()...build()*/, false);
@Override
public void put(JWKSet jwkSet) {
this.cache.put(jwkSetUri, jwkSet);
}
@Override
public JWKSet get() {
return this.cache.get(jwkSetUri, JWKSet.class);
}
@Override
public boolean requiresRefresh() {
return this.cache.get(jwkSetUri) == null;
}
};
RemoteJWKSet<SecurityContext> jwkSource = new RemoteJWKSet<>(
jwkSetUrl, restOperationsResourceRetriever, jwkSetCache);
JWSVerificationKeySelector<SecurityContext> newJwsKeySelector =
new JWSVerificationKeySelector<>(JWSAlgorithm.RS256, jwkSource);
jwtProcessor.setJWSKeySelector(newJwsKeySelector);
})
.build();
}
If this works (or gets close), it allows you to re-use the existing encapsulated implementation of ResourceRetriever
.
Thank you for the reply. I created a test that reproduce the issue here: https://github.com/spring-projects/spring-security/compare/main...tinolazreg:spring-security:issue_11621_tests?expand=1.
decodeWhenCacheAndUnknownKidShouldTriggerFetchOfJwkSet
ends up throwing an expection: org.springframework.security.oauth2.jwt.BadJwtException: An error occurred while attempting to decode the Jwt: Signed JWT rejected: Another algorithm expected, or no matching key(s) found
, since the JWK set is only fetched from the cache.
The second test decodeWithoutCacheSpecifiedAndUnknownKidShouldTriggerFetchOfJwkSet
works as expected, and handles a new KID being introduced properly.
Thanks for the tests @tinolazreg! I've opened a PR (#11638) for a possible fix based on my suggested workaround. Would you mind reviewing and verify if it would fix the issue?
Thank you for picking this up quickly!
I tested your branch against my local setup, and we still have issues when using a provided Cache. The problem now seems to be here: https://bitbucket.org/connect2id/nimbus-jose-jwt/src/43b5435c548d70ce250afe6f932a6a55b3833bb8/src/main/java/com/nimbusds/jose/jwk/source/RemoteJWKSet.java#lines-484. The if statement here evaluates to false, since the code tests if the instance of jwkSetCache.get()
is the same as an earlier jwkSetCache.get()
call in the same method.
I would assume that this is because SpringJwkSetCache.get() returns a new instance, while DefaultJwkSetCache.get() ends up returning the same instance.
Oh, right. I did that to ensure we're caching the same thing (JSON) and not some kind of object. I'll take a look at that after I come back from my trip in a week. Thanks for testing it for me!
Oh, right. I did that to ensure we're caching the same thing (JSON) and not some kind of object. I'll take a look at that after I come back from my trip in a week. Thanks for testing it for me!
Any update on this? :)
Thanks for the nudge @tinolazreg! I am just getting back from my trip and this is definitely on the list. I'll let you know as soon as I have an update.
@tinolazreg I've pushed an update to my branch if you want to try it out. Thanks!
@tinolazreg I've pushed an update to my branch if you want to try it out. Thanks!
It works great, good job! I tested it towards my local setup now, and the jwk-set is refreshed as expected when encountering an unknown KID.