Add data.path fast path for FilePermission (#61302)

The recursive data.path FilePermission check is an extremely hot
codepath in Elasticsearch. Unfortunately the FilePermission check in
Java is extremely allocation heavy. As it iterates through different
file permissions, it allocates byte arrays for each Path component that
must be compared. This PR improves the situation by adding the recursive
data.path FilePermission it its own PermissionsCollection object which
is checked first.

plugins/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/TikaImpl.java

@@ -150,8 +150,8 @@ final class TikaImpl {
                 addReadPermissions(perms, set);
             }
             // jvm's java.io.tmpdir (needs read/write)
-            FilePermissionUtils.addDirectoryPath(perms, "java.io.tmpdir",
-                PathUtils.get(System.getProperty("java.io.tmpdir")), "read,readlink,write,delete");
+            FilePermissionUtils.addDirectoryPath(perms, "java.io.tmpdir", PathUtils.get(System.getProperty("java.io.tmpdir")),
+                "read,readlink,write,delete", false);
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
@@ -181,7 +181,7 @@ final class TikaImpl {
             for (URL url : resources) {
                 Path path = PathUtils.get(url.toURI());
                 if (Files.isDirectory(path)) {
-                    FilePermissionUtils.addDirectoryPath(perms, "class.path", path, "read,readlink");
+                    FilePermissionUtils.addDirectoryPath(perms, "class.path", path, "read,readlink", false);
                 } else {
                     FilePermissionUtils.addSingleFilePath(perms, path, "read,readlink");
                 }

qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/ESPolicyUnitTests.java

@@ -52,7 +52,7 @@ public class ESPolicyUnitTests extends ESTestCase {
         Permission all = new AllPermission();
         PermissionCollection allCollection = all.newPermissionCollection();
         allCollection.add(all);
-        ESPolicy policy = new ESPolicy(Collections.emptyMap(), allCollection, Collections.emptyMap(), true);
+        ESPolicy policy = new ESPolicy(Collections.emptyMap(), allCollection, Collections.emptyMap(), true, new Permissions());
         // restrict ourselves to NoPermission
         PermissionCollection noPermissions = new Permissions();
         assertFalse(policy.implies(new ProtectionDomain(null, noPermissions), new FilePermission("foo", "read")));
@@ -67,7 +67,7 @@ public class ESPolicyUnitTests extends ESTestCase {
     public void testNullLocation() throws Exception {
         assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         PermissionCollection noPermissions = new Permissions();
-        ESPolicy policy = new ESPolicy(Collections.emptyMap(), noPermissions, Collections.emptyMap(), true);
+        ESPolicy policy = new ESPolicy(Collections.emptyMap(), noPermissions, Collections.emptyMap(), true, new Permissions());
         assertFalse(policy.implies(new ProtectionDomain(new CodeSource(null, (Certificate[]) null), noPermissions),
                 new FilePermission("foo", "read")));
     }
@@ -75,11 +75,22 @@ public class ESPolicyUnitTests extends ESTestCase {
     public void testListen() {
         assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         final PermissionCollection noPermissions = new Permissions();
-        final ESPolicy policy = new ESPolicy(Collections.emptyMap(), noPermissions, Collections.emptyMap(), true);
+        final ESPolicy policy = new ESPolicy(Collections.emptyMap(), noPermissions, Collections.emptyMap(), true, new Permissions());
         assertFalse(
             policy.implies(
                 new ProtectionDomain(ESPolicyUnitTests.class.getProtectionDomain().getCodeSource(), noPermissions),
                 new SocketPermission("localhost:" + randomFrom(0, randomIntBetween(49152, 65535)), "listen")));
     }
 
+    @SuppressForbidden(reason = "to create FilePermission object")
+    public void testDataPathPermissionIsChecked() {
+        assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
+        final PermissionCollection dataPathPermission = new Permissions();
+        dataPathPermission.add(new FilePermission("/home/elasticsearch/data/-", "read"));
+        final ESPolicy policy = new ESPolicy(Collections.emptyMap(), new Permissions(), Collections.emptyMap(), true, dataPathPermission);
+        assertTrue(
+            policy.implies(
+                    new ProtectionDomain(new CodeSource(null, (Certificate[]) null), new Permissions()),
+                    new FilePermission("/home/elasticsearch/data/index/file.si", "read")));
+    }
 }

qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java

@@ -216,7 +216,7 @@ public class EvilSecurityTests extends ESTestCase {
             assumeNoException("test cannot create symbolic links with security manager enabled", e);
         }
         Permissions permissions = new Permissions();
-        FilePermissionUtils.addDirectoryPath(permissions, "testing", link, "read");
+        FilePermissionUtils.addDirectoryPath(permissions, "testing", link, "read", false);
         assertExactPermissions(new FilePermission(link.toString(), "read"), permissions);
         assertExactPermissions(new FilePermission(link.resolve("foo").toString(), "read"), permissions);
         assertExactPermissions(new FilePermission(target.toString(), "read"), permissions);

server/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java

@@ -47,10 +47,13 @@ final class ESPolicy extends Policy {
     final Policy untrusted;
     final Policy system;
     final PermissionCollection dynamic;
+    final PermissionCollection dataPathPermission;
     final Map<String,Policy> plugins;
 
-    ESPolicy(Map<String, URL> codebases, PermissionCollection dynamic, Map<String,Policy> plugins, boolean filterBadDefaults) {
+    ESPolicy(Map<String, URL> codebases, PermissionCollection dynamic, Map<String,Policy> plugins, boolean filterBadDefaults,
+             PermissionCollection dataPathPermission) {
         this.template = Security.readPolicy(getClass().getResource(POLICY_RESOURCE), codebases);
+        this.dataPathPermission = dataPathPermission;
         this.untrusted = Security.readPolicy(getClass().getResource(UNTRUSTED_RESOURCE), Collections.emptyMap());
         if (filterBadDefaults) {
             this.system = new SystemPolicy(Policy.getPolicy());
@@ -98,6 +101,12 @@ final class ESPolicy extends Policy {
             }
         }
 
+        // The FilePermission to check access to the path.data is the hottest permission check in
+        // Elasticsearch, so we check it first.
+        if (permission instanceof FilePermission && dataPathPermission.implies(permission)) {
+            return true;
+        }
+
         // otherwise defer to template + dynamic file permissions
         return template.implies(domain, permission) || dynamic.implies(permission) || system.implies(domain, permission);
     }

server/src/main/java/org/elasticsearch/bootstrap/FilePermissionUtils.java

@@ -54,15 +54,17 @@ public class FilePermissionUtils {
     }
 
     /**
-     * Add access to path (and all files underneath it); this also creates the directory if it does not exist.
+     * Add access to path with direct and/or recursive access. This also creates the directory if it does not exist.
      *
      * @param policy            current policy to add permissions to
      * @param configurationName the configuration name associated with the path (for error messages only)
      * @param path              the path itself
      * @param permissions       set of file permissions to grant to the path
+     * @param recursiveAccessOnly   indicates if the permission should provide recursive access to files underneath
      */
     @SuppressForbidden(reason = "only place where creating Java-9 compatible FilePermission objects is possible")
-    public static void addDirectoryPath(Permissions policy, String configurationName, Path path, String permissions) throws IOException {
+    public static void addDirectoryPath(Permissions policy, String configurationName, Path path, String permissions,
+                                        boolean recursiveAccessOnly) throws IOException {
         // paths may not exist yet, this also checks accessibility
         try {
             Security.ensureDirectoryExists(path);
@@ -70,8 +72,12 @@ public class FilePermissionUtils {
             throw new IllegalStateException("Unable to access '" + configurationName + "' (" + path + ")", e);
         }
 
-        // add each path twice: once for itself, again for files underneath it
-        policy.add(new FilePermission(path.toString(), permissions));
+        // For some file permissions (data.path) we create a Permissions object that only checks the concrete
+        // path. Adding the directory would only create more overhead for this fast path.
+        if (recursiveAccessOnly == false) {
+            // add access for path itself
+            policy.add(new FilePermission(path.toString(), permissions));
+        }
         policy.add(new FilePermission(path.toString() + path.getFileSystem().getSeparator() + "-", permissions));
         /*
          * The file permission model since JDK 9 requires this due to the removal of pathname canonicalization. See also
@@ -79,9 +85,12 @@ public class FilePermissionUtils {
          */
         final Path realPath = path.toRealPath();
         if (path.toString().equals(realPath.toString()) == false) {
-            policy.add(new FilePermission(realPath.toString(), permissions));
+            if (recursiveAccessOnly == false) {
+                // add access for path itself
+                policy.add(new FilePermission(realPath.toString(), permissions));
+            }
+            // add access for files underneath
             policy.add(new FilePermission(realPath.toString() + realPath.getFileSystem().getSeparator() + "-", permissions));
         }
     }
-
 }

server/src/main/java/org/elasticsearch/bootstrap/Security.java

@@ -118,7 +118,8 @@ final class Security {
 
         // enable security policy: union of template and environment-based paths, and possibly plugin permissions
         Map<String, URL> codebases = getCodebaseJarMap(JarHell.parseClassPath());
-        Policy.setPolicy(new ESPolicy(codebases, createPermissions(environment), getPluginPermissions(environment), filterBadDefaults));
+        Policy.setPolicy(new ESPolicy(codebases, createPermissions(environment), getPluginPermissions(environment), filterBadDefaults,
+            createRecursiveDataPathPermission(environment)));
 
         // enable security manager
         final String[] classesThatCanExit =
@@ -254,6 +255,14 @@ final class Security {
         return policy;
     }
 
+    private static Permissions createRecursiveDataPathPermission(Environment environment) throws IOException {
+        Permissions policy = new Permissions();
+        for (Path path : environment.dataFiles()) {
+            addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete", true);
+        }
+        return policy;
+    }
+
     /** Adds access to classpath jars/classes for jar hell scan, etc */
     @SuppressForbidden(reason = "accesses fully qualified URLs to configure security")
     static void addClasspathPermissions(Permissions policy) throws IOException {
@@ -268,7 +277,7 @@ final class Security {
             }
             // resource itself
             if (Files.isDirectory(path)) {
-                addDirectoryPath(policy, "class.path", path, "read,readlink");
+                addDirectoryPath(policy, "class.path", path, "read,readlink", false);
             } else {
                 addSingleFilePath(policy, path, "read,readlink");
             }
@@ -280,21 +289,21 @@ final class Security {
      */
     static void addFilePermissions(Permissions policy, Environment environment) throws IOException {
         // read-only dirs
-        addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.binFile(), "read,readlink");
-        addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.libFile(), "read,readlink");
-        addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.modulesFile(), "read,readlink");
-        addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsFile(), "read,readlink");
-        addDirectoryPath(policy, "path.conf'", environment.configFile(), "read,readlink");
+        addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.binFile(), "read,readlink", false);
+        addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.libFile(), "read,readlink", false);
+        addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.modulesFile(), "read,readlink", false);
+        addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsFile(), "read,readlink", false);
+        addDirectoryPath(policy, "path.conf'", environment.configFile(), "read,readlink", false);
         // read-write dirs
-        addDirectoryPath(policy, "java.io.tmpdir", environment.tmpFile(), "read,readlink,write,delete");
-        addDirectoryPath(policy, Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile(), "read,readlink,write,delete");
+        addDirectoryPath(policy, "java.io.tmpdir", environment.tmpFile(), "read,readlink,write,delete", false);
+        addDirectoryPath(policy, Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile(), "read,readlink,write,delete", false);
         if (environment.sharedDataFile() != null) {
             addDirectoryPath(policy, Environment.PATH_SHARED_DATA_SETTING.getKey(), environment.sharedDataFile(),
-                "read,readlink,write,delete");
+                "read,readlink,write,delete", false);
         }
         final Set<Path> dataFilesPaths = new HashSet<>();
         for (Path path : environment.dataFiles()) {
-            addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete");
+            addDirectoryPath(policy, Environment.PATH_DATA_SETTING.getKey(), path, "read,readlink,write,delete", false);
             /*
              * We have to do this after adding the path because a side effect of that is that the directory is created; the Path#toRealPath
              * invocation will fail if the directory does not already exist. We use Path#toRealPath to follow symlinks and handle issues
@@ -310,7 +319,7 @@ final class Security {
             }
         }
         for (Path path : environment.repoFiles()) {
-            addDirectoryPath(policy, Environment.PATH_REPO_SETTING.getKey(), path, "read,readlink,write,delete");
+            addDirectoryPath(policy, Environment.PATH_REPO_SETTING.getKey(), path, "read,readlink,write,delete", false);
         }
         if (environment.pidFile() != null) {
             // we just need permission to remove the file if its elsewhere.

test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java

@@ -109,7 +109,7 @@ public class BootstrapForTesting {
                 Permissions perms = new Permissions();
                 Security.addClasspathPermissions(perms);
                 // java.io.tmpdir
-                FilePermissionUtils.addDirectoryPath(perms, "java.io.tmpdir", javaTmpDir, "read,readlink,write,delete");
+                FilePermissionUtils.addDirectoryPath(perms, "java.io.tmpdir", javaTmpDir, "read,readlink,write,delete", false);
                 // custom test config file
                 if (Strings.hasLength(System.getProperty("tests.config"))) {
                     FilePermissionUtils.addSingleFilePath(perms, PathUtils.get(System.getProperty("tests.config")), "read,readlink");
@@ -147,7 +147,7 @@ public class BootstrapForTesting {
                     addClassCodebase(codebases, "elasticsearch-rest-client", "org.elasticsearch.client.RestClient");
                 }
                 final Policy testFramework = Security.readPolicy(Bootstrap.class.getResource("test-framework.policy"), codebases);
-                final Policy esPolicy = new ESPolicy(codebases, perms, getPluginPermissions(), true);
+                final Policy esPolicy = new ESPolicy(codebases, perms, getPluginPermissions(), true, new Permissions());
                 Policy.setPolicy(new Policy() {
                     @Override
                     public boolean implies(ProtectionDomain domain, Permission permission) {