Allow certificate with duplicate principals in truststore.

The previous implementation uses principal as a key for a hashmap storing one certificate per entry. To preserve lookups, the value is now a List of certificates.

Additional logic was added to build certification validation chains using signature verification rather than just principal.

Closes #33125

Signed-off-by: Matt Eaton <git@divinehawk.com>
This commit is contained in:
Matt Eaton 2024-10-04 19:51:24 +02:00 committed by Marek Posolda
parent 07cf71e818
commit 9f0a348e4c
6 changed files with 86 additions and 40 deletions

View file

@ -22,6 +22,7 @@ import org.keycloak.provider.Provider;
import java.security.cert.X509Certificate; import java.security.cert.X509Certificate;
import java.security.KeyStore; import java.security.KeyStore;
import java.util.List;
import java.util.Map; import java.util.Map;
import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.SSLSocketFactory;
import javax.security.auth.x500.X500Principal; import javax.security.auth.x500.X500Principal;
@ -40,10 +41,10 @@ public interface TruststoreProvider extends Provider {
/** /**
* @return root certificates from the configured truststore as a map where the key is the X500Principal of the corresponding X509Certificate * @return root certificates from the configured truststore as a map where the key is the X500Principal of the corresponding X509Certificate
*/ */
Map<X500Principal, X509Certificate> getRootCertificates(); Map<X500Principal, List<X509Certificate>> getRootCertificates();
/** /**
* @return intermediate certificates from the configured truststore as a map where the key is the X500Principal of the corresponding X509Certificate * @return intermediate certificates from the configured truststore as a map where the key is the X500Principal of the corresponding X509Certificate
*/ */
Map<X500Principal, X509Certificate> getIntermediateCertificates(); Map<X500Principal, List<X509Certificate>> getIntermediateCertificates();
} }

View file

