From 0d0617d44af197e627af378438357e2d43242a69 Mon Sep 17 00:00:00 2001 From: Takashi Norimatsu Date: Sun, 5 Apr 2020 14:05:30 +0900 Subject: [PATCH] KEYCLOAK-13720 Specify Signature Algorithm in Signed JWT Client Authentication --- .../client/JWTClientAuthenticator.java | 15 ++++++ .../oidc/OIDCAdvancedConfigWrapper.java | 8 +++ .../protocol/oidc/OIDCConfigAttributes.java | 2 + .../oidc/DescriptionConverter.java | 5 ++ .../client/OIDCClientRegistrationTest.java | 31 +++++++++++ .../oauth/ClientAuthSignedJWTTest.java | 52 +++++++++++++++++-- .../messages/admin-messages_en.properties | 2 + .../admin/resources/js/controllers/clients.js | 3 ++ .../partials/client-credentials-jwt.html | 15 ++++++ 9 files changed, 129 insertions(+), 4 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientAuthenticator.java index 16d263c49d..770dcaf520 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientAuthenticator.java @@ -42,6 +42,7 @@ import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.ClientModel; import org.keycloak.models.RealmModel; import org.keycloak.models.SingleUseTokenStoreProvider; +import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.OIDCLoginProtocolService; import org.keycloak.provider.ProviderConfigProperty; @@ -117,6 +118,20 @@ public class JWTClientAuthenticator extends AbstractClientAuthenticator { return; } + String expectedSignatureAlg = OIDCAdvancedConfigWrapper.fromClientModel(client).getTokenEndpointAuthSigningAlg(); + if (jws.getHeader().getAlgorithm() == null || jws.getHeader().getAlgorithm().name() == null) { + Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "invalid_client", "invalid signature algorithm"); + context.challenge(challengeResponse); + return; + } + + String actualSignatureAlg = jws.getHeader().getAlgorithm().name(); + if (expectedSignatureAlg != null && !expectedSignatureAlg.equals(actualSignatureAlg)) { + Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "invalid_client", "invalid signature algorithm"); + context.challenge(challengeResponse); + return; + } + // Get client key and validate signature PublicKey clientPublicKey = getSignatureValidationKey(client, context, jws); if (clientPublicKey == null) { diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCAdvancedConfigWrapper.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCAdvancedConfigWrapper.java index cfc396c286..3cf255b8e9 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCAdvancedConfigWrapper.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCAdvancedConfigWrapper.java @@ -149,6 +149,14 @@ public class OIDCAdvancedConfigWrapper { setAttribute(OIDCConfigAttributes.ID_TOKEN_ENCRYPTED_RESPONSE_ENC, encName); } + public String getTokenEndpointAuthSigningAlg() { + return getAttribute(OIDCConfigAttributes.TOKEN_ENDPOINT_AUTH_SIGNING_ALG); + } + + public void setTokenEndpointAuthSigningAlg(String algName) { + setAttribute(OIDCConfigAttributes.TOKEN_ENDPOINT_AUTH_SIGNING_ALG, algName); + } + private String getAttribute(String attrKey) { if (clientModel != null) { return clientModel.getAttribute(attrKey); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCConfigAttributes.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCConfigAttributes.java index 5ddac4e681..832a8a9d39 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCConfigAttributes.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCConfigAttributes.java @@ -48,6 +48,8 @@ public final class OIDCConfigAttributes { public static final String CLIENT_SESSION_MAX_LIFESPAN = "client.session.max.lifespan"; public static final String PKCE_CODE_CHALLENGE_METHOD = "pkce.code.challenge.method"; + public static final String TOKEN_ENDPOINT_AUTH_SIGNING_ALG = "token.endpoint.auth.signing.alg"; + private OIDCConfigAttributes() { } diff --git a/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java b/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java index bb1d9d5284..fb316f731e 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java @@ -133,6 +133,8 @@ public class DescriptionConverter { configWrapper.setIdTokenEncryptedResponseEnc(clientOIDC.getIdTokenEncryptedResponseEnc()); } + configWrapper.setTokenEndpointAuthSigningAlg(clientOIDC.getTokenEndpointAuthSigningAlg()); + return client; } @@ -222,6 +224,9 @@ public class DescriptionConverter { if (config.getIdTokenEncryptedResponseEnc() != null) { response.setIdTokenEncryptedResponseEnc(config.getIdTokenEncryptedResponseEnc()); } + if (config.getTokenEndpointAuthSigningAlg() != null) { + response.setTokenEndpointAuthSigningAlg(config.getTokenEndpointAuthSigningAlg()); + } List foundPairwiseMappers = PairwiseSubMapperUtils.getPairwiseSubMappers(client); SubjectType subjectType = foundPairwiseMappers.isEmpty() ? SubjectType.PUBLIC : SubjectType.PAIRWISE; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java index e29c97ef6a..2792d992a4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OIDCClientRegistrationTest.java @@ -347,6 +347,37 @@ public class OIDCClientRegistrationTest extends AbstractClientRegistrationTest { } } + @Test + public void testTokenEndpointSigningAlg() throws Exception { + OIDCClientRepresentation response = null; + OIDCClientRepresentation updated = null; + try { + OIDCClientRepresentation clientRep = createRep(); + clientRep.setTokenEndpointAuthSigningAlg(Algorithm.ES256.toString()); + + response = reg.oidc().create(clientRep); + Assert.assertEquals(Algorithm.ES256.toString(), response.getTokenEndpointAuthSigningAlg()); + + ClientRepresentation kcClient = getClient(response.getClientId()); + OIDCAdvancedConfigWrapper config = OIDCAdvancedConfigWrapper.fromClientRepresentation(kcClient); + Assert.assertEquals(Algorithm.ES256.toString(), config.getTokenEndpointAuthSigningAlg()); + + reg.auth(Auth.token(response)); + response.setTokenEndpointAuthSigningAlg(null); + updated = reg.oidc().update(response); + Assert.assertEquals(null, response.getTokenEndpointAuthSigningAlg()); + + kcClient = getClient(updated.getClientId()); + config = OIDCAdvancedConfigWrapper.fromClientRepresentation(kcClient); + Assert.assertEquals(null, config.getTokenEndpointAuthSigningAlg()); + } finally { + // revert + reg.auth(Auth.token(updated)); + updated.setTokenEndpointAuthSigningAlg(null); + reg.oidc().update(updated); + } + } + @Test public void testOIDCEndpointCreateWithSamlClient() throws Exception { ClientsResource clientsResource = adminClient.realm(TEST).clients(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java index 9a63d6d7a7..592907216f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java @@ -52,6 +52,7 @@ import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventType; +import org.keycloak.jose.jwe.JWEException; import org.keycloak.jose.jws.JWSBuilder; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; @@ -84,6 +85,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.UnsupportedEncodingException; import java.net.URL; import java.nio.file.Files; import java.security.KeyFactory; @@ -304,6 +306,27 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { testECDSASignatureLength(getClientSignedToken(Algorithm.ES512), Algorithm.ES512); } + @Test + public void testCodeToTokenRequestSuccessES256Enforced() throws Exception { + ClientResource clientResource = null; + ClientRepresentation clientRep = null; + try { + clientResource = ApiUtil.findClientByClientId(adminClient.realm("test"), "client2"); + clientRep = clientResource.toRepresentation(); + OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setTokenEndpointAuthSigningAlg(Algorithm.ES256); + clientResource.update(clientRep); + + testCodeToTokenRequestSuccess(Algorithm.ES256); + } catch (Exception e) { + Assert.fail(); + } finally { + clientResource = ApiUtil.findClientByClientId(adminClient.realm("test"), "client2"); + clientRep = clientResource.toRepresentation(); + OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setTokenEndpointAuthSigningAlg(null); + clientResource.update(clientRep); + } + } + private void testECDSASignatureLength(String clientSignedToken, String alg) { String encodedSignature = clientSignedToken.split("\\.",3)[2]; byte[] signature = Base64Url.decode(encodedSignature); @@ -911,10 +934,31 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { @Test public void testCodeToTokenRequestFailureRS256() throws Exception { - testCodeToTokenRequestFailure(Algorithm.RS256); + testCodeToTokenRequestFailure(Algorithm.RS256, "unauthorized_client", "client_credentials_setup_required"); } - private void testCodeToTokenRequestFailure(String algorithm) throws Exception { + @Test + public void testCodeToTokenRequestFailureES256Enforced() throws Exception { + ClientResource clientResource = null; + ClientRepresentation clientRep = null; + try { + clientResource = ApiUtil.findClientByClientId(adminClient.realm("test"), "client2"); + clientRep = clientResource.toRepresentation(); + OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setTokenEndpointAuthSigningAlg(Algorithm.ES256); + clientResource.update(clientRep); + + testCodeToTokenRequestFailure(Algorithm.RS256, "invalid_client", "invalid_client_credentials"); + } catch (Exception e) { + Assert.fail(); + } finally { + clientResource = ApiUtil.findClientByClientId(adminClient.realm("test"), "client2"); + clientRep = clientResource.toRepresentation(); + OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).setTokenEndpointAuthSigningAlg(null); + clientResource.update(clientRep); + } + } + + private void testCodeToTokenRequestFailure(String algorithm, String error, String description) throws Exception { ClientRepresentation clientRepresentation = app2; ClientResource clientResource = getClient(testRealm.getRealm(), clientRepresentation.getId()); clientRepresentation = clientResource.toRepresentation(); @@ -935,13 +979,13 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClient2SignedJWT()); assertEquals(400, response.getStatusCode()); - assertEquals("unauthorized_client", response.getError()); + assertEquals(error, response.getError()); events.expect(EventType.CODE_TO_TOKEN_ERROR) .client("client2") .session((String) null) .clearDetails() - .error("client_credentials_setup_required") + .error(description) .user((String) null) .assertEvent(); } finally { diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties index 6b5c42f457..a74cb8578f 100644 --- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties +++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties @@ -414,6 +414,8 @@ gen-client-private-key=Generate Client Private Key generate-private-key=Generate Private Key kid=Kid kid.tooltip=KID (Key ID) of the client public key from imported JWKS. +token-endpoint-auth-signing-alg=Signature Algorithm +token-endpoint-auth-signing-alg.tooltip=JWA algorithm, which the client needs to use when signing a JWT for authentication. If left blank, the client is allowed to use any algorithm. use-jwks-url=Use JWKS URL use-jwks-url.tooltip=If the switch is on, client public keys will be downloaded from given JWKS URL. This allows great flexibility because new keys will be always re-downloaded again when client generates new keypair. If the switch is off, public key (or certificate) from the Keycloak DB is used, so when client keypair changes, you always need to import new key (or certificate) to the Keycloak DB as well. jwks-url=JWKS URL diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js index 7768762c74..2c4d2df9db 100755 --- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/controllers/clients.js @@ -227,6 +227,8 @@ module.controller('ClientSignedJWTCtrl', function($scope, $location, Client, Cli } }, true); + $scope.tokenEndpointAuthSigningAlg = $scope.client.attributes['token.endpoint.auth.signing.alg']; + if ($scope.client.attributes["use.jwks.url"]) { if ($scope.client.attributes["use.jwks.url"] == "true") { $scope.useJwksUrl = true; @@ -240,6 +242,7 @@ module.controller('ClientSignedJWTCtrl', function($scope, $location, Client, Cli } $scope.save = function() { + $scope.client.attributes['token.endpoint.auth.signing.alg'] = $scope.tokenEndpointAuthSigningAlg; if ($scope.useJwksUrl == true) { $scope.client.attributes["use.jwks.url"] = "true"; diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/client-credentials-jwt.html b/themes/src/main/resources/theme/base/admin/resources/partials/client-credentials-jwt.html index b2a5bf44b5..98c167d595 100644 --- a/themes/src/main/resources/theme/base/admin/resources/partials/client-credentials-jwt.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/client-credentials-jwt.html @@ -1,5 +1,20 @@
+
+ +
+
+ +
+
+ {{:: 'token-endpoint-auth-signing-alg.tooltip' | translate}} +
+