From 80bbb0be103ebdf285f6d80ab7dcdeb5686060b5 Mon Sep 17 00:00:00 2001 From: Jan-Hendrik Dolling Date: Wed, 13 Nov 2024 14:03:00 +0100 Subject: [PATCH] fix: PEM files distributed as part of SAML adapter configs for mod_auth_mellon export Changing return type of ClientResource from String to Response to support different response types. Should not be breaking as this is just a class used internally by Keycloak integration tests. Closes #34276 Co-authored-by: ccudennec-otto Co-authored-by: radwa-otto Co-authored-by: IngoStrauch2020 Signed-off-by: Jan-Hendrik Dolling --- .../org/keycloak/common/util/PemUtils.java | 7 ++ .../admin/client/resource/ClientResource.java | 5 +- .../ModAuthMellonClientInstallation.java | 27 ++++- .../keycloak/testsuite/util/SamlUtils.java | 1 + .../testsuite/admin/PermissionsTest.java | 7 +- .../admin/client/InstallationTest.java | 99 +++++++++++++++---- 6 files changed, 120 insertions(+), 26 deletions(-) diff --git a/common/src/main/java/org/keycloak/common/util/PemUtils.java b/common/src/main/java/org/keycloak/common/util/PemUtils.java index 955f509bdf0..483e7a1d0ec 100755 --- a/common/src/main/java/org/keycloak/common/util/PemUtils.java +++ b/common/src/main/java/org/keycloak/common/util/PemUtils.java @@ -124,6 +124,13 @@ public class PemUtils { .toString(); } + public static String addCertificateBeginEnd(String certificate) { + return new StringBuilder(BEGIN_CERT + "\n") + .append(certificate) + .append("\n" + END_CERT) + .toString(); + } + public static String addRsaPrivateKeyBeginEnd(String privateKeyPem) { return new StringBuilder(PemUtils.BEGIN_RSA_PRIVATE_KEY + "\n") .append(privateKeyPem) diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ClientResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ClientResource.java index 7e7ce97a80e..b3fc32d2cce 100644 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ClientResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ClientResource.java @@ -31,6 +31,7 @@ import jakarta.ws.rs.Produces; import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; import org.keycloak.representations.adapters.action.GlobalRequestResult; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientScopeRepresentation; @@ -115,7 +116,7 @@ public interface ClientResource { @GET @Path("installation/providers/{providerId}") - String getInstallationProvider(@PathParam("providerId") String providerId); + Response getInstallationProvider(@PathParam("providerId") String providerId); @Path("session-count") @GET @@ -217,4 +218,4 @@ public interface ClientResource { @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) public void invalidateRotatedSecret(); -} \ No newline at end of file +} diff --git a/services/src/main/java/org/keycloak/protocol/saml/installation/ModAuthMellonClientInstallation.java b/services/src/main/java/org/keycloak/protocol/saml/installation/ModAuthMellonClientInstallation.java index ffc99af579d..95dca88789b 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/installation/ModAuthMellonClientInstallation.java +++ b/services/src/main/java/org/keycloak/protocol/saml/installation/ModAuthMellonClientInstallation.java @@ -17,7 +17,12 @@ package org.keycloak.protocol.saml.installation; +import java.nio.charset.StandardCharsets; +import java.util.regex.MatchResult; +import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.keycloak.Config; +import org.keycloak.common.util.PemUtils; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; @@ -59,12 +64,12 @@ public class ModAuthMellonClientInstallation implements ClientInstallationProvid if (samlClient.requiresClientSignature()) { if (samlClient.getClientSigningPrivateKey() != null) { zip.putNextEntry(new ZipEntry(clientDirName + "/client-private-key.pem")); - zip.write(samlClient.getClientSigningPrivateKey().getBytes()); + zip.write(createClientSigningPrivateKeyRfc7468Representation(samlClient.getClientSigningPrivateKey())); zip.closeEntry(); } if (samlClient.getClientSigningCertificate() != null) { zip.putNextEntry(new ZipEntry(clientDirName + "/client-cert.pem")); - zip.write(samlClient.getClientSigningCertificate().getBytes()); + zip.write(createClientSigningCertificateRfc7468Representation(samlClient.getClientSigningCertificate())); zip.closeEntry(); } } @@ -132,4 +137,22 @@ public class ModAuthMellonClientInstallation implements ClientInstallationProvid public String getId() { return "mod-auth-mellon"; } + + private static byte[] createClientSigningPrivateKeyRfc7468Representation(String clientSigningPrivateKey) { + String resultAsString = PemUtils.addPrivateKeyBeginEnd(wrapAt64Chars(clientSigningPrivateKey)); + return resultAsString.getBytes(StandardCharsets.US_ASCII); + } + + private static byte[] createClientSigningCertificateRfc7468Representation(String clientSigningCertificate) { + String resultAsString = PemUtils.addCertificateBeginEnd(wrapAt64Chars(clientSigningCertificate)); + return resultAsString.getBytes(StandardCharsets.US_ASCII); + } + + private static String wrapAt64Chars(String text) { + return Pattern.compile(".{1,64}") + .matcher(text) + .results() + .map(MatchResult::group) + .collect(Collectors.joining("\n")); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlUtils.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlUtils.java index f623dcfcfa7..e1ebc481fe1 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlUtils.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlUtils.java @@ -45,6 +45,7 @@ public class SamlUtils { .map(ClientRepresentation::getId) .map(res::get) .map(clientResource -> clientResource.getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR)) + .map(response -> response.readEntity(String.class)) .orElseThrow(() -> new RuntimeException("Missing descriptor")); SAMLParser parser = SAMLParser.getInstance(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java index de091e05af0..297e2e05f9a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/PermissionsTest.java @@ -552,9 +552,10 @@ public class PermissionsTest extends AbstractKeycloakTest { realm.clients().get(foo.getId()).toRepresentation(); } }, Resource.CLIENT, false); - invoke(new Invocation() { - public void invoke(RealmResource realm) { - realm.clients().get(foo.getId()).getInstallationProvider("nosuch"); + invoke(new InvocationWithResponse() { + public void invoke(RealmResource realm, AtomicReference responseRef) { + Response response = realm.clients().get(foo.getId()).getInstallationProvider("nosuch"); + responseRef.set(response); } }, Resource.CLIENT, false); invoke(new Invocation() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java index c5d6fdf4ec9..e2e73ccbfe6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/InstallationTest.java @@ -17,13 +17,18 @@ package org.keycloak.testsuite.admin.client; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.StringReader; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import jakarta.ws.rs.core.Response; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -44,9 +49,11 @@ import org.w3c.dom.NodeList; import org.xml.sax.InputSource; import org.xml.sax.SAXException; -import jakarta.ws.rs.NotFoundException; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.keycloak.common.Profile.Feature.AUTHORIZATION; import static org.keycloak.testsuite.auth.page.AuthRealm.TEST; import static org.keycloak.testsuite.util.ServerURLs.getAuthServerContextRoot; @@ -103,27 +110,31 @@ public class InstallationTest extends AbstractClientTest { @Test public void testOidcJBossXml() { - String xml = oidcClient.getInstallationProvider("keycloak-oidc-jboss-subsystem"); + Response response = oidcClient.getInstallationProvider("keycloak-oidc-jboss-subsystem"); + String xml = response.readEntity(String.class); assertOidcInstallationConfig(xml); assertThat(xml, containsString("")); assertThat(xml, containsString("SPECIFY YOUR entityID!")); assertThat(xml, not(containsString(KeyUtils.findActiveSigningKey(testRealmResource()).getCertificate()))); @@ -188,7 +203,8 @@ public class InstallationTest extends AbstractClientTest { @Test public void testSamlAdapterCli() { - String cli = samlClient.getInstallationProvider("keycloak-saml-subsystem-cli"); + Response response = samlClient.getInstallationProvider("keycloak-saml-subsystem-cli"); + String cli = response.readEntity(String.class); assertThat(cli, containsString("/subsystem=keycloak-saml/secure-deployment=YOUR-WAR.war/")); assertThat(cli, containsString("SPECIFY YOUR entityID!")); assertThat(cli, not(containsString(KeyUtils.findActiveSigningKey(testRealmResource()).getCertificate()))); @@ -197,7 +213,8 @@ public class InstallationTest extends AbstractClientTest { @Test public void testSamlMetadataSpDescriptor() throws Exception { - String xml = samlClient.getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR); + Response response = samlClient.getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR); + String xml = response.readEntity(String.class); Document doc = getDocumentFromXmlString(xml); assertElements(doc, METADATA_NSURI.get(), "EntityDescriptor", null); assertElements(doc, METADATA_NSURI.get(), "SPSSODescriptor", null); @@ -206,7 +223,8 @@ public class InstallationTest extends AbstractClientTest { @Test public void testSamlJBossXml() { - String xml = samlClient.getInstallationProvider("keycloak-saml-subsystem"); + Response response = samlClient.getInstallationProvider("keycloak-saml-subsystem"); + String xml = response.readEntity(String.class); assertThat(xml, containsString(" attrNamesAndValues = new HashMap<>(); attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get()); attrNamesAndValues.put("Location", "ERROR:ENDPOINT_NOT_SET"); @@ -231,7 +250,8 @@ public class InstallationTest extends AbstractClientTest { //fallback to adminUrl updater.setAdminUrl("https://admin-url").update(); assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT); - doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR)); + response = updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR); + doc = getDocumentFromXmlString(response.readEntity(String.class)); attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get()); attrNamesAndValues.put("Location", "https://admin-url"); assertElements(doc, METADATA_NSURI.get(), "SingleLogoutService", attrNamesAndValues); @@ -246,7 +266,8 @@ public class InstallationTest extends AbstractClientTest { .update(); assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT); - doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR)); + response = updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR); + doc = getDocumentFromXmlString(response.readEntity(String.class)); attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_POST_BINDING.get()); attrNamesAndValues.put("Location", "https://saml-logout-post-url"); assertElements(doc, METADATA_NSURI.get(), "SingleLogoutService", attrNamesAndValues); @@ -268,7 +289,8 @@ public class InstallationTest extends AbstractClientTest { assertThat(updater.getResource().toRepresentation().getAttributes().get(SamlConfigAttributes.SAML_FORCE_POST_BINDING), equalTo("false")); //error fallback - Document doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR)); + Response response = updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR); + Document doc = getDocumentFromXmlString(response.readEntity(String.class)); Map attrNamesAndValues = new HashMap<>(); attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get()); attrNamesAndValues.put("Location", "ERROR:ENDPOINT_NOT_SET"); @@ -279,7 +301,8 @@ public class InstallationTest extends AbstractClientTest { //fallback to adminUrl updater.setAdminUrl("https://admin-url").update(); assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT); - doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR)); + response = updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR); + doc = getDocumentFromXmlString(response.readEntity(String.class)); attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get()); attrNamesAndValues.put("Location", "https://admin-url"); assertElements(doc, METADATA_NSURI.get(), "SingleLogoutService", attrNamesAndValues); @@ -293,7 +316,8 @@ public class InstallationTest extends AbstractClientTest { .setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "https://saml-logout-redirect-url") .update(); assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT); - doc = getDocumentFromXmlString(updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR)); + response = updater.getResource().getInstallationProvider(SamlSPDescriptorClientInstallation.SAML_CLIENT_INSTALATION_SP_DESCRIPTOR); + doc = getDocumentFromXmlString(response.readEntity(String.class)); attrNamesAndValues.put("Binding", JBossSAMLURIConstants.SAML_HTTP_REDIRECT_BINDING.get()); attrNamesAndValues.put("Location", "https://saml-logout-redirect-url"); assertElements(doc, METADATA_NSURI.get(), "SingleLogoutService", attrNamesAndValues); @@ -305,6 +329,43 @@ public class InstallationTest extends AbstractClientTest { assertAdminEvents.assertEvent(getRealmId(), OperationType.UPDATE, AdminEventPaths.clientResourcePath(samlClientId), ResourceType.CLIENT); } + @Test + public void testPemsInModAuthMellonExportShouldBeFormattedInRfc7468() throws IOException { + Response response = samlClient.getInstallationProvider("mod-auth-mellon"); + byte[] result = response.readEntity(byte[].class); + + String clientPrivateKey = null; + String clientCert = null; + try (ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(result))) { + ZipEntry entry; + while ((entry = zis.getNextEntry()) != null) { + if (entry.getName().endsWith("client-private-key.pem")) { + clientPrivateKey = new String(zis.readAllBytes(), StandardCharsets.US_ASCII); + } + else if (entry.getName().endsWith("client-cert.pem")) { + clientCert = new String(zis.readAllBytes(), StandardCharsets.US_ASCII); + } + } + } + + assertNotNull(clientPrivateKey); + assertNotNull(clientCert); + assertRfc7468PrivateKey(clientPrivateKey); + assertRfc7468Cert(clientCert); + } + + private void assertRfc7468PrivateKey(String result) { + assertTrue(result.startsWith("-----BEGIN PRIVATE KEY-----")); + assertTrue(result.endsWith("-----END PRIVATE KEY-----")); + result.lines().forEach(line -> assertTrue(line.length() <= 64)); + } + + private void assertRfc7468Cert(String result) { + assertTrue(result.startsWith("-----BEGIN CERTIFICATE-----")); + assertTrue(result.endsWith("-----END CERTIFICATE-----")); + result.lines().forEach(line -> assertTrue(line.length() <= 64)); + } + private Document getDocumentFromXmlString(String xml) throws SAXException, ParserConfigurationException, IOException { DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); dbf.setNamespaceAware(true);