Fix entitlement checks for relative links (elastic#124133) (elastic#124480)

Author: mosche

libs/entitlement/qa/entitled-plugin/src/main/java/org/elasticsearch/entitlement/qa/entitled/EntitledActions.java

@@ -66,6 +66,19 @@ public static Path createTempSymbolicLink() throws IOException {
         return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir());
     }
 
+    public static Path createK8sLikeMount() throws IOException {
+        Path baseDir = readDir().resolve("k8s");
+        var versionedDir = Files.createDirectories(baseDir.resolve("..version"));
+        var actualFileMount = Files.createFile(versionedDir.resolve("mount-" + random.nextLong() + ".tmp"));
+
+        var dataDir = Files.createSymbolicLink(baseDir.resolve("..data"), versionedDir.getFileName());
+        // mount-0.tmp -> ..data/mount-0.tmp -> ..version/mount-0.tmp
+        return Files.createSymbolicLink(
+            baseDir.resolve(actualFileMount.getFileName()),
+            dataDir.getFileName().resolve(actualFileMount.getFileName())
+        );
+    }
+
     public static URLConnection createHttpURLConnection() throws IOException {
         return URI.create("http://127.0.0.1:12345/").toURL().openConnection();
     }

libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/NioFilesActions.java

@@ -140,6 +140,16 @@ static void checkFilesCreateSymbolicLink() throws IOException {
         }
     }
 
+    @EntitlementTest(expectedAccess = PLUGINS)
+    static void checkFilesCreateRelativeSymbolicLink() throws IOException {
+        var directory = EntitledActions.createTempDirectoryForWrite();
+        try {
+            Files.createSymbolicLink(directory.resolve("link"), Path.of("target"));
+        } catch (UnsupportedOperationException | FileSystemException e) {
+            // OK not to implement symbolic link in the filesystem
+        }
+    }
+
     @EntitlementTest(expectedAccess = PLUGINS)
     static void checkFilesCreateLink() throws IOException {
         var directory = EntitledActions.createTempDirectoryForWrite();
@@ -150,6 +160,17 @@ static void checkFilesCreateLink() throws IOException {
         }
     }
 
