Add permission to secure access to certain config files (elastic#107827)

This adds a SecuredFileAccessPermission that Elasticsearch and plugins can use to limit access to certain files, so that only code that also has that same permission can access the specified files

docs/changelog/107827.yaml

@@ -0,0 +1,5 @@
+pr: 107827
+summary: Add permission to secure access to certain config files
+area: Security
+type: bug
+issues: []

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

@@ -10,6 +10,7 @@
 
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.test.ESTestCase;
+import org.junit.BeforeClass;
 
 import java.io.FilePermission;
 import java.net.SocketPermission;
@@ -19,11 +20,15 @@
 import java.security.Permission;
 import java.security.PermissionCollection;
 import java.security.Permissions;
+import java.security.Policy;
 import java.security.ProtectionDomain;
 import java.security.cert.Certificate;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+
+import static java.util.Map.entry;
+import static org.elasticsearch.bootstrap.ESPolicy.POLICY_RESOURCE;
 
 /**
  * Unit tests for ESPolicy: these cannot run with security manager,
@@ -32,6 +37,13 @@
 public class ESPolicyUnitTests extends ESTestCase {
 
     static final Map<String, URL> TEST_CODEBASES = BootstrapForTesting.getCodebases();
+    static Policy DEFAULT_POLICY;
+
+    @BeforeClass
+    public static void setupPolicy() {
+        assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
+        DEFAULT_POLICY = PolicyUtil.readPolicy(ESPolicy.class.getResource(POLICY_RESOURCE), TEST_CODEBASES);
+    }
 
     /**
      * Test policy with null codesource.
@@ -42,12 +54,11 @@ public class ESPolicyUnitTests extends ESTestCase {
      */
     @SuppressForbidden(reason = "to create FilePermission object")
     public void testNullCodeSource() throws Exception {
-        assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         // create a policy with AllPermission
         Permission all = new AllPermission();
         PermissionCollection allCollection = all.newPermissionCollection();
         allCollection.add(all);
-        ESPolicy policy = new ESPolicy(TEST_CODEBASES, allCollection, Collections.emptyMap(), true, List.of(), List.of());
+        ESPolicy policy = new ESPolicy(DEFAULT_POLICY, allCollection, Map.of(), true, List.of(), Map.of());
         // restrict ourselves to NoPermission
         PermissionCollection noPermissions = new Permissions();
         assertFalse(policy.implies(new ProtectionDomain(null, noPermissions), new FilePermission("foo", "read")));
@@ -58,9 +69,8 @@ public void testNullCodeSource() throws Exception {
      */
     @SuppressForbidden(reason = "to create FilePermission object")
     public void testNullLocation() throws Exception {
-        assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         PermissionCollection noPermissions = new Permissions();
-        ESPolicy policy = new ESPolicy(TEST_CODEBASES, noPermissions, Collections.emptyMap(), true, List.of(), List.of());
+        ESPolicy policy = new ESPolicy(DEFAULT_POLICY, noPermissions, Map.of(), true, List.of(), Map.of());
         assertFalse(
             policy.implies(
                 new ProtectionDomain(new CodeSource(null, (Certificate[]) null), noPermissions),
@@ -70,9 +80,8 @@ public void testNullLocation() throws Exception {
     }
 
     public void testListen() {
-        assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         final PermissionCollection noPermissions = new Permissions();
-        final ESPolicy policy = new ESPolicy(TEST_CODEBASES, noPermissions, Collections.emptyMap(), true, List.of(), List.of());
+        final ESPolicy policy = new ESPolicy(DEFAULT_POLICY, noPermissions, Map.of(), true, List.of(), Map.of());
         assertFalse(
             policy.implies(
                 new ProtectionDomain(ESPolicyUnitTests.class.getProtectionDomain().getCodeSource(), noPermissions),
@@ -83,14 +92,13 @@ public void testListen() {
 
     @SuppressForbidden(reason = "to create FilePermission object")
     public void testDataPathPermissionIsChecked() {
-        assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
         final ESPolicy policy = new ESPolicy(
-            TEST_CODEBASES,
+            DEFAULT_POLICY,
             new Permissions(),
-            Collections.emptyMap(),
+            Map.of(),
             true,
             List.of(new FilePermission("/home/elasticsearch/data/-", "read")),
-            List.of()
+            Map.of()
         );
         assertTrue(
             policy.implies(
@@ -101,27 +109,52 @@ public void testDataPathPermissionIsChecked() {
     }
 
     @SuppressForbidden(reason = "to create FilePermission object")
-    public void testForbiddenFilesAreForbidden() {
-        assumeTrue("test cannot run with security manager", System.getSecurityManager() == null);
-
-        FilePermission configPerm = new FilePermission("/home/elasticsearch/config/-", "read");
-        PermissionCollection coll = configPerm.newPermissionCollection();
-        coll.add(configPerm);
+    public void testSecuredAccess() {
+        String file1 = "/home/elasticsearch/config/pluginFile1.yml";
+        URL codebase1 = randomFrom(TEST_CODEBASES.values());
+        String file2 = "/home/elasticsearch/config/pluginFile2.yml";
+        URL codebase2 = randomValueOtherThan(codebase1, () -> randomFrom(TEST_CODEBASES.values()));
+        String dir1 = "/home/elasticsearch/config/pluginDir/";
+        URL codebase3 = randomValueOtherThanMany(Set.of(codebase1, codebase2)::contains, () -> randomFrom(TEST_CODEBASES.values()));
+        URL otherCodebase = randomValueOtherThanMany(
+            Set.of(codebase1, codebase2, codebase3)::contains,
+            () -> randomFrom(TEST_CODEBASES.values())
+        );
 
         ESPolicy policy = new ESPolicy(
-            TEST_CODEBASES,
-            coll,
-            Collections.emptyMap(),
+            DEFAULT_POLICY,
+            new Permissions(),
+            Map.of(),
             true,
             List.of(),
-            List.of(new FilePermission("/home/elasticsearch/config/forbidden.yml", "read"))
-        );
-        ProtectionDomain pd = new ProtectionDomain(
-            new CodeSource(randomBoolean() ? null : randomFrom(TEST_CODEBASES.values()), (Certificate[]) null),
-            new Permissions()
+            Map.ofEntries(entry(file1, Set.of(codebase1)), entry(file2, Set.of(codebase1, codebase2)), entry(dir1 + "*", Set.of(codebase3)))
         );
 
-        assertTrue(policy.implies(pd, new FilePermission("/home/elasticsearch/config/config.yml", "read")));
-        assertFalse(policy.implies(pd, new FilePermission("/home/elasticsearch/config/forbidden.yml", "read")));
+        ProtectionDomain nullDomain = new ProtectionDomain(new CodeSource(null, (Certificate[]) null), new Permissions());
+        ProtectionDomain codebase1Domain = new ProtectionDomain(new CodeSource(codebase1, (Certificate[]) null), new Permissions());
+        ProtectionDomain codebase2Domain = new ProtectionDomain(new CodeSource(codebase2, (Certificate[]) null), new Permissions());
+        ProtectionDomain codebase3Domain = new ProtectionDomain(new CodeSource(codebase3, (Certificate[]) null), new Permissions());
+        ProtectionDomain otherCodebaseDomain = new ProtectionDomain(new CodeSource(otherCodebase, (Certificate[]) null), new Permissions());
+
+        Set<String> actions = Set.of("read", "write", "read,write", "delete", "read,write,execute,readlink,delete");
+
+        assertFalse(policy.implies(nullDomain, new FilePermission(file1, randomFrom(actions))));
+        assertFalse(policy.implies(otherCodebaseDomain, new FilePermission(file1, randomFrom(actions))));
+        assertTrue(policy.implies(codebase1Domain, new FilePermission(file1, randomFrom(actions))));
+        assertFalse(policy.implies(codebase2Domain, new FilePermission(file1, randomFrom(actions))));
+        assertFalse(policy.implies(codebase3Domain, new FilePermission(file1, randomFrom(actions))));
+
+        assertFalse(policy.implies(nullDomain, new FilePermission(file2, randomFrom(actions))));
+        assertFalse(policy.implies(otherCodebaseDomain, new FilePermission(file2, randomFrom(actions))));
+        assertTrue(policy.implies(codebase1Domain, new FilePermission(file2, randomFrom(actions))));
+        assertTrue(policy.implies(codebase2Domain, new FilePermission(file2, randomFrom(actions))));
+        assertFalse(policy.implies(codebase3Domain, new FilePermission(file2, randomFrom(actions))));
+
+        String dirFile = dir1 + "file.yml";
+        assertFalse(policy.implies(nullDomain, new FilePermission(dirFile, randomFrom(actions))));
+        assertFalse(policy.implies(otherCodebaseDomain, new FilePermission(dirFile, randomFrom(actions))));
+        assertFalse(policy.implies(codebase1Domain, new FilePermission(dirFile, randomFrom(actions))));
+        assertFalse(policy.implies(codebase2Domain, new FilePermission(dirFile, randomFrom(actions))));
+        assertTrue(policy.implies(codebase3Domain, new FilePermission(dirFile, randomFrom(actions))));
     }
 }

server/src/main/java/org/elasticsearch/SecuredFileAccessPermission.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch;
+
+import java.security.BasicPermission;
+
+/**
+ * A permission granted to ensure secured access to a file in the config directory.
+ * <p>
+ * By granting this permission, all code that does not have the same permission on the same file
+ * will be denied all read/write access to that file.
+ * Note that you also need to wrap any access to the secured files in an {@code AccessController.doPrivileged()} block
+ * as Elasticsearch itself is denied access to files secured by plugins.
+ */
+public class SecuredFileAccessPermission extends BasicPermission {
+    public SecuredFileAccessPermission(String path) {
+        super(path, "");
+    }
+}

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

@@ -21,10 +21,13 @@
 import java.security.Permissions;
 import java.security.Policy;
 import java.security.ProtectionDomain;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 /** custom policy for union of static and dynamic permissions */
 final class ESPolicy extends Policy {
@@ -34,25 +37,28 @@ final class ESPolicy extends Policy {
     /** limited policy for scripts */
     static final String UNTRUSTED_RESOURCE = "untrusted.policy";
 
+    private static final String ALL_FILE_MASK = "read,readlink,write,delete,execute";
+
     final Policy template;
     final Policy untrusted;
     final Policy system;
     final PermissionCollection dynamic;
     final PermissionCollection dataPathPermission;
-    final PermissionCollection forbiddenFilePermission;
-    final Map<String, Policy> plugins;
+    final Map<URL, Policy> plugins;
+    final PermissionCollection allSecuredFiles;
+    final Map<FilePermission, Set<URL>> securedFiles;
 
+    @SuppressForbidden(reason = "Need to access and check file permissions directly")
     ESPolicy(
-        Map<String, URL> codebases,
+        Policy template,
         PermissionCollection dynamic,
-        Map<String, Policy> plugins,
+        Map<URL, Policy> plugins,
         boolean filterBadDefaults,
         List<FilePermission> dataPathPermissions,
-        List<FilePermission> forbiddenFilePermissions
+        Map<String, Set<URL>> securedFiles
     ) {
-        this.template = PolicyUtil.readPolicy(getClass().getResource(POLICY_RESOURCE), codebases);
+        this.template = template;
         this.dataPathPermission = createPermission(dataPathPermissions);
-        this.forbiddenFilePermission = createPermission(forbiddenFilePermissions);
         this.untrusted = PolicyUtil.readPolicy(getClass().getResource(UNTRUSTED_RESOURCE), Collections.emptyMap());
         if (filterBadDefaults) {
             this.system = new SystemPolicy(Policy.getPolicy());
@@ -61,6 +67,27 @@ final class ESPolicy extends Policy {
         }
         this.dynamic = dynamic;
         this.plugins = plugins;
+
+        this.securedFiles = securedFiles.entrySet()
+            .stream()
+            .collect(Collectors.toUnmodifiableMap(e -> new FilePermission(e.getKey(), ALL_FILE_MASK), e -> Set.copyOf(e.getValue())));
+        this.allSecuredFiles = createPermission(this.securedFiles.keySet());
+    }
+
+    private static PermissionCollection createPermission(Collection<FilePermission> permissions) {
+        PermissionCollection coll;
+        var it = permissions.iterator();
+        if (it.hasNext() == false) {
+            coll = new Permissions();
+        } else {
+            Permission p = it.next();
+            coll = p.newPermissionCollection();
+            coll.add(p);
+            it.forEachRemaining(coll::add);
+        }
+
+        coll.setReadOnly();
+        return coll;
     }
 
     private static PermissionCollection createPermission(List<FilePermission> permissions) {
@@ -87,20 +114,26 @@ public boolean implies(ProtectionDomain domain, Permission permission) {
             return false;
         }
 
-        // completely deny access to specific files that are forbidden
-        if (forbiddenFilePermission.implies(permission)) {
-            return false;
+        URL location = codeSource.getLocation();
+        if (allSecuredFiles.implies(permission)) {
+            /*
+             * Check if location can access this secured file
+             * The permission this is generated from, SecuredFileAccessPermission, doesn't have a mask,
+             * it just grants all access (and so disallows all access from others)
+             * It's helpful to use the infrastructure around FilePermission here to do the directory structure check with implies
+             * so we use ALL_FILE_MASK mask to check if we can do something with this file, whatever the actual operation we're requesting
+             */
+            return canAccessSecuredFile(location, new FilePermission(permission.getName(), ALL_FILE_MASK));
         }
 
-        URL location = codeSource.getLocation();
         if (location != null) {
             // run scripts with limited permissions
             if (BootstrapInfo.UNTRUSTED_CODEBASE.equals(location.getFile())) {
                 return untrusted.implies(domain, permission);
             }
             // check for an additional plugin permission: plugin policy is
             // only consulted for its codesources.
-            Policy plugin = plugins.get(location.getFile());
+            Policy plugin = plugins.get(location);
             if (plugin != null && plugin.implies(domain, permission)) {
                 return true;
             }
@@ -122,6 +155,29 @@ public boolean implies(ProtectionDomain domain, Permission permission) {
         return template.implies(domain, permission) || dynamic.implies(permission) || system.implies(domain, permission);
     }
 
+    @SuppressForbidden(reason = "We get given an URL by the security infrastructure")
+    private boolean canAccessSecuredFile(URL location, FilePermission permission) {
+        if (location == null) {
+            return false;
+        }
+
+        // check the source
+        Set<URL> accessibleSources = securedFiles.get(permission);
+        if (accessibleSources != null) {
+            // simple case - single-file referenced directly
+            return accessibleSources.contains(location);
+        } else {
+            // there's a directory reference in there somewhere
+            // do a manual search :(
+            // there may be several permissions that potentially match,
+            // grant access if any of them cover this file
+            return securedFiles.entrySet()
+                .stream()
+                .filter(e -> e.getKey().implies(permission))
+                .anyMatch(e -> e.getValue().contains(location));
+        }
+    }
+
     private static void hadoopHack() {
         for (StackTraceElement element : Thread.currentThread().getStackTrace()) {
             if ("org.apache.hadoop.util.Shell".equals(element.getClassName()) && "runCommand".equals(element.getMethodName())) {

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

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.bootstrap;
 
+import org.elasticsearch.SecuredFileAccessPermission;
 import org.elasticsearch.core.IOUtils;
 import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.core.SuppressForbidden;
@@ -50,6 +51,7 @@
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import javax.management.MBeanPermission;
 import javax.management.MBeanServerPermission;
@@ -60,6 +62,8 @@
 import javax.security.auth.kerberos.DelegationPermission;
 import javax.security.auth.kerberos.ServicePermission;
 
+import static java.util.Map.entry;
+
 public class PolicyUtil {
 
     // this object is checked by reference, so the value in the list does not matter
@@ -158,20 +162,15 @@ public boolean test(Permission permission) {
         // is used to mean names are accepted. We do not use this model for all permissions because many permission
         // classes have their own meaning for some form of wildcard matching of the name, which we want to delegate
         // to those permissions if possible.
-        Map<String, List<String>> classPermissions = Map.of(
-            URLPermission.class,
-            ALLOW_ALL_NAMES,
-            DelegationPermission.class,
-            ALLOW_ALL_NAMES,
-            ServicePermission.class,
-            ALLOW_ALL_NAMES,
-            PrivateCredentialPermission.class,
-            ALLOW_ALL_NAMES,
-            SQLPermission.class,
-            List.of("callAbort", "setNetworkTimeout"),
-            ClassPermission.class,
-            ALLOW_ALL_NAMES
-        ).entrySet().stream().collect(Collectors.toMap(e -> e.getKey().getCanonicalName(), Map.Entry::getValue));
+        Map<String, List<String>> classPermissions = Stream.of(
+            entry(URLPermission.class, ALLOW_ALL_NAMES),
+            entry(DelegationPermission.class, ALLOW_ALL_NAMES),
+            entry(ServicePermission.class, ALLOW_ALL_NAMES),
+            entry(PrivateCredentialPermission.class, ALLOW_ALL_NAMES),
+            entry(SQLPermission.class, List.of("callAbort", "setNetworkTimeout")),
+            entry(ClassPermission.class, ALLOW_ALL_NAMES),
+            entry(SecuredFileAccessPermission.class, ALLOW_ALL_NAMES)
+        ).collect(Collectors.toMap(e -> e.getKey().getCanonicalName(), Map.Entry::getValue));
         PermissionCollection pluginPermissionCollection = new Permissions();
         namedPermissions.forEach(pluginPermissionCollection::add);
         pluginPermissionCollection.setReadOnly();