KEYCLOAK-14207 Client Policy - Executor : Enforce more secure client signature algorithm when client registration

services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmEnforceExecutor.java

@@ -0,0 +1,135 @@
+/*
+ * Copyright 2020 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.services.clientpolicy.executor;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.commons.compress.utils.Sets;
+import org.jboss.logging.Logger;
+
+import org.keycloak.OAuthErrorException;
+import org.keycloak.component.ComponentModel;
+import org.keycloak.crypto.Algorithm;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.protocol.oidc.OIDCConfigAttributes;
+import org.keycloak.representations.idm.ClientRepresentation;
+import org.keycloak.services.clientpolicy.AdminClientRegisterContext;
+import org.keycloak.services.clientpolicy.AdminClientUpdateContext;
+import org.keycloak.services.clientpolicy.ClientPolicyContext;
+import org.keycloak.services.clientpolicy.ClientPolicyException;
+import org.keycloak.services.clientpolicy.ClientPolicyLogger;
+import org.keycloak.services.clientpolicy.DynamicClientRegisterContext;
+import org.keycloak.services.clientpolicy.DynamicClientUpdateContext;
+
+public class SecureSigningAlgorithmEnforceExecutor implements ClientPolicyExecutorProvider {
+
+    private static final Logger logger = Logger.getLogger(SecureSigningAlgorithmEnforceExecutor.class);
+
+    private final KeycloakSession session;
+    private final ComponentModel componentModel;
+
+    private static final List<String> sigTargets = Arrays.asList(
+            OIDCConfigAttributes.USER_INFO_RESPONSE_SIGNATURE_ALG,
+            OIDCConfigAttributes.REQUEST_OBJECT_SIGNATURE_ALG,
+            OIDCConfigAttributes.ID_TOKEN_SIGNED_RESPONSE_ALG,
+            OIDCConfigAttributes.TOKEN_ENDPOINT_AUTH_SIGNING_ALG);
+
+    private static final List<String> sigTargetsAdminRestApiOnly = Arrays.asList(
+            OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG);
+
+    public SecureSigningAlgorithmEnforceExecutor(KeycloakSession session, ComponentModel componentModel) {
+        this.session = session;
+        this.componentModel = componentModel;
+    }
+
+    @Override
+    public String getName() {
+        return componentModel.getName();
+    }
+
+    @Override
+    public String getProviderId() {
+        return componentModel.getProviderId();
+    }
+
+    @Override
+    public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException {
+        switch (context.getEvent()) {
+        case REGISTER:
+            if (context instanceof AdminClientRegisterContext) {
+                verifySecureSigningAlgorithm(((AdminClientRegisterContext)context).getProposedClientRepresentation(), true, false);
+            } else if (context instanceof DynamicClientRegisterContext) {
+                verifySecureSigningAlgorithm(((DynamicClientRegisterContext)context).getProposedClientRepresentation(), false, false);
+            } else {
+                throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "not allowed input format.");
+            }
+            break;
+        case UPDATE:
+            if (context instanceof AdminClientUpdateContext) {
+                verifySecureSigningAlgorithm(((AdminClientUpdateContext)context).getProposedClientRepresentation(), true, true);
+            } else if (context instanceof DynamicClientUpdateContext) {
+                verifySecureSigningAlgorithm(((DynamicClientUpdateContext)context).getProposedClientRepresentation(), false, true);
+            } else {
+                throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "not allowed input format.");
+            }
+            break;
+        default:
+            return;
+        }
+    }
+
+    private void verifySecureSigningAlgorithm(ClientRepresentation clientRep, boolean byAdminRestApi, boolean isUpdate) throws ClientPolicyException {
+        if (clientRep.getAttributes() == null) {
+            throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "no signature algorithm was specified.");
+        }
+
+        for (String sigTarget : sigTargets) {
+            verifySecureSigningAlgorithm(sigTarget, clientRep.getAttributes().get(sigTarget));
+        }
+
+        // no client metadata found in RFC 7591 OAuth Dynamic Client Registration Metadata
+        if (byAdminRestApi) {
+            for (String sigTarget : sigTargetsAdminRestApiOnly) {
+                verifySecureSigningAlgorithm(sigTarget, clientRep.getAttributes().get(sigTarget));
+            }
+        }
+    }
+
+    private void verifySecureSigningAlgorithm(String sigTarget, String sigAlg) throws ClientPolicyException {
+        if (sigAlg == null) {
+            ClientPolicyLogger.logv(logger, "Signing algorithm not specified explicitly. signature target = {0}", sigTarget);
+            return;
+        }
+        switch (sigAlg) {
+        case Algorithm.PS256:
+        case Algorithm.PS384:
+        case Algorithm.PS512:
+        case Algorithm.ES256:
+        case Algorithm.ES384:
+        case Algorithm.ES512:
+            ClientPolicyLogger.logv(logger, "Passed. signature target = {0}, signature algorithm = {1}", sigTarget, sigAlg);
+            return;
+        }
+        ClientPolicyLogger.logv(logger, "NOT allowed signatureAlgorithm. signature target = {0}, signature algorithm = {1}", sigTarget, sigAlg);
+        throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "not allowed signature algorithm.");
+    }
+
+}

services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmEnforceExecutorFactory.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright 2020 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.services.clientpolicy.executor;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.keycloak.Config.Scope;
+import org.keycloak.component.ComponentModel;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.KeycloakSessionFactory;
+import org.keycloak.provider.ProviderConfigProperty;
+
+public class SecureSigningAlgorithmEnforceExecutorFactory implements ClientPolicyExecutorProviderFactory {
+
+    public static final String PROVIDER_ID = "securesignalg-enforce-executor";
+
+    @Override
+    public ClientPolicyExecutorProvider create(KeycloakSession session, ComponentModel model) {
+        return new SecureSigningAlgorithmEnforceExecutor(session, model);
+    }
+
+    @Override
+    public void init(Scope config) {
+    }
+
+    @Override
+    public void postInit(KeycloakSessionFactory factory) {
+    }
+
+    @Override
+    public void close() {
+    }
+
+    @Override
+    public String getId() {
+        return PROVIDER_ID;
+    }
+
+    @Override
+    public String getHelpText() {
+        return "It refuses the client whose signature algorithms are considered not to be secure. It accepts ES256, ES384, ES512, PS256, PS384 and PS512.";
+    }
+
+    @Override
+    public List<ProviderConfigProperty> getConfigProperties() {
+        return Collections.emptyList();
+    }
+
+}

services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory

@@ -2,4 +2,5 @@ org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory
 org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory
 org.keycloak.services.clientpolicy.executor.SecureClientAuthEnforceExecutorFactory
 org.keycloak.services.clientpolicy.executor.PKCEEnforceExecutorFactory
-org.keycloak.services.clientpolicy.executor.SecureSessionEnforceExecutorFactory
+org.keycloak.services.clientpolicy.executor.SecureSessionEnforceExecutorFactory
+org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmEnforceExecutorFactory

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPolicyBasicsTest.java

@@ -29,6 +29,7 @@ import java.security.MessageDigest;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.function.Consumer;
 
@@ -62,6 +63,7 @@ import org.keycloak.events.EventType;
 import org.keycloak.jose.jws.Algorithm;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
+import org.keycloak.protocol.oidc.OIDCConfigAttributes;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.utils.OIDCResponseType;
 import org.keycloak.representations.AccessToken;
@@ -90,6 +92,7 @@ import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutor;
 import org.keycloak.services.clientpolicy.executor.SecureRequestObjectExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.SecureResponseTypeExecutorFactory;
 import org.keycloak.services.clientpolicy.executor.SecureSessionEnforceExecutorFactory;
+import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmEnforceExecutorFactory;
 import org.keycloak.testsuite.AbstractKeycloakTest;
 import org.keycloak.testsuite.AssertEvents;
 import org.keycloak.testsuite.admin.ApiUtil;
@@ -860,6 +863,121 @@ public class ClientPolicyBasicsTest extends AbstractKeycloakTest {
         }
     }
 
+    @Test
+    public void testSecureSigningAlgorithmEnforceExecutor() throws ClientRegistrationException, ClientPolicyException {
+        String policyName = "MyPolicy";
+        createPolicy(policyName, DefaultClientPolicyProviderFactory.PROVIDER_ID, null, null, null);
+        logger.info("... Created Policy : " + policyName);
+
+        createCondition("ClientUpdateContextCondition", ClientUpdateContextConditionFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> {
+            setConditionRegistrationMethods(provider, new ArrayList<>(Arrays.asList(
+                    ClientUpdateContextConditionFactory.BY_AUTHENTICATED_USER,
+                    ClientUpdateContextConditionFactory.BY_INITIAL_ACCESS_TOKEN,
+                    ClientUpdateContextConditionFactory.BY_REGISTRATION_ACCESS_TOKEN)));
+        });
+        registerCondition("ClientUpdateContextCondition", policyName);
+        logger.info("... Registered Condition : ClientUpdateContextConditionFactory");
+
+        createExecutor("SecureSigningAlgorithmEnforceExecutor", SecureSigningAlgorithmEnforceExecutorFactory.PROVIDER_ID, null, (ComponentRepresentation provider) -> {
+        });
+        registerExecutor("SecureSigningAlgorithmEnforceExecutor", policyName);
+        logger.info("... Registered Executor : SecureSigningAlgorithmEnforceExecutor");
+
+        String cAppAdminId = null;
+        String cAppDynamicId = null;
+        try {
+
+            // create by Admin REST API - fail
+            try {
+                createClientByAdmin("App-by-Admin", (ClientRepresentation clientRep) -> {
+                    clientRep.setDefaultRoles((String[]) Arrays.asList("sample-client-role-beta").toArray(new String[1]));
+                    clientRep.setSecret("secretBeta");
+                    clientRep.setAttributes(new HashMap<>());
+                    clientRep.getAttributes().put(OIDCConfigAttributes.USER_INFO_RESPONSE_SIGNATURE_ALG, Algorithm.none.name());
+                });
+                fail();
+            } catch (ClientPolicyException e) {
+                assertEquals(Errors.INVALID_REGISTRATION, e.getMessage());
+            }
+
+            // create by Admin REST API - success
+            cAppAdminId = createClientByAdmin("App-by-Admin", (ClientRepresentation clientRep) -> {
+                clientRep.setAttributes(new HashMap<>());
+                clientRep.getAttributes().put(OIDCConfigAttributes.USER_INFO_RESPONSE_SIGNATURE_ALG, Algorithm.PS256.name());
+                clientRep.getAttributes().put(OIDCConfigAttributes.REQUEST_OBJECT_SIGNATURE_ALG, Algorithm.ES256.name());
+                clientRep.getAttributes().put(OIDCConfigAttributes.ID_TOKEN_SIGNED_RESPONSE_ALG, Algorithm.ES256.name());
+                clientRep.getAttributes().put(OIDCConfigAttributes.TOKEN_ENDPOINT_AUTH_SIGNING_ALG, Algorithm.ES256.name());
+                clientRep.getAttributes().put(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG, Algorithm.ES256.name());
+            });
+
+            // update by Admin REST API - fail
+            try {
+            updateClientByAdmin(cAppAdminId, (ClientRepresentation clientRep) -> {
+                clientRep.setAttributes(new HashMap<>());
+                clientRep.getAttributes().put(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG, Algorithm.RS512.name());
+            });
+            } catch (Exception e) {
+                assertEquals("HTTP 400 Bad Request", e.getMessage());
+            }
+            ClientRepresentation cRep = getClientByAdmin(cAppAdminId);
+            assertEquals(Algorithm.ES256.name(), cRep.getAttributes().get(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG));
+
+            // update by Admin REST API - success
+            updateClientByAdmin(cAppAdminId, (ClientRepresentation clientRep) -> {
+                clientRep.setAttributes(new HashMap<>());
+                clientRep.getAttributes().put(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG, Algorithm.PS384.name());
+            });
+            cRep = getClientByAdmin(cAppAdminId);
+            assertEquals(Algorithm.PS384.name(), cRep.getAttributes().get(OIDCConfigAttributes.ACCESS_TOKEN_SIGNED_RESPONSE_ALG));
+
+            // create dynamically - fail
+            try {
+                createClientByAdmin("App-in-Dynamic", (ClientRepresentation clientRep) -> {
+                    clientRep.setDefaultRoles((String[]) Arrays.asList("sample-client-role-beta").toArray(new String[1]));
+                    clientRep.setSecret("secretBeta");
+                    clientRep.setAttributes(new HashMap<>());
+                    clientRep.getAttributes().put(OIDCConfigAttributes.USER_INFO_RESPONSE_SIGNATURE_ALG, Algorithm.RS384.name());
+                });
+                fail();
+            } catch (ClientPolicyException e) {
+                assertEquals(Errors.INVALID_REGISTRATION, e.getMessage());
+            }
+
+            // create dynamically - success
+            cAppDynamicId = createClientDynamically("App-in-Dynamic", (OIDCClientRepresentation clientRep) -> {
+                clientRep.setUserinfoSignedResponseAlg(Algorithm.ES256.name());
+                clientRep.setRequestObjectSigningAlg(Algorithm.ES256.name());
+                clientRep.setIdTokenSignedResponseAlg(Algorithm.PS256.name());
+                clientRep.setTokenEndpointAuthSigningAlg(Algorithm.PS256.name());
+            });
+            events.expect(EventType.CLIENT_REGISTER).client(cAppDynamicId).user(Matchers.isEmptyOrNullString()).assertEvent();
+            getClientDynamically(cAppDynamicId);
+
+            // update dynamically - fail
+            try {
+                 updateClientDynamically(cAppDynamicId, (OIDCClientRepresentation clientRep) -> {
+                     clientRep.setIdTokenSignedResponseAlg(Algorithm.RS256.name());
+                 });
+                fail();
+            } catch (ClientRegistrationException e) {
+                assertEquals("Failed to send request", e.getMessage());
+            }
+            OIDCClientRepresentation oidcCRep = getClientDynamically(cAppDynamicId);
+            assertEquals(Algorithm.PS256.name(), oidcCRep.getIdTokenSignedResponseAlg());
+
+            // update dynamically - success
+            updateClientDynamically(cAppDynamicId, (OIDCClientRepresentation clientRep) -> {
+                clientRep.setIdTokenSignedResponseAlg(Algorithm.ES384.name());
+            });
+            oidcCRep = getClientDynamically(cAppDynamicId);
+            assertEquals(Algorithm.ES384.name(), oidcCRep.getIdTokenSignedResponseAlg());
+
+        } finally {
+            deleteClientByAdmin(cAppAdminId);
+            deleteClientDynamically(cAppDynamicId);
+        }
+    }
+
     private AuthorizationEndpointRequestObject createValidRequestObjectForSecureRequestObjectExecutor(String clientId) throws URISyntaxException {
         AuthorizationEndpointRequestObject requestObject = new AuthorizationEndpointRequestObject();
         requestObject.id(KeycloakModelUtils.generateId());