Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/129738.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 129738
summary: Watch SSL files instead of directories
area: TLS
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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<>();
}
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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
Expand Down
Loading