Include denial reason info in checkFileRead

Author: prdoyle

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

@@ -227,28 +227,29 @@ public void checkFileRead(Class<?> callerClass, Path path, boolean followLinks)
         ModuleEntitlements entitlements = policyManager.getEntitlements(requestingClass);
 
         Path realPath = null;
-        boolean canRead = entitlements.fileAccess().canRead(path);
-        if (canRead && followLinks) {
+        String denialReason = entitlements.fileAccess().canRead(path)? null : "Forbidden by policies";
+        if (denialReason == null && followLinks) {
             try {
                 realPath = path.toRealPath();
                 if (realPath.equals(path) == false) {
-                    canRead = entitlements.fileAccess().canRead(realPath);
+                    denialReason = entitlements.fileAccess().canRead(realPath)? null : "Symlink target forbidden by policies";
                 }
             } catch (NoSuchFileException e) {
                 throw e; // rethrow
             } catch (IOException e) {
-                canRead = false;
+                denialReason = e.toString();
             }
         }
 
-        if (canRead == false) {
+        if (denialReason != null) {
             notEntitled(
                 Strings.format(
-                    "component [%s], module [%s], class [%s], entitlement [file], operation [read], path [%s]",
+                    "component [%s], module [%s], class [%s], entitlement [file], operation [read], path [%s], reason [%s]",
                     entitlements.componentName(),
                     entitlements.moduleName(),
                     requestingClass,
-                    realPath == null ? path : Strings.format("%s -> %s", path, realPath)
+                    realPath == null ? path : Strings.format("%s -> %s", path, realPath),
+                    denialReason
                 ),
                 requestingClass,
                 entitlements

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyCheckerImplTests.java

@@ -9,15 +9,31 @@
 
 package org.elasticsearch.entitlement.runtime.policy;
 
+import org.elasticsearch.entitlement.runtime.api.NotEntitledException;
+import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement;
+import org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.FileData;
 import org.elasticsearch.test.ESTestCase;
+import org.hamcrest.text.MatchesPattern;
 
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
+import static java.nio.file.LinkOption.NOFOLLOW_LINKS;
 import static org.elasticsearch.entitlement.runtime.policy.PolicyManagerTests.NO_ENTITLEMENTS_MODULE;
 import static org.elasticsearch.entitlement.runtime.policy.PolicyManagerTests.TEST_PATH_LOOKUP;
+import static org.elasticsearch.entitlement.runtime.policy.PolicyManagerTests.createEmptyTestServerPolicy;
 import static org.elasticsearch.entitlement.runtime.policy.PolicyManagerTests.makeClassInItsOwnModule;
+import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.text.MatchesPattern.matchesPattern;
 
 public class PolicyCheckerImplTests extends ESTestCase {
     public void testRequestingClassFastPath() throws IOException, ClassNotFoundException {
@@ -54,8 +70,68 @@ public void testRequestingModuleWithStackWalk() throws IOException, ClassNotFoun
         );
     }
 
+    /**
+     * Set up a situation where the file read entitlement check encounters
+     * a strange {@link IOException} that is not {@link NoSuchFileException}.
+     * Ensure that the resulting {@link NotEntitledException} makes
+     * some effort to indicate the nature of the problem.
+     */
+    public void testIOExceptionFollowingSymlink() throws IOException {
+        Path dir = createTempDir();
+        Path symlink = dir.resolve("symlink");
+        Path allegedDir = dir.resolve("not_a_dir");
+        Path target = allegedDir.resolve("target");
+        Files.createFile(allegedDir); // Not a dir!
+        Files.createSymbolicLink(symlink, target);
+
+        PathLookup testPathLookup = new PathLookup() {
+            @Override
+            public Path pidFile() {
+                throw new UnsupportedOperationException();
+            }
+
+            @Override
+            public Stream<Path> getBaseDirPaths(BaseDir baseDir) {
+                return Stream.empty();
+            }
+
+            @Override
+            public Stream<Path> resolveSettingPaths(BaseDir baseDir, String settingName) {
+                throw new UnsupportedOperationException();
+            }
+
+            @Override
+            public boolean isPathOnDefaultFilesystem(Path path) {
+                // We need this to be on the default filesystem or it will be trivially allowed
+                return true;
+            }
+        };
+
+        var policyManager = new TestPolicyManager(
+            createEmptyTestServerPolicy(),
+            List.of(),
+            Map.of("testComponent", new Policy("testPolicy", List.of(new Scope("testModule", List.of(new FilesEntitlement(List.of(FileData.ofPath(symlink, READ)))))))),
+            c-> PolicyManager.PolicyScope.plugin("testComponent", "testModule"),
+            testPathLookup,
+            List.of(),
+            List.of()
+        );
+        policyManager.setActive(true);
+        policyManager.setTriviallyAllowingTestCode(false);
+
+        var checker = checker(NO_ENTITLEMENTS_MODULE, policyManager, testPathLookup);
+        var exception = assertThrows(NotEntitledException.class, ()->checker.checkFileRead(getClass(), symlink, true));
+        assertThat("The reason should be an exception of some sort",
+            exception.getMessage(), matchesPattern(".*reason.*Exception.*"));
+    }
+
     private static PolicyCheckerImpl checker(Module entitlementsModule) {
-        return new PolicyCheckerImpl(Set.of(), entitlementsModule, null, TEST_PATH_LOOKUP);
+        // TODO: TEST_PATH_LOOKUP is always null at this point!
+        return checker(entitlementsModule, null, TEST_PATH_LOOKUP);
+    }
+
+    private static PolicyCheckerImpl checker(Module entitlementsModule, PolicyManager policyManager, PathLookup testPathLookup) {
+        return new PolicyCheckerImpl(Set.of(), entitlementsModule, policyManager, testPathLookup);
     }
 
 }