Rethrow NoSuchFileException if encountering an invalid symlink when checking file entitlements

libs/entitlement/bridge/src/main/java/org/elasticsearch/entitlement/bridge/EntitlementChecker.java

@@ -64,6 +64,7 @@
 import java.nio.file.FileVisitOption;
 import java.nio.file.FileVisitor;
 import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.OpenOption;
 import java.nio.file.Path;
 import java.nio.file.WatchEvent;
@@ -1203,7 +1204,7 @@ void checkNewByteChannel(
     void checkType(Class<?> callerClass, FileStore that);
 
     // path
-    void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options);
+    void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) throws NoSuchFileException;
 
     void checkPathRegister(Class<?> callerClass, Path that, WatchService watcher, WatchEvent.Kind<?>... events);
 

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

@@ -63,7 +63,11 @@ public static Path createTempDirectoryForWrite() throws IOException {
     }
 
     public static Path createTempSymbolicLink() throws IOException {
-        return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir());
+        return createTempSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir());
+    }
+
+    public static Path createTempSymbolicLink(Path link, Path target) throws IOException {
+        return Files.createSymbolicLink(link, target);
     }
 
     public static Path createK8sLikeMount() throws IOException {

libs/entitlement/qa/entitlement-test-plugin/build.gradle

@@ -24,6 +24,7 @@ dependencies {
   compileOnly project(':server')
   compileOnly project(':libs:logging')
   compileOnly project(":libs:entitlement:qa:entitled-plugin")
+  implementation project(":libs:entitlement")
 }
 
 tasks.named("javadoc").configure {

libs/entitlement/qa/entitlement-test-plugin/src/main/java/module-info.java

@@ -11,6 +11,7 @@
     requires org.elasticsearch.server;
     requires org.elasticsearch.base;
     requires org.elasticsearch.logging;
+    requires org.elasticsearch.entitlement;
     requires org.elasticsearch.entitlement.qa.entitled;
 
     // Modules we'll attempt to use in order to exercise entitlements

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

@@ -9,6 +9,8 @@
 
 package org.elasticsearch.entitlement.qa.test;
 
+import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
+
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -27,5 +29,7 @@ enum ExpectedAccess {
 
     ExpectedAccess expectedAccess();
 
+    Class<? extends Exception> expectedExceptionIfDenied() default NotEntitledException.class;
+
     int fromJavaVersion() default -1;
 }

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

@@ -62,6 +62,10 @@ static Path readFile() {
         return testRootDir.resolve("read_file");
     }
 
+    static Path readFileAlwaysAllowed() {
+        return readDir().resolve("always_allowed");
+    }
+
     static Path readWriteFile() {
         return testRootDir.resolve("read_write_file");
     }

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

@@ -14,8 +14,10 @@
 import java.io.IOException;
 import java.nio.file.FileSystems;
 import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.WatchEvent;
 
+import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.ALWAYS_DENIED;
 import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.PLUGINS;
 
 @SuppressWarnings({ "unused" /* called via reflection */, "rawtypes" })
@@ -26,6 +28,12 @@ static void checkToRealPath() throws IOException {
         FileCheckActions.readFile().toRealPath();
     }
 
+    @EntitlementTest(expectedAccess = ALWAYS_DENIED, expectedExceptionIfDenied = NoSuchFileException.class)
+    static void checkToRealPathForInvalidTarget() throws IOException {
+        EntitledActions.createTempSymbolicLink(FileCheckActions.readFileAlwaysAllowed(), FileCheckActions.readDir().resolve("invalid"))
+            .toRealPath();
+    }
+
     @EntitlementTest(expectedAccess = PLUGINS)
     static void checkToRealPathWithK8sLikeMount() throws IOException, Exception {
         EntitledActions.createK8sLikeMount().toRealPath();

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

@@ -13,6 +13,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.core.CheckedRunnable;
 import org.elasticsearch.core.SuppressForbidden;
+import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
 import org.elasticsearch.logging.LogManager;
 import org.elasticsearch.logging.Logger;
 import org.elasticsearch.rest.BaseRestHandler;
@@ -68,20 +69,25 @@
 public class RestEntitlementsCheckAction extends BaseRestHandler {
     private static final Logger logger = LogManager.getLogger(RestEntitlementsCheckAction.class);
 
-    record CheckAction(CheckedRunnable<Exception> action, EntitlementTest.ExpectedAccess expectedAccess, Integer fromJavaVersion) {
+    record CheckAction(
+        CheckedRunnable<Exception> action,
+        EntitlementTest.ExpectedAccess expectedAccess,
+        Class<? extends Exception> expectedExceptionIfDenied,
+        Integer fromJavaVersion
+    ) {
         /**
          * These cannot be granted to plugins, so our test plugins cannot test the "allowed" case.
          */
         static CheckAction deniedToPlugins(CheckedRunnable<Exception> action) {
-            return new CheckAction(action, SERVER_ONLY, null);
+            return new CheckAction(action, SERVER_ONLY, NotEntitledException.class, null);
         }
 
         static CheckAction forPlugins(CheckedRunnable<Exception> action) {
-            return new CheckAction(action, PLUGINS, null);
+            return new CheckAction(action, PLUGINS, NotEntitledException.class, null);
         }
 
         static CheckAction alwaysDenied(CheckedRunnable<Exception> action) {
-            return new CheckAction(action, ALWAYS_DENIED, null);
+            return new CheckAction(action, ALWAYS_DENIED, NotEntitledException.class, null);
         }
     }
 
@@ -128,7 +134,12 @@ static CheckAction alwaysDenied(CheckedRunnable<Exception> action) {
             entry("responseCache_setDefault", alwaysDenied(RestEntitlementsCheckAction::setDefaultResponseCache)),
             entry(
                 "createInetAddressResolverProvider",
-                new CheckAction(VersionSpecificNetworkChecks::createInetAddressResolverProvider, SERVER_ONLY, 18)
+                new CheckAction(
+                    VersionSpecificNetworkChecks::createInetAddressResolverProvider,
+                    SERVER_ONLY,
+                    NotEntitledException.class,
+                    18
+                )
             ),
             entry("createURLStreamHandlerProvider", alwaysDenied(RestEntitlementsCheckAction::createURLStreamHandlerProvider)),
             entry("createURLWithURLStreamHandler", alwaysDenied(RestEntitlementsCheckAction::createURLWithURLStreamHandler)),
@@ -237,7 +248,12 @@ private static Stream<Entry<String, CheckAction>> getTestEntries(Class<?> action
                 }
             };
             Integer fromJavaVersion = testAnnotation.fromJavaVersion() == -1 ? null : testAnnotation.fromJavaVersion();
-            entries.add(entry(method.getName(), new CheckAction(runnable, testAnnotation.expectedAccess(), fromJavaVersion)));
+            entries.add(
+                entry(
+                    method.getName(),
+                    new CheckAction(runnable, testAnnotation.expectedAccess(), testAnnotation.expectedExceptionIfDenied(), fromJavaVersion)
+                )
+            );
         }
         return entries.stream();
     }
@@ -436,8 +452,17 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
         }
 
         return channel -> {
-            logger.info("Calling check action [{}]", actionName);
-            checkAction.action().run();
+            logger.info("Calling check action [{}]", checkAction);
+            try {
+                checkAction.action().run();
+            } catch (Exception e) {
+                if (checkAction.expectedExceptionIfDenied.isInstance(e) == false) {
+                    throw e;
+                }
+                logger.debug("Check action [{}] denied with expected exception", actionName, e);
+                channel.sendResponse(new RestResponse(channel, RestStatus.FORBIDDEN, e));
+                return;
+            }
             logger.debug("Check action [{}] returned", actionName);
             channel.sendResponse(new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName)));
         };

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

