Reworked the enabling of the CRL function

jwaeab · web-flow · commit 43cfa67386e9 · 2025-04-10T14:43:37.000+02:00
Closes #932
diff --git a/CHANGES.md b/CHANGES.md
@@ -2,6 +2,7 @@
 
 ## Version 6.0.4
 
+* CRL not properly applied when using certificate files - Issue #932
 * Return default values if schema changes in Cassandra have not been completed - Issue #922
 
 ## Version 6.0.3
diff --git a/application/src/main/java/com/ericsson/bss/cassandra/ecchronos/application/CRLFileManager.java b/application/src/main/java/com/ericsson/bss/cassandra/ecchronos/application/CRLFileManager.java
@@ -164,6 +164,7 @@ private void refreshCRLs()
         if (!fileModified() && myAttempt == 1)
         {
             // No file changes and no reattempts needed, so no refresh is necessary at this time
+            LOG.debug("No CRL file changes detected; skipping refresh");
             return;
         }
 
@@ -180,16 +181,17 @@ private void refreshCRLs()
             long endTime = System.nanoTime();
             long duration = TimeUnit.NANOSECONDS.toMillis(endTime - startTime);
             // CRL file was refreshed; notify and log the details
-            updateFileMetadata();
-            notifyRefresh();
             LOG.info("CRL file refreshed in {} ms; read {} record(s), discarded {} duplicate(s)",
                     duration, this.myCRLs.size(), dupes);
         }
         catch (IOException | CRLException e)
         {
             myCRLs.clear();
-            LOG.error("Failed to read CRL file; any cached CRLs have been purged", e);
+            LOG.error("Failed to read CRL file; any previously cached CRLs have been purged");
         }
+        // Make sure listeners are aware of the changes
+        updateFileMetadata();
+        notifyRefresh();
     }
 
 }
diff --git a/application/src/main/java/com/ericsson/bss/cassandra/ecchronos/application/CustomCRLValidator.java b/application/src/main/java/com/ericsson/bss/cassandra/ecchronos/application/CustomCRLValidator.java
@@ -120,7 +120,7 @@ public void validateCertificate(final X509Certificate cert, final X509Certificat
                     + cert.getIssuerX500Principal().getName());
         }
 
-        // If this point is reached, the certificate is valid and not revoked
+        // If this point is reached, the certificate is not revoked
     }
 
     public void addRefreshListener(final Runnable listener)
diff --git a/application/src/main/java/com/ericsson/bss/cassandra/ecchronos/application/CustomX509TrustManager.java b/application/src/main/java/com/ericsson/bss/cassandra/ecchronos/application/CustomX509TrustManager.java
@@ -156,13 +156,14 @@ public void onRefresh()
             revalidateServerTrust();
             revalidateClientTrust();
             myCRLValidator.resetAttempts();
