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,15 @@ public static Path createTempDirectoryForWrite() throws IOException {
     }
 
     public static Path createTempSymbolicLink() throws IOException {
-        return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), readWriteDir());
+        return createTempSymbolicLink(readWriteDir());
+    }
+
+    public static Path createTempSymbolicLink(Path target) throws IOException {
+        return Files.createSymbolicLink(readDir().resolve("entitlements-link-" + random.nextLong()), target);
+    }
+
+    public static Path pathToRealPath(Path path) throws IOException {
+        return path.toRealPath();
     }
 
     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/PathActions.java

@@ -10,12 +10,17 @@
 package org.elasticsearch.entitlement.qa.test;
 
 import org.elasticsearch.entitlement.qa.entitled.EntitledActions;
+import org.elasticsearch.entitlement.runtime.policy.PolicyManager;
 
 import java.io.IOException;
 import java.nio.file.FileSystems;
 import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
 import java.nio.file.WatchEvent;
+import java.util.Arrays;
 
+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 +31,18 @@ static void checkToRealPath() throws IOException {
         FileCheckActions.readFile().toRealPath();
     }
 
+    @EntitlementTest(expectedAccess = ALWAYS_DENIED, expectedExceptionIfDenied = NoSuchFileException.class)
+    static void checkToRealPathForInvalidTarget() throws IOException {
+        Path invalidLink = EntitledActions.createTempSymbolicLink(FileCheckActions.readDir().resolve("invalid"));
+        try {
+            EntitledActions.pathToRealPath(invalidLink); // throws NoSuchFileException when checking entitlements due to invalid target
+        } catch (NoSuchFileException e) {
+            assert Arrays.stream(e.getStackTrace()).anyMatch(t -> t.getClassName().equals(PolicyManager.class.getName()))
+                : "Expected NoSuchFileException to be thrown by entitlements check";
+            throw e;
+        }
+    }
+
     @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();
     }
@@ -437,9 +453,19 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
 
         return channel -> {
             logger.info("Calling check action [{}]", actionName);
-            checkAction.action().run();
-            logger.debug("Check action [{}] returned", actionName);
-            channel.sendResponse(new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName)));
+            RestResponse response;
+            try {
+                checkAction.action().run();
+                response = new RestResponse(RestStatus.OK, Strings.format("Succesfully executed action [%s]", actionName));
+            } catch (Exception e) {
+                var statusCode = checkAction.expectedExceptionIfDenied.isInstance(e)
+                    ? RestStatus.FORBIDDEN
+                    : RestStatus.INTERNAL_SERVER_ERROR;
+                response = new RestResponse(channel, statusCode, e);
+                response.addHeader("expectedException", checkAction.expectedExceptionIfDenied.getName());
+            }
+            logger.debug("Check action [{}] returned status [{}]", actionName, response.status().getStatus());
+            channel.sendResponse(response);
         };
     }
 

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

@@ -11,14 +11,17 @@
 
 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 {
@@ -69,8 +72,34 @@ 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, statusCodeMatcher(403));
         }
     }
+
+    private static Matcher<ResponseException> statusCodeMatcher(int statusCode) {
+        return new TypeSafeMatcher<>() {
+            String expectedException = null;
+
+            @Override
+            protected boolean matchesSafely(ResponseException item) {
+                Response resp = item.getResponse();
+                expectedException = resp.getHeader("expectedException");
+                return resp.getStatusLine().getStatusCode() == statusCode && expectedException != null;
+            }
+
+            @Override
+            public void describeTo(Description description) {
+                description.appendValue(statusCode).appendText(" due to ").appendText(expectedException);
+            }
+
+            @Override
+            protected void describeMismatchSafely(ResponseException item, Description description) {
+                description.appendText("was ")
+                    .appendValue(item.getResponse().getStatusLine().getStatusCode())
+                    .appendText("\n")
+                    .appendValue(item.getMessage());
+            }
+        };
+    }
 }

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;
             }
         }