refactor(SubjectCertificateTrustedValidator): use standard JCE PKIX classes for certificate trust validation (#5)

Signed-off-by: Mart Somermaa <mrts@users.noreply.github.com>

Author: mrts

src/main/java/org/webeid/security/exceptions/JceException.java

@@ -0,0 +1,9 @@
+package org.webeid.security.exceptions;
+
+import java.security.GeneralSecurityException;
+
+public class JceException extends TokenValidationException {
+    public JceException(GeneralSecurityException e) {
+        super("Java Cryptography Extension loading or configuration failed", e);
+    }
+}

src/main/java/org/webeid/security/validator/AuthTokenValidationConfiguration.java

@@ -46,7 +46,7 @@ final class AuthTokenValidationConfiguration {
 
     private URI siteOrigin;
     private Cache<String, LocalDateTime> nonceCache;
-    private Collection<X509Certificate> trustedCACertificates = new HashSet<>();
+    private Collection<X509Certificate> trustedRootCACertificates = new HashSet<>();
     private boolean isUserCertificateRevocationCheckWithOcspEnabled = true;
     private Duration ocspRequestTimeout = Duration.ofSeconds(5);
     private Duration allowedClientClockSkew = Duration.ofMinutes(3);
@@ -63,7 +63,7 @@ final class AuthTokenValidationConfiguration {
     private AuthTokenValidationConfiguration(AuthTokenValidationConfiguration other) {
         this.siteOrigin = other.siteOrigin;
         this.nonceCache = other.nonceCache;
-        this.trustedCACertificates = new HashSet<>(other.trustedCACertificates);
+        this.trustedRootCACertificates = new HashSet<>(other.trustedRootCACertificates);
         this.isUserCertificateRevocationCheckWithOcspEnabled = other.isUserCertificateRevocationCheckWithOcspEnabled;
         this.ocspRequestTimeout = other.ocspRequestTimeout;
         this.allowedClientClockSkew = other.allowedClientClockSkew;
@@ -89,8 +89,8 @@ Cache<String, LocalDateTime> getNonceCache() {
         return nonceCache;
     }
 
-    Collection<X509Certificate> getTrustedCACertificates() {
-        return trustedCACertificates;
+    Collection<X509Certificate> getTrustedRootCACertificates() {
+        return trustedRootCACertificates;
     }
 
     boolean isUserCertificateRevocationCheckWithOcspEnabled() {
@@ -148,8 +148,8 @@ void validate() {
         Objects.requireNonNull(siteOrigin, "Origin URI must not be null");
         OriginValidator.validateIsOriginURL(siteOrigin);
         Objects.requireNonNull(nonceCache, "Nonce cache must not be null");
-        if (trustedCACertificates.isEmpty()) {
-            throw new IllegalArgumentException("At least one trusted certificate authority must be provided");
+        if (trustedRootCACertificates.isEmpty()) {
+            throw new IllegalArgumentException("At least one trusted root certificate authority must be provided");
         }
         requirePositiveDuration(ocspRequestTimeout, "OCSP request timeout");
         requirePositiveDuration(allowedClientClockSkew, "Allowed client clock skew");

src/main/java/org/webeid/security/validator/AuthTokenValidatorBuilder.java

@@ -25,6 +25,7 @@
 import org.bouncycastle.asn1.ASN1ObjectIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.webeid.security.exceptions.JceException;
 
 import javax.cache.Cache;
 import java.net.URI;
@@ -72,18 +73,18 @@ public AuthTokenValidatorBuilder withNonceCache(Cache<String, LocalDateTime> cac
     }
 
     /**
-     * Sets the list of trusted user certificate Certificate Authorities.
-     * In order for the user certificate to be considered valid, the issuer of the user certificate
-     * must be present in this list.
+     * Sets the list of trusted user certificate root Certificate Authorities.
+     * In order for the user certificate to be considered valid, the root certificate of the issuer
+     * of the user certificate must be present in this list.
      * <p>
-     * At least one trusted Certificate Authority must be provided as mandatory configuration parameter.
+     * At least one trusted root Certificate Authority must be provided as mandatory configuration parameter.
      *
-     * @param certificates trusted Certificate Authority certificates
+     * @param certificates trusted root Certificate Authority certificates
      * @return the builder instance for method chaining
      */
-    public AuthTokenValidatorBuilder withTrustedCertificateAuthorities(X509Certificate... certificates) {
-        Collections.addAll(configuration.getTrustedCACertificates(), certificates);
-        LOG.debug("Trusted certificate authorities set to {}", configuration.getTrustedCACertificates());
+    public AuthTokenValidatorBuilder withTrustedRootCertificateAuthorities(X509Certificate... certificates) {
+        Collections.addAll(configuration.getTrustedRootCACertificates(), certificates);
+        LOG.debug("Trusted root certificate authorities set to {}", configuration.getTrustedRootCACertificates());
         return this;
     }
 
@@ -128,6 +129,19 @@ public AuthTokenValidatorBuilder withOcspRequestTimeout(Duration ocspRequestTime
         return this;
     }
 
+    /**
+     * Sets the list of OCSP URLs for which the nonce protocol extension will be disabled.
+     * The OCSP URL is extracted from the user certificate and some OCSP services don't support the nonce extension.
+     *
+     * @param urls OCSP URLs for which the nonce protocol extension will be disabled
+     * @return the builder instance for method chaining
+     */
+    public AuthTokenValidatorBuilder withNonceDisabledOcspUrls(URI... urls) {
+        Collections.addAll(configuration.getNonceDisabledOcspUrls(), urls);
+        LOG.debug("OCSP URLs for which the nonce protocol extension is disabled set to {}", configuration.getNonceDisabledOcspUrls());
+        return this;
+    }
+
     /**
      * Sets the tolerated clock skew of the client computer when verifying the token expiration field {@code exp}.
      * <p>
@@ -162,8 +176,9 @@ public AuthTokenValidatorBuilder withSiteCertificateSha256Fingerprint(String cer
      * @return the configured authentication token validator object
      * @throws NullPointerException     when required parameters are null
      * @throws IllegalArgumentException when any parameter is invalid
+     * @throws RuntimeException         when JCE configuration is invalid
      */
-    public AuthTokenValidator build() throws NullPointerException, IllegalArgumentException {
+    public AuthTokenValidator build() throws NullPointerException, IllegalArgumentException, JceException {
         configuration.validate();
         return new AuthTokenValidatorImpl(configuration);
     }

src/main/java/org/webeid/security/validator/AuthTokenValidatorImpl.java

@@ -26,15 +26,22 @@
 import okhttp3.OkHttpClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.webeid.security.exceptions.JceException;
 import org.webeid.security.exceptions.TokenParseException;
 import org.webeid.security.exceptions.TokenValidationException;
 import org.webeid.security.validator.validators.*;
 
+import java.security.GeneralSecurityException;
+import java.security.cert.CertStore;
+import java.security.cert.CollectionCertStoreParameters;
+import java.security.cert.TrustAnchor;
 import java.security.cert.X509Certificate;
+import java.util.Set;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 /**
- * Provides default implementation of {@link AuthTokenValidator}.
+ * Provides the default implementation of {@link AuthTokenValidator}.
  */
 final class AuthTokenValidatorImpl implements AuthTokenValidator {
 
@@ -51,19 +58,19 @@ final class AuthTokenValidatorImpl implements AuthTokenValidator {
     private final Supplier<OkHttpClient> httpClientSupplier;
     private final ValidatorBatch simpleSubjectCertificateValidators;
     private final ValidatorBatch tokenBodyValidators;
+    private final Set<TrustAnchor> trustedRootCACertificateAnchors;
+    private final CertStore trustedRootCACertificateCertStore;
 
     /**
      * @param configuration configuration parameters for the token validator
      */
-    AuthTokenValidatorImpl(AuthTokenValidationConfiguration configuration) {
+    AuthTokenValidatorImpl(AuthTokenValidationConfiguration configuration) throws JceException {
         // Copy the configuration object to make AuthTokenValidatorImpl immutable and thread-safe.
         this.configuration = configuration.copy();
-        /*
-         * Lazy initialization, avoid constructing the OkHttpClient object when certificate revocation check is not enabled.
-         * Returns a supplier which caches the instance retrieved during the first call to get() and returns
-         * that value on subsequent calls to get(). The returned supplier is thread-safe.
-         * The OkHttpClient build() method will be invoked at most once.
-         */
+        // Lazy initialization, avoid constructing the OkHttpClient object when certificate revocation check is not enabled.
+        // Returns a supplier which caches the instance retrieved during the first call to get() and returns
+        // that value on subsequent calls to get(). The returned supplier is thread-safe.
+        // The OkHttpClient build() method will be invoked at most once.
         this.httpClientSupplier = Suppliers.memoize(() -> new OkHttpClient.Builder()
             .connectTimeout(configuration.getOcspRequestTimeout())
             .callTimeout(configuration.getOcspRequestTimeout())
@@ -82,6 +89,20 @@ final class AuthTokenValidatorImpl implements AuthTokenValidator {
             new SiteCertificateFingerprintValidator(configuration.getSiteCertificateSha256Fingerprint())::validateSiteCertificateFingerprint
         );
 
+        // Create and cache trusted root CA certificate JCA objects for SubjectCertificateTrustedValidator.
+        trustedRootCACertificateAnchors = configuration.getTrustedRootCACertificates()
+            .stream()
+            .map(cert -> new TrustAnchor(cert, null))
+            .collect(Collectors.toSet());
+        try {
+            // We use the default JCE provider as there is no reason to use Bouncy Castle, moreover BC requires
+            // the validated certificate to be in the certificate store which breaks the clean immutable usage of
+            // trustedRootCACertificateCertStore in SubjectCertificateTrustedValidator.
+            trustedRootCACertificateCertStore = CertStore.getInstance("Collection",
+                new CollectionCertStoreParameters(configuration.getTrustedRootCACertificates()));
+        } catch (GeneralSecurityException e) {
+            throw new JceException(e);
+        }
     }
 
     @Override
@@ -128,7 +149,7 @@ public X509Certificate validate(String authToken) throws TokenValidationExceptio
      */
     private ValidatorBatch getCertTrustValidators() {
         final SubjectCertificateTrustedValidator certTrustedValidator =
-            new SubjectCertificateTrustedValidator(configuration.getTrustedCACertificates());
+            new SubjectCertificateTrustedValidator(trustedRootCACertificateAnchors, trustedRootCACertificateCertStore);
         return ValidatorBatch.createFrom(
             certTrustedValidator::validateCertificateTrusted
         ).addOptional(configuration.isUserCertificateRevocationCheckWithOcspEnabled(),

src/main/java/org/webeid/security/validator/validators/SubjectCertificateNotRevokedValidator.java

@@ -77,7 +77,7 @@ public void validateCertificateNotRevoked(AuthTokenValidatorData actualTokenData
             final OCSPReq request = new OcspRequestBuilder()
                 .certificate(certificate)
                 .enableOcspNonce(!ocspNonceDisabled)
-                .issuer(Objects.requireNonNull(trustValidator.getSubjectCertificateIssuerCertificate()))
+                .issuer(Objects.requireNonNull(trustValidator.getSubjectCertificateIssuerRootCertificate()))
                 .build();
 
             LOG.debug("Sending OCSP request");

src/main/java/org/webeid/security/validator/validators/SubjectCertificateTrustedValidator.java

@@ -24,27 +24,32 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.webeid.security.exceptions.JceException;
 import org.webeid.security.exceptions.UserCertificateNotTrustedException;
 import org.webeid.security.validator.AuthTokenValidatorData;
 
-import javax.security.auth.x500.X500Principal;
-import java.security.GeneralSecurityException;
-import java.security.cert.X509Certificate;
-import java.util.Collection;
-import java.util.Map;
-import java.util.function.Function;
-import java.util.stream.Collectors;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.*;
+import java.util.Set;
 
+/**
+ * Validator that validates that the user certificate from the authentication token is signed by a trusted certificate authority.
+ * <p>
+ * We use the default JCE provider as there is no reason to use Bouncy Castle, moreover BC requires the validated certificate
+ * to be in the certificate store which breaks the clean immutable usage of {@code trustedRootCACertificateCertStore}.
+ */
 public final class SubjectCertificateTrustedValidator {
 
     private static final Logger LOG = LoggerFactory.getLogger(SubjectCertificateTrustedValidator.class);
 
-    private final Map<X500Principal, X509Certificate> trustedCACertificates;
-    private X509Certificate trustedCACertificate;
+    private final Set<TrustAnchor> trustedRootCACertificateAnchors;
+    private final CertStore trustedRootCACertificateCertStore;
+    private X509Certificate subjectCertificateIssuerRootCertificate;
 
-    public SubjectCertificateTrustedValidator(Collection<X509Certificate> trustedCACertificates) {
-        this.trustedCACertificates = trustedCACertificates.stream()
-            .collect(Collectors.toMap(X509Certificate::getSubjectX500Principal, Function.identity()));
+    public SubjectCertificateTrustedValidator(Set<TrustAnchor> trustedRootCACertificateAnchors, CertStore trustedRootCACertificateCertStore) {
+        this.trustedRootCACertificateAnchors = trustedRootCACertificateAnchors;
+        this.trustedRootCACertificateCertStore = trustedRootCACertificateCertStore;
     }
 
     /**
@@ -53,29 +58,33 @@ public SubjectCertificateTrustedValidator(Collection<X509Certificate> trustedCAC
      * @param actualTokenData authentication token data that contains the user certificate.
      * @throws UserCertificateNotTrustedException when user certificate is not signed by a trusted CA or is valid after CA certificate.
      */
-    public void validateCertificateTrusted(AuthTokenValidatorData actualTokenData) throws UserCertificateNotTrustedException {
+    public void validateCertificateTrusted(AuthTokenValidatorData actualTokenData) throws UserCertificateNotTrustedException, JceException {
 
-        final X509Certificate userCertificate = actualTokenData.getSubjectCertificate();
-        final X509Certificate caCertificate = trustedCACertificates.get(userCertificate.getIssuerX500Principal());
+        final X509Certificate certificate = actualTokenData.getSubjectCertificate();
 
-        if (caCertificate == null) {
-            throw new UserCertificateNotTrustedException("User certificate CA is not in the trusted CA list");
-        }
+        final X509CertSelector selector = new X509CertSelector();
+        selector.setCertificate(certificate);
 
         try {
-            userCertificate.verify(caCertificate.getPublicKey());
-            if (userCertificate.getNotAfter().after(caCertificate.getNotAfter())) {
-                throw new UserCertificateNotTrustedException("Trusted CA certificate expires earlier than the user certificate");
-            }
-            this.trustedCACertificate = caCertificate;
-            LOG.debug("User certificate is signed with a trusted CA certificate");
-        } catch (GeneralSecurityException e) {
-            LOG.trace("Error verifying signer's certificate {} against CA certificate {}", userCertificate.getSubjectDN(), caCertificate.getSubjectDN());
+            final PKIXBuilderParameters pkixBuilderParameters = new PKIXBuilderParameters(trustedRootCACertificateAnchors, selector);
+            pkixBuilderParameters.setRevocationEnabled(false);
+            pkixBuilderParameters.addCertStore(trustedRootCACertificateCertStore);
+
+            // See the comment in AuthTokenValidatorImpl constructor why we use the default JCE provider.
+            final CertPathBuilder certPathBuilder = CertPathBuilder.getInstance(CertPathBuilder.getDefaultType());
+            final PKIXCertPathBuilderResult result = (PKIXCertPathBuilderResult) certPathBuilder.build(pkixBuilderParameters);
+
+            subjectCertificateIssuerRootCertificate = result.getTrustAnchor().getTrustedCert();
+
+        } catch (InvalidAlgorithmParameterException | NoSuchAlgorithmException e) {
+            throw new JceException(e);
+        } catch (CertPathBuilderException e) {
+            LOG.trace("Error verifying signer's certificate {}: {}", certificate.getSubjectDN(), e);
             throw new UserCertificateNotTrustedException();
         }
     }
 
-    public X509Certificate getSubjectCertificateIssuerCertificate() {
-        return trustedCACertificate;
+    public X509Certificate getSubjectCertificateIssuerRootCertificate() {
+        return subjectCertificateIssuerRootCertificate;
     }
 }

src/test/java/org/webeid/security/testutil/AbstractTestWithCache.java

@@ -40,14 +40,18 @@ public abstract class AbstractTestWithCache {
     private static final String INCORRECT_TEST_NONCE_KEY = CORRECT_TEST_NONCE_KEY + "incorrect";
     private static final String TOO_SHORT_TEST_NONCE_KEY = "1234567812345678123456781234567";
 
-    private final CacheManager cacheManager = Caching.getCachingProvider(CaffeineCachingProvider.class.getName()).getCacheManager();
-    private final CompleteConfiguration<String, LocalDateTime> cacheConfig = new MutableConfiguration<String, LocalDateTime>().setTypes(String.class, LocalDateTime.class);
+    private static final CacheManager cacheManager = Caching.getCachingProvider(CaffeineCachingProvider.class.getName()).getCacheManager();
+    private static final CompleteConfiguration<String, LocalDateTime> cacheConfig = new MutableConfiguration<String, LocalDateTime>().setTypes(String.class, LocalDateTime.class);
 
     protected Cache<String, LocalDateTime> cache;
 
+    public static Cache<String, LocalDateTime> createCache(String cacheName) {
+        return cacheManager.createCache(cacheName, cacheConfig);
+    }
+
     @BeforeEach
     protected void setup() {
-        cache = cacheManager.createCache(CACHE_NAME, cacheConfig);
+        cache = createCache(CACHE_NAME);
     }
 
     public void putCorrectNonceToCache() {

src/test/java/org/webeid/security/testutil/AbstractTestWithMockedDateValidatorAndCorrectNonce.java

@@ -23,6 +23,7 @@
 package org.webeid.security.testutil;
 
 import org.junit.jupiter.api.BeforeEach;
+import org.webeid.security.exceptions.JceException;
 import org.webeid.security.validator.AuthTokenValidator;
 
 import java.security.cert.CertificateException;
@@ -39,7 +40,7 @@ public void setup() {
         super.setup();
         try {
             validator = getAuthTokenValidator(cache);
-        } catch (CertificateException e) {
+        } catch (CertificateException | JceException e) {
             throw new RuntimeException(e);
         }
     }

src/test/java/org/webeid/security/testutil/AbstractTestWithValidator.java

@@ -23,6 +23,7 @@
 package org.webeid.security.testutil;
 
 import org.junit.jupiter.api.BeforeEach;
+import org.webeid.security.exceptions.JceException;
 import org.webeid.security.validator.AuthTokenValidator;
 
 import java.security.cert.CertificateException;
@@ -39,7 +40,7 @@ protected void setup() {
         super.setup();
         try {
             validator = getAuthTokenValidator(cache);
-        } catch (CertificateException e) {
+        } catch (CertificateException | JceException e) {
             throw new RuntimeException(e);
         }
     }