From af8354267b1bdb76189e4a5bc213423c095d2607 Mon Sep 17 00:00:00 2001 From: Luca Leonardo Scorcia Date: Mon, 13 Sep 2021 12:12:38 +0200 Subject: [PATCH] KEYCLOAK-16462 X509 Auth: add option to revalidate certificate trust --- ...actX509ClientCertificateAuthenticator.java | 3 + ...ClientCertificateAuthenticatorFactory.java | 9 +- .../x509/CertificateValidator.java | 113 +++++++++++++++++- .../x509/ValidateX509CertificateUsername.java | 1 + .../x509/X509AuthenticatorConfigModel.java | 9 ++ .../X509ClientCertificateAuthenticator.java | 1 + .../x509/AbstractX509AuthenticationTest.java | 5 + .../testsuite/x509/X509BrowserLoginTest.java | 5 + 8 files changed, 141 insertions(+), 5 deletions(-) diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticator.java index e93e5b0397..13c455b882 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticator.java @@ -85,6 +85,7 @@ public abstract class AbstractX509ClientCertificateAuthenticator implements Auth public static final String CERTIFICATE_EXTENDED_KEY_USAGE = "x509-cert-auth.extendedkeyusage"; static final String DEFAULT_MATCH_ALL_EXPRESSION = "(.*?)(?:$)"; public static final String CONFIRMATION_PAGE_DISALLOWED = "x509-cert-auth.confirmation-page-disallowed"; + public static final String REVALIDATE_CERTIFICATE = "x509-cert-auth.revalidate-certificate-enabled"; protected Response createInfoResponse(AuthenticationFlowContext context, String infoMessage, Object ... parameters) { @@ -110,6 +111,8 @@ public abstract class AbstractX509ClientCertificateAuthenticator implements Auth .oCSPEnabled(config.getOCSPEnabled()) .oCSPResponseCertificate(config.getOCSPResponderCertificate()) .oCSPResponderURI(config.getOCSPResponder()) + .trustValidation() + .enabled(config.getRevalidateCertificateEnabled()) .timestampValidation() .enabled(config.isCertValidationEnabled()); } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticatorFactory.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticatorFactory.java index cafa966b52..6bad5ad0f0 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticatorFactory.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticatorFactory.java @@ -207,6 +207,12 @@ public abstract class AbstractX509ClientCertificateAuthenticatorFactory implemen identityConfirmationPageDisallowed.setLabel("Bypass identity confirmation"); identityConfirmationPageDisallowed.setHelpText("By default, the users are prompted to confirm their identity extracted from X509 client certificate. The identity confirmation prompt is skipped if the option is switched on."); + ProviderConfigProperty revalidateCertificateEnabled = new ProviderConfigProperty(); + revalidateCertificateEnabled.setType(BOOLEAN_TYPE); + revalidateCertificateEnabled.setName(REVALIDATE_CERTIFICATE); + revalidateCertificateEnabled.setLabel("Revalidate Client Certificate"); + revalidateCertificateEnabled.setHelpText("Forces revalidation of the client certificate according to the certificates defined in the truststore. This is useful when behind a non-validating proxy or when the number of allowed certificate chains would be too large for mutual SSL negotiation."); + configProperties = asList(mappingMethodList, canonicalDn, serialnumberHex, @@ -222,7 +228,8 @@ public abstract class AbstractX509ClientCertificateAuthenticatorFactory implemen ocspResponderCert, keyUsage, extendedKeyUsage, - identityConfirmationPageDisallowed); + identityConfirmationPageDisallowed, + revalidateCertificateEnabled); } @Override diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java index a327ad36df..16289dc8d7 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java @@ -47,16 +47,25 @@ import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; import java.security.GeneralSecurityException; -import java.security.cert.CRLException; +import java.security.cert.CertPathBuilder; +import java.security.cert.CertPathBuilderException; import java.security.cert.CertPathValidatorException; +import java.security.cert.CertStore; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; -import java.security.cert.X509CRL; +import java.security.cert.CollectionCertStoreParameters; +import java.security.cert.CRLException; +import java.security.cert.PKIXBuilderParameters; +import java.security.cert.PKIXCertPathBuilderResult; +import java.security.cert.TrustAnchor; import java.security.cert.X509Certificate; +import java.security.cert.X509CertSelector; +import java.security.cert.X509CRL; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.Hashtable; import java.util.LinkedList; import java.util.List; @@ -388,6 +397,7 @@ public class CertificateValidator { boolean _ocspEnabled; OCSPChecker ocspChecker; boolean _timestampValidationEnabled; + boolean _trustValidationEnabled; public CertificateValidator() { @@ -400,7 +410,8 @@ public class CertificateValidator { boolean oCSPCheckingEnabled, OCSPChecker ocspChecker, KeycloakSession session, - boolean timestampValidationEnabled) { + boolean timestampValidationEnabled, + boolean trustValidationEnabled) { _certChain = certChain; _keyUsageBits = keyUsageBits; _extendedKeyUsage = extendedKeyUsage; @@ -411,6 +422,7 @@ public class CertificateValidator { this.ocspChecker = ocspChecker; this.session = session; _timestampValidationEnabled = timestampValidationEnabled; + _trustValidationEnabled = trustValidationEnabled; if (ocspChecker == null) throw new IllegalArgumentException("ocspChecker"); @@ -487,6 +499,7 @@ public class CertificateValidator { validateKeyUsage(_certChain, _keyUsageBits); return this; } + public CertificateValidator validateExtendedKeyUsage() throws GeneralSecurityException { validateExtendedKeyUsage(_certChain, _extendedKeyUsage); return this; @@ -519,6 +532,79 @@ public class CertificateValidator { return this; } + public CertificateValidator validateTrust() throws GeneralSecurityException { + if (!_trustValidationEnabled) + return this; + + TruststoreProvider truststoreProvider = session.getProvider(TruststoreProvider.class); + if (truststoreProvider == null || truststoreProvider.getTruststore() == null) { + logger.error("Cannot validate client certificate trust: Truststore not available"); + } + else + { + Set trustedRootCerts = truststoreProvider.getRootCertificates().entrySet().stream().map(t -> t.getValue()).collect(Collectors.toSet()); + Set trustedIntermediateCerts = truststoreProvider.getIntermediateCertificates().entrySet().stream().map(t -> t.getValue()).collect(Collectors.toSet()); + + logger.debugf("Found %d trusted root certs, %d trusted intermediate certs", trustedRootCerts.size(), trustedIntermediateCerts.size()); + + verifyCertificateTrust(_certChain, trustedRootCerts, trustedIntermediateCerts); + } + + return this; + } + + /** + * Attempts to build a certification chain for given certificate and to verify + * it. Relies on a set of root CA certificates (trust anchors) and a set of + * intermediate certificates (to be used as part of the chain). + * @param certChain - client chain presented for validation. cert to validate is assumed to be the first in the chain + * @param trustedRootCerts - set of trusted root CA certificates + * @param trustedIntermediateCerts - set of intermediate certificates + * @return the certification chain (if verification is successful) + * @throws GeneralSecurityException - if the verification is not successful + * (e.g. certification path cannot be built or some certificate in the + * chain is expired) + */ + private static PKIXCertPathBuilderResult verifyCertificateTrust(X509Certificate[] certChain, Set trustedRootCerts, + Set trustedIntermediateCerts) throws GeneralSecurityException { + + // Create the selector that specifies the starting certificate + X509CertSelector selector = new X509CertSelector(); + selector.setCertificate(certChain[0]); + + // Create the trust anchors (set of root CA certificates) + Set trustAnchors = new HashSet(); + for (X509Certificate trustedRootCert : trustedRootCerts) { + trustAnchors.add(new TrustAnchor(trustedRootCert, null)); + } + + // Configure the PKIX certificate builder algorithm parameters + PKIXBuilderParameters pkixParams = + new PKIXBuilderParameters(trustAnchors, selector); + + // Disable CRL checks (this is done manually as additional step) + pkixParams.setRevocationEnabled(false); + + // Specify a list of intermediate certificates + Set intermediateCerts = new HashSet(); + for (X509Certificate intermediateCert : trustedIntermediateCerts) { + intermediateCerts.add(intermediateCert); + } + // Client certificates have to be added to the list of intermediate certs + for (X509Certificate clientCert : certChain) { + intermediateCerts.add(clientCert); + } + CertStore intermediateCertStore = CertStore.getInstance("Collection", + new CollectionCertStoreParameters(intermediateCerts), "BC"); + pkixParams.addCertStore(intermediateCertStore); + + // Build and verify the certification chain + CertPathBuilder builder = CertPathBuilder.getInstance("PKIX", "BC"); + PKIXCertPathBuilderResult result = + (PKIXCertPathBuilderResult) builder.build(pkixParams); + return result; + } + private X509Certificate findCAInTruststore(X500Principal issuer) throws GeneralSecurityException { TruststoreProvider truststoreProvider = session.getProvider(TruststoreProvider.class); if (truststoreProvider == null || truststoreProvider.getTruststore() == null) { @@ -593,6 +679,7 @@ public class CertificateValidator { } } } + private static List getCRLDistributionPoints(X509Certificate cert) { try { return CRLUtils.getCRLDistributionPoints(cert); @@ -650,6 +737,7 @@ public class CertificateValidator { String _responderUri; X509Certificate _responderCert; boolean _timestampValidationEnabled; + boolean _trustValidationEnabled; public CertificateValidatorBuilder() { _extendedKeyUsage = new LinkedList<>(); @@ -831,6 +919,19 @@ public class CertificateValidator { } } + public class TrustValidationBuilder { + + CertificateValidatorBuilder _parent; + protected TrustValidationBuilder(CertificateValidatorBuilder parent) { + _parent = parent; + } + + public CertificateValidatorBuilder enabled(boolean value) { + _trustValidationEnabled = value; + return _parent; + } + } + public CertificateValidatorBuilder session(KeycloakSession session) { this.session = session; return this; @@ -852,13 +953,17 @@ public class CertificateValidator { return new TimestampValidationBuilder(this); } + public TrustValidationBuilder trustValidation() { + return new TrustValidationBuilder(this); + } + public CertificateValidator build(X509Certificate[] certs) { if (_crlLoader == null) { _crlLoader = new CRLFileLoader(session, ""); } return new CertificateValidator(certs, _keyUsageBits, _extendedKeyUsage, _crlCheckingEnabled, _crldpEnabled, _crlLoader, _ocspEnabled, - new BouncyCastleOCSPChecker(session, _responderUri, _responderCert), session, _timestampValidationEnabled); + new BouncyCastleOCSPChecker(session, _responderUri, _responderCert), session, _timestampValidationEnabled, _trustValidationEnabled); } } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/ValidateX509CertificateUsername.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/ValidateX509CertificateUsername.java index 577f7514b0..cd214f20df 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/ValidateX509CertificateUsername.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/ValidateX509CertificateUsername.java @@ -74,6 +74,7 @@ public class ValidateX509CertificateUsername extends AbstractX509ClientCertifica CertificateValidator.CertificateValidatorBuilder builder = certificateValidationParameters(context.getSession(), config); CertificateValidator validator = builder.build(certs); validator.checkRevocationStatus() + .validateTrust() .validateKeyUsage() .validateExtendedKeyUsage() .validateTimestamps(); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509AuthenticatorConfigModel.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509AuthenticatorConfigModel.java index 634c04a811..d95c9bc67d 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509AuthenticatorConfigModel.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509AuthenticatorConfigModel.java @@ -271,4 +271,13 @@ public class X509AuthenticatorConfigModel extends AuthenticatorConfigModel { getConfig().put(SERIALNUMBER_HEX, Boolean.toString(value)); return this; } + + public boolean getRevalidateCertificateEnabled() { + return Boolean.parseBoolean(getConfig().get(REVALIDATE_CERTIFICATE)); + } + + public X509AuthenticatorConfigModel setRevalidateCertificateEnabled(boolean value) { + getConfig().put(REVALIDATE_CERTIFICATE, Boolean.toString(value)); + return this; + } } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509ClientCertificateAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509ClientCertificateAuthenticator.java index 8aebcfa75e..282ef1ea4e 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509ClientCertificateAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509ClientCertificateAuthenticator.java @@ -85,6 +85,7 @@ public class X509ClientCertificateAuthenticator extends AbstractX509ClientCertif CertificateValidator.CertificateValidatorBuilder builder = certificateValidationParameters(context.getSession(), config); CertificateValidator validator = builder.build(certs); validator.checkRevocationStatus() + .validateTrust() .validateKeyUsage() .validateExtendedKeyUsage() .validateTimestamps(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/AbstractX509AuthenticationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/AbstractX509AuthenticationTest.java index eb6aa45e0f..6c3e2abc37 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/AbstractX509AuthenticationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/AbstractX509AuthenticationTest.java @@ -434,6 +434,11 @@ public abstract class AbstractX509AuthenticationTest extends AbstractTestRealmKe .setExtendedKeyUsage(extendedKeyUsage); } + protected static X509AuthenticatorConfigModel createLoginSubjectEmailWithRevalidateCert(boolean revalidateCertEnabled) { + return createLoginSubjectEmail2UsernameOrEmailConfig() + .setRevalidateCertificateEnabled(revalidateCertEnabled); + } + protected static X509AuthenticatorConfigModel createLoginSubjectCN2UsernameOrEmailConfig() { return new X509AuthenticatorConfigModel() .setConfirmationPageAllowed(true) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509BrowserLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509BrowserLoginTest.java index d1f041f998..05a6ca1376 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509BrowserLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509BrowserLoginTest.java @@ -114,6 +114,11 @@ public class X509BrowserLoginTest extends AbstractX509AuthenticationTest { x509BrowserLogin(createLoginSubjectEmailWithExtendedKeyUsage("serverAuth"), userId, "test-user@localhost", "test-user@localhost"); } + @Test + public void loginWithRevalidateCertEnabledCertIsTrusted() throws Exception { + x509BrowserLogin(createLoginSubjectEmailWithRevalidateCert(true), userId, "test-user@localhost", "test-user@localhost"); + } + @Test public void loginIgnoreX509IdentityContinueToFormLogin() throws Exception { // Set the X509 authenticator configuration