@ -29,6 +29,8 @@ import java.io.InputStream;
import java.net.URI; import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.security.GeneralSecurityException; import java.security.GeneralSecurityException;
import java.security.InvalidKeyException;
import java.security.SignatureException;
import java.security.cert.CRLException; import java.security.cert.CRLException;
import java.security.cert.CertPathBuilder; import java.security.cert.CertPathBuilder;
import java.security.cert.CertPathValidatorException; import java.security.cert.CertPathValidatorException;
@ -589,8 +591,8 @@ public class CertificateValidator {
} }
else else
{ {
Set<X509Certificate> trustedRootCerts = truststoreProvider.getRootCertificates().entrySet().stream().map(t -> t.getValue()).collect(Collectors.toSet()); Set<X509Certificate> trustedRootCerts = truststoreProvider.getRootCertificates().entrySet().stream().flatMap(t -> t.getValue().stream()).collect(Collectors.toSet());
Set<X509Certificate> trustedIntermediateCerts = truststoreProvider.getIntermediateCertificates().entrySet().stream().map(t -> t.getValue()).collect(Collectors.toSet()); Set<X509Certificate> trustedIntermediateCerts = truststoreProvider.getIntermediateCertificates().entrySet().stream().flatMap(t -> t.getValue().stream()).collect(Collectors.toSet());
logger.debugf("Found %d trusted root certs, %d trusted intermediate certs", trustedRootCerts.size(), trustedIntermediateCerts.size()); logger.debugf("Found %d trusted root certs, %d trusted intermediate certs", trustedRootCerts.size(), trustedIntermediateCerts.size());
@ -651,16 +653,30 @@ public class CertificateValidator {
return result; return result;
} }
private X509Certificate findCAInTruststore(X500Principal issuer) throws GeneralSecurityException { private X509Certificate findCAInTruststore(X509Certificate cert) throws GeneralSecurityException {
TruststoreProvider truststoreProvider = session.getProvider(TruststoreProvider.class); TruststoreProvider truststoreProvider = session.getProvider(TruststoreProvider.class);
if (truststoreProvider == null || truststoreProvider.getTruststore() == null) { if (truststoreProvider == null || truststoreProvider.getTruststore() == null) {
return null; return null;
} }
Map<X500Principal, X509Certificate> rootCerts = truststoreProvider.getRootCertificates(); Map<X500Principal, List<X509Certificate>> rootCerts = truststoreProvider.getRootCertificates();
X509Certificate ca = rootCerts.get(issuer); X500Principal issuer = cert.getIssuerX500Principal();
if (ca == null) { List<X509Certificate> cas = rootCerts.get(issuer);
X509Certificate ca = null;
if (cas == null) {
// fallback to lookup the issuer from the list of intermediary CAs // fallback to lookup the issuer from the list of intermediary CAs
ca = truststoreProvider.getIntermediateCertificates().get(issuer); cas = truststoreProvider.getIntermediateCertificates().get(issuer);
}
// find the one that signed the cert
if (cas != null) {
for (X509Certificate cacert : cas) {
try {
cert.verify(cacert.getPublicKey());
} catch (InvalidKeyException | SignatureException e) {
continue;
}
ca = cacert;
break;
}
} }
if (ca != null) { if (ca != null) {
ca.checkValidity(); ca.checkValidity();
@ -687,7 +703,7 @@ public class CertificateValidator {
} else { } else {
// only one cert => find the CA certificate using the truststore SPI // only one cert => find the CA certificate using the truststore SPI
cert = certs[0]; cert = certs[0];
issuer = findCAInTruststore(cert.getIssuerX500Principal()); issuer = findCAInTruststore(cert);
if (issuer == null) { if (issuer == null) {
throw new GeneralSecurityException( throw new GeneralSecurityException(
String.format("No trusted CA in certificate found: %s. Add it to truststore SPI if valid.", String.format("No trusted CA in certificate found: %s. Add it to truststore SPI if valid.",

View file

@ -10,6 +10,7 @@ import org.keycloak.truststore.TruststoreProviderFactory;
import java.security.cert.X509Certificate; import java.security.cert.X509Certificate;
import java.util.Set; import java.util.Set;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
/** /**
* The factory and the corresponding providers extract a client certificate * The factory and the corresponding providers extract a client certificate
@ -84,8 +85,11 @@ public class NginxProxySslClientCertificateLookupFactory extends AbstractClientC
TruststoreProvider provider = truststoreFactory.create(kcSession); TruststoreProvider provider = truststoreFactory.create(kcSession);
if (provider != null && provider.getTruststore() != null) { if (provider != null && provider.getTruststore() != null) {
trustedRootCerts.addAll(provider.getRootCertificates().values()); Set<X509Certificate> rootCertificates = provider.getRootCertificates().entrySet().stream().flatMap(t -> t.getValue().stream()).collect(Collectors.toSet());
intermediateCerts.addAll(provider.getIntermediateCertificates().values()); Set<X509Certificate> intermediateCertficiates = provider.getIntermediateCertificates().entrySet().stream().flatMap(t -> t.getValue().stream()).collect(Collectors.toSet());
trustedRootCerts.addAll(rootCertificates);
intermediateCerts.addAll(intermediateCertficiates);
logger.debug("Keycloak truststore loaded for NGINX x509cert-lookup provider."); logger.debug("Keycloak truststore loaded for NGINX x509cert-lookup provider.");
isTruststoreLoaded = true; isTruststoreLoaded = true;

View file

@ -21,6 +21,7 @@ import org.keycloak.common.enums.HostnameVerificationPolicy;
import java.security.KeyStore; import java.security.KeyStore;
import java.security.cert.X509Certificate; import java.security.cert.X509Certificate;
import java.util.List;
import java.util.Map; import java.util.Map;
import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.SSLSocketFactory;
import javax.security.auth.x500.X500Principal; import javax.security.auth.x500.X500Principal;
@ -33,10 +34,10 @@ public class FileTruststoreProvider implements TruststoreProvider {
private final HostnameVerificationPolicy policy; private final HostnameVerificationPolicy policy;
private final SSLSocketFactory sslSocketFactory; private final SSLSocketFactory sslSocketFactory;
private final KeyStore truststore; private final KeyStore truststore;
private final Map<X500Principal, X509Certificate> rootCertificates; private final Map<X500Principal, List<X509Certificate>> rootCertificates;
private final Map<X500Principal, X509Certificate> intermediateCertificates; private final Map<X500Principal, List<X509Certificate>> intermediateCertificates;
public FileTruststoreProvider(KeyStore truststore, HostnameVerificationPolicy policy, Map<X500Principal, X509Certificate> rootCertificates, Map<X500Principal, X509Certificate> intermediateCertificates) { public FileTruststoreProvider(KeyStore truststore, HostnameVerificationPolicy policy, Map<X500Principal, List<X509Certificate>> rootCertificates,Map<X500Principal, List<X509Certificate>> intermediateCertificates) {
this.policy = policy; this.policy = policy;
this.truststore = truststore; this.truststore = truststore;
this.rootCertificates = rootCertificates; this.rootCertificates = rootCertificates;
@ -62,12 +63,12 @@ public class FileTruststoreProvider implements TruststoreProvider {
} }
@Override @Override
public Map<X500Principal, X509Certificate> getRootCertificates() { public Map<X500Principal, List<X509Certificate>> getRootCertificates() {
return rootCertificates; return rootCertificates;
} }
@Override @Override
public Map<X500Principal, X509Certificate> getIntermediateCertificates() { public Map<X500Principal, List<X509Certificate>> getIntermediateCertificates() {
return intermediateCertificates; return intermediateCertificates;
} }

View file

@ -37,6 +37,7 @@ import java.security.SignatureException;
import java.security.cert.Certificate; import java.security.cert.Certificate;
import java.security.cert.CertificateException; import java.security.cert.CertificateException;
import java.security.cert.X509Certificate; import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Enumeration; import java.util.Enumeration;
@ -176,8 +177,8 @@ public class FileTruststoreProviderFactory implements TruststoreProviderFactory
private static class TruststoreCertificatesLoader { private static class TruststoreCertificatesLoader {
private Map<X500Principal, X509Certificate> trustedRootCerts = new HashMap<>(); private Map<X500Principal, List<X509Certificate>> trustedRootCerts = new HashMap<>();
private Map<X500Principal, X509Certificate> intermediateCerts = new HashMap<>(); private Map<X500Principal, List<X509Certificate>> intermediateCerts = new HashMap<>();
public TruststoreCertificatesLoader(KeyStore truststore) { public TruststoreCertificatesLoader(KeyStore truststore) {
@ -213,11 +214,21 @@ public class FileTruststoreProviderFactory implements TruststoreProviderFactory
X509Certificate cax509cert = (X509Certificate) certificate; X509Certificate cax509cert = (X509Certificate) certificate;
if (isSelfSigned(cax509cert)) { if (isSelfSigned(cax509cert)) {
X500Principal principal = cax509cert.getSubjectX500Principal(); X500Principal principal = cax509cert.getSubjectX500Principal();
trustedRootCerts.put(principal, cax509cert); List<X509Certificate> certs = trustedRootCerts.get(principal);
if (certs == null) {
certs = new ArrayList<>();
trustedRootCerts.put(principal, certs);
}
certs.add(cax509cert);
log.debug("Trusted root CA found in truststore : alias : " + alias + " | Subject DN : " + principal); log.debug("Trusted root CA found in truststore : alias : " + alias + " | Subject DN : " + principal);
} else { } else {
X500Principal principal = cax509cert.getSubjectX500Principal(); X500Principal principal = cax509cert.getSubjectX500Principal();
intermediateCerts.put(principal, cax509cert); List<X509Certificate> certs = intermediateCerts.get(principal);
if (certs == null) {
certs = new ArrayList<>();
intermediateCerts.put(principal, certs);
}
certs.add(cax509cert);
log.debug("Intermediate CA found in truststore : alias : " + alias + " | Subject DN : " + principal); log.debug("Intermediate CA found in truststore : alias : " + alias + " | Subject DN : " + principal);
} }
} else } else

View file

@ -18,9 +18,12 @@
package org.keycloak.utils; package org.keycloak.utils;
import java.security.GeneralSecurityException; import java.security.GeneralSecurityException;
import java.security.InvalidKeyException;
import java.security.SignatureException;
import java.security.cert.X509CRL; import java.security.cert.X509CRL;
import java.security.cert.X509Certificate; import java.security.cert.X509Certificate;
import java.util.Arrays; import java.util.Arrays;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
@ -71,7 +74,7 @@ public final class CRLUtils {
// Try to find the CRL issuer certificate in the truststore // Try to find the CRL issuer certificate in the truststore
if (crlSignatureCertificate == null) { if (crlSignatureCertificate == null) {
log.tracef("Not found CRL issuer '%s' in the CA chain of the certificate. Fallback to lookup CRL issuer in the truststore", crlIssuerPrincipal); log.tracef("Not found CRL issuer '%s' in the CA chain of the certificate. Fallback to lookup CRL issuer in the truststore", crlIssuerPrincipal);
crlSignatureCertificate = findCRLSignatureCertificateInTruststore(session, certs, crlIssuerPrincipal); crlSignatureCertificate = findCRLSignatureCertificateInTruststore(session, certs, crl);
} }
// Verify signature on CRL // Verify signature on CRL
@ -87,18 +90,31 @@ public final class CRLUtils {
} }
private static X509Certificate findCRLSignatureCertificateInTruststore(KeycloakSession session, X509Certificate[] certs, X500Principal crlIssuerPrincipal) throws GeneralSecurityException { private static X509Certificate findCRLSignatureCertificateInTruststore(KeycloakSession session, X509Certificate[] certs, X509CRL crl) throws GeneralSecurityException {
TruststoreProvider truststoreProvider = session.getProvider(TruststoreProvider.class); TruststoreProvider truststoreProvider = session.getProvider(TruststoreProvider.class);
if (truststoreProvider == null || truststoreProvider.getTruststore() == null) { if (truststoreProvider == null || truststoreProvider.getTruststore() == null) {
throw new GeneralSecurityException("Truststore not available"); throw new GeneralSecurityException("Truststore not available");
} }
Map<X500Principal, X509Certificate> rootCerts = truststoreProvider.getRootCertificates(); X500Principal crlIssuerPrincipal = crl.getIssuerX500Principal();
Map<X500Principal, X509Certificate> intermediateCerts = truststoreProvider.getIntermediateCertificates(); Map<X500Principal, List<X509Certificate>> rootCerts = truststoreProvider.getRootCertificates();
Map<X500Principal, List<X509Certificate>> intermediateCerts = truststoreProvider.getIntermediateCertificates();
X509Certificate crlSignatureCertificate = intermediateCerts.get(crlIssuerPrincipal); List<X509Certificate> crlSignatureCertificates = intermediateCerts.get(crlIssuerPrincipal);
if (crlSignatureCertificate == null) { X509Certificate crlSignatureCertificate = null;
crlSignatureCertificate = rootCerts.get(crlIssuerPrincipal);
if (crlSignatureCertificates == null) {
crlSignatureCertificates = rootCerts.get(crlIssuerPrincipal);
}
for (X509Certificate cacert : crlSignatureCertificates) {
try {
crl.verify(cacert.getPublicKey());
} catch (InvalidKeyException | SignatureException e) {
continue;
}
crlSignatureCertificate = cacert;
break;
} }
if (crlSignatureCertificate == null) { if (crlSignatureCertificate == null) {
@ -112,25 +128,22 @@ public final class CRLUtils {
.map(X509Certificate::getIssuerX500Principal) .map(X509Certificate::getIssuerX500Principal)
.collect(Collectors.toSet()); .collect(Collectors.toSet());
X509Certificate currentCRLAnchorCertificate = crlSignatureCertificate;
X500Principal currentCRLAnchorPrincipal = crlIssuerPrincipal; X500Principal currentCRLAnchorPrincipal = crlIssuerPrincipal;
for (X500Principal certificateCAPrincipal : certificateCAPrincipals) { for (X500Principal certificateCAPrincipal : certificateCAPrincipals) {
if (certificateCAPrincipal.equals(currentCRLAnchorPrincipal)) { if (certificateCAPrincipal.equals(currentCRLAnchorPrincipal)) {
log.tracef("Found trust anchor of the CRL issuer '%s' in the CA chain. Anchor is '%s'", crlIssuerPrincipal, currentCRLAnchorPrincipal); log.tracef("Found trust anchor of the CRL issuer '%s' in the CA chain. Anchor is '%s'", crlIssuerPrincipal, currentCRLAnchorPrincipal);
break; return crlSignatureCertificate;
} }
}
// Try to see the anchor // Anchor was not in the provided certificate chain, check the truststore
currentCRLAnchorPrincipal = currentCRLAnchorCertificate.getIssuerX500Principal(); List<X509Certificate> currentCRLAnchorCertificates = intermediateCerts.get(currentCRLAnchorPrincipal);
if (currentCRLAnchorCertificates == null) {
currentCRLAnchorCertificate = intermediateCerts.get(currentCRLAnchorPrincipal); currentCRLAnchorCertificates = rootCerts.get(currentCRLAnchorPrincipal);
if (currentCRLAnchorCertificate == null) { }
currentCRLAnchorCertificate = rootCerts.get(currentCRLAnchorPrincipal); if (currentCRLAnchorCertificates == null) {
} throw new GeneralSecurityException("Certificate for CRL issuer '" + crlIssuerPrincipal + "' available in the truststore, but doesn't have trust anchors with the CA chain.");
if (currentCRLAnchorCertificate == null) {
throw new GeneralSecurityException("Certificate for CRL issuer '" + crlIssuerPrincipal + "' available in the truststore, but doesn't have trust anchors with the CA chain.");
}
} }
return crlSignatureCertificate; return crlSignatureCertificate;