Fix error reporting for SSL resources outside of config dir (#64235)

The SSLConfigurationReloader is constructed before the SSLService in order
to avoid a potential race condition where files that were modified after the
SSL Service was created, but before the reloader was started would not be 
reloaded (see f9de7ce)

However, a consequence of this was that some of the error handling that
had been built into the SSL service (and related classes) would not be 
called if an exception was thrown from the reloader class. 

This commit changes the reloader class to catch-and-log for additional
exception types, so that the more feature-rich error handling in the other 
SSL configuration classes can be applied.
 
Co-authored-by: Tim Vernum <tim@adjective.org>

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloader.java

@@ -16,15 +16,16 @@ 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.security.AccessControlException;
 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.CompletableFuture;
 import java.util.concurrent.ExecutionException;
@@ -83,7 +84,8 @@ public final class SSLConfigurationReloader {
                                       ResourceWatcherService resourceWatcherService, Collection<SSLConfiguration> sslConfigurations) {
         Map<Path, List<SSLConfiguration>> pathToConfigurationsMap = new HashMap<>();
         for (SSLConfiguration sslConfiguration : sslConfigurations) {
-            for (Path directory : directoriesToMonitor(sslConfiguration.filesToMonitor(environment))) {
+            final List<Path> filesToMonitor = sslConfiguration.filesToMonitor(environment);
+            for (Path directory : directoriesToMonitor(filesToMonitor)) {
                 pathToConfigurationsMap.compute(directory, (path, list) -> {
                     if (list == null) {
                         list = new ArrayList<>();
@@ -94,16 +96,16 @@ public final class SSLConfigurationReloader {
             }
         }
 
-        for (Entry<Path, List<SSLConfiguration>> entry : pathToConfigurationsMap.entrySet()) {
-            ChangeListener changeListener = new ChangeListener(environment, List.copyOf(entry.getValue()), reloadConsumer);
-            FileWatcher fileWatcher = new FileWatcher(entry.getKey());
+        pathToConfigurationsMap.forEach((path, configurations) -> {
+            ChangeListener changeListener = new ChangeListener(environment, List.copyOf(configurations), reloadConsumer);
+            FileWatcher fileWatcher = new FileWatcher(path);
             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);
+            } catch (IOException | AccessControlException e) {
+                logger.error("failed to start watching directory [{}] for ssl configurations [{}] - {}", path, configurations, e);
             }
-        }
+        });
     }
 
     /**

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java

@@ -38,6 +38,7 @@ import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.watcher.ResourceWatcherService;
 import org.junit.After;
 import org.junit.Before;
+import org.mockito.Mockito;
 
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLHandshakeException;
@@ -53,6 +54,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
 import java.nio.file.StandardOpenOption;
+import java.security.AccessControlException;
 import java.security.AccessController;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
@@ -63,6 +65,7 @@ import java.security.PrivilegedExceptionAction;
 import java.security.UnrecoverableKeyException;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
@@ -503,6 +506,31 @@ public class SSLConfigurationReloaderTests extends ESTestCase {
         assertThat(sslService.sslContextHolder(config).sslContext(), sameInstance(context));
     }
 
+    /**
+     * Tests that the reloader doesn't throw an exception if a file is unreadable or configured to be outside of the config/ directory.
+     * These errors are handled correctly by the relevant {@link KeyConfig} and {@link TrustConfig} classes, so the reloader should
+     * simply log and continue.
+     */
+    public void testFailureToReadFileDoesntFail() throws Exception {
+        Path tempDir = createTempDir();
+        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();
+        Environment env = TestEnvironment.newEnvironment(settings);
+
+        final ResourceWatcherService mockResourceWatcher = Mockito.mock(ResourceWatcherService.class);
+        Mockito.when(mockResourceWatcher.add(Mockito.any(), Mockito.any()))
+            .thenThrow(randomBoolean() ? new AccessControlException("access denied in test") : new IOException("file error for testing"));
+        final Collection<SSLConfiguration> configurations = SSLService.getSSLConfigurations(settings).values();
+        try {
+            new SSLConfigurationReloader(env, null, mockResourceWatcher, configurations);
+        } catch (Exception e) {
+            fail("SSLConfigurationReloader threw exception, but is expected to catch and log file access errors instead:" + e);
+        }
+    }
+
     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