Entitlement policies correct handling of prefixes that are not directories (elastic#121598) (elastic#121809)

* Fix FileAccessTree for prefixes that aren't parents

* Support backslashes

* Whoops, nio

* Move normalization responsibility to FileEntitlement

* Normalize to native separators

* Avoid forbidden API

Author: prdoyle

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

@@ -17,8 +17,11 @@
 import java.util.List;
 import java.util.Objects;
 
+import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
+
 public final class FileAccessTree {
     public static final FileAccessTree EMPTY = new FileAccessTree(List.of());
+    private static final String FILE_SEPARATOR = getDefaultFileSystem().getSeparator();
 
     private final String[] readPaths;
     private final String[] writePaths;
@@ -27,11 +30,11 @@ private FileAccessTree(List<FileEntitlement> fileEntitlements) {
         List<String> readPaths = new ArrayList<>();
         List<String> writePaths = new ArrayList<>();
         for (FileEntitlement fileEntitlement : fileEntitlements) {
-            var mode = fileEntitlement.mode();
-            if (mode == FileEntitlement.Mode.READ_WRITE) {
-                writePaths.add(fileEntitlement.path());
+            String path = normalizePath(Path.of(fileEntitlement.path()));
+            if (fileEntitlement.mode() == FileEntitlement.Mode.READ_WRITE) {
+                writePaths.add(path);
             }
-            readPaths.add(fileEntitlement.path());
+            readPaths.add(path);
         }
 
         readPaths.sort(String::compareTo);
@@ -46,14 +49,20 @@ public static FileAccessTree of(List<FileEntitlement> fileEntitlements) {
     }
 
     boolean canRead(Path path) {
-        return checkPath(normalize(path), readPaths);
+        return checkPath(normalizePath(path), readPaths);
     }
 
     boolean canWrite(Path path) {
-        return checkPath(normalize(path), writePaths);
+        return checkPath(normalizePath(path), writePaths);
     }
 
-    private static String normalize(Path path) {
+    /**
+     * @return the "canonical" form of the given {@code path}, to be used for entitlement checks.
+     */
+    static String normalizePath(Path path) {
+        // Note that toAbsolutePath produces paths separated by the default file separator,
+        // so on Windows, if the given path uses forward slashes, this consistently
+        // converts it to backslashes.
         return path.toAbsolutePath().normalize().toString();
     }
 
@@ -64,7 +73,7 @@ private static boolean checkPath(String path, String[] paths) {
         int ndx = Arrays.binarySearch(paths, path);
         if (ndx < -1) {
             String maybeParent = paths[-ndx - 2];
-            return path.startsWith(maybeParent);
+            return path.startsWith(maybeParent) && path.startsWith(FILE_SEPARATOR, maybeParent.length());
         }
         return ndx >= 0;
     }

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

@@ -12,10 +12,12 @@
 import org.elasticsearch.entitlement.runtime.policy.ExternalEntitlement;
 import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException;
 
-import java.nio.file.Paths;
-
 /**
- * Describes a file entitlement with a path and mode.
+ * Describes entitlement to access files at a particular location.
+ *
+ * @param path the location of the files. For directories, implicitly includes access to
+ *            all contained files and (recursively) subdirectories.
+ * @param mode the type of operation
  */
 public record FileEntitlement(String path, Mode mode) implements Entitlement {
 
@@ -24,14 +26,6 @@ public enum Mode {
         READ_WRITE
     }
 
-    public FileEntitlement {
-        path = normalizePath(path);
-    }
-
-    private static String normalizePath(String path) {
-        return Paths.get(path).toAbsolutePath().normalize().toString();
-    }
-
     private static Mode parseMode(String mode) {
         if (mode.equals("read")) {
             return Mode.READ;

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

@@ -16,6 +16,7 @@
 import java.nio.file.Path;
 import java.util.List;
 
+import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
 import static org.hamcrest.Matchers.is;
 
 public class FileAccessTreeTests extends ESTestCase {
@@ -41,7 +42,9 @@ public void testRead() {
         var tree = FileAccessTree.of(List.of(entitlement("foo", "read")));
         assertThat(tree.canRead(path("foo")), is(true));
         assertThat(tree.canRead(path("foo/subdir")), is(true));
+        assertThat(tree.canRead(path("food")), is(false));
         assertThat(tree.canWrite(path("foo")), is(false));
+        assertThat(tree.canWrite(path("food")), is(false));
 
         assertThat(tree.canRead(path("before")), is(false));
         assertThat(tree.canRead(path("later")), is(false));
@@ -51,7 +54,9 @@ public void testWrite() {
         var tree = FileAccessTree.of(List.of(entitlement("foo", "read_write")));
         assertThat(tree.canWrite(path("foo")), is(true));
         assertThat(tree.canWrite(path("foo/subdir")), is(true));
+        assertThat(tree.canWrite(path("food")), is(false));
         assertThat(tree.canRead(path("foo")), is(true));
+        assertThat(tree.canRead(path("food")), is(false));
 
         assertThat(tree.canWrite(path("before")), is(false));
         assertThat(tree.canWrite(path("later")), is(false));
@@ -83,6 +88,22 @@ public void testNormalizePath() {
         assertThat(tree.canRead(path("")), is(false));
     }
 
+    public void testForwardSlashes() {
+        String sep = getDefaultFileSystem().getSeparator();
+        var tree = FileAccessTree.of(List.of(entitlement("a/b", "read"), entitlement("m" + sep + "n", "read")));
+
+        // Native separators work
+        assertThat(tree.canRead(path("a" + sep + "b")), is(true));
+        assertThat(tree.canRead(path("m" + sep + "n")), is(true));
+
+        // Forward slashes also work
+        assertThat(tree.canRead(path("a/b")), is(true));
+        assertThat(tree.canRead(path("m/n")), is(true));
+
+        // In case the native separator is a backslash, don't treat that as an escape
+        assertThat(tree.canRead(path("m\n")), is(false));
+    }
+
     FileEntitlement entitlement(String path, String mode) {
         Path p = path(path);
         return FileEntitlement.create(p.toString(), mode);