NFC-128 Add revocation checking for intermediate CA certificates

Author: aarmam

README.md

@@ -343,13 +343,13 @@ Starting from format version `web-eid:1.1`, the authentication token may additio
 
 In addition to the fields described above, the `web-eid:1.1` format defines the following additional fields:
 
-- `unverifiedIntermediateCertificates`: an array of base64-encoded DER intermediate CA certificates that make up the trust chain of the authentication certificate in `unverifiedCertificate`. Like the authentication certificate, these certificates are received from the client side and cannot be trusted; they are only used as candidate certificates when building the certification path, which must still terminate at a trusted certificate authority. The field is optional, but when present it must not be empty. When it is present, `unverifiedSigningCertificates` may be omitted.
+- `unverifiedIntermediateCertificates`: an array of base64-encoded DER intermediate CA certificates that make up the trust chain of the authentication certificate in `unverifiedCertificate`. Like the authentication certificate, these certificates are received from the client side and cannot be trusted; they are only used as candidate certificates when building the certification path, which must still terminate at a trusted certificate authority. Every intermediate selected for the path is revocation-checked with OCSP or CRLs and the token is rejected if its status cannot be determined. The field is optional, but when present it must not be empty. When it is present, `unverifiedSigningCertificates` may be omitted.
 
 - `unverifiedSigningCertificates`: an array of the eID user's signing certificates, presented alongside the authentication certificate. For `web-eid:1.1` tokens this field is required unless `unverifiedIntermediateCertificates` is present, and when present it must not be empty. Each entry contains:
 
     - `certificate`: the base64-encoded DER signing certificate. During validation it must have the same subject and issuer as the authentication certificate, be valid, contain the non-repudiation key usage bit and be signed by a trusted certificate authority.
 
-    - `intermediateCertificates`: an optional array of base64-encoded DER intermediate CA certificates that make up the trust chain of this signing certificate. When present it must not be empty and, as with `unverifiedIntermediateCertificates`, the certificates are only used as candidates when building the certification path to a trusted CA.
+    - `intermediateCertificates`: an optional array of base64-encoded DER intermediate CA certificates that make up the trust chain of this signing certificate. When present it must not be empty and, as with `unverifiedIntermediateCertificates`, the certificates are only used as candidates when building the certification path to a trusted CA and every selected intermediate is revocation-checked.
 
     - `supportedSignatureAlgorithms`: the signature algorithms that the signing certificate's key supports. Each entry has a `cryptoAlgorithm` (`ECC` or `RSA`), a `hashFunction` (one of `SHA-224`, `SHA-256`, `SHA-384`, `SHA-512`, `SHA3-224`, `SHA3-256`, `SHA3-384`, `SHA3-512`) and a `paddingScheme` (`NONE`, `PKCS1.5` or `PSS`).
 

src/main/java/eu/webeid/security/certificate/CertificateValidator.java

@@ -30,13 +30,20 @@
 import java.security.GeneralSecurityException;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertPath;
 import java.security.cert.CertPathBuilder;
 import java.security.cert.CertPathBuilderException;
+import java.security.cert.CertPathValidator;
+import java.security.cert.CertPathValidatorException;
 import java.security.cert.CertStore;
 import java.security.cert.Certificate;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateFactory;
 import java.security.cert.CollectionCertStoreParameters;
 import java.security.cert.PKIXBuilderParameters;
 import java.security.cert.PKIXCertPathBuilderResult;
+import java.security.cert.PKIXParameters;
+import java.security.cert.PKIXRevocationChecker;
 import java.security.cert.TrustAnchor;
 import java.security.cert.X509CertSelector;
 import java.security.cert.X509Certificate;
