Add exclusive access files for security module (elastic#123676) (elastic#124485)

* Add exclusive access files for security module (elastic#123676)

This commit fills out missing entitlements for the security module.
Specifically they are config files which require exclusive access.

* Use a consistent ordering in policy manager exclulsive tests

* use better assertion

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>

Author: rjernst

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java

@@ -23,8 +23,12 @@
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.function.BiConsumer;
 
 import static java.util.Comparator.comparing;
@@ -42,27 +46,47 @@ record ExclusiveFileEntitlement(String componentName, String moduleName, FilesEn
     /**
      * An intermediary structure to help globally validate exclusive paths, and then build exclusive paths for individual modules.
      */
-    record ExclusivePath(String componentName, String moduleName, String path) {
+    record ExclusivePath(String componentName, Set<String> moduleNames, String path) {
 
         @Override
         public String toString() {
-            return "[[" + componentName + "] [" + moduleName + "] [" + path + "]]";
+            return "[[" + componentName + "] " + moduleNames + " [" + path + "]]";
         }
     }
 
     static List<ExclusivePath> buildExclusivePathList(List<ExclusiveFileEntitlement> exclusiveFileEntitlements, PathLookup pathLookup) {
-        List<ExclusivePath> exclusivePaths = new ArrayList<>();
+        Map<String, ExclusivePath> exclusivePaths = new HashMap<>();
         for (ExclusiveFileEntitlement efe : exclusiveFileEntitlements) {
             for (FilesEntitlement.FileData fd : efe.filesEntitlement().filesData()) {
                 if (fd.exclusive()) {
                     List<Path> paths = fd.resolvePaths(pathLookup).toList();
                     for (Path path : paths) {
-                        exclusivePaths.add(new ExclusivePath(efe.componentName(), efe.moduleName(), normalizePath(path)));
+                        String normalizedPath = normalizePath(path);
+                        var exclusivePath = exclusivePaths.computeIfAbsent(
+                            normalizedPath,
+                            k -> new ExclusivePath(efe.componentName(), new HashSet<>(), normalizedPath)
+                        );
+                        if (exclusivePath.componentName().equals(efe.componentName()) == false) {
+                            throw new IllegalArgumentException(
+                                "Path ["
+                                    + normalizedPath
+                                    + "] is already exclusive to ["
+                                    + exclusivePath.componentName()
+                                    + "]"
+                                    + exclusivePath.moduleNames
+                                    + ", cannot add exclusive access for ["
+                                    + efe.componentName()
+                                    + "]["
+                                    + efe.moduleName
+                                    + "]"
+                            );
+                        }
+                        exclusivePath.moduleNames.add(efe.moduleName());
                     }
                 }
             }
         }
-        return exclusivePaths.stream().sorted(comparing(ExclusivePath::path, PATH_ORDER)).distinct().toList();
+        return exclusivePaths.values().stream().sorted(comparing(ExclusivePath::path, PATH_ORDER)).distinct().toList();
     }
 
     static void validateExclusivePaths(List<ExclusivePath> exclusivePaths) {
@@ -97,7 +121,7 @@ private FileAccessTree(
     ) {
         List<String> updatedExclusivePaths = new ArrayList<>();
         for (ExclusivePath exclusivePath : exclusivePaths) {
-            if (exclusivePath.componentName().equals(componentName) == false || exclusivePath.moduleName().equals(moduleName) == false) {
+            if (exclusivePath.componentName().equals(componentName) == false || exclusivePath.moduleNames().contains(moduleName) == false) {
                 updatedExclusivePaths.add(exclusivePath.path());
             }
         }

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java

@@ -164,7 +164,8 @@ public PathSettingFileData withExclusive(boolean exclusive) {
         public Stream<Path> resolveRelativePaths(PathLookup pathLookup) {
             Stream<String> result = pathLookup.settingResolver()
                 .apply(setting)
-                .filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false);
+                .filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false)
+                .distinct();
             return result.map(Path::of);
         }
 

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java

@@ -26,6 +26,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
 import static org.elasticsearch.entitlement.runtime.policy.FileAccessTree.buildExclusivePathList;
@@ -386,7 +387,7 @@ public void testDuplicateExclusivePaths() {
             original.moduleName(),
             new FilesEntitlement(List.of(originalFileData.withPlatform(WINDOWS)))
         );
-        var originalExclusivePath = new ExclusivePath("component1", "module1", normalizePath(path("/a/b")));
+        var originalExclusivePath = new ExclusivePath("component1", Set.of("module1"), normalizePath(path("/a/b")));
 
         // Some basic tests
 
@@ -406,27 +407,14 @@ public void testDuplicateExclusivePaths() {
         var distinctEntitlements = List.of(original, differentComponent, differentModule, differentPath);
         var distinctPaths = List.of(
             originalExclusivePath,
-            new ExclusivePath("component2", original.moduleName(), originalExclusivePath.path()),
-            new ExclusivePath(original.componentName(), "module2", originalExclusivePath.path()),
-            new ExclusivePath(original.componentName(), original.moduleName(), normalizePath(path("/c/d")))
+            new ExclusivePath("component2", Set.of(original.moduleName()), originalExclusivePath.path()),
+            new ExclusivePath(original.componentName(), Set.of("module2"), originalExclusivePath.path()),
+            new ExclusivePath(original.componentName(), Set.of(original.moduleName()), normalizePath(path("/c/d")))
         );
-        assertEquals(
-            "Distinct elements should not be combined",
-            distinctPaths,
-            buildExclusivePathList(distinctEntitlements, TEST_PATH_LOOKUP)
-        );
-
-        // Do merge things we should
-
-        List<ExclusiveFileEntitlement> interleavedEntitlements = new ArrayList<>();
-        distinctEntitlements.forEach(e -> {
-            interleavedEntitlements.add(e);
-            interleavedEntitlements.add(original);
-        });
-        assertEquals(
-            "Identical elements should be combined wherever they are in the list",
-            distinctPaths,
-            buildExclusivePathList(interleavedEntitlements, TEST_PATH_LOOKUP)
+        var iae = expectThrows(IllegalArgumentException.class, () -> buildExclusivePathList(distinctEntitlements, TEST_PATH_LOOKUP));
+        assertThat(
+            iae.getMessage(),
+            equalTo("Path [/a/b] is already exclusive to [component1][module1], cannot add exclusive access for [component2][module1]")
         );
 
         var equivalentEntitlements = List.of(original, differentMode, differentPlatform);
@@ -486,7 +474,7 @@ static FilesEntitlement entitlement(Map<String, String> value) {
     static List<ExclusivePath> exclusivePaths(String componentName, String moduleName, String... paths) {
         List<ExclusivePath> exclusivePaths = new ArrayList<>();
         for (String path : paths) {
-            exclusivePaths.add(new ExclusivePath(componentName, moduleName, normalizePath(path(path))));
+            exclusivePaths.add(new ExclusivePath(componentName, Set.of(moduleName), normalizePath(path(path))));
         }
         return exclusivePaths;
     }

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java

@@ -38,6 +38,8 @@
 import static org.elasticsearch.entitlement.runtime.policy.PolicyManager.ALL_UNNAMED;
 import static org.elasticsearch.entitlement.runtime.policy.PolicyManager.SERVER_COMPONENT_NAME;
 import static org.hamcrest.Matchers.aMapWithSize;
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.sameInstance;
 
@@ -444,9 +446,9 @@ public void testDuplicateEntitlements() {
     }
 
     public void testFilesEntitlementsWithExclusive() {
-        var baseTestPath = Path.of("/tmp").toAbsolutePath();
-        var testPath1 = Path.of("/tmp/test").toAbsolutePath();
-        var testPath2 = Path.of("/tmp/test/foo").toAbsolutePath();
+        var baseTestPath = Path.of("/base").toAbsolutePath();
+        var testPath1 = Path.of("/base/test").toAbsolutePath();
+        var testPath2 = Path.of("/base/test/foo").toAbsolutePath();
         var iae = expectThrows(
             IllegalArgumentException.class,
             () -> new PolicyManager(
@@ -458,7 +460,7 @@ public void testFilesEntitlementsWithExclusive() {
                         "test",
                         List.of(
                             new Scope(
-                                "test",
+                                "test.module1",
                                 List.of(
                                     new FilesEntitlement(
                                         List.of(FilesEntitlement.FileData.ofPath(testPath1, FilesEntitlement.Mode.READ).withExclusive(true))
@@ -472,7 +474,7 @@ public void testFilesEntitlementsWithExclusive() {
                         "test",
                         List.of(
                             new Scope(
-                                "test",
+                                "test.module2",
                                 List.of(
                                     new FilesEntitlement(
                                         List.of(FilesEntitlement.FileData.ofPath(testPath1, FilesEntitlement.Mode.READ).withExclusive(true))
@@ -490,8 +492,15 @@ public void testFilesEntitlementsWithExclusive() {
                 Set.of()
             )
         );
-        assertTrue(iae.getMessage().contains("duplicate/overlapping exclusive paths found in files entitlements:"));
-        assertTrue(iae.getMessage().contains(Strings.format("[test] [%s]]", testPath1.toString())));
+        assertThat(
+            iae.getMessage(),
+            allOf(
+                containsString("Path [/base/test] is already exclusive"),
+                containsString("[plugin1][test.module1]"),
+                containsString("[plugin2][test.module2]"),
+                containsString("cannot add exclusive access")
+            )
+        );
 
         iae = expectThrows(
             IllegalArgumentException.class,

server/src/main/java/org/elasticsearch/index/analysis/Analysis.java

@@ -66,7 +66,6 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.security.AccessControlException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -261,7 +260,7 @@ public static List<String> getWordList(
         } catch (IOException ioe) {
             String message = Strings.format("IOException while reading %s: %s", settingPath, path);
             throw new IllegalArgumentException(message, ioe);
-        } catch (AccessControlException ace) {
+        } catch (SecurityException ace) {
             throw new IllegalArgumentException(Strings.format("Access denied trying to read file %s: %s", settingPath, path), ace);
         }
     }

server/src/main/java/org/elasticsearch/watcher/FileWatcher.java

@@ -20,7 +20,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.attribute.BasicFileAttributes;
-import java.security.AccessControlException;
 import java.util.Arrays;
 import java.util.stream.StreamSupport;
 
@@ -256,7 +255,7 @@ private Observer createChild(Path file, boolean initial) throws IOException {
                 FileObserver child = new FileObserver(file);
                 child.init(initial);
                 return child;
-            } catch (AccessControlException e) {
+            } catch (SecurityException e) {
                 // don't have permissions, use a placeholder
                 logger.debug(() -> Strings.format("Don't have permissions to watch path [%s]", file), e);
                 return new DeniedObserver(file);

server/src/test/java/org/elasticsearch/index/shard/StoreRecoveryTests.java

@@ -45,7 +45,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.attribute.BasicFileAttributes;
-import java.security.AccessControlException;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.function.Predicate;
@@ -259,7 +258,7 @@ public boolean hardLinksSupported(Path path) throws IOException {
             BasicFileAttributes sourceAttr = Files.readAttributes(path.resolve("foo.bar"), BasicFileAttributes.class);
             // we won't get here - no permission ;)
             return destAttr.fileKey() != null && destAttr.fileKey().equals(sourceAttr.fileKey());
-        } catch (AccessControlException ex) {
+        } catch (SecurityException ex) {
             return true; // if we run into that situation we know it's supported.
         } catch (UnsupportedOperationException ex) {
             return false;

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/PrivilegedFileWatcher.java

@@ -10,6 +10,7 @@
 import org.elasticsearch.watcher.FileWatcher;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -34,6 +35,15 @@ public PrivilegedFileWatcher(Path path) {
         super(path);
     }
 
+    public PrivilegedFileWatcher(Path path, boolean checkFileContents) {
+        super(path, checkFileContents);
+    }
+
+    @Override
+    protected InputStream newInputStream(Path path) throws IOException {
+        return Files.newInputStream(path);
+    }
+
     @Override
     protected boolean fileExists(Path path) {
         return doPrivileged((PrivilegedAction<Boolean>) () -> Files.exists(path));

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/file/FileUserRolesStore.java

@@ -20,6 +20,7 @@
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.support.NoOpLogger;
 import org.elasticsearch.xpack.core.security.support.Validation;
+import org.elasticsearch.xpack.security.PrivilegedFileWatcher;
 import org.elasticsearch.xpack.security.support.SecurityFiles;
 
 import java.io.IOException;
@@ -57,7 +58,7 @@ public class FileUserRolesStore {
         file = resolveFile(config.env());
         userRoles = parseFileLenient(file, logger);
         listeners = new CopyOnWriteArrayList<>(Collections.singletonList(listener));
-        FileWatcher watcher = new FileWatcher(file.getParent());
+        FileWatcher watcher = new PrivilegedFileWatcher(file.getParent());
         watcher.addListener(new FileListener());
         try {
             watcherService.add(watcher, ResourceWatcherService.Frequency.HIGH);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/FileServiceAccountTokenStore.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.xpack.core.security.action.service.TokenInfo.TokenSource;
 import org.elasticsearch.xpack.core.security.authc.support.Hasher;
 import org.elasticsearch.xpack.core.security.support.NoOpLogger;
+import org.elasticsearch.xpack.security.PrivilegedFileWatcher;
 import org.elasticsearch.xpack.security.authc.service.ServiceAccount.ServiceAccountId;
 import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
 import org.elasticsearch.xpack.security.support.FileLineParser;
@@ -59,7 +60,7 @@ public FileServiceAccountTokenStore(
         super(env.settings(), threadPool);
         this.clusterService = clusterService;
         file = resolveFile(env);
-        FileWatcher watcher = new FileWatcher(file.getParent());
+        FileWatcher watcher = new PrivilegedFileWatcher(file.getParent());
         watcher.addListener(new FileReloadListener(file, this::tryReload));
         try {
             resourceWatcherService.add(watcher, ResourceWatcherService.Frequency.HIGH);