Server Argument for Disable Cert Expiry Check (#3552)

- Enable the server argument for disable
  certificate expiry check file to override
  the default value.

- Add Test cases to test the argument
  with valid and expired certs.

- Minor Cleanups

infrastructure/src/main/java/org/corfudb/infrastructure/CorfuServer.java

@@ -64,6 +64,7 @@ public class CorfuServer {
                     + "[-b] [-g -o <username_file> -j <password_file>] "
                     + "[-k <seqcache>] [-T <threads>] [-B <size>] [-i <channel-implementation>] "
                     + "[-H <seconds>] [-I <cluster-id>] [-x <ciphers>] [-z <tls-protocols>]] "
+                    + "[--disable-cert-expiry-check-file=<file_path>]"
                     + "[--metrics]"
                     + "[--health-port=<health_port>]"
                     + "[--snapshot-batch=<batch-size>] [--lock-lease=<lease-duration>]"
@@ -167,6 +168,9 @@ public class CorfuServer {
                     + "              Comma separated list of TLS protocols to use.\n"
                     + "                                                                          "
                     + "              [default: TLSv1.1,TLSv1.2].\n"
+                    + " --disable-cert-expiry-check-file=<file_path>                             "
+                    + "              Path to Disable Cert Expiry Check File. If this file is     "
+                    + "              present, the certificate expiry checks are disabled.\n      "
                     + " --base-server-threads=<base_server_threads>                              "
                     + "              Number of threads dedicated for the base server [default: 1].\n"
                     + " --log-size-quota-percentage=<max_log_size_percentage>                    "

infrastructure/src/main/java/org/corfudb/infrastructure/CorfuServerNode.java

@@ -31,6 +31,7 @@
 import javax.annotation.Nonnull;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Map;
@@ -352,6 +353,9 @@ protected void initChannel(@Nonnull Channel ch) throws Exception {
                                 .map(Paths::get)
                                 .orElse(TrustStoreConfig.DEFAULT_DISABLE_CERT_EXPIRY_CHECK_FILE);
 
+                        log.trace("getServerChannelInitializer: certExpiryFile path is {}, isCertExpiryCheckEnabled is {}.",
+                                certExpiryFile, !Files.exists(certExpiryFile));
+
                         TrustStoreConfig trustStoreConfig = TrustStoreConfig.from(
                                 context.getServerConfig(String.class, ConfigParamNames.TRUST_STORE),
                                 context.getServerConfig(String.class, ConfigParamNames.TRUST_STORE_PASS_FILE),

infrastructure/src/main/java/org/corfudb/infrastructure/logreplication/infrastructure/CorfuInterClusterReplicationServer.java

@@ -65,6 +65,7 @@ public class CorfuInterClusterReplicationServer implements Runnable {
                     + "[-b] [-g -o <username_file> -j <password_file>] "
                     + "[-k <seqcache>] [-T <threads>] [-B <size>] [-i <channel-implementation>] "
                     + "[-H <seconds>] [-I <cluster-id>] [-x <ciphers>] [-z <tls-protocols>]] "
+                    + "[--disable-cert-expiry-check-file=<file_path>]"
                     + "[--metrics]"
                     + "[-P <prefix>] [-R <retention>] <port>\n"
                     + "\n"
@@ -159,6 +160,9 @@ public class CorfuInterClusterReplicationServer implements Runnable {
                     + "              Comma separated list of TLS protocols to use.\n"
                     + "                                                                          "
                     + "              [default: TLSv1.1,TLSv1.2].\n"
+                    + " --disable-cert-expiry-check-file=<file_path>                             "
+                    + "              Path to Disable Cert Expiry Check File. If this file is     "
+                    + "              present, the certificate expiry checks are disabled.\n      "
                     + " --base-server-threads=<base_server_threads>                              "
                     + "              Number of threads dedicated for the base server [default: 1].\n"
                     + " --log-size-quota-percentage=<max_log_size_percentage>                    "

runtime/src/main/java/org/corfudb/security/tls/ReloadableTrustManager.java

@@ -36,21 +36,21 @@ public ReloadableTrustManager(TrustStoreConfig trustStoreConfig) throws SSLExcep
 
     @Override
     public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
-        checkValidity(chain);
-
         reloadTrustStoreWrapper();
+        checkValidity(chain);
         trustManager.checkClientTrusted(chain, authType);
     }
 
     @Override
     public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
-        checkValidity(chain);
         reloadTrustStoreWrapper();
+        checkValidity(chain);
         trustManager.checkServerTrusted(chain, authType);
     }
 
     private void checkValidity(X509Certificate[] chain) throws CertificateExpiredException, CertificateNotYetValidException {
         if (isCertExpiryCheckEnabled()) {
+            log.trace("checkValidity: CertExpiryCheck is Enabled.");
             for (X509Certificate cert : chain) {
                 cert.checkValidity();
             }

runtime/src/test/java/org/corfudb/security/tls/ReloadableTrustManagerTest.java

@@ -16,7 +16,9 @@
 import java.security.cert.X509Certificate;
 
 import static org.corfudb.security.tls.TlsTestContext.CLIENT_CERT;
+import static org.corfudb.security.tls.TlsTestContext.CLIENT_CERT_ECDSA_EXPIRED;
 import static org.corfudb.security.tls.TlsTestContext.CLIENT_TRUST_NO_SERVER;
+import static org.corfudb.security.tls.TlsTestContext.CLIENT_TRUST_ECDSA_EXPIRED_NO_SERVER;
 import static org.corfudb.security.tls.TlsTestContext.CLIENT_TRUST_WITH_SERVER;
 import static org.corfudb.security.tls.TlsTestContext.SERVER_CERT;
 import static org.corfudb.security.tls.TlsTestContext.SERVER_TRUST_NO_CLIENT;
@@ -26,14 +28,29 @@
 public class ReloadableTrustManagerTest {
 
     @Test
-    public void testServerCheckClient() throws Exception {
+    public void testServerCheckClientRSA() throws Exception {
         withDisabledCheckExpiry(() -> {
             ReloadableTrustManager manager = new ReloadableTrustManager(SERVER_TRUST_WITH_CLIENT);
             X509Certificate cert = getCertificate(CLIENT_CERT, true);
             manager.checkClientTrusted(new X509Certificate[]{cert}, "RSA");
         });
     }
 
+    /**
+     * Test that server can validate the expired client certificate of type ECDSA
+     * when certificate expiry check is disabled.
+     *
+     * @throws Exception any exceptions caught while validating the certificate
+     */
+    @Test
+    public void testServerCheckClientEC() throws Exception {
+        withDisabledCheckExpiry(() -> {
+            ReloadableTrustManager manager = new ReloadableTrustManager(CLIENT_TRUST_ECDSA_EXPIRED_NO_SERVER);
+            X509Certificate cert = getCertificate(CLIENT_CERT_ECDSA_EXPIRED, false);
+            manager.checkClientTrusted(new X509Certificate[]{cert}, "ECDHE_RSA");
+        });
+    }
+
     @Test
     public void testServerCheckClientForValidCerts() throws Exception {
         ReloadableTrustManager manager = new ReloadableTrustManager(ValidCerts.TRUST_STORE_CONFIG);

runtime/src/test/java/org/corfudb/security/tls/TlsTestContext.java

@@ -2,24 +2,11 @@
 
 import org.corfudb.security.tls.TlsUtils.CertStoreConfig.TrustStoreConfig;
 
-import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
 public final class TlsTestContext {
 
-    static final class ValidCerts {
-        public static final Path CERT_DIR = Paths.get("src/test/resources/security/valid_certs");
-        public static final Path PASSWORD_FILE = CERT_DIR.resolve("pass.txt");
-        public static final Path RUNTIME_CERT = CERT_DIR.resolve("runtime.cert");
-
-        public static final TrustStoreConfig TRUST_STORE_CONFIG = new TrustStoreConfig(
-                CERT_DIR.resolve("truststore.jks"),
-                PASSWORD_FILE,
-                TrustStoreConfig.DEFAULT_DISABLE_CERT_EXPIRY_CHECK_FILE
-        );
-    }
-
     public static final Path CERT_DIR = Paths.get("src/test/resources/security/reload");
     public static final Path PASSWORD_FILE = CERT_DIR.resolve("password");
 
@@ -31,6 +18,7 @@ static final class ValidCerts {
     public static final TrustStoreConfig SERVER_TRUST_NO_CLIENT = buildTrustStore("server_trust_no_client.jks");
     public static final TrustStoreConfig CLIENT_TRUST_WITH_SERVER = buildTrustStore(CLIENT_TRUST_WITH_SERVER_FILE_NAME);
     public static final TrustStoreConfig CLIENT_TRUST_NO_SERVER = buildTrustStore("client_trust_no_server.jks");
+    public static final TrustStoreConfig CLIENT_TRUST_ECDSA_EXPIRED_NO_SERVER = buildTrustStore("client_ecdsa_expired.jks");
 
     public static final TrustStoreConfig FAKE_LOCATION_AND_PASS = TrustStoreConfig.from(
             "definitely fake location",
@@ -44,8 +32,20 @@ static final class ValidCerts {
     );
 
     public static final Path CLIENT_CERT = CERT_DIR.resolve("client.cert");
+    public static final Path CLIENT_CERT_ECDSA_EXPIRED = CERT_DIR.resolve("client_ecdsa_expired.cert");
     public static final Path SERVER_CERT = CERT_DIR.resolve("server.cert");
 
+    static final class ValidCerts {
+        public static final Path CERT_DIR = Paths.get("src/test/resources/security/valid_certs");
+        public static final Path PASSWORD_FILE = CERT_DIR.resolve("pass.txt");
+        public static final Path RUNTIME_CERT = CERT_DIR.resolve("runtime.cert");
+
+        public static final TrustStoreConfig TRUST_STORE_CONFIG = new TrustStoreConfig(
+                CERT_DIR.resolve("truststore.jks"),
+                PASSWORD_FILE,
+                TrustStoreConfig.DEFAULT_DISABLE_CERT_EXPIRY_CHECK_FILE
+        );
+    }
     private TlsTestContext() {
         //prevent creating new instances
     }
@@ -57,24 +57,4 @@ private static TrustStoreConfig buildTrustStore(String trustStoreFile) {
                 DISABLE_CERT_EXPIRY_CHECK_FILE_NAME
         );
     }
-
-    public static void disableCertExpiryCheck(TestAction testAction) throws Exception {
-        File disableCertExpiryCheck = SERVER_TRUST_WITH_CLIENT
-                .getDisableCertExpiryCheckFile()
-                .toAbsolutePath()
-                .toFile();
-
-        try {
-            disableCertExpiryCheck.createNewFile();
-            disableCertExpiryCheck.deleteOnExit();
-
-            testAction.run();
-        } finally {
-            disableCertExpiryCheck.delete();
-        }
-    }
-
-    public interface TestAction {
-        void run() throws Exception;
-    }
 }

runtime/src/test/resources/security/reload/client_ecdsa_expired.cert

@@ -0,0 +1,13 @@
+-----BEGIN CERTIFICATE-----
+MIICADCCAYSgAwIBAgIECuZKtzAMBggqhkjOPQQDAwUAMFYxEDAOBgNVBAgTB0NB
+IGM9VVMxEjAQBgNVBAcTCVBhbG8gQWx0bzEPMA0GA1UEChMGVk13YXJlMQ0wCwYD
+VQQLEwROU0JVMQ4wDAYDVQQDEwVDb3JmdTAeFw0xMzAyMjQxMDUwMzlaFw0xMzAy
+MjUxMDUwMzlaMFYxEDAOBgNVBAgTB0NBIGM9VVMxEjAQBgNVBAcTCVBhbG8gQWx0
+bzEPMA0GA1UEChMGVk13YXJlMQ0wCwYDVQQLEwROU0JVMQ4wDAYDVQQDEwVDb3Jm
+dTB2MBAGByqGSM49AgEGBSuBBAAiA2IABGs7FauxQDj/UzJB/MCKsa78k6rMYYSY
+7ess/1o/gUV30WHQvDA66h/lDIrgub+n2hRU28Wama502TmNwG8/6lpiotoW9ji6
+qgVx9U52AX1teTOIFpKzNiIQZQvBI1BTUKMhMB8wHQYDVR0OBBYEFBk4oSzWbDzF
+1VSA4/U4P6iCC8U0MAwGCCqGSM49BAMDBQADaAAwZQIxAPFwkLgcQi+mOHEn+QRi
+Qe7cB/ACax0bboi4uWnkRY2GKlAEeVK8tLC4pcyskNklIwIwSSanwxEY8ZceMtfi
+5BmMnOM9TuoZT3wPYdw9BgfUODZyKrcv40+Wajl9NFGiGa1O
+-----END CERTIFICATE-----

test/src/test/java/org/corfudb/integration/AbstractIT.java

@@ -507,6 +507,7 @@ public static class CorfuServerRunner {
         private String metricsConfigFile = "";
         private boolean single = true;
         private boolean tlsEnabled = false;
+        private boolean tlsMutualAuthEnabled = false;
         private boolean noAutoCommit = true;
         private String keyStore = null;
         private String keyStorePassword = null;
@@ -515,6 +516,7 @@ public static class CorfuServerRunner {
         private String trustStore = null;
         private String logSizeLimitPercentage = null;
         private String trustStorePassword = null;
+        private String disableCertExpiryCheckFile = null;
         private String compressionCodec = null;
         private boolean disableHost = false;
         private String networkInterface = null;
@@ -574,6 +576,12 @@ public String getOptionsString() {
                 if (trustStorePassword != null) {
                     command.append(" -w ").append(trustStorePassword);
                 }
+                if (tlsMutualAuthEnabled) {
+                    command.append(" -b");
+                }
+                if (disableCertExpiryCheckFile != null) {
+                    command.append(" --disable-cert-expiry-check-file=").append(disableCertExpiryCheckFile);
+                }
             }
 
             if (networkInterface != null) {
@@ -628,11 +636,13 @@ public static class CorfuReplicationServerRunner {
         private int port = DEFAULT_LOG_REPLICATION_PORT;
         private String metricsConfigFile = "";
         private boolean tlsEnabled = false;
+        private boolean tlsMutualAuthEnabled = false;
         private String keyStore = null;
         private String keyStorePassword = null;
         private String logLevel = "INFO";
         private String trustStore = null;
         private String trustStorePassword = null;
+        private String disableCertExpiryCheckFile = null;
         private String compressionCodec = null;
         private String pluginConfigFilePath = null;
         private String logPath = null;
@@ -679,6 +689,12 @@ public String getOptionsString() {
                 if (trustStorePassword != null) {
                     command.append(" -w ").append(trustStorePassword);
                 }
+                if (tlsMutualAuthEnabled) {
+                    command.append(" -b");
+                }
+                if (disableCertExpiryCheckFile != null) {
+                    command.append(" --disable-cert-expiry-check-file=").append(disableCertExpiryCheckFile);
+                }
             }
 
             if (pluginConfigFilePath != null) {