@@ -89,20 +96,64 @@ public static X509Certificate validateIsSignedByTrustedCA(X509Certificate certif
             final CertPathBuilder certPathBuilder = CertPathBuilder.getInstance(CertPathBuilder.getDefaultType());
             final PKIXCertPathBuilderResult result = (PKIXCertPathBuilderResult) certPathBuilder.build(pkixBuilderParameters);
 
+            validateIntermediateCertificatesNotRevoked(
+                result,
+                trustedCACertificateAnchors,
+                trustedCACertificateCertStore,
+                additionalIntermediateCertificates,
+                now
+            );
+
             final X509Certificate trustedCACert = result.getTrustAnchor().getTrustedCert();
 
             // Verify that the trusted CA cert is presently valid before returning the result.
             certificateIsValidOnDate(trustedCACert, now, "Trusted CA");
 
             return getIssuerCertificate(result, trustedCACert);
 
-        } catch (InvalidAlgorithmParameterException | NoSuchAlgorithmException e) {
+        } catch (InvalidAlgorithmParameterException | NoSuchAlgorithmException | CertificateException e) {
             throw new JceException(e);
-        } catch (CertPathBuilderException e) {
+        } catch (CertPathBuilderException | CertPathValidatorException e) {
             throw new CertificateNotTrustedException(certificate, e);
         }
     }
 
