From e3468945ce2c89f6233875cc53bcabea62c2d77b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Tue, 25 Feb 2025 13:39:58 +0100 Subject: [PATCH] [Entitlements] Follows links during FileAccessTree creation (#123357) --- .../runtime/policy/FileAccessTree.java | 48 +++++++++++++++---- .../runtime/policy/FileAccessTreeTests.java | 44 +++++++++++++++++ 2 files changed, 84 insertions(+), 8 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 d0eded74556b7..336a00643e979 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 @@ -10,18 +10,27 @@ package org.elasticsearch.entitlement.runtime.policy; import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement; +import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode; +import org.elasticsearch.logging.LogManager; +import org.elasticsearch.logging.Logger; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; import java.util.List; import java.util.Objects; +import java.util.function.BiConsumer; import static org.elasticsearch.core.PathUtils.getDefaultFileSystem; public final class FileAccessTree { + private static final Logger logger = LogManager.getLogger(FileAccessTree.class); private static final String FILE_SEPARATOR = getDefaultFileSystem().getSeparator(); private final String[] readPaths; @@ -30,6 +39,27 @@ public final class FileAccessTree { private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup) { List readPaths = new ArrayList<>(); List writePaths = new ArrayList<>(); + BiConsumer addPath = (path, mode) -> { + var normalized = normalizePath(path); + if (mode == Mode.READ_WRITE) { + writePaths.add(normalized); + } + readPaths.add(normalized); + }; + BiConsumer addPathAndMaybeLink = (path, mode) -> { + addPath.accept(path, mode); + // also try to follow symlinks. Lucene does this and writes to the target path. + if (Files.exists(path)) { + try { + Path realPath = path.toRealPath(); + if (realPath.equals(path) == false) { + addPath.accept(realPath, mode); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + }; for (FilesEntitlement.FileData fileData : filesEntitlement.filesData()) { var platform = fileData.platform(); if (platform != null && platform.isCurrent() == false) { @@ -38,18 +68,20 @@ private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup) var mode = fileData.mode(); var paths = fileData.resolvePaths(pathLookup); paths.forEach(path -> { - var normalized = normalizePath(path); - if (mode == FilesEntitlement.Mode.READ_WRITE) { - writePaths.add(normalized); + if (path == null) { + // TODO: null paths shouldn't be allowed, but they can occur due to repo paths + return; } - readPaths.add(normalized); + addPathAndMaybeLink.accept(path, mode); }); } - // everything has access to the temp dir - String tempDir = normalizePath(pathLookup.tempDir()); - readPaths.add(tempDir); - writePaths.add(tempDir); + // everything has access to the temp dir and the jdk + addPathAndMaybeLink.accept(pathLookup.tempDir(), Mode.READ_WRITE); + + // TODO: watcher uses javax.activation which looks for known mime types configuration, should this be global or explicit in watcher? + Path jdk = Paths.get(System.getProperty("java.home")); + addPathAndMaybeLink.accept(jdk.resolve("conf"), Mode.READ); readPaths.sort(PATH_ORDER); writePaths.sort(PATH_ORDER); 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 71ec497b9ec13..98fd98b75719e 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 @@ -10,11 +10,15 @@ package org.elasticsearch.entitlement.runtime.policy; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement; import org.elasticsearch.test.ESTestCase; import org.junit.BeforeClass; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -23,6 +27,7 @@ import static org.elasticsearch.core.PathUtils.getDefaultFileSystem; import static org.hamcrest.Matchers.is; +@ESTestCase.WithoutSecurityManager public class FileAccessTreeTests extends ESTestCase { static Path root; @@ -211,6 +216,45 @@ public void testForwardSlashes() { assertThat(tree.canRead(path("m/n")), is(true)); } + public void testJdkAccess() { + Path jdkDir = Paths.get(System.getProperty("java.home")); + var confDir = jdkDir.resolve("conf"); + var tree = accessTree(FilesEntitlement.EMPTY); + + assertThat(tree.canRead(confDir), is(true)); + assertThat(tree.canWrite(confDir), is(false)); + assertThat(tree.canRead(jdkDir), is(false)); + } + + @SuppressForbidden(reason = "don't care about the directory location in tests") + public void testFollowLinks() throws IOException { + Path baseSourceDir = Files.createTempDirectory("fileaccess_source"); + Path source1Dir = baseSourceDir.resolve("source1"); + Files.createDirectory(source1Dir); + Path source2Dir = baseSourceDir.resolve("source2"); + Files.createDirectory(source2Dir); + + Path baseTargetDir = Files.createTempDirectory("fileaccess_target"); + Path readTarget = baseTargetDir.resolve("read_link"); + Path writeTarget = baseTargetDir.resolve("write_link"); + Files.createSymbolicLink(readTarget, source1Dir); + Files.createSymbolicLink(writeTarget, source2Dir); + var tree = accessTree(entitlement(readTarget.toString(), "read", writeTarget.toString(), "read_write")); + + assertThat(tree.canRead(baseSourceDir), is(false)); + assertThat(tree.canRead(baseTargetDir), is(false)); + + assertThat(tree.canRead(readTarget), is(true)); + assertThat(tree.canWrite(readTarget), is(false)); + assertThat(tree.canRead(source1Dir), is(true)); + assertThat(tree.canWrite(source1Dir), is(false)); + + assertThat(tree.canRead(writeTarget), is(true)); + assertThat(tree.canWrite(writeTarget), is(true)); + assertThat(tree.canRead(source2Dir), is(true)); + assertThat(tree.canWrite(source2Dir), is(true)); + } + public void testTempDirAccess() { var tree = FileAccessTree.of(FilesEntitlement.EMPTY, TEST_PATH_LOOKUP); assertThat(tree.canRead(TEST_PATH_LOOKUP.tempDir()), is(true));