+    @EntitlementTest(expectedAccess = PLUGINS)
+    static void checkFilesCreateRelativeLink() throws IOException {
+        var directory = EntitledActions.createTempDirectoryForWrite();
+        var target = directory.resolve("target");
+        try {
+            Files.createLink(directory.resolve("link"), Path.of("target"));
+        } catch (UnsupportedOperationException | FileSystemException e) {
+            // OK not to implement symbolic link in the filesystem
+        }
+    }
+
     @EntitlementTest(expectedAccess = PLUGINS)
     static void checkFilesDelete() throws IOException {
         var file = EntitledActions.createTempFileForWrite();

libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/PathActions.java

@@ -9,6 +9,8 @@
 
 package org.elasticsearch.entitlement.qa.test;
 
+import org.elasticsearch.entitlement.qa.entitled.EntitledActions;
+
 import java.io.IOException;
 import java.nio.file.FileSystems;
 import java.nio.file.LinkOption;
@@ -24,6 +26,11 @@ static void checkToRealPath() throws IOException {
         FileCheckActions.readFile().toRealPath();
     }
 
+    @EntitlementTest(expectedAccess = PLUGINS)
+    static void checkToRealPathWithK8sLikeMount() throws IOException, Exception {
+        EntitledActions.createK8sLikeMount().toRealPath();
+    }
+
     @EntitlementTest(expectedAccess = PLUGINS)
     static void checkToRealPathNoFollow() throws IOException {
         FileCheckActions.readFile().toRealPath(LinkOption.NOFOLLOW_LINKS);

libs/entitlement/qa/src/javaRestTest/java/org/elasticsearch/entitlement/qa/EntitlementsTestRule.java

@@ -40,6 +40,7 @@ class EntitlementsTestRule implements TestRule {
                 "files",
                 List.of(
                     Map.of("path", tempDir.resolve("read_dir"), "mode", "read_write"),
+                    Map.of("path", tempDir.resolve("read_dir").resolve("k8s").resolve("..data"), "mode", "read", "exclusive", true),
                     Map.of("path", tempDir.resolve("read_write_dir"), "mode", "read_write"),
                     Map.of("path", tempDir.resolve("read_file"), "mode", "read"),
                     Map.of("path", tempDir.resolve("read_write_file"), "mode", "read_write")

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java

@@ -19,7 +19,6 @@
 import java.io.FileDescriptor;
 import java.io.FileFilter;
 import java.io.FilenameFilter;
-import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.PrintStream;
@@ -74,7 +73,6 @@
 import java.nio.file.FileStore;
 import java.nio.file.FileVisitOption;
 import java.nio.file.FileVisitor;
-import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.OpenOption;
 import java.nio.file.Path;
@@ -2050,16 +2048,21 @@ public void checkSelectorProviderInheritedChannel(Class<?> callerClass, Selector
         policyManager.checkCreateTempFile(callerClass);
     }
 
+    private static Path resolveLinkTarget(Path path, Path target) {
+        var parent = path.getParent();
+        return parent == null ? target : parent.resolve(target);
+    }
+
     @Override
     public void check$java_nio_file_Files$$createSymbolicLink(Class<?> callerClass, Path link, Path target, FileAttribute<?>... attrs) {
-        policyManager.checkFileRead(callerClass, target);
         policyManager.checkFileWrite(callerClass, link);
+        policyManager.checkFileRead(callerClass, resolveLinkTarget(link, target));
     }
 
     @Override
     public void check$java_nio_file_Files$$createLink(Class<?> callerClass, Path link, Path existing) {
-        policyManager.checkFileRead(callerClass, existing);
         policyManager.checkFileWrite(callerClass, link);
+        policyManager.checkFileRead(callerClass, resolveLinkTarget(link, existing));
     }
 
     @Override
@@ -2548,13 +2551,13 @@ public void checkCreateDirectory(Class<?> callerClass, FileSystemProvider that,
     @Override
     public void checkCreateSymbolicLink(Class<?> callerClass, FileSystemProvider that, Path link, Path target, FileAttribute<?>... attrs) {
         policyManager.checkFileWrite(callerClass, link);
-        policyManager.checkFileRead(callerClass, target);
+        policyManager.checkFileRead(callerClass, resolveLinkTarget(link, target));
     }
 
     @Override
     public void checkCreateLink(Class<?> callerClass, FileSystemProvider that, Path link, Path existing) {
         policyManager.checkFileWrite(callerClass, link);
-        policyManager.checkFileRead(callerClass, existing);
+        policyManager.checkFileRead(callerClass, resolveLinkTarget(link, existing));
     }
 
     @Override
@@ -2748,14 +2751,7 @@ public void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... o
                 followLinks = false;
             }
         }
-        if (followLinks) {
-            try {
-                policyManager.checkFileRead(callerClass, Files.readSymbolicLink(that));
-            } catch (IOException | UnsupportedOperationException e) {
-                // that is not a link, or unrelated IOException or unsupported
-            }
-        }
-        policyManager.checkFileRead(callerClass, that);
+        policyManager.checkFileRead(callerClass, that, followLinks);
     }
 
     @Override

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

@@ -31,6 +31,7 @@
 import org.elasticsearch.logging.Logger;
 
 import java.io.File;
+import java.io.IOException;
 import java.lang.StackWalker.StackFrame;
 import java.lang.module.ModuleFinder;
 import java.lang.module.ModuleReference;
@@ -325,6 +326,10 @@ private static boolean isPathOnDefaultFilesystem(Path path) {
     }
 
     public void checkFileRead(Class<?> callerClass, Path path) {
+        checkFileRead(callerClass, path, false);
+    }
+
+    public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks) {
         if (isPathOnDefaultFilesystem(path) == false) {
             return;
         }
@@ -334,14 +339,28 @@ public void checkFileRead(Class<?> callerClass, Path path) {
         }
 
         ModuleEntitlements entitlements = getEntitlements(requestingClass);
-        if (entitlements.fileAccess().canRead(path) == false) {
+
+        Path realPath = null;
+        boolean canRead = entitlements.fileAccess().canRead(path);
+        if (canRead && followLinks) {
+            try {
+                realPath = path.toRealPath();
+            } catch (IOException e) {
+                // target not found or other IO error
+            }
+            if (realPath != null && realPath.equals(path) == false) {
+                canRead = entitlements.fileAccess().canRead(realPath);
+            }
+        }
+
+        if (canRead == false) {
             notEntitled(
                 Strings.format(
                     "Not entitled: component [%s], module [%s], class [%s], entitlement [file], operation [read], path [%s]",
                     entitlements.componentName(),
                     requestingClass.getModule().getName(),
                     requestingClass,
-                    path
+                    realPath == null ? path : Strings.format("%s -> %s", path, realPath)
                 ),
                 callerClass
             );