Skip to content

Commit

Permalink
Handle TLS file updates during startup
Browse files Browse the repository at this point in the history 
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 and the class is now
primarily a utility class that no longer needs to be instantiated.

Closes elastic#54867
  • Loading branch information
@jaymode
jaymode committed Apr 9, 2020
1 parent 6fd6895 commit 60788f1
Show file tree
Hide file tree
Showing 8 changed files with 284 additions and 146 deletions.
24 changes: 21 additions & 3 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
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.SpecialPermission;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionResponse;
Expand Down Expand Up @@ -65,6 +66,7 @@
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;
Expand All @@ -80,6 +82,9 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
Expand Down Expand Up @@ -233,10 +238,23 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
IndexNameExpressionResolver expressionResolver) {
List<Object> components = new ArrayList<>();

final SSLService sslService = new SSLService(environment);
final Map<String, SSLConfiguration> sslConfigurations = SSLService.getSSLConfigurations(environment.settings());
final CompletableFuture<SSLService> sslServiceFuture = new CompletableFuture<>();
final Consumer<SSLConfiguration> reloadConsumer = sslConfiguration -> {
try {
final SSLService sslService = sslServiceFuture.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);
}
};
SSLConfigurationReloader.startWatching(environment, reloadConsumer, resourceWatcherService, sslConfigurations.values());
final SSLService sslService = new SSLService(environment, sslConfigurations);
sslServiceFuture.complete(sslService);
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);

setLicenseService(new LicenseService(settings, clusterService, getClock(),
environment, resourceWatcherService, getLicenseState()));
Expand Down
81 changes: 35 additions & 46 deletions .../plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,68 +17,55 @@

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
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.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<Path, ChangeListener> pathToChangeListenerMap = new ConcurrentHashMap<>();
private final Environment environment;
private final ResourceWatcherService resourceWatcherService;
private final SSLService sslService;

public SSLConfigurationReloader(Environment env, SSLService sslService, ResourceWatcherService resourceWatcher) {
this.environment = env;
this.resourceWatcherService = resourceWatcher;
this.sslService = sslService;
startWatching(sslService.getLoadedSSLConfigurations());
}
private SSLConfigurationReloader() { }

/**
* 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<SSLConfiguration> sslConfigurations) {
public static void startWatching(Environment environment, Consumer<SSLConfiguration> reloadConsumer,
ResourceWatcherService resourceWatcherService, Collection<SSLConfiguration> sslConfigurations) {
Map<Path, List<SSLConfiguration>> 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<Path, List<SSLConfiguration>> entry : pathToConfigurationsMap.entrySet()) {
ChangeListener changeListener = new ChangeListener(environment, List.copyOf(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);
}
}
}

/**
Expand All @@ -92,15 +79,17 @@ private static Set<Path> directoriesToMonitor(List<Path> filePaths) {
return paths;
}

private class ChangeListener implements FileChangesListener {
private static class ChangeListener implements FileChangesListener {

private final CopyOnWriteArraySet<SSLConfiguration> sslConfigurations = new CopyOnWriteArraySet<>();
private final Environment environment;
private final List<SSLConfiguration> sslConfigurations;
private final Consumer<SSLConfiguration> 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<SSLConfiguration> sslConfigurations,
Consumer<SSLConfiguration> reloadConsumer) {
this.environment = environment;
this.sslConfigurations = sslConfigurations;
this.reloadConsumer = reloadConsumer;
}

@Override
Expand All @@ -118,7 +107,7 @@ public void onFileChanged(Path file) {
boolean reloaded = false;
for (SSLConfiguration sslConfiguration : sslConfigurations) {
if (sslConfiguration.filesToMonitor(environment).contains(file)) {
reloadSSLContext(sslConfiguration);
reloadConsumer.accept(sslConfiguration);
reloaded = true;
}
}
Expand Down
90 changes: 47 additions & 43 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
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;
Expand Down Expand Up @@ -120,27 +119,34 @@ public class SSLService {
* always maps to the same {@link SSLContextHolder}, even if it is being used within a different context-name.
*/
private final Map<SSLConfiguration, SSLContextHolder> sslContexts;
private final SetOnce<SSLConfiguration> transportSSLConfiguration = new SetOnce<>();

