Add support for certificates hot reload (opensearch-project#4880)

Signed-off-by: Andrey Pleskach <ples@aiven.io>
willyborankin · Nov 25, 2024 · c22002a · c22002a
1 parent a8447cc
commit c22002a
Show file tree

Hide file tree

Showing 8 changed files with 362 additions and 11 deletions.
diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
@@ -223,7 +223,10 @@
 import static org.opensearch.security.setting.DeprecatedSettings.checkForDeprecatedSetting;
 import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX;
 import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_USE_CLUSTER_STATE;
+import static org.opensearch.security.support.ConfigConstants.SECURITY_SSL_CERTIFICATES_HOT_RELOAD_ENABLED;
+import static org.opensearch.security.support.ConfigConstants.SECURITY_SSL_CERT_RELOAD_ENABLED;
 import static org.opensearch.security.support.ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION;
+
 // CS-ENFORCE-SINGLE
 
 public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
@@ -313,7 +316,11 @@ private static boolean useClusterStateToInitSecurityConfig(final Settings settin
      * @return true if ssl cert reload is enabled else false
      */
     private static boolean isSslCertReloadEnabled(final Settings settings) {
-        return settings.getAsBoolean(ConfigConstants.SECURITY_SSL_CERT_RELOAD_ENABLED, false);
+        return settings.getAsBoolean(SECURITY_SSL_CERT_RELOAD_ENABLED, false);
+    }
+
+    private boolean sslCertificatesHotReloadEnabled(final Settings settings) {
+        return settings.getAsBoolean(SECURITY_SSL_CERTIFICATES_HOT_RELOAD_ENABLED, false);
     }
 
     @SuppressWarnings("removal")
@@ -1203,6 +1210,19 @@ public Collection<Object> createComponents(
         components.add(passwordHasher);
 
         components.add(sslSettingsManager);
+        if (isSslCertReloadEnabled(settings) && sslCertificatesHotReloadEnabled(settings)) {
+            throw new OpenSearchException(
+                "Either "
+                    + SECURITY_SSL_CERT_RELOAD_ENABLED
+                    + " or "
+                    + SECURITY_SSL_CERTIFICATES_HOT_RELOAD_ENABLED
+                    + " can be set to true, but not both."
+            );
+        }
+
+        if (sslCertificatesHotReloadEnabled(settings) && !isSslCertReloadEnabled(settings)) {
+            sslSettingsManager.addSslConfigurationsChangeListener(resourceWatcherService);
+        }
 
         final var allowDefaultInit = settings.getAsBoolean(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, false);
         final var useClusterState = useClusterStateToInitSecurityConfig(settings);

diff --git a/src/main/java/org/opensearch/security/ssl/SslContextHandler.java b/src/main/java/org/opensearch/security/ssl/SslContextHandler.java
@@ -21,6 +21,9 @@
 import java.util.stream.Stream;
 import javax.net.ssl.SSLEngine;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
 import org.opensearch.security.ssl.config.Certificate;
 import org.opensearch.transport.NettyAllocator;
 
@@ -30,6 +33,8 @@
 
 public class SslContextHandler {
 
+    private final static Logger LOGGER = LogManager.getLogger(SslContextHandler.class);
+
     private SslContext sslContext;
 
     private final SslConfiguration sslConfiguration;
@@ -78,7 +83,7 @@ Stream<Certificate> keyMaterialCertificates(final List<Certificate> certificates
         return certificates.stream().filter(Certificate::hasKey);
     }
 
-    void reloadSslContext() throws CertificateException {
+    boolean reloadSslContext() throws CertificateException {
         final var newCertificates = sslConfiguration.certificates();
 
         boolean hasChanges = false;
@@ -89,11 +94,13 @@ void reloadSslContext() throws CertificateException {
         final var newKeyMaterialCertificates = keyMaterialCertificates(newCertificates).collect(Collectors.toList());
 
         if (notSameCertificates(loadedAuthorityCertificates, newAuthorityCertificates)) {
+            LOGGER.debug("Certification authority has changed");
             hasChanges = true;
             validateDates(newAuthorityCertificates);
         }
 
         if (notSameCertificates(loadedKeyMaterialCertificates, newKeyMaterialCertificates)) {
+            LOGGER.debug("Key material and access certificate has changed");
             hasChanges = true;
             validateNewKeyMaterialCertificates(
                 loadedKeyMaterialCertificates,
@@ -111,6 +118,7 @@ void reloadSslContext() throws CertificateException {
             loadedCertificates.clear();
             loadedCertificates.addAll(newCertificates);
         }
+        return hasChanges;
     }
 
     private boolean notSameCertificates(final List<Certificate> loadedCertificates, final List<Certificate> newCertificates) {

diff --git a/src/main/java/org/opensearch/security/ssl/SslSettingsManager.java b/src/main/java/org/opensearch/security/ssl/SslSettingsManager.java
@@ -11,11 +11,15 @@
 
 package org.opensearch.security.ssl;
 
+import java.io.IOException;
+import java.nio.file.Path;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 import javax.crypto.Cipher;
 
 import com.google.common.collect.ImmutableMap;
@@ -29,6 +33,9 @@
 import org.opensearch.security.ssl.config.CertType;
 import org.opensearch.security.ssl.config.SslCertificatesLoader;
 import org.opensearch.security.ssl.config.SslParameters;
+import org.opensearch.watcher.FileChangesListener;
+import org.opensearch.watcher.FileWatcher;
+import org.opensearch.watcher.ResourceWatcherService;
 
 import io.netty.handler.ssl.ClientAuth;
 import io.netty.handler.ssl.OpenSsl;
@@ -108,13 +115,13 @@ private Map<CertType, SslContextHandler> buildSslContexts(final Environment envi
 
     public synchronized void reloadSslContext(final CertType certType) {
         sslContextHandler(certType).ifPresentOrElse(sscContextHandler -> {
-            LOGGER.info("Reloading {} SSL context", certType.name());
             try {
-                sscContextHandler.reloadSslContext();
+                if (sscContextHandler.reloadSslContext()) {
+                    LOGGER.info("{} SSL context reloaded", certType.name());
+                }
             } catch (CertificateException e) {
                 throw new OpenSearchException(e);
             }
-            LOGGER.info("{} SSL context reloaded", certType.name());
         }, () -> LOGGER.error("Missing SSL Context for {}", certType.name()));
     }
 
@@ -180,6 +187,50 @@ private Map<CertType, SslConfiguration> loadConfigurations(final Environment env
         return configurationBuilder.build();
     }
 
+    public void addSslConfigurationsChangeListener(final ResourceWatcherService resourceWatcherService) {
+        for (final var directoryToMonitor : directoriesToMonitor()) {
+            final var fileWatcher = new FileWatcher(directoryToMonitor);
+            fileWatcher.addListener(new FileChangesListener() {
+                @Override
+                public void onFileCreated(final Path file) {
+                    onFileChanged(file);
+                }
+
+                @Override
+                public void onFileDeleted(final Path file) {
+                    onFileChanged(file);
+                }
+
+                @Override
+                public void onFileChanged(final Path file) {
+                    for (final var e : sslSettingsContexts.entrySet()) {
+                        final var certType = e.getKey();
+                        final var sslConfiguration = e.getValue().sslConfiguration();
+                        if (sslConfiguration.dependentFiles().contains(file)) {
+                            SslSettingsManager.this.reloadSslContext(certType);
+                        }
+                    }
+                }
+            });
+            try {
+                resourceWatcherService.add(fileWatcher, ResourceWatcherService.Frequency.HIGH);
+                LOGGER.info("Added SSL configuration change listener for: {}", directoryToMonitor);
+            } catch (IOException e) {
+                // TODO: should we fail here, or are error logs sufficient?
+                throw new OpenSearchException("Couldn't add SSL configurations change listener", e);
+            }
+        }
+    }
+
+    private Set<Path> directoriesToMonitor() {
+        return sslSettingsContexts.values()
+            .stream()
+            .map(SslContextHandler::sslConfiguration)
+            .flatMap(c -> c.dependentFiles().stream())
+            .map(Path::getParent)
+            .collect(Collectors.toSet());
+    }
+
     private boolean clientNode(final Settings settings) {
         return !"node".equals(settings.get(OpenSearchSecuritySSLPlugin.CLIENT_TYPE));
     }

diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java
@@ -287,6 +287,7 @@ public class ConfigConstants {
     public static final String SECURITY_SSL_DUAL_MODE_SKIP_SECURITY = OPENDISTRO_SECURITY_CONFIG_PREFIX + "passive_security";
     public static final String LEGACY_OPENDISTRO_SECURITY_CONFIG_SSL_DUAL_MODE_ENABLED = "opendistro_security_config.ssl_dual_mode_enabled";
     public static final String SECURITY_SSL_CERT_RELOAD_ENABLED = "plugins.security.ssl_cert_reload_enabled";
+    public static final String SECURITY_SSL_CERTIFICATES_HOT_RELOAD_ENABLED = "plugins.security.ssl.certificates_hot_reload.enabled";
     public static final String SECURITY_DISABLE_ENVVAR_REPLACEMENT = "plugins.security.disable_envvar_replacement";
     public static final String SECURITY_DFM_EMPTY_OVERRIDES_ALL = "plugins.security.dfm_empty_overrides_all";
 

diff --git a/src/test/java/org/opensearch/security/ssl/CertificatesRule.java b/src/test/java/org/opensearch/security/ssl/CertificatesRule.java
@@ -69,16 +69,28 @@ public class CertificatesRule extends ExternalResource {
 
     private PrivateKey accessCertificatePrivateKey;
 
+    private final boolean generateDefaultCertificates;
+
+    public CertificatesRule() {
+        this(true);
+    }
+
+    public CertificatesRule(final boolean generateDefaultCertificates) {
+        this.generateDefaultCertificates = generateDefaultCertificates;
+    }
+
     @Override
     protected void before() throws Throwable {
         super.before();
         temporaryFolder.create();
         configRootFolder = temporaryFolder.newFolder("esHome").toPath();
-        final var keyPair = generateKeyPair();
-        caCertificateHolder = generateCaCertificate(keyPair);
-        final var keyAndCertificate = generateAccessCertificate(keyPair);
-        accessCertificatePrivateKey = keyAndCertificate.v1();
-        accessCertificateHolder = keyAndCertificate.v2();
+        if (generateDefaultCertificates) {
+            final var keyPair = generateKeyPair();
+            caCertificateHolder = generateCaCertificate(keyPair);
+            final var keyAndCertificate = generateAccessCertificate(keyPair);
+            accessCertificatePrivateKey = keyAndCertificate.v1();
+            accessCertificateHolder = keyAndCertificate.v2();
+        }
     }
 
     @Override