Watch SSL files instead of directories (#129738)
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)
This commit is contained in:
parent
e245c176a2
commit
8b62a55f2f
|
@ -0,0 +1,5 @@
|
|||
pr: 129738
|
||||
summary: Watch SSL files instead of directories
|
||||
area: TLS
|
||||
type: bug
|
||||
issues: []
|
|
@ -57,6 +57,11 @@ public class FileWatcher extends AbstractResourceWatcher<FileChangesListener> {
|
|||
rootFileObserver = new FileObserver(path);
|
||||
}
|
||||
|
||||
// For testing
|
||||
public Path getPath() {
|
||||
return path;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clears any state with the FileWatcher, making all files show up as new
|
||||
*/
|
||||
|
|
|
@ -22,10 +22,8 @@ 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.Set;
|
||||
import java.util.concurrent.ExecutionException;
|
||||
import java.util.concurrent.Future;
|
||||
import java.util.function.Consumer;
|
||||
|
@ -80,7 +78,7 @@ public final class SSLConfigurationReloader {
|
|||
}
|
||||
|
||||
/**
|
||||
* 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 @@ public final class SSLConfigurationReloader {
|
|||
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 @@ public final class SSLConfigurationReloader {
|
|||
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;
|
||||
|
|
|
@ -37,6 +37,8 @@ import org.elasticsearch.test.http.MockResponse;
|
|||
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.Certificate;
|
|||
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.SSLSession;
|
|||
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 class SSLConfigurationReloaderTests extends ESTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
|
|
Loading…
Reference in New Issue