From 180ce8ccd8a66a0c1db73ce5b4da7f3fef7688ca Mon Sep 17 00:00:00 2001 From: Jimmy Chakkalakal Date: Tue, 11 Aug 2026 10:25:33 +0100 Subject: [PATCH] Stop storing client private keys in the database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generate endpoint stored client private keys as clear-text client attributes. The server never uses these keys for any cryptographic operation — it only needs the public key/certificate to verify signature. This change: - Stops persisting private keys in generate, upload, and SAML client creation endpoints - Strips private keys from getKeyInfo responses - Deprecates the generate, download, and generate-and-download endpoints - Updates admin UI to reflect that private keys are downloaded once and not stored - Updates documentation with guidance to generate keys externally and examples for converting keystores to JWK format for JWKS_URL usage Closes #49239 Signed-off-by: Jimmy Chakkalakal --- .../con-confidential-client-credentials.adoc | 89 ++++++++++++++----- .../saml/proc-creating-saml-client.adoc | 4 +- .../ClientAttributeCertificateResource.java | 12 +++ .../admin/messages/messages_en.properties | 4 +- .../src/clients/keys/ExportSamlKeyDialog.tsx | 1 + js/apps/admin-ui/src/clients/keys/Keys.tsx | 1 + .../admin-ui/src/clients/keys/SamlKeys.tsx | 15 +++- .../src/clients/keys/SamlKeysDialog.tsx | 4 +- .../jwt-authorization-grant.spec.ts | 2 +- js/eslint.config.js | 1 + .../src/resources/clients.ts | 12 +++ .../test/clients.spec.ts | 4 +- .../protocol/saml/SamlProtocolFactory.java | 2 - .../KeycloakSamlClientInstallation.java | 4 +- .../KeycloakSamlSubsystemCliInstallation.java | 4 +- .../oidc/DescriptionConverter.java | 3 +- .../ClientAttributeCertificateResource.java | 37 ++++++-- .../services/util/CertificateInfoHelper.java | 31 +++++-- .../tests/admin/client/CredentialsTest.java | 29 +++--- .../tests/admin/client/InstallationTest.java | 11 +-- .../AbstractClientAuthSignedJWTTest.java | 4 +- 21 files changed, 204 insertions(+), 70 deletions(-) diff --git a/docs/documentation/server_admin/topics/clients/oidc/con-confidential-client-credentials.adoc b/docs/documentation/server_admin/topics/clients/oidc/con-confidential-client-credentials.adoc index 4ac53416ada0..bc2689daed73 100644 --- a/docs/documentation/server_admin/topics/clients/oidc/con-confidential-client-credentials.adoc +++ b/docs/documentation/server_admin/topics/clients/oidc/con-confidential-client-credentials.adoc @@ -27,41 +27,90 @@ image:images/client-credentials-jwt.png[Signed JWT] In this authenticator you can enforce the *Signature algorithm* used by the client (any algorithm is valid by default) and the *Max expiration* allowed for the JWT token (tokens received after this period will not be accepted because they are too old, note that tokens should be issued right before the authentication, 60 seconds by default). -When choosing this credential type you will have to also generate a private key and certificate for the client in the tab `Keys`. The private key will be used to sign the JWT, while the certificate is used by the server to verify the signature. +When choosing this credential type you will have to configure the client's public key or certificate in the *Keys* tab. The client uses the private key to sign the JWT, while {project_name} uses the certificate or public key to verify the signature. {project_name} does not need and does not store the client's private key. -.Keys tab -image:images/client-oidc-keys.png[Keys tab] +The recommended approach is to generate the key pair externally and provide {project_name} with the public key using one of the methods below. -Click on the `Generate new keys` button to start this process. +*Using a JWKS URL (recommended)* -.Generate keys -image:images/generate-client-keys.png[] +Click *Use JWKS URL* and provide the URL where the public key is published in https://datatracker.ietf.org/doc/html/rfc7517[JWK] format. With this option, if the key is ever changed, {project_name} reimports the key automatically. -. Select the archive format you want to use. -. Enter a *key password*. -. Enter a *store password*. -. Click *Generate*. +To generate a key pair and convert it to JWK format: + +. Generate a keystore with a key pair: ++ +[source,bash] +---- +keytool -genkeypair -alias mykey -keyalg RSA -keysize 4096 \ + -validity 1095 -storetype PKCS12 \ + -keystore keystore.p12 -storepass changeit \ + -dname "CN=my-client" +---- + +. Export the certificate from the keystore: ++ +[source,bash] +---- +keytool -exportcert -alias mykey -keystore keystore.p12 \ + -storepass changeit -rfc -file certificate.pem +---- + +. Convert the keystore to JWK format. You can use a library or tool such as https://connect2id.com/products/nimbus-jose-jwt[Nimbus JOSE+JWT] or https://github.com/nicosabena/pem-to-jwk[pem-to-jwk]. For example, using `openssl` and a JWK conversion tool: ++ +[source,bash] +---- +# Extract the public key in PEM format +openssl x509 -in certificate.pem -pubkey -noout > publickey.pem + +# Convert to JWK format using a tool of your choice +# The result should be a JSON Web Key Set (JWKS) like: +# { +# "keys": [ +# { +# "kty": "RSA", +# "use": "sig", +# "kid": "mykey", +# "n": "...", +# "e": "AQAB" +# } +# ] +# } +---- -When you generate the keys, {project_name} will store the certificate and you download the private key and certificate for your client. +. Host the resulting JWKS JSON at a URL accessible by {project_name} and configure it as the *JWKS URL*. -You can also generate keys using an external tool and then import the client's certificate by clicking *Import Certificate*. +*Importing a certificate or public key* + +You can import the client's certificate or public key by clicking *Import Certificate* in the *Keys* tab. .Import certificate image:images/import-client-cert.png[Import Certificate] -. Select the archive format of the certificate. -. Enter the store password. +. Select the archive format of the certificate (*Certificate PEM*, *Public Key PEM*, *JSON Web Key Set*, or a keystore format). +. Enter the store password (for keystore formats). . Select the certificate file by clicking *Import File*. . Click *Import*. -Importing a certificate is unnecessary if you click *Use JWKS URL*. In this case, you can provide the URL where the public key is published in https://datatracker.ietf.org/doc/html/rfc7517[JWK] format. With this option, if the key is ever changed, {project_name} reimports the key. +Only the certificate or public key is stored by {project_name}. If a keystore containing a private key is uploaded, the private key is discarded. -If you are using a client secured by {project_name} adapter, you can configure the JWKS URL in this format, assuming that https://myhost.com/myapp is the root URL of your client application: +*Generating keys in {project_name} (deprecated)* -[source,bash,subs=+attributes] ----- -https://myhost.com/myapp/k_jwks ----- +NOTE: The key generation features in the admin console are deprecated. Clients should generate their own key pairs and provide only the public key or certificate to {project_name}. + +.Keys tab +image:images/client-oidc-keys.png[Keys tab] + +Click on the `Generate new keys` button to start this process. + +.Generate keys +image:images/generate-client-keys.png[] + +. Select the archive format you want to use. +. Enter a *key password*. +. Enter a *store password*. +. Click *Generate*. + +When you generate the keys, {project_name} stores only the certificate. The private key is included in the downloaded keystore and is not stored on the server. Save the downloaded keystore securely -- it cannot be retrieved from {project_name} later. See link:{developerguide_link}[{developerguide_name}] for more details. diff --git a/docs/documentation/server_admin/topics/clients/saml/proc-creating-saml-client.adoc b/docs/documentation/server_admin/topics/clients/saml/proc-creating-saml-client.adoc index 9de83c424bfb..538f8b90071e 100644 --- a/docs/documentation/server_admin/topics/clients/saml/proc-creating-saml-client.adoc +++ b/docs/documentation/server_admin/topics/clients/saml/proc-creating-saml-client.adoc @@ -147,7 +147,9 @@ If the option *Use metadata descriptor URL* is enabled in the *Signature and Enc *Encrypt Assertions*:: Encrypts the assertions in SAML documents with the specified client public key. Default algorithms used for encryption are configured with security in mind. If you need a different configuration, the encryption details can be modified in the *Settings* tab, section *Signature and Encryption*. The encryption options are only visible when this *Encrypt Assertions* option is enabled. + -The key used to encrypt the assertions is controlled in the same way as in the case of *Client Signature Required*. If *Use metadata descriptor URL* is enabled, the key is doenloaded and cached by {project_name}. If that option is disabled, you need to import or generate the key when activating the *Encrypt Assertions* option. +The key used to encrypt the assertions is controlled in the same way as in the case of *Client Signature Required*. If *Use metadata descriptor URL* is enabled, the key is downloaded and cached by {project_name}. If that option is disabled, you need to import or generate the key when activating the *Encrypt Assertions* option. + +NOTE: When keys are generated in {project_name}, only the certificate is stored on the server. The private key is included in the downloaded file and is not retained by {project_name}. Clients should save the private key securely. The recommended approach is to generate key pairs externally and upload only the certificate to {project_name}. == Advanced tab diff --git a/integration/admin-client-core/src/main/java/org/keycloak/admin/client/resource/ClientAttributeCertificateResource.java b/integration/admin-client-core/src/main/java/org/keycloak/admin/client/resource/ClientAttributeCertificateResource.java index d3be3a13aebf..14d241290e9d 100644 --- a/integration/admin-client-core/src/main/java/org/keycloak/admin/client/resource/ClientAttributeCertificateResource.java +++ b/integration/admin-client-core/src/main/java/org/keycloak/admin/client/resource/ClientAttributeCertificateResource.java @@ -46,7 +46,11 @@ public interface ClientAttributeCertificateResource { * Generate a new certificate with new key pair * * @return + * + * @deprecated Clients should generate their own key pairs and upload only the public key/certificate. + * */ + @Deprecated @POST @Path("generate") @Produces(MediaType.APPLICATION_JSON) @@ -82,7 +86,11 @@ public interface ClientAttributeCertificateResource { * @param config Keystore configuration as JSON. Parameters "keySize" and "validity" of the config are supported since Keycloak 26.3. Key size is 4096 by default and validity is 3 years by default. * For older versions than Keycloak 26.3, the key size is 2048 and validity is 10 years. * @return + * + * @deprecated Clients should manage their own private keys. + * */ + @Deprecated @POST @Path("/download") @Produces(MediaType.APPLICATION_OCTET_STREAM) @@ -97,7 +105,11 @@ public interface ClientAttributeCertificateResource { * * @param config Keystore configuration as JSON * @return + * + * @deprecated Clients should generate their own key pairs and upload only the public key/certificate + * */ + @Deprecated @POST @Path("/generate-and-download") @Produces(MediaType.APPLICATION_OCTET_STREAM) diff --git a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties index f461738f386a..28170b51325b 100644 --- a/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties +++ b/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties @@ -1033,8 +1033,8 @@ missingAttributes=No {{label}} have been defined yet. Click the below button to testConnectionError=Error\! {{error}} authenticatedAccessPoliciesHelp=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. deleteClientPolicyProfileSuccess=Profile successfully removed from the policy. -reGenerateSigningExplain=If you regenerate the signing key for client, the Keycloak database will be updated, and you may need to download a new adapter for this client. -reGenerateEncryptionExplain=If you regenerate the encryption key for client, the Keycloak database will be updated, and you may need to download a new adapter for this client. +reGenerateSigningExplain=If you regenerate the signing key for client, a new certificate will be stored in the Keycloak database. The private key will be downloaded once and is not stored on the server. You may need to reconfigure your client application. +reGenerateEncryptionExplain=If you regenerate the encryption key for client, a new certificate will be stored in the Keycloak database. The private key will be downloaded once and is not stored on the server. You may need to reconfigure your client application. evaluate=Evaluate enableLdapv3Password=Enable the LDAPv3 password modify extended operation status=Status diff --git a/js/apps/admin-ui/src/clients/keys/ExportSamlKeyDialog.tsx b/js/apps/admin-ui/src/clients/keys/ExportSamlKeyDialog.tsx index 936976fd4cda..e67b5b7d505d 100644 --- a/js/apps/admin-ui/src/clients/keys/ExportSamlKeyDialog.tsx +++ b/js/apps/admin-ui/src/clients/keys/ExportSamlKeyDialog.tsx @@ -33,6 +33,7 @@ export const ExportSamlKeyDialog = ({ const download = async (config: KeyStoreConfig) => { try { + // eslint-disable-next-line @typescript-eslint/no-deprecated const keyStore = await adminClient.clients.downloadKey( { id: clientId, diff --git a/js/apps/admin-ui/src/clients/keys/Keys.tsx b/js/apps/admin-ui/src/clients/keys/Keys.tsx index 453713a6d300..ccbcca8e6ec8 100644 --- a/js/apps/admin-ui/src/clients/keys/Keys.tsx +++ b/js/apps/admin-ui/src/clients/keys/Keys.tsx @@ -84,6 +84,7 @@ export const Keys = ({ const generate = async (config: KeyStoreConfig) => { try { + // eslint-disable-next-line @typescript-eslint/no-deprecated const keyStore = await adminClient.clients.generateAndDownloadKey( { id: clientId, diff --git a/js/apps/admin-ui/src/clients/keys/SamlKeys.tsx b/js/apps/admin-ui/src/clients/keys/SamlKeys.tsx index 6d816d58f9a6..feb6aa93ca69 100644 --- a/js/apps/admin-ui/src/clients/keys/SamlKeys.tsx +++ b/js/apps/admin-ui/src/clients/keys/SamlKeys.tsx @@ -220,20 +220,27 @@ export const SamlKeys = ({ clientId, save }: SamlKeysProps) => { const generate = async (attr: KeyTypes) => { const index = KEYS.indexOf(attr); try { - const info = [...(keyInfo || [])]; - info[index] = await adminClient.clients.generateKey({ + // eslint-disable-next-line @typescript-eslint/no-deprecated + const generatedKey = await adminClient.clients.generateKey({ id: clientId, attr, }); - setKeyInfo(info); saveAs( - new Blob([info[index].privateKey!], { + new Blob([generatedKey.privateKey!], { type: "application/octet-stream", }), "private.key", ); + const serverInfo = await adminClient.clients.getKeyInfo({ + id: clientId, + attr, + }); + const info = [...(keyInfo || [])]; + info[index] = serverInfo; + setKeyInfo(info); + addAlert(t("generateSuccess"), AlertVariant.success); } catch (error) { addError("generateError", error); diff --git a/js/apps/admin-ui/src/clients/keys/SamlKeysDialog.tsx b/js/apps/admin-ui/src/clients/keys/SamlKeysDialog.tsx index be9ec044ef97..40249247c430 100644 --- a/js/apps/admin-ui/src/clients/keys/SamlKeysDialog.tsx +++ b/js/apps/admin-ui/src/clients/keys/SamlKeysDialog.tsx @@ -98,17 +98,19 @@ export const SamlKeysDialog = ({ const generate = async () => { try { + // eslint-disable-next-line @typescript-eslint/no-deprecated const key = await adminClient.clients.generateKey({ id, attr, }); - setKeys(key); saveAs( new Blob([key.privateKey!], { type: "application/octet-stream", }), "private.key", ); + // Clear private key from display state — it is not stored on server + setKeys({ ...key, privateKey: undefined }); addAlert(t("generateSuccess"), AlertVariant.success); } catch (error) { diff --git a/js/apps/admin-ui/test/identity-providers/jwt-authorization-grant.spec.ts b/js/apps/admin-ui/test/identity-providers/jwt-authorization-grant.spec.ts index cccdab833fa5..73e0c60c975e 100644 --- a/js/apps/admin-ui/test/identity-providers/jwt-authorization-grant.spec.ts +++ b/js/apps/admin-ui/test/identity-providers/jwt-authorization-grant.spec.ts @@ -130,7 +130,7 @@ test.describe.serial("JWT Authorization Grant identity provider test", () => { await expect( page.getByTestId("config.publicKeySignatureVerifier"), - ).toHaveValue(/{ "keys" : /); + ).toHaveValue(/{\s*"keys"\s*:\s*/); await clickSaveButton(page); await assertNotificationMessage(page, "Provider successfully updated"); diff --git a/js/eslint.config.js b/js/eslint.config.js index 95636ac41d77..dc87491f0dbd 100644 --- a/js/eslint.config.js +++ b/js/eslint.config.js @@ -146,6 +146,7 @@ export default tseslint.config( files: ["libs/keycloak-admin-client/test/**"], rules: { "@typescript-eslint/no-unused-expressions": "off", + "@typescript-eslint/no-deprecated": "off", }, }, { diff --git a/js/libs/keycloak-admin-client/src/resources/clients.ts b/js/libs/keycloak-admin-client/src/resources/clients.ts index 2200eeffd1b8..1af522a15e9e 100644 --- a/js/libs/keycloak-admin-client/src/resources/clients.ts +++ b/js/libs/keycloak-admin-client/src/resources/clients.ts @@ -996,6 +996,10 @@ export class Clients extends Resource<{ realm?: string }> { urlParamKeys: ["id", "attr"], }); + /** + * @deprecated Clients should generate their own key pairs and upload only the + * public key/certificate. The private key is no longer stored on the server. + */ public generateKey = this.makeRequest< { id: string; attr: string }, CertificateRepresentation @@ -1005,6 +1009,10 @@ export class Clients extends Resource<{ realm?: string }> { urlParamKeys: ["id", "attr"], }); + /** + * @deprecated Private keys are no longer stored on the server. The downloaded + * keystore will only contain the certificate. + */ public downloadKey = this.makeUpdateRequest< { id: string; attr: string }, KeyStoreConfig, @@ -1018,6 +1026,10 @@ export class Clients extends Resource<{ realm?: string }> { }, }); + /** + * @deprecated Clients should generate their own key pairs and upload only the + * public key/certificate. + */ public generateAndDownloadKey = this.makeUpdateRequest< { id: string; attr: string }, KeyStoreConfig, diff --git a/js/libs/keycloak-admin-client/test/clients.spec.ts b/js/libs/keycloak-admin-client/test/clients.spec.ts index f0009fe44bd7..475efa6a18b5 100644 --- a/js/libs/keycloak-admin-client/test/clients.spec.ts +++ b/js/libs/keycloak-admin-client/test/clients.spec.ts @@ -968,12 +968,14 @@ describe("Clients", () => { expect(certificate).to.be.ok; expect(certificate.certificate).to.be.ok; + expect(certificate.privateKey).to.be.ok; const info = await kcAdminClient.clients.getKeyInfo({ id: currentClient.id!, attr, }); - expect(info).to.be.eql(certificate); + expect(info.privateKey).to.be.undefined; + expect(info.certificate).to.be.eql(certificate.certificate); }); it("download key", async () => { diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolFactory.java b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolFactory.java index 911dbf2dddf6..fb25620145d5 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolFactory.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolFactory.java @@ -199,8 +199,6 @@ public void setupClientDefaults(ClientRepresentation clientRep, ClientModel newC && (!client.isUseMetadataDescriptorUrl() || client.getMetadataDescriptorUrl() != null)) { CertificateRepresentation info = KeycloakModelUtils.generateKeyPairCertificate(newClient.getClientId()); client.setClientSigningCertificate(info.getCertificate()); - client.setClientSigningPrivateKey(info.getPrivateKey()); - } if (clientRep.isFrontchannelLogout() == null) { diff --git a/services/src/main/java/org/keycloak/protocol/saml/installation/KeycloakSamlClientInstallation.java b/services/src/main/java/org/keycloak/protocol/saml/installation/KeycloakSamlClientInstallation.java index e9c2d024ce72..6732e435cb46 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/installation/KeycloakSamlClientInstallation.java +++ b/services/src/main/java/org/keycloak/protocol/saml/installation/KeycloakSamlClientInstallation.java @@ -59,7 +59,7 @@ public static void baseXml(KeycloakSession session, RealmModel realm, ClientMode buffer.append(" \n"); buffer.append(" \n"); if (samlClient.getClientSigningPrivateKey() == null) { - buffer.append(" PRIVATE KEY NOT SET UP OR KNOWN\n"); + buffer.append(" PRIVATE KEY NOT STORED ON SERVER - REPLACE WITH YOUR OWN PRIVATE KEY\n"); } else { buffer.append(" ").append(samlClient.getClientSigningPrivateKey()).append("\n"); } @@ -77,7 +77,7 @@ public static void baseXml(KeycloakSession session, RealmModel realm, ClientMode buffer.append(" \n"); buffer.append(" \n"); if (samlClient.getClientEncryptingPrivateKey() == null) { - buffer.append(" PRIVATE KEY NOT SET UP OR KNOWN\n"); + buffer.append(" PRIVATE KEY NOT STORED ON SERVER - REPLACE WITH YOUR OWN PRIVATE KEY\n"); } else { buffer.append(" ").append(samlClient.getClientEncryptingPrivateKey()).append("\n"); } diff --git a/services/src/main/java/org/keycloak/protocol/saml/installation/KeycloakSamlSubsystemCliInstallation.java b/services/src/main/java/org/keycloak/protocol/saml/installation/KeycloakSamlSubsystemCliInstallation.java index 6520746f1409..1ed3c8afdcd4 100644 --- a/services/src/main/java/org/keycloak/protocol/saml/installation/KeycloakSamlSubsystemCliInstallation.java +++ b/services/src/main/java/org/keycloak/protocol/saml/installation/KeycloakSamlSubsystemCliInstallation.java @@ -57,7 +57,7 @@ public Response generateInstallation(KeycloakSession session, RealmModel realm, builder.append("/subsystem=keycloak-saml/secure-deployment=YOUR-WAR.war/SP=") .append(quote(entityId)) .append("/Key=KEY1:add(signing=true, \\\nPrivateKeyPem=") - .append(quote(samlClient.getClientSigningPrivateKey() == null ? "PRIVATE KEY NOT SET UP OR KNOWN" : samlClient.getClientSigningPrivateKey())) + .append(quote(samlClient.getClientSigningPrivateKey() == null ? "PRIVATE KEY NOT STORED ON SERVER - REPLACE WITH YOUR OWN PRIVATE KEY" : samlClient.getClientSigningPrivateKey())) .append(", \\\nCertificatePem=") .append(quote(samlClient.getClientSigningCertificate() == null ? "YOU MUST CONFIGURE YOUR_CLIENT's SIGNING CERTIFICATE" : samlClient.getClientSigningCertificate())) .append(")\n\n"); @@ -66,7 +66,7 @@ public Response generateInstallation(KeycloakSession session, RealmModel realm, builder.append("/subsystem=keycloak-saml/secure-deployment=YOUR-WAR.war/SP=") .append(quote(entityId)) .append("/Key=KEY2:add(encryption=true,PrivateKeyPem=") - .append(quote(samlClient.getClientEncryptingPrivateKey() == null ? "PRIVATE KEY NOT SET UP OR KNOWN" : samlClient.getClientEncryptingPrivateKey())) + .append(quote(samlClient.getClientEncryptingPrivateKey() == null ? "PRIVATE KEY NOT STORED ON SERVER - REPLACE WITH YOUR OWN PRIVATE KEY" : samlClient.getClientEncryptingPrivateKey())) .append(")\n\n"); } 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 c40d3712330e..96200cde9136 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/oidc/DescriptionConverter.java @@ -323,7 +323,8 @@ private static boolean setPublicKey(OIDCClientRepresentation clientOIDC, ClientR JWK publicKeyJWk = JWKSUtils.getKeyForUse(keySet, JWK.Use.SIG); try { - configWrapper.setJwksString(JsonSerialization.writeValueAsPrettyString(clientOIDC.getJwks())); + configWrapper.setJwksString(CertificateInfoHelper.stripPrivateKeyParams( + JsonSerialization.writeValueAsPrettyString(clientOIDC.getJwks()))); } catch (IOException e) { throw new ClientRegistrationException("Illegal jwks format"); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientAttributeCertificateResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientAttributeCertificateResource.java index e2b5ba9b1113..04d7b8a5c36c 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientAttributeCertificateResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientAttributeCertificateResource.java @@ -105,22 +105,28 @@ public CertificateRepresentation getKeyInfo() { /** * Generate a new certificate with new key pair * + * @deprecated Clients should generate their own key pairs and upload only the public key/certificate. + * * @return */ + @Deprecated @POST @NoCache @Path("generate") @Produces(MediaType.APPLICATION_JSON) @Tag(name = KeycloakOpenAPI.Admin.Tags.CLIENT_ATTRIBUTE_CERTIFICATE) - @Operation( summary = "Generate a new certificate with new key pair") + @Operation(summary = "Generate a new certificate with new key pair", deprecated = true) public CertificateRepresentation generate() { auth.clients().requireConfigure(client); CertificateRepresentation info = KeycloakModelUtils.generateKeyPairCertificate(client.getClientId()); + CertificateRepresentation infoForStorage = new CertificateRepresentation(); + infoForStorage.setCertificate(info.getCertificate()); + CertificateInfoHelper.updateClientModelCertificateInfo(client, infoForStorage, attributePrefix); - CertificateInfoHelper.updateClientModelCertificateInfo(client, info, attributePrefix); - - adminEvent.operation(OperationType.ACTION).resourcePath(session.getContext().getUri()).representation(info).success(); + CertificateRepresentation sanitized = new CertificateRepresentation(); + sanitized.setCertificate(info.getCertificate()); + adminEvent.operation(OperationType.ACTION).resourcePath(session.getContext().getUri()).representation(sanitized).success(); return info; } @@ -175,9 +181,18 @@ private void updateCertFromRequest(CertificateRepresentation info) { if (OIDCLoginProtocol.LOGIN_PROTOCOL.equals(client.getProtocol()) && info.getJwks() != null) { CertificateInfoHelper.updateClientModelJwksString(client, attributePrefix, info.getJwks()); } else { - CertificateInfoHelper.updateClientModelCertificateInfo(client, info, attributePrefix); + CertificateRepresentation infoForStorage = new CertificateRepresentation(); + infoForStorage.setCertificate(info.getCertificate()); + infoForStorage.setPublicKey(info.getPublicKey()); + infoForStorage.setKid(info.getKid()); + CertificateInfoHelper.updateClientModelCertificateInfo(client, infoForStorage, attributePrefix); } - adminEvent.operation(OperationType.ACTION).resourcePath(session.getContext().getUri()).representation(info).success(); + CertificateRepresentation sanitized = new CertificateRepresentation(); + sanitized.setCertificate(info.getCertificate()); + sanitized.setPublicKey(info.getPublicKey()); + sanitized.setKid(info.getKid()); + sanitized.setJwks(info.getJwks()); + adminEvent.operation(OperationType.ACTION).resourcePath(session.getContext().getUri()).representation(sanitized).success(); } /** @@ -185,14 +200,17 @@ private void updateCertFromRequest(CertificateRepresentation info) { * * @param config Keystore configuration as JSON * @return + * + * @deprecated Clients should manage their own private keys. */ + @Deprecated @POST @NoCache @Path("/download") @Produces(MediaType.APPLICATION_OCTET_STREAM) @Consumes(MediaType.APPLICATION_JSON) @Tag(name = KeycloakOpenAPI.Admin.Tags.CLIENT_ATTRIBUTE_CERTIFICATE) - @Operation( summary = "Get a keystore file for the client, containing private key and public certificate") + @Operation( summary = "Get a keystore file for the client, containing the public certificate", deprecated = true) public byte[] getKeystore(@Parameter(description = "Keystore configuration as JSON") final KeyStoreConfig config) { auth.clients().requireView(client); @@ -224,7 +242,10 @@ public byte[] getKeystore(@Parameter(description = "Keystore configuration as JS * * @param config Keystore configuration as JSON * @return + * + * @deprecated Clients should generate their own key pairs and upload only the public key/certificate. */ + @Deprecated @POST @NoCache @Path("/generate-and-download") @@ -235,7 +256,7 @@ public byte[] getKeystore(@Parameter(description = "Keystore configuration as JS "Generate a new keypair and certificate, and get the private key file\n" + "\n" + "Generates a keypair and certificate and serves the private key in a specified keystore format.\n" + - "Only generated public certificate is saved in Keycloak DB - the private key is not.") + "Only generated public certificate is saved in Keycloak DB - the private key is not.", deprecated = true) public byte[] generateAndGetKeystore(@Parameter(description = "Keystore configuration as JSON") final KeyStoreConfig config) { auth.clients().requireConfigure(client); diff --git a/services/src/main/java/org/keycloak/services/util/CertificateInfoHelper.java b/services/src/main/java/org/keycloak/services/util/CertificateInfoHelper.java index 70007ead486a..ef166932deed 100644 --- a/services/src/main/java/org/keycloak/services/util/CertificateInfoHelper.java +++ b/services/src/main/java/org/keycloak/services/util/CertificateInfoHelper.java @@ -25,6 +25,7 @@ import java.security.UnrecoverableKeyException; import java.security.cert.X509Certificate; import java.util.HashMap; +import java.util.Set; import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.core.MultivaluedMap; @@ -71,7 +72,6 @@ public class CertificateInfoHelper { // CLIENT MODEL METHODS public static CertificateRepresentation getCertificateFromClient(ClientModel client, String attributePrefix) { - String privateKeyAttribute = attributePrefix + "." + PRIVATE_KEY; String certificateAttribute = attributePrefix + "." + X509CERTIFICATE; String publicKeyAttribute = attributePrefix + "." + PUBLIC_KEY; String kidAttribute = attributePrefix + "." + KID; @@ -84,7 +84,6 @@ public static CertificateRepresentation getCertificateFromClient(ClientModel cli CertificateRepresentation rep = new CertificateRepresentation(); rep.setCertificate(client.getAttribute(certificateAttribute)); rep.setPublicKey(client.getAttribute(publicKeyAttribute)); - rep.setPrivateKey(client.getAttribute(privateKeyAttribute)); rep.setKid(client.getAttribute(kidAttribute)); return rep; @@ -105,11 +104,15 @@ public static CertificateRepresentation jwksStringToSigCertificateRepresentation throw new IllegalStateException("Certificate not found for use sig"); } - // set the public key as before and also the full jwks + for (JWK key : keySet.getKeys()) { + key.getOtherClaims().keySet().removeAll(JWK_PRIVATE_KEY_PARAMS); + } + String publicOnlyJwks = JsonSerialization.writeValueAsPrettyString(keySet); + PublicKey publicKey = JWKParser.create(publicKeyJwk).toPublicKey(); String publicKeyPem = KeycloakModelUtils.getPemFromKey(publicKey); CertificateRepresentation info = new CertificateRepresentation(); - info.setJwks(jwks); + info.setJwks(publicOnlyJwks); info.setPublicKey(publicKeyPem); info.setKid(publicKeyJwk.getKeyId()); return info; @@ -162,7 +165,25 @@ public static void updateClientModelJwksString(ClientModel client, String attrib setOrRemoveAttr(client, certificateAttribute, null); setOrRemoveAttr(client, kidAttribute, null); setOrRemoveAttr(client, OIDCConfigAttributes.USE_JWKS_STRING, Boolean.TRUE.toString()); - setOrRemoveAttr(client, OIDCConfigAttributes.JWKS_STRING, jwks); + setOrRemoveAttr(client, OIDCConfigAttributes.JWKS_STRING, stripPrivateKeyParams(jwks)); + } + + private static final Set JWK_PRIVATE_KEY_PARAMS = Set.of("d", "p", "q", "dp", "dq", "qi", "oth", "k"); + + public static String stripPrivateKeyParams(String jwks) { + try { + JSONWebKeySet keySet = JsonSerialization.readValue(jwks, JSONWebKeySet.class); + if (keySet != null && keySet.getKeys() != null) { + for (JWK key : keySet.getKeys()) { + key.getOtherClaims().keySet().removeAll(JWK_PRIVATE_KEY_PARAMS); + } + return JsonSerialization.writeValueAsPrettyString(keySet); + } + return jwks; + } catch (IOException e) { + logger.warn("Failed to parse JWKS for private key stripping, storing as-is", e); + return jwks; + } } private static void setOrRemoveAttr(ClientModel client, String attrName, String attrValue) { diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/client/CredentialsTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/client/CredentialsTest.java index 282c9679c60b..459afcf8a9c7 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/client/CredentialsTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/client/CredentialsTest.java @@ -39,6 +39,7 @@ import org.keycloak.events.admin.ResourceType; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.KeyStoreConfig; +import org.keycloak.representations.idm.AdminEventRepresentation; import org.keycloak.representations.idm.CertificateRepresentation; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.CredentialRepresentation; @@ -62,6 +63,7 @@ import org.junit.jupiter.api.io.TempDir; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -137,11 +139,21 @@ public void testGetAndRegenerateRegistrationAccessToken() { public void testGetCertificateResource() { ClientAttributeCertificateResource certRsc = accountClient.getCertficateResource("jwt.credential"); CertificateRepresentation cert = certRsc.generate(); - CertificateRepresentation certFromGet = certRsc.getKeyInfo(); - assertEquals(cert.getCertificate(), certFromGet.getCertificate()); - assertEquals(cert.getPrivateKey(), certFromGet.getPrivateKey()); - AdminEventAssertion.assertEvent(adminEvents.poll(), OperationType.ACTION, AdminEventPaths.clientCertificateGenerateSecretPath(accountClient.toRepresentation().getId(), "jwt.credential"), cert, ResourceType.CLIENT); + // Response should contain the private key + assertNotNull(cert.getPrivateKey(), "generate should return private key"); + assertNotNull(cert.getCertificate(), "generate should return certificate"); + + // But server should not have stored it + CertificateRepresentation certFromGet = certRsc.getKeyInfo(); + assertNull(certFromGet.getPrivateKey(), "private key should not be stored on server"); + assertEquals(cert.getCertificate(), certFromGet.getCertificate(), "certificate should match"); + // Admin event should not contain the private key + AdminEventRepresentation event = adminEvents.poll(); + assertFalse(event.getRepresentation().contains(cert.getPrivateKey()), "admin event should not contain private key"); + CertificateRepresentation expectedEventRep = new CertificateRepresentation(); + expectedEventRep.setCertificate(cert.getCertificate()); + AdminEventAssertion.assertEvent(event, OperationType.ACTION, AdminEventPaths.clientCertificateGenerateSecretPath(accountClient.toRepresentation().getId(), "jwt.credential"), expectedEventRep, ResourceType.CLIENT); } @Test @@ -176,7 +188,7 @@ public void testUploadKeyAndCertificate() throws Exception { // Get the certificate - to make sure cert was properly updated cert = certRsc.getKeyInfo(); assertEquals(generatedKeystore.getCertificateInfo().getCertificate(), cert.getCertificate(), "cert properly set"); - assertEquals(generatedKeystore.getCertificateInfo().getPrivateKey(), cert.getPrivateKey(), "privateKey properly set"); + assertNull(cert.getPrivateKey(), "private key should not be stored on server"); // Upload a different certificate via /upload-certificate, privateKey should be nullified MultipartFormDataOutput form = new MultipartFormDataOutput(); @@ -240,20 +252,17 @@ public void testDownloadKeystore() throws Exception { KeyStoreConfig config = new KeyStoreConfig(); config.setFormat(preferredKeystoreType.toString()); config.setKeyAlias("alias"); - config.setKeyPassword("keyPass"); config.setStorePassword("storePass"); byte[] result = certRsc.getKeystore(config); KeyStore keyStore = CryptoIntegration.getProvider().getKeyStore(preferredKeystoreType); keyStore.load(new ByteArrayInputStream(result), "storePass".toCharArray()); - Key key = keyStore.getKey("alias", "keyPass".toCharArray()); + assertFalse(keyStore.isKeyEntry("alias"), "private key should not be in keystore"); Certificate cert = keyStore.getCertificate("alias"); assertInstanceOf(X509Certificate.class, cert, "Certificat is X509"); - String keyPem = KeycloakModelUtils.getPemFromKey(key); - String certPem = KeycloakModelUtils.getPemFromCertificate((X509Certificate) cert); - assertEquals(certrep.getPrivateKey(), keyPem, "key match"); + String certPem = KeycloakModelUtils.getPemFromCertificate((X509Certificate) cert); assertEquals(certrep.getCertificate(), certPem, "cert match"); } diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/client/InstallationTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/client/InstallationTest.java index 3ce2596b0914..e1fe7f3c7dd4 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/client/InstallationTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/client/InstallationTest.java @@ -368,18 +368,11 @@ public void testPemsInModAuthMellonExportShouldBeFormattedInRfc7468() throws IOE } } - Assertions.assertNotNull(clientPrivateKey); - Assertions.assertNotNull(clientCert); - assertRfc7468PrivateKey(clientPrivateKey); + Assertions.assertNull(clientPrivateKey, "private key should not be included in export"); + Assertions.assertNotNull(clientCert, "certificate should be included in export"); assertRfc7468Cert(clientCert); } - private void assertRfc7468PrivateKey(String result) { - Assertions.assertTrue(result.startsWith("-----BEGIN PRIVATE KEY-----")); - Assertions.assertTrue(result.endsWith("-----END PRIVATE KEY-----")); - result.lines().forEach(line -> Assertions.assertTrue(line.length() <= 64)); - } - private void assertRfc7468Cert(String result) { Assertions.assertTrue(result.startsWith("-----BEGIN CERTIFICATE-----")); Assertions.assertTrue(result.endsWith("-----END CERTIFICATE-----")); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AbstractClientAuthSignedJWTTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AbstractClientAuthSignedJWTTest.java index 1a20f3a85a69..e797668cd578 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AbstractClientAuthSignedJWTTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AbstractClientAuthSignedJWTTest.java @@ -552,7 +552,9 @@ protected void testUploadKeystore(String keystoreFormat, String filePath, String } else if (keystoreFormat.equals(CertificateInfoHelper.JSON_WEB_KEY_SET)) { Assertions.assertEquals("true", client.getAttributes().get(OIDCConfigAttributes.USE_JWKS_STRING)); String jwks = new String(Files.readAllBytes(keystoreFile.toPath())); - Assertions.assertEquals(jwks, client.getAttributes().get(OIDCConfigAttributes.JWKS_STRING)); + JSONWebKeySet expected = JsonSerialization.readValue(jwks, JSONWebKeySet.class); + JSONWebKeySet actual = JsonSerialization.readValue(client.getAttributes().get(OIDCConfigAttributes.JWKS_STRING), JSONWebKeySet.class); + Assertions.assertEquals(JsonSerialization.writeValueAsString(expected), JsonSerialization.writeValueAsString(actual)); CertificateRepresentation info = getClient(testRealm.getRealm(), client.getId()) .getCertficateResource(JWTClientAuthenticator.ATTR_PREFIX).getKeyInfo(); Assertions.assertNotNull(info.getPublicKey());