JCE: fix PKIXCertPathValidator revocation status index and PKIXRevocationChecker mutable map for SunJCE interop

Author: cconlon

src/main/java/com/wolfssl/provider/jce/WolfCryptPKIXCertPathValidator.java

@@ -733,28 +733,56 @@ public TrustAnchor findTrustAnchor(PKIXParameters params,
         return anchorFound;
     }
 
+    /**
+     * Throw CertPathValidatorException for undetermined revocation status.
+     *
+     * Used when revocation checking is enabled but no CRLs are available
+     * and no PKIXRevocationChecker is configured to handle OCSP.
+     *
+     * @param message error message describing the revocation failure
+     * @param certPath the CertPath being validated (for exception reporting)
+     * @param certs list of certificates from certPath
+     *
+     * @throws CertPathValidatorException always thrown with
+     *         UNDETERMINED_REVOCATION_STATUS reason
+     */
+    private void throwUndeterminedRevocationStatus(String message,
+        CertPath certPath, List<X509Certificate> certs)
+        throws CertPathValidatorException {
+
+        /* Report index of last cert in path (closest to trust anchor)
+         * to match SunJCE behavior. */
+        int failIndex = 0;
+        if (certs != null && certs.size() > 1) {
+            failIndex = certs.size() - 1;
+        }
+        throw new CertPathValidatorException(message, null, certPath,
+            failIndex, BasicReason.UNDETERMINED_REVOCATION_STATUS);
+    }
+
     /**
      * Check if revocation has been enabled in PKIXParameters, and if so
      * find and load any CRLs in params.getCertStores().
      *
      * When a PKIXRevocationChecker has been registered via
      * addCertPathChecker(), that checker handles revocation checking. CRL
      * checking in the native CertManager is only enabled if:
-     *   - No PKIXRevocationChecker is present (default CRL behavior), OR
+     *   - No PKIXRevocationChecker is present (default CRL behavior), or
      *   - PKIXRevocationChecker has PREFER_CRLS option set
      *
      * @param params parameters used to check if revocation is enabled
-     *        and if so load any CRLs available
+     *        and, if so load any CRLs available
      * @param cm WolfSSLCertManager to load CRLs into
-     * @param targetCert peer/leaf cert used to find matching CRL
+     * @param certPath the CertPath being validated (for exception reporting)
+     * @param certs list of certificates from certPath
      * @param pathCheckers list of registered CertPathCheckers
      *
      * @throws CertPathValidatorException if error is encountered during
      *        revocation checking or CRL loading
      */
     private void checkRevocationEnabledAndLoadCRLs(
         PKIXParameters params, WolfSSLCertManager cm,
-        X509Certificate targetCert,
+        CertPath certPath, List<X509Certificate> certs,
         List<PKIXCertPathChecker> pathCheckers)
         throws CertPathValidatorException {
 
@@ -810,12 +838,22 @@ private void checkRevocationEnabledAndLoadCRLs(
 
             /* Enable CRL in native WolfSSLCertManager */
             cm.CertManagerEnableCRL(WolfCrypt.WOLFSSL_CRL_CHECK);
-
             log("CRL support enabled in native WolfSSLCertManager");
 
             stores = params.getCertStores();
             if (stores == null || stores.isEmpty()) {
                 log("no CertStores in PKIXParameters to load CRLs");
+
+                /* If revocation is enabled but no CRLs and no
+                 * PKIXRevocationChecker to handle OCSP, we cannot determine
+                 * revocation status. Per RFC 5280, this should fail. */
+                if (!hasRevocationChecker) {
+                    throwUndeterminedRevocationStatus(
+                        "Revocation checking enabled but no CRLs available " +
+                        "and no PKIXRevocationChecker configured for OCSP",
+                        certPath, certs);
+                }
+
                 return;
             }
 
@@ -850,7 +888,7 @@ private void checkRevocationEnabledAndLoadCRLs(
 
             /* Create CRL selector to help match target X509Certificate */
             X509CRLSelector selector = new X509CRLSelector();
-            selector.setCertificateChecking(targetCert);
+            selector.setCertificateChecking(certs.get(0));
 
             try {
                 /* Find and load any matching CRLs */
@@ -868,6 +906,16 @@ private void checkRevocationEnabledAndLoadCRLs(
             }
 
             log("loaded " + loadedCount + " CRLs into WolfSSLCertManager");
+
+            /* If no CRLs were loaded and no PKIXRevocationChecker is handling
+             * OCSP, we cannot determine revocation status. */
+            if (loadedCount == 0 && !hasRevocationChecker) {
+                throwUndeterminedRevocationStatus(
+                    "Revocation checking enabled but no CRLs found in " +
+                    "CertStores and no PKIXRevocationChecker configured " +
+                    "for OCSP",
+                    certPath, certs);
+            }
         }
         else {
             log("revocation not enabled in PKIXParameters");
@@ -1040,7 +1088,7 @@ public CertPathValidatorResult engineValidate(
              * will try to find/verify CRL against trusted roots on load.
              * Pass pathCheckers so we can skip CRL setup when a
              * PKIXRevocationChecker is handling revocation via OCSP. */
-            checkRevocationEnabledAndLoadCRLs(pkixParams, cm, certs.get(0),
+            checkRevocationEnabledAndLoadCRLs(pkixParams, cm, certPath, certs,
                 pathCheckers);
 
             /* Verify cert chain */

src/main/java/com/wolfssl/provider/jce/WolfCryptPKIXRevocationChecker.java

@@ -545,11 +545,16 @@ public void setOcspResponses(Map<X509Certificate, byte[]> responses) {
     /**
      * Get pre-loaded OCSP responses.
      *
+     * Returns the internal mutable map, not an unmodifiable copy.
+     * JDK sun.security.validator.PKIXValidator.addResponses() expects
+     * to be able to add OCSP responses to this map when using the internal
+     * Validator API.
+     *
      * @return Map of certificates to OCSP response bytes
      */
     @Override
     public Map<X509Certificate, byte[]> getOcspResponses() {
-        return Collections.unmodifiableMap(this.ocspResponses);
+        return this.ocspResponses;
     }
 
     /**

src/test/java/com/wolfssl/provider/jce/test/WolfCryptPKIXCertPathValidatorTest.java

@@ -1675,6 +1675,146 @@ public void testAlgorithmConstraintsWithSHAVariant() throws Exception {
         }
     }
 
+    /**
+     * Test that revocation checking fails with UNDETERMINED_REVOCATION_STATUS
+     * when revocation is enabled but no CRLs are available and no
+     * PKIXRevocationChecker is registered for OCSP.
+     */
+    @Test
+    public void testRevocationEnabledNoCRLsFailsWithUndeterminedStatus()
+        throws FileNotFoundException, KeyStoreException, IOException,
+               NoSuchAlgorithmException, CertificateException,
+               InvalidAlgorithmParameterException, NoSuchProviderException,
+               Exception {
+
+        KeyStore store = null;
+        CertificateFactory certFactory = null;
+        InputStream fis = null;
+        Certificate serverCert = null;
+        List<Certificate> certList = new ArrayList<>();
+
+        if (!WolfCrypt.CrlEnabled()) {
+            /* Native CRL not enabled, skip test */
+            System.out.println("CertPathValidator revocation status test " +
+                "skipped, CRL not compiled in");
+            return;
+        }
+
+        /* Use example KeyStore that verifies server-cert.der */
+        store = createKeyStoreFromFile(jksCaServerRSA2048, keyStorePass);
+        if (store == null || store.size() != 1) {
+            throw new Exception("Error creating KeyStore");
+        }
+
+        certFactory = CertificateFactory.getInstance("X.509");
+
+        /* Import server-cert.der into Certificate object */
+        fis = new FileInputStream(serverCertDer);
+        serverCert = certFactory.generateCertificate(fis);
+        certList.add(serverCert);
+        fis.close();
+
+        /* Create PKIXParameters with trusted KeyStore */
+        PKIXParameters params = new PKIXParameters(store);
+
+        /* Enable revocation checking but DO NOT add any CRLs or
+         * PKIXRevocationChecker. This should cause validation to fail
+         * with UNDETERMINED_REVOCATION_STATUS. */
+        params.setRevocationEnabled(true);
+        params.setCertStores(null);
+
+        /* Validate cert chain, should fail */
+        CertPath path = certFactory.generateCertPath(certList);
+        CertPathValidator cpv =
+            CertPathValidator.getInstance("PKIX", provider);
+
+        try {
+            cpv.validate(path, params);
+            fail("Expected CertPathValidatorException with " +
+                 "UNDETERMINED_REVOCATION_STATUS when revocation is enabled " +
+                 "but no CRLs or OCSP checker is available");
+
+        } catch (CertPathValidatorException e) {
+            /* Expected - verify reason is UNDETERMINED_REVOCATION_STATUS */
+            assertEquals("Expected BasicReason.UNDETERMINED_REVOCATION_STATUS",
+                BasicReason.UNDETERMINED_REVOCATION_STATUS, e.getReason());
+        }
+    }
+
+    /**
+     * Test that revocation checking fails with UNDETERMINED_REVOCATION_STATUS
+     * when revocation is enabled and CertStores exist but contain no CRLs.
+     */
+    @Test
+    public void testRevocationEnabledEmptyCertStoreFailsWithUndeterminedStatus()
+        throws FileNotFoundException, KeyStoreException, IOException,
+               NoSuchAlgorithmException, CertificateException,
+               InvalidAlgorithmParameterException, NoSuchProviderException,
+               Exception {
+
+        KeyStore store = null;
+        CertificateFactory certFactory = null;
+        InputStream fis = null;
+        Certificate serverCert = null;
+        List<Certificate> certList = new ArrayList<>();
+        CertStore emptyCertStore = null;
+        List<CertStore> certStores = null;
+
+        if (!WolfCrypt.CrlEnabled()) {
+            /* Native CRL not enabled, skip test */
+            System.out.println("CertPathValidator revocation status test " +
+                "skipped, CRL not compiled in");
+            return;
+        }
+
+        /* Use example KeyStore that verifies server-cert.der */
+        store = createKeyStoreFromFile(jksCaServerRSA2048, keyStorePass);
+        if (store == null || store.size() != 1) {
+            throw new Exception("Error creating KeyStore");
+        }
+
+        certFactory = CertificateFactory.getInstance("X.509");
+
+        /* Import server-cert.der into Certificate object */
+        fis = new FileInputStream(serverCertDer);
+        serverCert = certFactory.generateCertificate(fis);
+        certList.add(serverCert);
+        fis.close();
+
+        /* Create PKIXParameters with trusted KeyStore */
+        PKIXParameters params = new PKIXParameters(store);
+
+        /* Enable revocation checking with empty CertStore (no CRLs).
+         * This should cause validation to fail with
+         * UNDETERMINED_REVOCATION_STATUS. */
+        params.setRevocationEnabled(true);
+
+        /* Create empty CertStore */
+        Collection<CRL> emptyCrls = new HashSet<>();
+        emptyCertStore = CertStore.getInstance("Collection",
+            new CollectionCertStoreParameters(emptyCrls));
+        certStores = new ArrayList<>();
+        certStores.add(emptyCertStore);
+        params.setCertStores(certStores);
+
+        /* Validate cert chain, should fail */
+        CertPath path = certFactory.generateCertPath(certList);
+        CertPathValidator cpv =
+            CertPathValidator.getInstance("PKIX", provider);
+
+        try {
+            cpv.validate(path, params);
+            fail("Expected CertPathValidatorException with " +
+                 "UNDETERMINED_REVOCATION_STATUS when revocation is enabled " +
+                 "but CertStore contains no CRLs");
+
+        } catch (CertPathValidatorException e) {
+            /* Expected - verify reason is UNDETERMINED_REVOCATION_STATUS */
+            assertEquals("Expected BasicReason.UNDETERMINED_REVOCATION_STATUS",
+                BasicReason.UNDETERMINED_REVOCATION_STATUS, e.getReason());
+        }
+    }
+
     /**
      * Test that zero-length cert paths are valid per RFC 5280. This occurs
      * when CertPathBuilder determines the trust anchor itself is the target.

src/test/java/com/wolfssl/provider/jce/test/WolfCryptPKIXRevocationCheckerTest.java

@@ -319,13 +319,12 @@ public void testRevocationCheckerGetSetOcspResponses() throws Exception {
         assertEquals(1, responses.size());
         assertTrue(responses.containsKey(testCert));
 
-        /* Returned map should be unmodifiable */
-        try {
-            responses.put(testCert, new byte[] {0x04});
-            fail("Expected UnsupportedOperationException");
-        } catch (UnsupportedOperationException e) {
-            /* expected */
-        }
+        /* Returned map must be mutable for compatibility with JDK
+         * sun.security.validator.PKIXValidator.addResponses() which adds
+         * OCSP responses directly to the map returned by getOcspResponses(). */
+        responses.put(testCert, new byte[] {0x04});
+        assertEquals(1, responses.size());
+        assertArrayEquals(new byte[] {0x04}, responses.get(testCert));
 
         /* Set null should clear to empty map */
         checker.setOcspResponses(null);