+    private static void validateIntermediateCertificatesNotRevoked(
+        PKIXCertPathBuilderResult pathBuilderResult,
+        Set<TrustAnchor> trustedCACertificateAnchors,
+        CertStore trustedCACertificateCertStore,
+        List<X509Certificate> additionalIntermediateCertificates,
+        Date now
+    ) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, CertificateException,
+        CertPathValidatorException {
+        final List<? extends Certificate> certificatePath = pathBuilderResult.getCertPath().getCertificates();
+        if (certificatePath.size() <= 1) {
+            return; // leaf chains directly to a trust anchor; no non-anchor intermediate to validate
+        }
+
+        // The existing application OCSP flow checks the end-entity certificate. Validate only the CA suffix here,
+        // excluding both the end entity at index 0 and the trust anchor, which is not part of the built path.
+        final CertPath intermediateCertificatePath = CertificateFactory.getInstance("X.509")
+            .generateCertPath(certificatePath.subList(1, certificatePath.size()));
+        final PKIXParameters revocationCheckingParameters = new PKIXParameters(trustedCACertificateAnchors);
+        revocationCheckingParameters.setDate(now);
+        // An explicitly added checker is active regardless of this flag and avoids installing a second default checker.
+        revocationCheckingParameters.setRevocationEnabled(false);
+        revocationCheckingParameters.addCertStore(trustedCACertificateCertStore);
+        if (additionalIntermediateCertificates != null && !additionalIntermediateCertificates.isEmpty()) {
+            revocationCheckingParameters.addCertStore(CertStore.getInstance(
+                "Collection", new CollectionCertStoreParameters(additionalIntermediateCertificates)));
+        }
+
+        final CertPathValidator certPathValidator = CertPathValidator.getInstance(CertPathValidator.getDefaultType());
+        final PKIXRevocationChecker revocationChecker =
+            (PKIXRevocationChecker) certPathValidator.getRevocationChecker();
+        // The default checker prefers OCSP and falls back to CRLs. SOFT_FAIL is deliberately not enabled: a token-
+        // supplied intermediate whose revocation status cannot be established must not become part of a trusted path.
+        revocationCheckingParameters.addCertPathChecker(revocationChecker);
+        certPathValidator.validate(intermediateCertificatePath, revocationCheckingParameters);
+    }
+
     private static X509Certificate getIssuerCertificate(PKIXCertPathBuilderResult path, X509Certificate trustedCACert) {
         // The built path is ordered from the subject towards the anchor and excludes the anchor, so index 1 (when
         // present) is the subject's direct issuer; otherwise the subject was issued directly by the trust anchor.

src/test/java/eu/webeid/security/certificate/CertificateValidatorTest.java

@@ -22,10 +22,13 @@
 
 package eu.webeid.security.certificate;
 
+import eu.webeid.security.exceptions.CertificateNotTrustedException;
 import org.bouncycastle.asn1.x500.X500Name;
 import org.bouncycastle.asn1.x509.BasicConstraints;
 import org.bouncycastle.asn1.x509.Extension;
 import org.bouncycastle.asn1.x509.KeyUsage;
+import org.bouncycastle.cert.X509v2CRLBuilder;
+import org.bouncycastle.cert.jcajce.JcaX509CRLConverter;
 import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter;
 import org.bouncycastle.cert.jcajce.JcaX509ExtensionUtils;
 import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder;
@@ -39,15 +42,19 @@
 import java.security.KeyPairGenerator;
 import java.security.PrivateKey;
 import java.security.PublicKey;
+import java.security.cert.CertPathValidatorException;
 import java.security.cert.CertStore;
+import java.security.cert.CollectionCertStoreParameters;
 import java.security.cert.TrustAnchor;
+import java.security.cert.X509CRL;
 import java.security.cert.X509Certificate;
 import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Set;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 class CertificateValidatorTest {
 
@@ -60,6 +67,10 @@ class CertificateValidatorTest {
     private static X509Certificate intermediateCertificateB; // signed by C
     private static X509Certificate intermediateCertificateA; // signed by B, direct issuer of the leaf
     private static X509Certificate leafCertificate;          // signed by A
+    private static X509CRL rootCrl;
+    private static X509CRL intermediateCCrl;
+    private static X509CRL intermediateBCrl;
+    private static X509CRL intermediateBRevokingACrl;
 
     @BeforeAll
     static void setUp() throws Exception {
@@ -86,15 +97,21 @@ static void setUp() throws Exception {
             intermediateBName, intermediateBKeyPair.getPrivate(), intermediateBKeyPair.getPublic(), true, BigInteger.valueOf(4));
         leafCertificate = generateCertificate(leafName, leafKeyPair.getPublic(),
             intermediateAName, intermediateAKeyPair.getPrivate(), intermediateAKeyPair.getPublic(), false, BigInteger.valueOf(5));
+
+        rootCrl = generateCrl(rootName, rootKeyPair.getPrivate());
+        intermediateCCrl = generateCrl(intermediateCName, intermediateCKeyPair.getPrivate());
+        intermediateBCrl = generateCrl(intermediateBName, intermediateBKeyPair.getPrivate());
+        intermediateBRevokingACrl = generateCrl(
+            intermediateBName, intermediateBKeyPair.getPrivate(), intermediateCertificateA.getSerialNumber());
     }
 
     @Test
     void whenChainHasTokenSuppliedIntermediate_thenReturnsDirectIssuerNotTrustAnchor() throws Exception {
         final Set<TrustAnchor> anchors = Collections.singleton(new TrustAnchor(rootCertificate, null));
-        final CertStore emptyStore = CertificateValidator.buildCertStoreFromCertificates(Collections.emptyList());
+        final CertStore revocationStore = buildCrlStore(rootCrl, intermediateCCrl, intermediateBCrl);
 
         final X509Certificate issuer = CertificateValidator.validateIsSignedByTrustedCA(
-            leafCertificate, anchors, emptyStore,
+            leafCertificate, anchors, revocationStore,
             List.of(intermediateCertificateA, intermediateCertificateB, intermediateCertificateC), NOW);
 
         // The leaf is issued by intermediate A, whose chain (A -> B -> C) leads to the root trust anchor. The issuer
@@ -119,15 +136,48 @@ void whenChainHasMultipleTokenSuppliedIntermediatesAndGrandparentIsPinned_thenVa
         // Token supplies the full A -> B -> C intermediate chain; the top (C) is configured as the trust anchor.
         // The path builds leaf -> A -> B -> C, and the issuer returned for OCSP is the direct issuer (A).
         final Set<TrustAnchor> anchors = Collections.singleton(new TrustAnchor(intermediateCertificateC, null));
-        final CertStore emptyStore = CertificateValidator.buildCertStoreFromCertificates(Collections.emptyList());
+        final CertStore revocationStore = buildCrlStore(intermediateCCrl, intermediateBCrl);
 
         final X509Certificate issuer = CertificateValidator.validateIsSignedByTrustedCA(
-            leafCertificate, anchors, emptyStore,
+            leafCertificate, anchors, revocationStore,
             List.of(intermediateCertificateA, intermediateCertificateB, intermediateCertificateC), NOW);
 
         assertThat(issuer).isEqualTo(intermediateCertificateA);
     }
 
+    @Test
+    void whenTokenSuppliedIntermediateIsRevoked_thenRejectsCertificateChain() throws Exception {
+        final Set<TrustAnchor> anchors = Collections.singleton(new TrustAnchor(rootCertificate, null));
+        final CertStore revocationStore = buildCrlStore(
+            rootCrl, intermediateCCrl, intermediateBRevokingACrl);
+
+        assertThat(intermediateBRevokingACrl.isRevoked(intermediateCertificateA)).isTrue();
+
+        assertThatThrownBy(() -> CertificateValidator.validateIsSignedByTrustedCA(
+            leafCertificate, anchors, revocationStore,
+            List.of(intermediateCertificateA, intermediateCertificateB, intermediateCertificateC), NOW))
+            .isInstanceOf(CertificateNotTrustedException.class)
+            .satisfies(exception -> {
+                assertThat(exception.getCause()).isInstanceOf(CertPathValidatorException.class);
+                final CertPathValidatorException validationException =
+                    (CertPathValidatorException) exception.getCause();
+                assertThat(validationException.getReason())
+                    .isEqualTo(CertPathValidatorException.BasicReason.REVOKED);
+            });
+    }
+
+    @Test
+    void whenTokenSuppliedIntermediateRevocationStatusIsUnknown_thenRejectsCertificateChain() throws Exception {
+        final Set<TrustAnchor> anchors = Collections.singleton(new TrustAnchor(rootCertificate, null));
+        final CertStore emptyStore = CertificateValidator.buildCertStoreFromCertificates(Collections.emptyList());
+
+        assertThatThrownBy(() -> CertificateValidator.validateIsSignedByTrustedCA(
+            leafCertificate, anchors, emptyStore,
+            List.of(intermediateCertificateA, intermediateCertificateB, intermediateCertificateC), NOW))
+            .isInstanceOf(CertificateNotTrustedException.class)
+            .hasCauseInstanceOf(CertPathValidatorException.class);
+    }
+
     private static KeyPair generateKeyPair() throws Exception {
         final KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA");
         keyPairGenerator.initialize(2048);
@@ -149,4 +199,19 @@ private static X509Certificate generateCertificate(X500Name subject, PublicKey s
         final ContentSigner signer = new JcaContentSignerBuilder("SHA256withRSA").build(issuerPrivateKey);
         return new JcaX509CertificateConverter().getCertificate(builder.build(signer));
     }
+
+    private static X509CRL generateCrl(X500Name issuer, PrivateKey issuerPrivateKey,
+                                       BigInteger... revokedCertificateSerials) throws Exception {
+        final X509v2CRLBuilder builder = new X509v2CRLBuilder(issuer, NOT_BEFORE);
+        builder.setNextUpdate(NOT_AFTER);
+        for (final BigInteger serial : revokedCertificateSerials) {
+            builder.addCRLEntry(serial, NOT_BEFORE, 0);
+        }
+        final ContentSigner signer = new JcaContentSignerBuilder("SHA256withRSA").build(issuerPrivateKey);
+        return new JcaX509CRLConverter().getCRL(builder.build(signer));
+    }
+
+    private static CertStore buildCrlStore(X509CRL... crls) throws Exception {
+        return CertStore.getInstance("Collection", new CollectionCertStoreParameters(List.of(crls)));
+    }
 }