diff --git a/docs/changelog/129738.yaml b/docs/changelog/129738.yaml new file mode 100644 index 0000000000000..38a33b5aefb3f --- /dev/null +++ b/docs/changelog/129738.yaml @@ -0,0 +1,5 @@ +pr: 129738 +summary: Watch SSL files instead of directories +area: TLS +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/watcher/FileWatcher.java b/server/src/main/java/org/elasticsearch/watcher/FileWatcher.java index 4620e65534d3e..a8fbf3659e7b7 100644 --- a/server/src/main/java/org/elasticsearch/watcher/FileWatcher.java +++ b/server/src/main/java/org/elasticsearch/watcher/FileWatcher.java @@ -57,6 +57,11 @@ public FileWatcher(Path path, boolean checkFileContents) { rootFileObserver = new FileObserver(path); } + // For testing + public Path getPath() { + return path; + } + /** * Clears any state with the FileWatcher, making all files show up as new */ 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 e75fe0ab26f35..c84d492f4c17f 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 @@ -22,10 +22,8 @@ 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.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.function.Consumer; @@ -80,7 +78,7 @@ private static Consumer reloadConsumer(Future futu } /** - * Collects all of the directories that need to be monitored for the provided {@link SslConfiguration} instances and ensures that + * Collects all of the files that need to be monitored for the provided {@link SslConfiguration} instances and ensures that * they are being watched for changes */ private static void startWatching( @@ -91,8 +89,8 @@ private static void startWatching( Map> pathToConfigurationsMap = new HashMap<>(); for (SslConfiguration sslConfiguration : sslConfigurations) { final Collection filesToMonitor = sslConfiguration.getDependentFiles(); - for (Path directory : directoriesToMonitor(filesToMonitor)) { - pathToConfigurationsMap.compute(directory, (path, list) -> { + for (Path file : filesToMonitor) { + pathToConfigurationsMap.compute(file, (path, list) -> { if (list == null) { list = new ArrayList<>(); } @@ -109,22 +107,11 @@ private static void startWatching( try { resourceWatcherService.add(fileWatcher, Frequency.HIGH); } catch (IOException | SecurityException e) { - logger.error("failed to start watching directory [{}] for ssl configurations [{}] - {}", path, configurations, e); + logger.error("failed to start watching file [{}] for ssl configurations [{}] - {}", path, configurations, e); } }); } - /** - * Returns a unique set of directories that need to be monitored based on the provided file paths - */ - private static Set directoriesToMonitor(Iterable filePaths) { - Set paths = new HashSet<>(); - for (Path path : filePaths) { - paths.add(path.getParent()); - } - return paths; - } - private static class ChangeListener implements FileChangesListener { private final List sslConfigurations; 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 68987bbd19815..04c983775a082 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 @@ -37,6 +37,8 @@ import org.elasticsearch.test.http.MockWebServer; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.watcher.FileWatcher; +import org.elasticsearch.watcher.ResourceWatcher; import org.elasticsearch.watcher.ResourceWatcherService; import org.junit.After; import org.junit.Before; @@ -66,7 +68,9 @@ import java.security.cert.CertificateException; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.TimeUnit; @@ -79,6 +83,7 @@ import javax.net.ssl.SSLSocket; import static org.elasticsearch.test.TestMatchers.throwableWithMessage; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.sameInstance; @@ -559,6 +564,38 @@ public void testFailureToReadFileDoesntFail() throws Exception { } } + /** + * Due to exclusive access entitlements + * (see {@link org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.FileData#exclusive}), + * it is not safe to monitor a directory or any files that are not an explicit part of this SSL configuration. + */ + public void testReloaderOnlyWatchesSpecifiedFiles() throws Exception { + final Set watchedPaths = new HashSet<>(); + final ResourceWatcherService mockResourceWatcher = Mockito.mock(ResourceWatcherService.class); + Mockito.when(mockResourceWatcher.add(Mockito.any(ResourceWatcher.class), Mockito.any(ResourceWatcherService.Frequency.class))) + .then(inv -> { + final FileWatcher fileWatcher = asInstanceOf(FileWatcher.class, inv.getArguments()[0]); + watchedPaths.add(fileWatcher.getPath()); + return null; + }); + + final Path tempDir = createTempDir(); + final Path clientCertPath = tempDir.resolve("testclient.crt"); + Settings settings = baseKeystoreSettings(tempDir, null).putList( + "xpack.security.transport.ssl.certificate_authorities", + clientCertPath.toString() + ).put("path.home", createTempDir()).build(); + + final Environment env = newEnvironment(settings); + final Collection configurations = SSLService.getSSLConfigurations(env).values(); + new SSLConfigurationReloader(ignore -> {}, mockResourceWatcher, configurations); + + assertThat( + watchedPaths, + containsInAnyOrder(tempDir.resolve("testclient.pem"), tempDir.resolve("testclient.crt"), tempDir.resolve("testclientcert.crt")) + ); + } + private Settings.Builder baseKeystoreSettings(Path tempDir, MockSecureSettings secureSettings) throws IOException { final Path keyPath = tempDir.resolve("testclient.pem"); final Path certPath = tempDir.resolve("testclientcert.crt"); // testclient.crt filename already used in #testPEMTrustReloadException