Watch SSL files instead of directories (#129738) (#129987)

With the introduction of entitlements (#120243) and exclusive file
access (#123087) it is no longer safe to watch a whole directory.

In a lot of deployments, the parent directory for SSL config files
will be the main config directory, which also contains exclusive files
such as SAML realm metadata or File realm users. Watching that
directory will cause entitlement warnings because it is not
permissible for core/ssl-config to read files that are exclusively
owned by the security module (or other modules)

Author: tvernum

docs/changelog/129738.yaml

@@ -0,0 +1,5 @@
+pr: 129738
+summary: Watch SSL files instead of directories
+area: TLS
+type: bug
+issues: []

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
      */

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<SslConfiguration> reloadConsumer(Future<SSLService> 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<Path, List<SslConfiguration>> pathToConfigurationsMap = new HashMap<>();
         for (SslConfiguration sslConfiguration : sslConfigurations) {
             final Collection<Path> 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<Path> directoriesToMonitor(Iterable<Path> filePaths) {
-        Set<Path> paths = new HashSet<>();
-        for (Path path : filePaths) {
-            paths.add(path.getParent());
-        }
-        return paths;
-    }
-
     private static class ChangeListener implements FileChangesListener {
 
         private final List<SslConfiguration> sslConfigurations;

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<Path> 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<SslConfiguration> 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