@@ -11,18 +11,25 @@
 
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.Response;
+import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.entitlement.qa.EntitlementsTestRule.PolicyBuilder;
 import org.elasticsearch.test.rest.ESRestTestCase;
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.TypeSafeMatcher;
 
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 
-import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 
 public abstract class AbstractEntitlementsIT extends ESRestTestCase {
 
+    static final PolicyBuilder ALWAYS_ALLOWED_TEST_ENTITLEMENTS = (builder, tempDir) -> builder.value(
+        Map.of("files", List.of(Map.of("path", tempDir.resolve("read_dir").resolve("always_allowed"), "mode", "read")))
+    );
+
     static final PolicyBuilder ALLOWED_TEST_ENTITLEMENTS = (builder, tempDir) -> {
         builder.value("create_class_loader");
         builder.value("set_https_connection_properties");
@@ -69,8 +76,28 @@ public void testAction() throws IOException {
             Response result = executeCheck();
             assertThat(result.getStatusLine().getStatusCode(), equalTo(200));
         } else {
-            var exception = expectThrows(IOException.class, this::executeCheck);
-            assertThat(exception.getMessage(), containsString("not_entitled_exception"));
+            var exception = expectThrows(ResponseException.class, this::executeCheck);
+            assertThat(exception, FORBIDDEN_STATUS_CODE_MATCHER);
         }
     }
+
+    private static final Matcher<ResponseException> FORBIDDEN_STATUS_CODE_MATCHER = new TypeSafeMatcher<>() {
+        @Override
+        protected boolean matchesSafely(ResponseException item) {
+            return item.getResponse().getStatusLine().getStatusCode() == 403;
+        }
+
+        @Override
+        public void describeTo(Description description) {
+            description.appendText("403 (FORBIDDEN)");
+        }
+
+        @Override
+        protected void describeMismatchSafely(ResponseException item, Description description) {
+            description.appendText("was ")
+                .appendValue(item.getResponse().getStatusLine().getStatusCode())
+                .appendText("\n")
+                .appendValue(item.getMessage());
+        }
+    };
 }

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

