From 405ff0ce27210ab3a00804e1371b083cce832576 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Fri, 17 Apr 2020 20:10:33 -0600 Subject: [PATCH] Handle TLS file updates during startup (#55330) This change reworks the loading and monitoring of files that are used for the construction of SSLContexts so that updates to these files are not lost if the updates occur during startup. Previously, the SSLService would parse the settings, build the SSLConfiguration objects, and construct the SSLContexts prior to the SSLConfigurationReloader starting to monitor these files for changes. This allowed for a small window where updates to these files may never be observed until the node restarted. To remove the potential miss of a change to these files, the code now parses the settings and builds SSLConfiguration instances prior to the construction of the SSLService. The files back the SSLConfiguration instances are then registered for monitoring and finally the SSLService is constructed from the previously parse SSLConfiguration instances. As the SSLService is not constructed when the code starts monitoring the files for changes, a CompleteableFuture is used to obtain a reference to the SSLService; this allows for construction of the SSLService to complete and ensures that we do not miss any file updates during the construction of the SSLService. While working on this change, the SSLConfigurationReloader was also refactored to reflect how it is currently used. When the SSLConfigurationReloader was originally written the files that it monitored could change during runtime. This is no longer the case as we stopped the monitoring of files that back dynamic SSLContext instances. In order to support the ability for items to change during runtime, the class made use of concurrent data structures. The use of these concurrent datastructures has been removed. Closes #54867 Backport of #54999 --- .../elasticsearch/xpack/core/XPackPlugin.java | 25 ++-- .../core/ssl/SSLConfigurationReloader.java | 117 +++++++++++------- .../xpack/core/ssl/SSLService.java | 100 +++++++-------- .../ssl/SSLConfigurationReloaderTests.java | 94 +++++++------- .../test/SecuritySettingsSource.java | 2 + .../authc/ldap/LdapSessionFactoryTests.java | 4 +- .../ssl/SSLReloadDuringStartupIntegTests.java | 116 +++++++++++++++++ .../xpack/ssl/SSLReloadIntegTests.java | 1 - 8 files changed, 303 insertions(+), 156 deletions(-) create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadDuringStartupIntegTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java index 16522caf612..19326cc5859 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java @@ -66,6 +66,7 @@ import org.elasticsearch.xpack.core.rest.action.RestReloadAnalyzersAction; import org.elasticsearch.xpack.core.rest.action.RestXPackInfoAction; import org.elasticsearch.xpack.core.rest.action.RestXPackUsageAction; import org.elasticsearch.xpack.core.security.authc.TokenMetadata; +import org.elasticsearch.xpack.core.ssl.SSLConfiguration; import org.elasticsearch.xpack.core.ssl.SSLConfigurationReloader; import org.elasticsearch.xpack.core.ssl.SSLService; import org.elasticsearch.xpack.core.watcher.WatcherMetadata; @@ -87,8 +88,8 @@ import java.util.stream.StreamSupport; public class XPackPlugin extends XPackClientPlugin implements ExtensiblePlugin, RepositoryPlugin, EnginePlugin { - private static Logger logger = LogManager.getLogger(XPackPlugin.class); - private static DeprecationLogger deprecationLogger = new DeprecationLogger(logger); + private static final Logger logger = LogManager.getLogger(XPackPlugin.class); + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger); public static final String XPACK_INSTALLED_NODE_ATTR = "xpack.installed"; @@ -259,11 +260,7 @@ public class XPackPlugin extends XPackClientPlugin implements ExtensiblePlugin, Supplier repositoriesServiceSupplier) { List components = new ArrayList<>(); - final SSLService sslService = new SSLService(environment); - setSslService(sslService); - // just create the reloader as it will pull all of the loaded ssl configurations and start watching them - new SSLConfigurationReloader(environment, sslService, resourceWatcherService); - + final SSLService sslService = createSSLService(environment, resourceWatcherService); setLicenseService(new LicenseService(settings, clusterService, getClock(), environment, resourceWatcherService, getLicenseState())); @@ -387,4 +384,18 @@ public class XPackPlugin extends XPackClientPlugin implements ExtensiblePlugin, settings.add(SourceOnlySnapshotRepository.SOURCE_ONLY); return settings; } + + /** + * Handles the creation of the SSLService along with the necessary actions to enable reloading + * of SSLContexts when configuration files change on disk. + */ + private SSLService createSSLService(Environment environment, ResourceWatcherService resourceWatcherService) { + final Map sslConfigurations = SSLService.getSSLConfigurations(environment.settings()); + final SSLConfigurationReloader reloader = + new SSLConfigurationReloader(environment, resourceWatcherService, sslConfigurations.values()); + final SSLService sslService = new SSLService(environment, sslConfigurations); + reloader.setSSLService(sslService); + setSslService(sslService); + return sslService; + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloader.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloader.java index 2a786f12e59..07cbfcabeae 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloader.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloader.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.core.ssl; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.env.Environment; import org.elasticsearch.watcher.FileChangesListener; import org.elasticsearch.watcher.FileWatcher; @@ -14,71 +15,95 @@ import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.watcher.ResourceWatcherService.Frequency; import javax.net.ssl.SSLContext; - import java.io.IOException; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.function.Consumer; /** * Ensures that the files backing an {@link SSLConfiguration} are monitored for changes and the underlying key/trust material is reloaded * and the {@link SSLContext} has existing sessions invalidated to force the use of the new key/trust material */ -public class SSLConfigurationReloader { +public final class SSLConfigurationReloader { private static final Logger logger = LogManager.getLogger(SSLConfigurationReloader.class); - private final ConcurrentHashMap pathToChangeListenerMap = new ConcurrentHashMap<>(); - private final Environment environment; - private final ResourceWatcherService resourceWatcherService; - private final SSLService sslService; + private final CompletableFuture sslServiceFuture = new CompletableFuture<>(); - public SSLConfigurationReloader(Environment env, SSLService sslService, ResourceWatcherService resourceWatcher) { - this.environment = env; - this.resourceWatcherService = resourceWatcher; - this.sslService = sslService; - startWatching(sslService.getLoadedSSLConfigurations()); + public SSLConfigurationReloader(Environment environment, + ResourceWatcherService resourceWatcherService, + Collection sslConfigurations) { + startWatching(environment, reloadConsumer(sslServiceFuture), resourceWatcherService, sslConfigurations); + } + + // for testing + SSLConfigurationReloader(Environment environment, + Consumer reloadConsumer, + ResourceWatcherService resourceWatcherService, + Collection sslConfigurations) { + startWatching(environment, reloadConsumer, resourceWatcherService, sslConfigurations); + } + + public void setSSLService(SSLService sslService) { + final boolean completed = sslServiceFuture.complete(sslService); + if (completed == false) { + throw new IllegalStateException("ssl service future was already completed!"); + } + } + + private static Consumer reloadConsumer(CompletableFuture future) { + return sslConfiguration -> { + try { + final SSLService sslService = future.get(); + logger.debug("reloading ssl configuration [{}]", sslConfiguration); + sslService.reloadSSLContext(sslConfiguration); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } catch (ExecutionException e) { + throw new ElasticsearchException("failed to obtain ssl service", e); + } + }; } /** * Collects all of the directories that need to be monitored for the provided {@link SSLConfiguration} instances and ensures that * they are being watched for changes */ - private void startWatching(Collection sslConfigurations) { + private static void startWatching(Environment environment, Consumer reloadConsumer, + ResourceWatcherService resourceWatcherService, Collection sslConfigurations) { + Map> pathToConfigurationsMap = new HashMap<>(); for (SSLConfiguration sslConfiguration : sslConfigurations) { for (Path directory : directoriesToMonitor(sslConfiguration.filesToMonitor(environment))) { - pathToChangeListenerMap.compute(directory, (path, listener) -> { - if (listener != null) { - listener.addSSLConfiguration(sslConfiguration); - return listener; + pathToConfigurationsMap.compute(directory, (path, list) -> { + if (list == null) { + list = new ArrayList<>(); } - - ChangeListener changeListener = new ChangeListener(); - changeListener.addSSLConfiguration(sslConfiguration); - FileWatcher fileWatcher = new FileWatcher(path); - fileWatcher.addListener(changeListener); - try { - resourceWatcherService.add(fileWatcher, Frequency.HIGH); - return changeListener; - } catch (IOException e) { - logger.error("failed to start watching directory [{}] for ssl configuration [{}]", path, sslConfiguration); - } - return null; + list.add(sslConfiguration); + return list; }); } } - } - /** - * Reloads the ssl context associated with this configuration. It is visible so that tests can override as needed - */ - void reloadSSLContext(SSLConfiguration configuration) { - logger.debug("reloading ssl configuration [{}]", configuration); - sslService.sslContextHolder(configuration).reload(); + for (Entry> entry : pathToConfigurationsMap.entrySet()) { + ChangeListener changeListener = new ChangeListener(environment, Collections.unmodifiableList(entry.getValue()), reloadConsumer); + FileWatcher fileWatcher = new FileWatcher(entry.getKey()); + fileWatcher.addListener(changeListener); + try { + resourceWatcherService.add(fileWatcher, Frequency.HIGH); + } catch (IOException e) { + logger.error("failed to start watching directory [{}] for ssl configurations [{}]", entry.getKey(), sslConfigurations); + } + } } /** @@ -92,15 +117,17 @@ public class SSLConfigurationReloader { return paths; } - private class ChangeListener implements FileChangesListener { + private static class ChangeListener implements FileChangesListener { - private final CopyOnWriteArraySet sslConfigurations = new CopyOnWriteArraySet<>(); + private final Environment environment; + private final List sslConfigurations; + private final Consumer reloadConsumer; - /** - * Adds the given ssl configuration to those that have files within the directory watched by this change listener - */ - private void addSSLConfiguration(SSLConfiguration sslConfiguration) { - sslConfigurations.add(sslConfiguration); + private ChangeListener(Environment environment, List sslConfigurations, + Consumer reloadConsumer) { + this.environment = environment; + this.sslConfigurations = sslConfigurations; + this.reloadConsumer = reloadConsumer; } @Override @@ -118,7 +145,7 @@ public class SSLConfigurationReloader { boolean reloaded = false; for (SSLConfiguration sslConfiguration : sslConfigurations) { if (sslConfiguration.filesToMonitor(environment).contains(file)) { - reloadSSLContext(sslConfiguration); + reloadConsumer.accept(sslConfiguration); reloaded = true; } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index bb81095a296..ddda618f34a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -12,7 +12,6 @@ import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy; import org.apache.http.nio.reactor.IOSession; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.lucene.util.SetOnce; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.common.CheckedSupplier; @@ -119,7 +118,6 @@ public class SSLService { * always maps to the same {@link SSLContextHolder}, even if it is being used within a different context-name. */ private final Map sslContexts; - private final SetOnce transportSSLConfiguration = new SetOnce<>(); private final Environment env; /** @@ -127,7 +125,19 @@ public class SSLService { * @see #SSLService(Settings, Environment) */ public SSLService(Environment environment) { - this(environment.settings(), environment); + this(environment, getSSLConfigurations(environment.settings())); + } + + /** + * Create a new SSLService using the provided {@link SSLConfiguration} instances. The ssl + * contexts created from these configurations will be cached. + */ + public SSLService(Environment environment, Map sslConfigurations) { + this.env = environment; + this.settings = environment.settings(); + this.diagnoseTrustExceptions = shouldEnableDiagnoseTrust(); + this.sslConfigurations = sslConfigurations; + this.sslContexts = loadSSLConfigurations(this.sslConfigurations); } /** @@ -135,11 +145,11 @@ public class SSLService { * for use later */ public SSLService(Settings settings, Environment environment) { - this.settings = settings; this.env = environment; + this.settings = settings; this.diagnoseTrustExceptions = shouldEnableDiagnoseTrust(); - this.sslConfigurations = new HashMap<>(); - this.sslContexts = loadSSLConfigurations(); + this.sslConfigurations = getSSLConfigurations(this.settings); + this.sslContexts = loadSSLConfigurations(this.sslConfigurations); } private SSLService(Settings settings, Environment environment, Map sslConfigurations, @@ -159,12 +169,6 @@ public class SSLService { public SSLService createDynamicSSLService() { return new SSLService(settings, env, sslConfigurations, sslContexts) { - @Override - Map loadSSLConfigurations() { - // we don't need to load anything... - return Collections.emptyMap(); - } - /** * Returns the existing {@link SSLContextHolder} for the configuration * @throws IllegalArgumentException if not found @@ -337,6 +341,10 @@ public class SSLService { return sslContextHolder(configuration).sslContext(); } + public void reloadSSLContext(SSLConfiguration configuration) { + sslContextHolder(configuration).reload(); + } + /** * Returns the existing {@link SSLContextHolder} for the configuration * @@ -481,27 +489,43 @@ public class SSLService { return trustManager; } - /** - * Parses the settings to load all SSLConfiguration objects that will be used. - */ - Map loadSSLConfigurations() { - Map sslContextHolders = new HashMap<>(); + public static Map getSSLConfigurations(Settings settings) { + final Map sslSettingsMap = getSSLSettingsMap(settings); + final Map sslConfigurationMap = new HashMap<>(sslSettingsMap.size()); + sslSettingsMap.forEach((key, sslSettings) -> { + if (key.endsWith(".")) { + // Drop trailing '.' so that any exception messages are consistent + key = key.substring(0, key.length() - 1); + } + sslConfigurationMap.put(key, new SSLConfiguration(sslSettings)); + }); + return Collections.unmodifiableMap(sslConfigurationMap); + } - Map sslSettingsMap = new HashMap<>(); + static Map getSSLSettingsMap(Settings settings) { + final Map sslSettingsMap = new HashMap<>(); sslSettingsMap.put(XPackSettings.HTTP_SSL_PREFIX, getHttpTransportSSLSettings(settings)); sslSettingsMap.put("xpack.http.ssl", settings.getByPrefix("xpack.http.ssl.")); sslSettingsMap.putAll(getRealmsSSLSettings(settings)); sslSettingsMap.putAll(getMonitoringExporterSettings(settings)); sslSettingsMap.put(WatcherField.EMAIL_NOTIFICATION_SSL_PREFIX, settings.getByPrefix(WatcherField.EMAIL_NOTIFICATION_SSL_PREFIX)); + sslSettingsMap.put(XPackSettings.TRANSPORT_SSL_PREFIX, settings.getByPrefix(XPackSettings.TRANSPORT_SSL_PREFIX)); + sslSettingsMap.putAll(getTransportProfileSSLSettings(settings)); + return Collections.unmodifiableMap(sslSettingsMap); + } - sslSettingsMap.forEach((key, sslSettings) -> loadConfiguration(key, sslSettings, sslContextHolders)); - - final Settings transportSSLSettings = settings.getByPrefix(XPackSettings.TRANSPORT_SSL_PREFIX); - final SSLConfiguration transportSSLConfiguration = - loadConfiguration(XPackSettings.TRANSPORT_SSL_PREFIX, transportSSLSettings, sslContextHolders); - this.transportSSLConfiguration.set(transportSSLConfiguration); - Map profileSettings = getTransportProfileSSLSettings(settings); - profileSettings.forEach((key, profileSetting) -> loadConfiguration(key, profileSetting, sslContextHolders)); + /** + * Parses the settings to load all SSLConfiguration objects that will be used. + */ + Map loadSSLConfigurations(Map sslConfigurationMap) { + final Map sslContextHolders = new HashMap<>(sslConfigurationMap.size()); + sslConfigurationMap.forEach((key, sslConfiguration) -> { + try { + sslContextHolders.computeIfAbsent(sslConfiguration, this::createSslContext); + } catch (Exception e) { + throw new ElasticsearchSecurityException("failed to load SSL configuration [{}]", e, key); + } + }); for (String context : Arrays.asList("xpack.security.transport.ssl", "xpack.security.http.ssl")) { validateServerConfiguration(context); @@ -510,21 +534,6 @@ public class SSLService { return Collections.unmodifiableMap(sslContextHolders); } - private SSLConfiguration loadConfiguration(String key, Settings settings, Map contextHolders) { - if (key.endsWith(".")) { - // Drop trailing '.' so that any exception messages are consistent - key = key.substring(0, key.length() - 1); - } - try { - final SSLConfiguration configuration = new SSLConfiguration(settings); - storeSslConfiguration(key, configuration); - contextHolders.computeIfAbsent(configuration, this::createSslContext); - return configuration; - } catch (Exception e) { - throw new ElasticsearchSecurityException("failed to load SSL configuration [{}]", e, key); - } - } - private void validateServerConfiguration(String prefix) { assert prefix.endsWith(".ssl"); SSLConfiguration configuration = getSSLConfiguration(prefix); @@ -552,13 +561,6 @@ public class SSLService { } } - private void storeSslConfiguration(String key, SSLConfiguration configuration) { - if (key.endsWith(".")) { - key = key.substring(0, key.length() - 1); - } - sslConfigurations.put(key, configuration); - } - /** * Returns information about each certificate that is referenced by any SSL configuration. @@ -766,7 +768,7 @@ public class SSLService { return sslSettings; } - private Settings getHttpTransportSSLSettings(Settings settings) { + private static Settings getHttpTransportSSLSettings(Settings settings) { Settings httpSSLSettings = settings.getByPrefix(XPackSettings.HTTP_SSL_PREFIX); if (httpSSLSettings.isEmpty()) { return httpSSLSettings; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java index 8f879fe049c..5e3c469d966 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java @@ -338,19 +338,17 @@ public class SSLConfigurationReloaderTests extends ESTestCase { final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl."); final AtomicReference exceptionRef = new AtomicReference<>(); final CountDownLatch latch = new CountDownLatch(1); - new SSLConfigurationReloader(env, sslService, resourceWatcherService) { - @Override - void reloadSSLContext(SSLConfiguration configuration) { - try { - super.reloadSSLContext(configuration); - } catch (Exception e) { - exceptionRef.set(e); - throw e; - } finally { - latch.countDown(); - } + final Consumer reloadConsumer = sslConfiguration -> { + try { + sslService.reloadSSLContext(sslConfiguration); + } catch (Exception e) { + exceptionRef.set(e); + throw e; + } finally { + latch.countDown(); } }; + new SSLConfigurationReloader(env, reloadConsumer, resourceWatcherService, SSLService.getSSLConfigurations(settings).values()); final SSLContext context = sslService.sslContextHolder(config).sslContext(); @@ -391,19 +389,17 @@ public class SSLConfigurationReloaderTests extends ESTestCase { final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl."); final AtomicReference exceptionRef = new AtomicReference<>(); final CountDownLatch latch = new CountDownLatch(1); - new SSLConfigurationReloader(env, sslService, resourceWatcherService) { - @Override - void reloadSSLContext(SSLConfiguration configuration) { - try { - super.reloadSSLContext(configuration); - } catch (Exception e) { - exceptionRef.set(e); - throw e; - } finally { - latch.countDown(); - } + final Consumer reloadConsumer = sslConfiguration -> { + try { + sslService.reloadSSLContext(sslConfiguration); + } catch (Exception e) { + exceptionRef.set(e); + throw e; + } finally { + latch.countDown(); } }; + new SSLConfigurationReloader(env, reloadConsumer, resourceWatcherService, SSLService.getSSLConfigurations(settings).values()); final SSLContext context = sslService.sslContextHolder(config).sslContext(); @@ -437,19 +433,17 @@ public class SSLConfigurationReloaderTests extends ESTestCase { final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl."); final AtomicReference exceptionRef = new AtomicReference<>(); final CountDownLatch latch = new CountDownLatch(1); - new SSLConfigurationReloader(env, sslService, resourceWatcherService) { - @Override - void reloadSSLContext(SSLConfiguration configuration) { - try { - super.reloadSSLContext(configuration); - } catch (Exception e) { - exceptionRef.set(e); - throw e; - } finally { - latch.countDown(); - } + final Consumer reloadConsumer = sslConfiguration -> { + try { + sslService.reloadSSLContext(sslConfiguration); + } catch (Exception e) { + exceptionRef.set(e); + throw e; + } finally { + latch.countDown(); } }; + new SSLConfigurationReloader(env, reloadConsumer, resourceWatcherService, SSLService.getSSLConfigurations(settings).values()); final SSLContext context = sslService.sslContextHolder(config).sslContext(); @@ -481,19 +475,17 @@ public class SSLConfigurationReloaderTests extends ESTestCase { final SSLConfiguration config = sslService.sslConfiguration(settings.getByPrefix("xpack.security.transport.ssl.")); final AtomicReference exceptionRef = new AtomicReference<>(); final CountDownLatch latch = new CountDownLatch(1); - new SSLConfigurationReloader(env, sslService, resourceWatcherService) { - @Override - void reloadSSLContext(SSLConfiguration configuration) { - try { - super.reloadSSLContext(configuration); - } catch (Exception e) { - exceptionRef.set(e); - throw e; - } finally { - latch.countDown(); - } + final Consumer reloadConsumer = sslConfiguration -> { + try { + sslService.reloadSSLContext(sslConfiguration); + } catch (Exception e) { + exceptionRef.set(e); + throw e; + } finally { + latch.countDown(); } }; + new SSLConfigurationReloader(env, reloadConsumer, resourceWatcherService, SSLService.getSSLConfigurations(settings).values()); final SSLContext context = sslService.sslContextHolder(config).sslContext(); @@ -532,16 +524,14 @@ public class SSLConfigurationReloaderTests extends ESTestCase { final CountDownLatch reloadLatch = new CountDownLatch(1); final SSLService sslService = new SSLService(settings, env); final SSLConfiguration config = sslService.getSSLConfiguration("xpack.security.transport.ssl"); - new SSLConfigurationReloader(env, sslService, resourceWatcherService) { - @Override - void reloadSSLContext(SSLConfiguration configuration) { - try { - super.reloadSSLContext(configuration); - } finally { - reloadLatch.countDown(); - } + final Consumer reloadConsumer = sslConfiguration -> { + try { + sslService.reloadSSLContext(sslConfiguration); + } finally { + reloadLatch.countDown(); } }; + new SSLConfigurationReloader(env, reloadConsumer, resourceWatcherService, SSLService.getSSLConfigurations(settings).values()); // Baseline checks preChecks.accept(sslService.sslContextHolder(config).sslContext()); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java index 146e5728a70..3d195d498fa 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.settings.SecureSettings; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.env.Environment; import org.elasticsearch.index.reindex.ReindexPlugin; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESIntegTestCase.Scope; @@ -133,6 +134,7 @@ public class SecuritySettingsSource extends NodeConfigurationSource { writeFile(xpackConf, "users_roles", configUsersRoles()); Settings.Builder builder = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), home) .put(XPackSettings.SECURITY_ENABLED.getKey(), true) .put(NetworkModule.TRANSPORT_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO) .put(NetworkModule.HTTP_TYPE_KEY, randomBoolean() ? SecurityField.NAME4 : SecurityField.NIO) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java index 059189f65ce..a9e64422481 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactoryTests.java @@ -273,8 +273,8 @@ public class LdapSessionFactoryTests extends LdapTestCase { SecureString userPass = new SecureString("pass"); try (ResourceWatcherService resourceWatcher = new ResourceWatcherService(settings, threadPool)) { - new SSLConfigurationReloader(environment, sslService, resourceWatcher); - + new SSLConfigurationReloader(environment, resourceWatcher, SSLService.getSSLConfigurations(environment.settings()).values()) + .setSSLService(sslService); Files.copy(fakeCa, ldapCaPath, StandardCopyOption.REPLACE_EXISTING); resourceWatcher.notifyNow(ResourceWatcherService.Frequency.HIGH); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadDuringStartupIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadDuringStartupIntegTests.java new file mode 100644 index 00000000000..84b09d56913 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadDuringStartupIntegTests.java @@ -0,0 +1,116 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.ssl; + +import org.elasticsearch.common.settings.MockSecureSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; +import org.elasticsearch.env.TestEnvironment; +import org.elasticsearch.test.InternalTestCluster.RestartCallback; +import org.elasticsearch.test.SecurityIntegTestCase; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.concurrent.CountDownLatch; + +public class SSLReloadDuringStartupIntegTests extends SecurityIntegTestCase { + + @Override + public Settings nodeSettings(int nodeOrdinal) { + Settings settings = super.nodeSettings(nodeOrdinal); + Environment tmpEnv = TestEnvironment.newEnvironment(settings); + // each node gets its own keystore under its home/config dir + Path origKeystorePath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"); + Path nodeSpecificPath = tmpEnv.configFile().resolve("testnode.jks"); + try { + Files.copy(origKeystorePath, nodeSpecificPath, StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + + Settings.Builder builder = Settings.builder() + .put(settings.filter((s) -> s.startsWith("xpack.security.transport.ssl.") == false), false); + MockSecureSettings secureSettings = new MockSecureSettings(); + builder.setSecureSettings(secureSettings); + builder.put("xpack.security.transport.ssl.keystore.path", nodeSpecificPath) + .put("resource.reload.interval.high", "1s") + .put("xpack.security.transport.ssl.enabled", true); + secureSettings.setString("xpack.security.transport.ssl.keystore.secure_password", "testnode"); + return builder.build(); + } + + @Override + protected boolean transportSSLEnabled() { + return true; + } + + public void testReloadDuringStartup() throws Exception { + final String node = randomFrom(internalCluster().getNodeNames()); + final Environment env = internalCluster().getInstance(Environment.class, node); + // this latch is to synchronize the start of the thread that updates the TLS config + // and the continuation of the node restart + final CountDownLatch latch = new CountDownLatch(2); + // this latch is used by the test to signal when the updated TLS configuration has been + // written so that the test should proceed with checks to ensure the node rejoins the cluster + final CountDownLatch writtenLatch = new CountDownLatch(1); + + // restart the node + internalCluster().restartNode(node, new RestartCallback() { + @Override + public Settings onNodeStopped(String nodeName) throws Exception { + Path origKeystorePath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.jks"); + Path keystorePath = env.configFile().resolve("testnode.jks"); + Path updatedKeystorePath = getDataPath("/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode_updated.jks"); + assertTrue(Files.exists(keystorePath)); + // replace the keystore with one that will fail during handshaking + copyAndAtomicMoveIfPossible(updatedKeystorePath, keystorePath); + new Thread(() -> { + latch.countDown(); + try { + latch.await(); + Thread.sleep(randomLongBetween(1L, 2000L)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + + // replace the bad keystore with a new copy of the good one + try { + copyAndAtomicMoveIfPossible(origKeystorePath, keystorePath); + writtenLatch.countDown(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }).start(); + + latch.countDown(); + try { + latch.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return super.onNodeStopped(nodeName); + } + }); + writtenLatch.await(); + ensureClusterSizeConsistency(); + ensureFullyConnectedCluster(); + } + + private void copyAndAtomicMoveIfPossible(Path source, Path target) throws IOException { + Path tmp = createTempFile(); + Files.copy(source, tmp, StandardCopyOption.REPLACE_EXISTING); + try { + Files.move(tmp, target, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + } catch (AtomicMoveNotSupportedException e) { + Files.move(tmp, target, StandardCopyOption.REPLACE_EXISTING); + } + } +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadIntegTests.java index 6354ddb5046..e9f7ae07430 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/ssl/SSLReloadIntegTests.java @@ -18,7 +18,6 @@ import org.elasticsearch.xpack.core.ssl.SSLService; import javax.net.ssl.SSLException; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; - import java.io.IOException; import java.net.SocketException; import java.nio.file.AtomicMoveNotSupportedException;