+            LOG.info("Certificates are not revoked by current CRL (any previous failed attempts was reset)");
         }
         catch (CertificateException e)
         {
             if (myCRLValidator.inStrictMode())
             {
                 // Strict mode: Log it, check for attempts made and eventually shut down if all attempts are consumed
-                LOG.warn("Provided certificate is rejected by CRL (strict mode, attempt {} of {} made)",
+                LOG.warn("Certificates are revoked by current CRL (strict mode, attempt {} of {} made)",
                         myCRLValidator.increaseAttempts(),
                         myCRLValidator.maxAttempts());
                 // If the last attempt was made, do a graceful shutdown
diff --git a/application/src/main/java/com/ericsson/bss/cassandra/ecchronos/application/ReloadingCertificateHandler.java b/application/src/main/java/com/ericsson/bss/cassandra/ecchronos/application/ReloadingCertificateHandler.java
@@ -38,15 +38,22 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.security.InvalidAlgorithmParameterException;
+import java.security.KeyFactory;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.security.PrivateKey;
 import java.security.UnrecoverableKeyException;
 import java.security.cert.CRLException;
+import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
+import java.security.cert.CertificateFactory;
+import java.security.spec.InvalidKeySpecException;
+import java.security.spec.PKCS8EncodedKeySpec;
 import java.util.Arrays;
+import java.util.Base64;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
@@ -59,6 +66,8 @@ public class ReloadingCertificateHandler implements CertificateHandler
     private final AtomicReference<Context> currentContext = new AtomicReference<>();
     private final Supplier<CqlTLSConfig> myCqlTLSConfigSupplier;
 
+    private static final String DEFAULT_STORE_TYPE_JKS = "JKS";
+
     public ReloadingCertificateHandler(final Supplier<CqlTLSConfig> cqlTLSConfigSupplier)
     {
         this.myCqlTLSConfigSupplier = cqlTLSConfigSupplier;
@@ -208,54 +217,136 @@ protected static SslContext createSSLContext(final CqlTLSConfig tlsConfig) throw
             CertificateException,
             UnrecoverableKeyException
     {
-
         SslContextBuilder builder = SslContextBuilder.forClient();
+        String storeType = tlsConfig.getStoreType().orElse(DEFAULT_STORE_TYPE_JKS);
+
         if (tlsConfig.isCertificateConfigured())
         {
+            LOG.info("PEM certificates configured for CQL connections, using internal store type {}", storeType);
+
+            // Get certificate and key files from config
             File certificateFile = new File(tlsConfig.getCertificatePath().get());
             File certificatePrivateKeyFile = new File(tlsConfig.getCertificatePrivateKeyPath().get());
             File trustCertificateFile = new File(tlsConfig.getTrustCertificatePath().get());
 
-            builder.keyManager(certificateFile, certificatePrivateKeyFile);
-            builder.trustManager(trustCertificateFile);
+            KeyStore keyStore = KeyStore.getInstance(storeType);
+            keyStore.load(null, null);
+
+            // Setup client certificates and its private key into a keystore/truststore
+            CertificateFactory cf = CertificateFactory.getInstance("X.509");
+            Certificate clientCert = cf.generateCertificate(new FileInputStream(certificateFile));
+            Certificate trustCert = cf.generateCertificate(new FileInputStream(trustCertificateFile));
+            PrivateKey privateKey = loadPrivateKey(certificatePrivateKeyFile);
+
+            LOG.info("Private key algorithm: {}", privateKey.getAlgorithm());
+
+            // Create the certificate chain and add it to the keystore
+            Certificate[] certChain = new Certificate[]{clientCert};
+            keyStore.setKeyEntry("client-key", privateKey, "".toCharArray(), certChain);
+
+            // Create KeyManagerFactory
+            KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
+            kmf.init(keyStore, "".toCharArray());
+
+            // Create TrustManagerFactory and load the trusted certificate into it
+            TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
+            KeyStore trustStore = KeyStore.getInstance(storeType);
+            trustStore.load(null, null);
+            trustStore.setCertificateEntry("trust-cert", trustCert);
+            tmf.init(trustStore);
+
+            // Finally, set the managers in the builder
+            builder.keyManager(kmf);
+            setTrustManagers(builder, tlsConfig, tmf);
         }
         else
         {
+            LOG.info("Keystore/truststore configured for CQL connections, expecting store type {}", storeType);
+
             KeyManagerFactory keyManagerFactory = getKeyManagerFactory(tlsConfig);
             builder.keyManager(keyManagerFactory);
-            TrustManagerFactory trustManagerFactory = getTrustManagerFactory(tlsConfig);
+            setTrustManagers(builder, tlsConfig, getTrustManagerFactory(tlsConfig));
+        }
+        if (tlsConfig.getCipherSuites().isPresent())
+        {
+            builder.ciphers(Arrays.asList(tlsConfig.getCipherSuites().get()));
+        }
+        return builder.protocols(tlsConfig.getProtocols()).build();
+    }
 
-            if (tlsConfig.getCRLConfig().getEnabled())
+    protected static void setTrustManagers(final SslContextBuilder builder,
+                                           final CqlTLSConfig config,
+                                           final TrustManagerFactory tmf)
+    {
+        if (config.getCRLConfig().getEnabled())
+        {
+            // If CRL is enabled, use the CustomX509TrustManager (CRL checking is added to the SSL context)
+            LOG.info("CRL enabled using strict mode: {}", config.getCRLConfig().getStrict());
+            TrustManager[] trustManagers = tmf.getTrustManagers();
+            for (int i = 0; i < trustManagers.length; i++)
             {
-                // If CRL is enabled, use the CustomX509TrustManager (CRL checking is added to the SSL context)
-                LOG.debug("CRL enabled using strict mode: {}", tlsConfig.getCRLConfig().getStrict());
-                TrustManager[] trustManagers = trustManagerFactory.getTrustManagers();
-                for (int i = 0; i < trustManagers.length; i++)
+                // Replace X509TrustManager with our custom one
+                if (trustManagers[i] instanceof X509TrustManager)
                 {
-                    if (trustManagers[i] instanceof X509TrustManager)
-                    {
-                        CustomCRLValidator validator = new CustomCRLValidator(tlsConfig.getCRLConfig());
-                        trustManagers[i] = new CustomX509TrustManager(
-                                (X509TrustManager) trustManagers[i],
-                                validator
-                        );
-                        // Add customized TrustManager
-                        builder.trustManager(trustManagers[i]);
-                    }
+                    CustomCRLValidator validator = new CustomCRLValidator(config.getCRLConfig());
+                    trustManagers[i] = new CustomX509TrustManager(
+                            (X509TrustManager) trustManagers[i],
+                            validator
+                    );
+                    builder.trustManager(trustManagers[i]);
                 }
             }
-            else
+        }
+        else
+        {
+            // No CRL, use TrustManagerFactory as-is
+            LOG.info("CRL not enabled");
+            builder.trustManager(tmf);
+        }
+    }
+
+    @SuppressWarnings("PMD.PreserveStackTrace")
+    protected static PrivateKey loadPrivateKey(final File file) throws IOException
+    {
+        try
+        {
+            // Supported key types are EC and RSA (DSA is legacy and should not be used)
+
+            // Read key file and remove the PEM header and any whitespaces
+            String key = Files.readString(file.toPath());
+            String privateKeyPEM = key
+                    .replaceAll("-----BEGIN .*PRIVATE KEY-----", "")
+                    .replaceAll("-----END .*PRIVATE KEY-----", "")
+                    .replaceAll("\\s", "");
+
+            // Decode the key into a nice byte array
+            byte[] decoded = Base64.getDecoder().decode(privateKeyPEM);
+            PKCS8EncodedKeySpec spec = new PKCS8EncodedKeySpec(decoded);
+
+            try
+            {
+                LOG.debug("Trying private key type EC");
+                KeyFactory kf = KeyFactory.getInstance("EC");
+                return kf.generatePrivate(spec);
+            }
+            catch (InvalidKeySpecException e1)
             {
-                // No CRL, use regular TrustManagerFqctory
-                LOG.debug("CRL not enabled");
-                builder.trustManager(trustManagerFactory);
+                LOG.debug("Trying private key type RSA");
+                try
+                {
+                    KeyFactory kf = KeyFactory.getInstance("RSA");
+                    return kf.generatePrivate(spec);
+                }
+                catch (InvalidKeySpecException e2)
+                {
+                    throw new IOException("Unsupported private key format", e2);
+                }
             }
         }
-        if (tlsConfig.getCipherSuites().isPresent())
+        catch (NoSuchAlgorithmException e)
         {
-            builder.ciphers(Arrays.asList(tlsConfig.getCipherSuites().get()));
+            throw new IOException("Failed to load the private key file", e);
         }
-        return builder.protocols(tlsConfig.getProtocols()).build();
     }
 
     protected static KeyManagerFactory getKeyManagerFactory(final CqlTLSConfig tlsConfig) throws IOException,
@@ -267,7 +358,7 @@ protected static KeyManagerFactory getKeyManagerFactory(final CqlTLSConfig tlsCo
         try (InputStream keystoreFile = new FileInputStream(tlsConfig.getKeyStorePath()))
         {
             KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(algorithm);
-            KeyStore keyStore = KeyStore.getInstance(tlsConfig.getStoreType().orElse("JKS"));
+            KeyStore keyStore = KeyStore.getInstance(tlsConfig.getStoreType().orElse(DEFAULT_STORE_TYPE_JKS));
             keyStore.load(keystoreFile, keystorePassword);
             keyManagerFactory.init(keyStore, keystorePassword);
             return keyManagerFactory;
@@ -283,7 +374,7 @@ protected static TrustManagerFactory getTrustManagerFactory(final CqlTLSConfig t
         try (InputStream truststoreFile = new FileInputStream(tlsConfig.getTrustStorePath()))
         {
             TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(algorithm);
-            KeyStore keyStore = KeyStore.getInstance(tlsConfig.getStoreType().orElse("JKS"));
+            KeyStore keyStore = KeyStore.getInstance(tlsConfig.getStoreType().orElse(DEFAULT_STORE_TYPE_JKS));
             keyStore.load(truststoreFile, truststorePassword);
             trustManagerFactory.init(keyStore);
             return trustManagerFactory;
diff --git a/docs/SETUP.md b/docs/SETUP.md
@@ -153,7 +153,7 @@ jmx:
     cipher_suites:
 ```
 
-CQL also supports certificates in PEM format.
+CQL also supports certificates in PEM format (EC and RSA algorithms only).
 
 ```yaml
 cql:

Original file line number	Diff line number	Diff line change
`@@ -164,6 +164,7 @@ private void refreshCRLs()`
`164`	`164`	`if (!fileModified() && myAttempt == 1)`
`165`	`165`	`{`
`166`	`166`	`// No file changes and no reattempts needed, so no refresh is necessary at this time`
	`167`	`+ LOG.debug("No CRL file changes detected; skipping refresh");`
`167`	`168`	`return;`
`168`	`169`	`}`
`169`	`170`
`@@ -180,16 +181,17 @@ private void refreshCRLs()`
`180`	`181`	`long endTime = System.nanoTime();`
`181`	`182`	`long duration = TimeUnit.NANOSECONDS.toMillis(endTime - startTime);`
`182`	`183`	`// CRL file was refreshed; notify and log the details`
`183`		`- updateFileMetadata();`
`184`		`- notifyRefresh();`
`185`	`184`	`LOG.info("CRL file refreshed in {} ms; read {} record(s), discarded {} duplicate(s)",`
`186`	`185`	`duration, this.myCRLs.size(), dupes);`
`187`	`186`	`}`
`188`	`187`	`catch (IOException \| CRLException e)`
`189`	`188`	`{`
`190`	`189`	`myCRLs.clear();`
`191`		`- LOG.error("Failed to read CRL file; any cached CRLs have been purged", e);`
	`190`	`+ LOG.error("Failed to read CRL file; any previously cached CRLs have been purged");`
`192`	`191`	`}`
	`192`	`+ // Make sure listeners are aware of the changes`
	`193`	`+ updateFileMetadata();`
	`194`	`+ notifyRefresh();`
`193`	`195`	`}`
`194`	`196`
`195`	`197`	`}`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ public void validateCertificate(final X509Certificate cert, final X509Certificat`
`120`	`120`	`+ cert.getIssuerX500Principal().getName());`
`121`	`121`	`}`
`122`	`122`
`123`		`- // If this point is reached, the certificate is valid and not revoked`
	`123`	`+ // If this point is reached, the certificate is not revoked`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`public void addRefreshListener(final Runnable listener)`
Original file line number	Diff line number	Diff line change
`@@ -156,13 +156,14 @@ public void onRefresh()`
`156`	`156`	`revalidateServerTrust();`
`157`	`157`	`revalidateClientTrust();`
`158`	`158`	`myCRLValidator.resetAttempts();`
	`159`	`+ LOG.info("Certificates are not revoked by current CRL (any previous failed attempts was reset)");`
`159`	`160`	`}`
`160`	`161`	`catch (CertificateException e)`
`161`	`162`	`{`
`162`	`163`	`if (myCRLValidator.inStrictMode())`
`163`	`164`	`{`
`164`	`165`	`// Strict mode: Log it, check for attempts made and eventually shut down if all attempts are consumed`
`165`		`- LOG.warn("Provided certificate is rejected by CRL (strict mode, attempt {} of {} made)",`
	`166`	`+ LOG.warn("Certificates are revoked by current CRL (strict mode, attempt {} of {} made)",`
`166`	`167`	`myCRLValidator.increaseAttempts(),`
`167`	`168`	`myCRLValidator.maxAttempts());`
`168`	`169`	`// If the last attempt was made, do a graceful shutdown`