/**
* Create a new SSLService that parses the settings for the ssl contexts that need to be created, creates them, and then caches them
* for use later
*/
public SSLService(Environment 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<String, SSLConfiguration> sslConfigurations) {
this.env = environment;
this.settings = env.settings();
this.diagnoseTrustExceptions = DIAGNOSE_TRUST_EXCEPTIONS_SETTING.get(environment.settings());
this.sslConfigurations = new HashMap<>();
this.sslContexts = loadSSLConfigurations();
this.sslConfigurations = sslConfigurations;
this.sslContexts = loadSSLConfigurations(this.sslConfigurations);
}

@Deprecated
public SSLService(Settings settings, Environment environment) {
this.env = environment;
this.settings = env.settings();
this.diagnoseTrustExceptions = DIAGNOSE_TRUST_EXCEPTIONS_SETTING.get(settings);
this.sslConfigurations = new HashMap<>();
this.sslContexts = loadSSLConfigurations();
this.sslConfigurations = getSSLConfigurations(this.settings);
this.sslContexts = loadSSLConfigurations(this.sslConfigurations);
}

private SSLService(Environment environment, Map<String, SSLConfiguration> sslConfigurations,
Expand All @@ -161,7 +167,7 @@ public SSLService createDynamicSSLService() {
return new SSLService(env, sslConfigurations, sslContexts) {

@Override
Map<SSLConfiguration, SSLContextHolder> loadSSLConfigurations() {
Map<SSLConfiguration, SSLContextHolder> loadSSLConfigurations(Map<String, SSLConfiguration> sslConfigurations) {
// we don't need to load anything...
return Collections.emptyMap();
}
Expand Down Expand Up @@ -342,6 +348,10 @@ public SSLContext sslContext(SSLConfiguration configuration) {
return sslContextHolder(configuration).sslContext();
}

public void reloadSSLContext(SSLConfiguration configuration) {
sslContextHolder(configuration).reload();
}

/**
* Returns the existing {@link SSLContextHolder} for the configuration
*
Expand Down Expand Up @@ -486,27 +496,43 @@ X509ExtendedTrustManager wrapWithDiagnostics(X509ExtendedTrustManager trustManag
return trustManager;
}

/**
* Parses the settings to load all SSLConfiguration objects that will be used.
*/
Map<SSLConfiguration, SSLContextHolder> loadSSLConfigurations() {
final Map<SSLConfiguration, SSLContextHolder> sslContextHolders = new HashMap<>();
public static Map<String, SSLConfiguration> getSSLConfigurations(Settings settings) {
final Map<String, Settings> sslSettingsMap = getSSLSettingsMap(settings);
final Map<String, SSLConfiguration> 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);
}

static Map<String, Settings> getSSLSettingsMap(Settings settings) {
final Map<String, Settings> 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<String, Settings> profileSettings = getTransportProfileSSLSettings(settings);
profileSettings.forEach((key, profileSetting) -> loadConfiguration(key, profileSetting, sslContextHolders));
/**
* Parses the settings to load all SSLConfiguration objects that will be used.
*/
Map<SSLConfiguration, SSLContextHolder> loadSSLConfigurations(Map<String, SSLConfiguration> sslConfigurationMap) {
final Map<SSLConfiguration, SSLContextHolder> 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 : List.of("xpack.security.transport.ssl", "xpack.security.http.ssl")) {
validateServerConfiguration(context);
Expand All @@ -515,21 +541,6 @@ Map<SSLConfiguration, SSLContextHolder> loadSSLConfigurations() {
return Collections.unmodifiableMap(sslContextHolders);
}

private SSLConfiguration loadConfiguration(String key, Settings settings, Map<SSLConfiguration, SSLContextHolder> 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);
Expand Down Expand Up @@ -557,13 +568,6 @@ private void validateServerConfiguration(String prefix) {
}
}

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.
Expand Down Expand Up @@ -771,7 +775,7 @@ private static Map<String, Settings> getTransportProfileSSLSettings(Settings set
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;
Expand Down
Loading

0 comments on commit 60788f1

Please sign in to comment.