diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java index f06a22790d8c..2bae779ab8ac 100755 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java @@ -42,6 +42,11 @@ import java.util.Set; import org.keycloak.adapters.cloned.HttpClientBuilder; import java.net.URI; +import java.security.cert.CertificateException; +import java.security.cert.CertificateExpiredException; +import java.security.cert.CertificateNotYetValidException; +import java.security.cert.X509Certificate; +import java.util.logging.Level; /** * @author Bill Burke @@ -205,8 +210,13 @@ private void processSigningKey(DefaultSamlDeployment.DefaultIDP idp, Key key, Re Certificate cert = null; try { cert = keyStore.getCertificate(key.getKeystore().getCertificateAlias()); + if (cert instanceof X509Certificate) { + ((X509Certificate) cert).checkValidity(); + } } catch (KeyStoreException e) { throw new RuntimeException(e); + } catch (CertificateException e) { + throw new RuntimeException(e); } publicKey = cert.getPublicKey(); } else { @@ -224,7 +234,12 @@ protected static PublicKey getPublicKeyFromPem(Key key) { if (key.getPublicKeyPem() != null) { publicKey = PemUtils.decodePublicKey(key.getPublicKeyPem().trim()); } else { - Certificate cert = PemUtils.decodeCertificate(key.getCertificatePem().trim()); + X509Certificate cert = PemUtils.decodeCertificate(key.getCertificatePem().trim()); + try { + cert.checkValidity(); + } catch (CertificateException ex) { + throw new RuntimeException(ex); + } publicKey = cert.getPublicKey(); } return publicKey; diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/rotation/SamlDescriptorPublicKeyLocator.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/rotation/SamlDescriptorPublicKeyLocator.java index 7a45fb790ceb..54be2169375f 100644 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/rotation/SamlDescriptorPublicKeyLocator.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/rotation/SamlDescriptorPublicKeyLocator.java @@ -36,6 +36,7 @@ import org.keycloak.dom.saml.v2.metadata.KeyTypes; import org.keycloak.rotation.KeyLocator; import org.keycloak.saml.processing.api.util.KeyInfoTools; +import java.security.cert.CertificateException; /** * This class defines a {@link KeyLocator} that looks up public keys and certificates in IdP's @@ -147,6 +148,11 @@ private synchronized PublicKey refreshCertificateCacheAndGet(String kid) { for (KeyInfo ki : signingCerts) { KeyName keyName = KeyInfoTools.getKeyName(ki); X509Certificate x509certificate = KeyInfoTools.getX509Certificate(ki); + try { + x509certificate.checkValidity(); + } catch (CertificateException ex) { + x509certificate = null; + } if (x509certificate != null && keyName != null) { LOG.tracef("Registering signing certificate %s", keyName.getName()); this.publicKeyCache.put(keyName.getName(), x509certificate.getPublicKey()); diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java index a65f86aac5d8..6aaecaf0dbcd 100755 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java +++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java @@ -88,9 +88,11 @@ import org.keycloak.rotation.KeyLocator; import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator; import org.keycloak.saml.validators.DestinationValidator; +import java.security.cert.CertificateException; import org.w3c.dom.Element; import java.util.*; +import javax.security.auth.x500.X500Principal; import javax.xml.crypto.dsig.XMLSignature; import org.w3c.dom.NodeList; @@ -218,9 +220,13 @@ protected KeyLocator getIDPKeyLocator() { List keys = new LinkedList<>(); for (String signingCertificate : config.getSigningCertificates()) { + X509Certificate cert = null; try { - X509Certificate cert = XMLSignatureUtil.getX509CertificateFromKeyInfoString(signingCertificate.replaceAll("\\s", "")); + cert = XMLSignatureUtil.getX509CertificateFromKeyInfoString(signingCertificate.replaceAll("\\s", "")); + cert.checkValidity(); keys.add(cert.getPublicKey()); + } catch (CertificateException e) { + logger.warnf("Ignoring invalid certificate: %s", cert); } catch (ProcessingException e) { throw new RuntimeException(e); } diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolUtils.java b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolUtils.java index 9a601739a2e5..825a31e94b92 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolUtils.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocolUtils.java @@ -29,11 +29,9 @@ import org.w3c.dom.Document; import javax.ws.rs.core.MultivaluedMap; -import javax.ws.rs.core.UriBuilder; import javax.ws.rs.core.UriInfo; import java.security.PublicKey; import java.security.Signature; -import java.security.cert.Certificate; import org.keycloak.dom.saml.v2.SAML2Object; import org.keycloak.dom.saml.v2.protocol.ExtensionsType; import org.keycloak.dom.saml.v2.protocol.RequestAbstractType; @@ -42,6 +40,8 @@ import org.keycloak.rotation.KeyLocator; import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; import org.w3c.dom.Element; /** @@ -114,9 +114,12 @@ public static PublicKey getPublicKey(ClientModel client, String attribute) throw private static PublicKey getPublicKey(String certPem) throws VerificationException { if (certPem == null) throw new VerificationException("Client does not have a public key."); - Certificate cert = null; + X509Certificate cert = null; try { cert = PemUtils.decodeCertificate(certPem); + cert.checkValidity(); + } catch (CertificateException ex) { + throw new VerificationException("Certificate is not valid."); } catch (Exception e) { throw new VerificationException("Could not decode cert", e); } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java index 4d46bc827ecc..106aa5b0b2ae 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/ClientAttributeUpdater.java @@ -1,9 +1,14 @@ package org.keycloak.testsuite.updaters; +import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.ClientResource; +import org.keycloak.admin.client.resource.ClientsResource; import org.keycloak.representations.idm.ClientRepresentation; import java.io.Closeable; import java.util.HashMap; +import java.util.List; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.Assert.assertThat; /** * @@ -16,6 +21,15 @@ public class ClientAttributeUpdater { private final ClientRepresentation rep; private final ClientRepresentation origRep; + public static ClientAttributeUpdater forClient(Keycloak adminClient, String realm, String clientId) { + ClientsResource clients = adminClient.realm(realm).clients(); + List foundClients = clients.findByClientId(clientId); + assertThat(foundClients, hasSize(1)); + ClientResource clientRes = clients.get(foundClients.get(0).getId()); + + return new ClientAttributeUpdater(clientRes); + } + public ClientAttributeUpdater(ClientResource clientResource) { this.clientResource = clientResource; this.origRep = clientResource.toRepresentation(); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java index 2e98e6e25916..1b9df30fc0a8 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/updaters/RealmAttributeUpdater.java @@ -41,6 +41,16 @@ public RealmAttributeUpdater removeAttribute(String name) { return this; } + public RealmAttributeUpdater setPublicKey(String key) { + this.rep.setPublicKey(key); + return this; + } + + public RealmAttributeUpdater setPrivateKey(String key) { + this.rep.setPrivateKey(key); + return this; + } + public Closeable update() { realmResource.update(rep); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java index 726910629278..7256be9ab7ea 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClient.java @@ -248,7 +248,7 @@ public HttpUriRequest createSamlSignedResponse(URI samlEndpoint, String relaySta @Override public HttpUriRequest createSamlSignedRequest(URI samlEndpoint, String relayState, Document samlRequest, String realmPrivateKey, String realmPublicKey) { - return null; + throw new UnsupportedOperationException("Not implemented yet."); } }; diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/CreateAuthnRequestStepBuilder.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/CreateAuthnRequestStepBuilder.java index aa85821179ab..0a0dc5d4fdea 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/CreateAuthnRequestStepBuilder.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/CreateAuthnRequestStepBuilder.java @@ -39,6 +39,8 @@ public class CreateAuthnRequestStepBuilder extends SamlDocumentStepBuilder