From dc3b037e3a9c9d77aedc144cfc14e329a31e748b Mon Sep 17 00:00:00 2001 From: mposolda Date: Thu, 20 Apr 2023 16:35:09 +0200 Subject: [PATCH] Incorrect Signature algorithms presented by Client Authenticator closes #15853 Co-authored-by: Jon Koops --- .../info/CryptoInfoRepresentation.java | 24 ++++++++++++++++++- .../public/locales/en/clients-help.json | 2 +- .../src/clients/credentials/Credentials.tsx | 4 +++- .../src/clients/credentials/SignedJWT.tsx | 17 +++++++++---- .../src/defs/serverInfoRepesentation.ts | 2 ++ .../ClientSignatureVerifierProvider.java | 4 ++++ ...metricClientSignatureVerifierProvider.java | 9 +++++++ .../ECDSAClientSignatureVerifierProvider.java | 10 ++++++++ ...SecretClientSignatureVerifierProvider.java | 10 ++++++++ .../admin/info/ServerInfoAdminResource.java | 17 ++++++++++++- .../testsuite/admin/ServerInfoTest.java | 6 +++++ 11 files changed, 96 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/keycloak/representations/info/CryptoInfoRepresentation.java b/core/src/main/java/org/keycloak/representations/info/CryptoInfoRepresentation.java index 97f77db909..8c582ddd2f 100644 --- a/core/src/main/java/org/keycloak/representations/info/CryptoInfoRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/info/CryptoInfoRepresentation.java @@ -34,7 +34,11 @@ public class CryptoInfoRepresentation { private String cryptoProvider; private List supportedKeystoreTypes; - public static CryptoInfoRepresentation create() { + private List clientSignatureSymmetricAlgorithms; + + private List clientSignatureAsymmetricAlgorithms; + + public static CryptoInfoRepresentation create(List clientSignatureSymmetricAlgorithms, List clientSignatureAsymmetricAlgorithms) { CryptoInfoRepresentation info = new CryptoInfoRepresentation(); CryptoProvider cryptoProvider = CryptoIntegration.getProvider(); @@ -42,6 +46,8 @@ public class CryptoInfoRepresentation { info.supportedKeystoreTypes = CryptoIntegration.getProvider().getSupportedKeyStoreTypes() .map(KeystoreUtil.KeystoreFormat::toString) .collect(Collectors.toList()); + info.clientSignatureSymmetricAlgorithms = clientSignatureSymmetricAlgorithms; + info.clientSignatureAsymmetricAlgorithms = clientSignatureAsymmetricAlgorithms; return info; } @@ -61,4 +67,20 @@ public class CryptoInfoRepresentation { public void setSupportedKeystoreTypes(List supportedKeystoreTypes) { this.supportedKeystoreTypes = supportedKeystoreTypes; } + + public List getClientSignatureSymmetricAlgorithms() { + return clientSignatureSymmetricAlgorithms; + } + + public void setClientSignatureSymmetricAlgorithms(List clientSignatureSymmetricAlgorithms) { + this.clientSignatureSymmetricAlgorithms = clientSignatureSymmetricAlgorithms; + } + + public List getClientSignatureAsymmetricAlgorithms() { + return clientSignatureAsymmetricAlgorithms; + } + + public void setClientSignatureAsymmetricAlgorithms(List clientSignatureAsymmetricAlgorithms) { + this.clientSignatureAsymmetricAlgorithms = clientSignatureAsymmetricAlgorithms; + } } diff --git a/js/apps/admin-ui/public/locales/en/clients-help.json b/js/apps/admin-ui/public/locales/en/clients-help.json index 1968c23658..94dda03c05 100644 --- a/js/apps/admin-ui/public/locales/en/clients-help.json +++ b/js/apps/admin-ui/public/locales/en/clients-help.json @@ -53,7 +53,7 @@ "count": "Specifies how many clients can be created using the token", "client-authenticator-type": "Client Authenticator used for authentication of this client against Keycloak server", "registration-access-token": "The registration access token provides access for clients to the client registration service.", - "signature-algorithm": "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.", + "signature-algorithm": "JWA algorithm, which the client needs to use when signing a JWT for authentication. If left blank, the client is allowed to use any appropriate algorithm for the particular client authenticator.", "anonymousAccessPolicies": "Those Policies are used when the Client Registration Service is invoked by unauthenticated request. This means that the request does not contain Initial Access Token nor Bearer Token.", "authenticatedAccessPolicies": "Those Policies are used when Client Registration Service is invoked by authenticated request. This means that the request contains Initial Access Token or Bearer Token.", "allowRegexComparison": "If OFF, then the Subject DN from given client certificate must exactly match the given DN from the 'Subject DN' property as described in the RFC8705 specification. The Subject DN can be in the RFC2553 or RFC1779 format. If ON, then the Subject DN from given client certificate should match regex specified by 'Subject DN' property.", diff --git a/js/apps/admin-ui/src/clients/credentials/Credentials.tsx b/js/apps/admin-ui/src/clients/credentials/Credentials.tsx index 054618c062..b377aedaef 100644 --- a/js/apps/admin-ui/src/clients/credentials/Credentials.tsx +++ b/js/apps/admin-ui/src/clients/credentials/Credentials.tsx @@ -185,7 +185,9 @@ export const Credentials = ({ client, save, refresh }: CredentialsProps) => { /> {(clientAuthenticatorType === "client-jwt" || - clientAuthenticatorType === "client-secret-jwt") && } + clientAuthenticatorType === "client-secret-jwt") && ( + + )} {clientAuthenticatorType === "client-jwt" && ( diff --git a/js/apps/admin-ui/src/clients/credentials/SignedJWT.tsx b/js/apps/admin-ui/src/clients/credentials/SignedJWT.tsx index 289da50676..4ba04d3590 100644 --- a/js/apps/admin-ui/src/clients/credentials/SignedJWT.tsx +++ b/js/apps/admin-ui/src/clients/credentials/SignedJWT.tsx @@ -10,14 +10,21 @@ import { import { useServerInfo } from "../../context/server-info/ServerInfoProvider"; import { HelpItem } from "ui-shared"; -import { convertAttributeNameToForm, sortProviders } from "../../util"; +import { convertAttributeNameToForm } from "../../util"; import { FormFields } from "../ClientDetails"; -export const SignedJWT = () => { +type SignedJWTProps = { + clientAuthenticatorType: string; +}; + +export const SignedJWT = ({ clientAuthenticatorType }: SignedJWTProps) => { const { control } = useFormContext(); - const providers = sortProviders( - useServerInfo().providers!.clientSignature.providers - ); + const { cryptoInfo } = useServerInfo(); + const providers = + clientAuthenticatorType === "client-jwt" + ? cryptoInfo?.clientSignatureAsymmetricAlgorithms ?? [] + : cryptoInfo?.clientSignatureSymmetricAlgorithms ?? []; + const { t } = useTranslation("clients"); const [open, isOpen] = useState(false); diff --git a/js/libs/keycloak-admin-client/src/defs/serverInfoRepesentation.ts b/js/libs/keycloak-admin-client/src/defs/serverInfoRepesentation.ts index 6dcf20e1d5..ca49c6cabf 100644 --- a/js/libs/keycloak-admin-client/src/defs/serverInfoRepesentation.ts +++ b/js/libs/keycloak-admin-client/src/defs/serverInfoRepesentation.ts @@ -73,4 +73,6 @@ export interface ProtocolMapperTypeRepresentation { export interface CryptoInfoRepresentation { cryptoProvider: string; supportedKeystoreTypes: string[]; + clientSignatureSymmetricAlgorithms: string[]; + clientSignatureAsymmetricAlgorithms: string[]; } diff --git a/server-spi-private/src/main/java/org/keycloak/crypto/ClientSignatureVerifierProvider.java b/server-spi-private/src/main/java/org/keycloak/crypto/ClientSignatureVerifierProvider.java index a4224cbdc7..065fe8dfa5 100644 --- a/server-spi-private/src/main/java/org/keycloak/crypto/ClientSignatureVerifierProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/crypto/ClientSignatureVerifierProvider.java @@ -28,4 +28,8 @@ public interface ClientSignatureVerifierProvider extends Provider { @Override default void close() { } + + String getAlgorithm(); + + boolean isAsymmetricAlgorithm(); } diff --git a/services/src/main/java/org/keycloak/crypto/AsymmetricClientSignatureVerifierProvider.java b/services/src/main/java/org/keycloak/crypto/AsymmetricClientSignatureVerifierProvider.java index fc549451a8..f3b5cdd7dc 100644 --- a/services/src/main/java/org/keycloak/crypto/AsymmetricClientSignatureVerifierProvider.java +++ b/services/src/main/java/org/keycloak/crypto/AsymmetricClientSignatureVerifierProvider.java @@ -35,4 +35,13 @@ public class AsymmetricClientSignatureVerifierProvider implements ClientSignatur return new ClientAsymmetricSignatureVerifierContext(session, client, input); } + @Override + public String getAlgorithm() { + return algorithm; + } + + @Override + public boolean isAsymmetricAlgorithm() { + return true; + } } diff --git a/services/src/main/java/org/keycloak/crypto/ECDSAClientSignatureVerifierProvider.java b/services/src/main/java/org/keycloak/crypto/ECDSAClientSignatureVerifierProvider.java index 8182e7e280..229015cd24 100644 --- a/services/src/main/java/org/keycloak/crypto/ECDSAClientSignatureVerifierProvider.java +++ b/services/src/main/java/org/keycloak/crypto/ECDSAClientSignatureVerifierProvider.java @@ -18,4 +18,14 @@ public class ECDSAClientSignatureVerifierProvider implements ClientSignatureVeri public SignatureVerifierContext verifier(ClientModel client, JWSInput input) throws VerificationException { return new ClientECDSASignatureVerifierContext(session, client, input); } + + @Override + public String getAlgorithm() { + return algorithm; + } + + @Override + public boolean isAsymmetricAlgorithm() { + return true; + } } diff --git a/services/src/main/java/org/keycloak/crypto/MacSecretClientSignatureVerifierProvider.java b/services/src/main/java/org/keycloak/crypto/MacSecretClientSignatureVerifierProvider.java index 430dbd2026..f7d05b07f9 100644 --- a/services/src/main/java/org/keycloak/crypto/MacSecretClientSignatureVerifierProvider.java +++ b/services/src/main/java/org/keycloak/crypto/MacSecretClientSignatureVerifierProvider.java @@ -34,4 +34,14 @@ public class MacSecretClientSignatureVerifierProvider implements ClientSignature public SignatureVerifierContext verifier(ClientModel client, JWSInput input) throws VerificationException { return new ClientMacSignatureVerifierContext(session, client, algorithm); } + + @Override + public String getAlgorithm() { + return algorithm; + } + + @Override + public boolean isAsymmetricAlgorithm() { + return false; + } } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java index 9812ba645c..c01cb5b058 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java @@ -23,6 +23,7 @@ import org.keycloak.broker.provider.IdentityProviderFactory; import org.keycloak.broker.social.SocialIdentityProvider; import org.keycloak.common.Profile; import org.keycloak.component.ComponentFactory; +import org.keycloak.crypto.ClientSignatureVerifierProvider; import org.keycloak.events.EventType; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; @@ -96,7 +97,21 @@ public class ServerInfoAdminResource { info.setSystemInfo(SystemInfoRepresentation.create(session.getKeycloakSessionFactory().getServerStartupTimestamp())); info.setMemoryInfo(MemoryInfoRepresentation.create()); info.setProfileInfo(ProfileInfoRepresentation.create()); - info.setCryptoInfo(CryptoInfoRepresentation.create()); + + // True - asymmetric algorithms, false - symmetric algorithms + Map> algorithms = session.getAllProviders(ClientSignatureVerifierProvider.class).stream() + .collect( + Collectors.toMap( + ClientSignatureVerifierProvider::isAsymmetricAlgorithm, + clientSignatureVerifier -> Collections.singletonList(clientSignatureVerifier.getAlgorithm()), + (l1, l2) -> listCombiner(l1, l2) + .stream() + .sorted() + .collect(Collectors.toList()), + HashMap::new + ) + ); + info.setCryptoInfo(CryptoInfoRepresentation.create(algorithms.get(false), algorithms.get(true))); setSocialProviders(info); setIdentityProviders(info); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java index d1b45e1268..1b70feb34a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java @@ -19,6 +19,7 @@ package org.keycloak.testsuite.admin; import org.junit.Test; import org.keycloak.common.Version; +import org.keycloak.crypto.Algorithm; import org.keycloak.keys.Attributes; import org.keycloak.keys.GeneratedRsaKeyProviderFactory; import org.keycloak.keys.KeyProvider; @@ -68,6 +69,11 @@ public class ServerInfoTest extends AbstractKeycloakTest { assertNotNull(info.getSystemInfo()); assertNotNull(info.getCryptoInfo()); Assert.assertNames(info.getCryptoInfo().getSupportedKeystoreTypes(), KeystoreUtils.getSupportedKeystoreTypes()); + Assert.assertNames(info.getCryptoInfo().getClientSignatureSymmetricAlgorithms(), Algorithm.HS256, Algorithm.HS384, Algorithm.HS512); + Assert.assertNames(info.getCryptoInfo().getClientSignatureAsymmetricAlgorithms(), + Algorithm.ES256, Algorithm.ES384, Algorithm.ES512, + Algorithm.PS256, Algorithm.PS384, Algorithm.PS512, + Algorithm.RS256, Algorithm.RS384, Algorithm.RS512); ComponentTypeRepresentation rsaGeneratedProviderInfo = info.getComponentTypes().get(KeyProvider.class.getName()) .stream()