@@ -18,7 +18,7 @@
 public class EntitlementsDeniedIT extends AbstractEntitlementsIT {
 
     @ClassRule
-    public static EntitlementsTestRule testRule = new EntitlementsTestRule(true, null);
+    public static EntitlementsTestRule testRule = new EntitlementsTestRule(true, ALWAYS_ALLOWED_TEST_ENTITLEMENTS);
 
     public EntitlementsDeniedIT(@Name("actionName") String actionName) {
         super(actionName, false);

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

@@ -18,7 +18,7 @@
 public class EntitlementsDeniedNonModularIT extends AbstractEntitlementsIT {
 
     @ClassRule
-    public static EntitlementsTestRule testRule = new EntitlementsTestRule(false, null);
+    public static EntitlementsTestRule testRule = new EntitlementsTestRule(false, ALWAYS_ALLOWED_TEST_ENTITLEMENTS);
 
     public EntitlementsDeniedNonModularIT(@Name("actionName") String actionName) {
         super(actionName, false);

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

@@ -74,6 +74,7 @@
 import java.nio.file.FileVisitOption;
 import java.nio.file.FileVisitor;
 import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.OpenOption;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -2744,7 +2745,7 @@ public void checkType(Class<?> callerClass, FileStore that) {
     }
 
     @Override
-    public void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) {
+    public void checkPathToRealPath(Class<?> callerClass, Path that, LinkOption... options) throws NoSuchFileException {
         boolean followLinks = true;
         for (LinkOption option : options) {
             if (option == LinkOption.NOFOLLOW_LINKS) {

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

@@ -35,6 +35,7 @@
 import java.lang.StackWalker.StackFrame;
 import java.lang.module.ModuleFinder;
 import java.lang.module.ModuleReference;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
@@ -326,10 +327,17 @@ private static boolean isPathOnDefaultFilesystem(Path path) {
     }
 
     public void checkFileRead(Class<?> callerClass, Path path) {
-        checkFileRead(callerClass, path, false);
+        try {
+            checkFileRead(callerClass, path, false);
+        } catch (NoSuchFileException e) {
+            assert false : "NoSuchFileException should only be thrown when following links";
+            var notEntitledException = new NotEntitledException(e.getMessage());
+            notEntitledException.addSuppressed(e);
+            throw notEntitledException;
+        }
     }
 
-    public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks) {
+    public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks) throws NoSuchFileException {
         if (isPathOnDefaultFilesystem(path) == false) {
             return;
         }
@@ -345,11 +353,13 @@ public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks)
         if (canRead && followLinks) {
             try {
                 realPath = path.toRealPath();
+                if (realPath.equals(path) == false) {
+                    canRead = entitlements.fileAccess().canRead(realPath);
+                }
+            } catch (NoSuchFileException e) {
+                throw e; // rethrow
             } catch (IOException e) {
-                // target not found or other IO error
-            }
-            if (realPath != null && realPath.equals(path) == false) {
-                canRead = entitlements.fileAccess().canRead(realPath);
+                canRead = false;
             }
         }