Refresh remote JWKs on all errors (#42850)

It turns out that key rotation on the OP, can manifest as both
a BadJWSException and a BadJOSEException in nimbus-jose-jwt. As
such we cannot depend on matching only BadJWSExceptions to
determine if we should poll the remote JWKs for an update.

This has the side-effect that a remote JWKs source will be polled
exactly one additional time too for errors that have to do with
configuration, or for errors that might be caused by not synched
clocks, forged JWTs, etc. ( These will throw a BadJWTException
which extends BadJOSEException also )
This commit is contained in:
Ioannis Kakavas 2019-06-11 10:55:50 +03:00
parent 6a77dde5ea
commit 1776d6e055
2 changed files with 28 additions and 9 deletions

View File

@ -12,7 +12,6 @@ import com.nimbusds.jose.jwk.JWKSelector;
import com.nimbusds.jose.jwk.JWKSet; import com.nimbusds.jose.jwk.JWKSet;
import com.nimbusds.jose.jwk.source.JWKSource; import com.nimbusds.jose.jwk.source.JWKSource;
import com.nimbusds.jose.proc.BadJOSEException; import com.nimbusds.jose.proc.BadJOSEException;
import com.nimbusds.jose.proc.BadJWSException;
import com.nimbusds.jose.proc.JWSVerificationKeySelector; import com.nimbusds.jose.proc.JWSVerificationKeySelector;
import com.nimbusds.jose.proc.SecurityContext; import com.nimbusds.jose.proc.SecurityContext;
import com.nimbusds.jose.util.IOUtils; import com.nimbusds.jose.util.IOUtils;
@ -241,7 +240,7 @@ public class OpenIdConnectAuthenticator {
} }
claimsListener.onResponse(enrichedVerifiedIdTokenClaims); claimsListener.onResponse(enrichedVerifiedIdTokenClaims);
} }
} catch (BadJWSException e) { } catch (BadJOSEException e) {
// We only try to update the cached JWK set once if a remote source is used and // We only try to update the cached JWK set once if a remote source is used and
// RSA or ECDSA is used for signatures // RSA or ECDSA is used for signatures
if (shouldRetry if (shouldRetry
@ -257,7 +256,7 @@ public class OpenIdConnectAuthenticator {
} else { } else {
claimsListener.onFailure(new ElasticsearchSecurityException("Failed to parse or validate the ID Token", e)); claimsListener.onFailure(new ElasticsearchSecurityException("Failed to parse or validate the ID Token", e));
} }
} catch (com.nimbusds.oauth2.sdk.ParseException | ParseException | BadJOSEException | JOSEException e) { } catch (com.nimbusds.oauth2.sdk.ParseException | ParseException | JOSEException e) {
claimsListener.onFailure(new ElasticsearchSecurityException("Failed to parse or validate the ID Token", e)); claimsListener.onFailure(new ElasticsearchSecurityException("Failed to parse or validate the ID Token", e));
} }
} }

View File

@ -320,7 +320,11 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token")); assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class)); assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("Expired JWT")); assertThat(e.getCause().getMessage(), containsString("Expired JWT"));
assertThat(callsToReloadJwk, equalTo(0)); if (jwk.getAlgorithm().getName().startsWith("HS")) {
assertThat(callsToReloadJwk, equalTo(0));
} else {
assertThat(callsToReloadJwk, equalTo(1));
}
} }
public void testImplicitFlowFailsNotYetIssuedToken() throws Exception { public void testImplicitFlowFailsNotYetIssuedToken() throws Exception {
@ -360,7 +364,11 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token")); assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class)); assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("JWT issue time ahead of current time")); assertThat(e.getCause().getMessage(), containsString("JWT issue time ahead of current time"));
assertThat(callsToReloadJwk, equalTo(0)); if (jwk.getAlgorithm().getName().startsWith("HS")) {
assertThat(callsToReloadJwk, equalTo(0));
} else {
assertThat(callsToReloadJwk, equalTo(1));
}
} }
public void testImplicitFlowFailsInvalidIssuer() throws Exception { public void testImplicitFlowFailsInvalidIssuer() throws Exception {
@ -399,7 +407,11 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token")); assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class)); assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("Unexpected JWT issuer")); assertThat(e.getCause().getMessage(), containsString("Unexpected JWT issuer"));
assertThat(callsToReloadJwk, equalTo(0)); if (jwk.getAlgorithm().getName().startsWith("HS")) {
assertThat(callsToReloadJwk, equalTo(0));
} else {
assertThat(callsToReloadJwk, equalTo(1));
}
} }
public void testImplicitFlowFailsInvalidAudience() throws Exception { public void testImplicitFlowFailsInvalidAudience() throws Exception {
@ -438,7 +450,11 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token")); assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class)); assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("Unexpected JWT audience")); assertThat(e.getCause().getMessage(), containsString("Unexpected JWT audience"));
assertThat(callsToReloadJwk, equalTo(0)); if (jwk.getAlgorithm().getName().startsWith("HS")) {
assertThat(callsToReloadJwk, equalTo(0));
} else {
assertThat(callsToReloadJwk, equalTo(1));
}
} }
public void testAuthenticateImplicitFlowFailsWithForgedRsaIdToken() throws Exception { public void testAuthenticateImplicitFlowFailsWithForgedRsaIdToken() throws Exception {
@ -611,7 +627,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token")); assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJOSEException.class)); assertThat(e.getCause(), instanceOf(BadJOSEException.class));
assertThat(e.getCause().getMessage(), containsString("Another algorithm expected, or no matching key(s) found")); assertThat(e.getCause().getMessage(), containsString("Another algorithm expected, or no matching key(s) found"));
assertThat(callsToReloadJwk, equalTo(0)); assertThat(callsToReloadJwk, equalTo(1));
} }
public void testImplicitFlowFailsWithUnsignedJwt() throws Exception { public void testImplicitFlowFailsWithUnsignedJwt() throws Exception {
@ -648,7 +664,11 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token")); assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class)); assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("Signed ID token expected")); assertThat(e.getCause().getMessage(), containsString("Signed ID token expected"));
assertThat(callsToReloadJwk, equalTo(0)); if (jwk.getAlgorithm().getName().startsWith("HS")) {
assertThat(callsToReloadJwk, equalTo(0));
} else {
assertThat(callsToReloadJwk, equalTo(1));
}
} }
public void testJsonObjectMerging() throws Exception { public void testJsonObjectMerging() throws Exception {