From 956a9f7e1963002a1b943718919e28117b4e34ab Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 21 Feb 2025 10:07:00 -0800 Subject: [PATCH 1/4] prune extraneous paths to prevent incorrect binary search --- .../runtime/policy/FileAccessTree.java | 20 +++++++++++++++++-- .../runtime/policy/FileAccessTreeTests.java | 18 +++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java index 98076af51ae60..8da54591c4c19 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java @@ -49,8 +49,24 @@ private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup) readPaths.sort(String::compareTo); writePaths.sort(String::compareTo); - this.readPaths = readPaths.toArray(new String[0]); - this.writePaths = writePaths.toArray(new String[0]); + this.readPaths = pruneSortedPaths(readPaths).toArray(new String[0]); + this.writePaths = pruneSortedPaths(writePaths).toArray(new String[0]); + } + + public static List pruneSortedPaths(List paths) { + List prunedReadPaths = new ArrayList<>(); + if (paths.isEmpty() == false) { + String currentPath = paths.get(0); + prunedReadPaths.add(currentPath); + for (int i = 1; i < paths.size(); ++i) { + String nextPath = paths.get(i); + if (nextPath.startsWith(currentPath) == false) { + prunedReadPaths.add(nextPath); + currentPath = nextPath; + } + } + } + return prunedReadPaths; } public static FileAccessTree of(FilesEntitlement filesEntitlement, PathLookup pathLookup) { diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java index 218fc0c956723..bea34204b4980 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java @@ -96,6 +96,24 @@ public void testReadWriteUnderRead() { assertThat(tree.canWrite(path("foo/bar")), is(true)); } + public void testPrunedPaths() { + var tree = accessTree(entitlement("foo", "read", "foo/baz", "read", "foo/bar", "read")); + assertThat(tree.canRead(path("foo")), is(true)); + assertThat(tree.canWrite(path("foo")), is(false)); + assertThat(tree.canRead(path("foo/bar")), is(true)); + assertThat(tree.canWrite(path("foo/bar")), is(false)); + assertThat(tree.canRead(path("foo/baz")), is(true)); + assertThat(tree.canWrite(path("foo/baz")), is(false)); + + tree = accessTree(entitlement("foo", "read", "foo/bar", "read_write")); + assertThat(tree.canRead(path("foo")), is(true)); + assertThat(tree.canWrite(path("foo")), is(false)); + assertThat(tree.canRead(path("foo/bar")), is(true)); + assertThat(tree.canWrite(path("foo/bar")), is(true)); + assertThat(tree.canRead(path("foo/baz")), is(true)); + assertThat(tree.canWrite(path("foo/baz")), is(false)); + } + public void testReadWithRelativePath() { for (var dir : List.of("config", "home")) { var tree = accessTree(entitlement(Map.of("relative_path", "foo", "mode", "read", "relative_to", dir))); From 90fcd728c3b5a9ab584bca056c54b0128f4793f8 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 21 Feb 2025 10:11:41 -0800 Subject: [PATCH 2/4] Update docs/changelog/123177.yaml --- docs/changelog/123177.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/123177.yaml diff --git a/docs/changelog/123177.yaml b/docs/changelog/123177.yaml new file mode 100644 index 0000000000000..3d1fb9f824fd5 --- /dev/null +++ b/docs/changelog/123177.yaml @@ -0,0 +1,5 @@ +pr: 123177 +summary: Prune extraneous files entitlements paths to prevent incorrect binary search +area: Infra/Entitlements +type: bug +issues: [] From a9d476ffcc538b61542e00e95eee3e2e5c461c23 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Fri, 21 Feb 2025 10:43:48 -0800 Subject: [PATCH 3/4] Delete docs/changelog/123177.yaml --- docs/changelog/123177.yaml | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 docs/changelog/123177.yaml diff --git a/docs/changelog/123177.yaml b/docs/changelog/123177.yaml deleted file mode 100644 index 3d1fb9f824fd5..0000000000000 --- a/docs/changelog/123177.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 123177 -summary: Prune extraneous files entitlements paths to prevent incorrect binary search -area: Infra/Entitlements -type: bug -issues: [] From 9ea1587af6f16c303901b98a4f3cc8d66a51c1b6 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 21 Feb 2025 16:32:34 -0800 Subject: [PATCH 4/4] address comments --- .../entitlement/runtime/policy/FileAccessTree.java | 2 +- .../entitlement/runtime/policy/FileAccessTreeTests.java | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java index 8da54591c4c19..660459f06d58b 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java @@ -53,7 +53,7 @@ private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup) this.writePaths = pruneSortedPaths(writePaths).toArray(new String[0]); } - public static List pruneSortedPaths(List paths) { + private static List pruneSortedPaths(List paths) { List prunedReadPaths = new ArrayList<>(); if (paths.isEmpty() == false) { String currentPath = paths.get(0); diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java index bea34204b4980..4eb3620c276ff 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java @@ -104,6 +104,9 @@ public void testPrunedPaths() { assertThat(tree.canWrite(path("foo/bar")), is(false)); assertThat(tree.canRead(path("foo/baz")), is(true)); assertThat(tree.canWrite(path("foo/baz")), is(false)); + // also test a non-existent subpath + assertThat(tree.canRead(path("foo/barf")), is(true)); + assertThat(tree.canWrite(path("foo/barf")), is(false)); tree = accessTree(entitlement("foo", "read", "foo/bar", "read_write")); assertThat(tree.canRead(path("foo")), is(true));