Add plugin permission validation (#64751)

Security manager policies within plugins currently can ask to grant any
permission (though we block some within the security manager itself at
runtime). Yet most of these permissions should never be necessary, and
some we would actively not want any plugins to be allowed to use. This
commit adds validation of plugins' policy files to restrict the
permissions allowed to be granted to a subset that is reasonable for
plugins to need. The allowed permissions are not ideal (still containing
things like suppressAccessChecks), but it is a step forward in defining
a stricter model for plugins that reduces the surface area of potential
abuse.

buildSrc/src/main/resources/checkstyle.xml

@@ -96,7 +96,7 @@
       <property name="ignoreComments" value="true" />
     </module>
     <module name="RegexpSinglelineJava">
-      <property name="format" value="java\.io\.Serializable" />
+      <property name="format" value="java\.io\.Serializable;" />
       <property name="message" value="References java.io.Serializable." />
       <property name="ignoreComments" value="true" />
     </module>

distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

@@ -850,10 +850,8 @@ class InstallPluginCommand extends EnvironmentAwareCommand {
     private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoot, Environment env, List<Path> deleteOnFailure)
         throws Exception {
         final PluginInfo info = loadPluginInfo(terminal, tmpRoot, env);
-
         checkCanInstallationProceed(terminal, Build.CURRENT.flavor(), info);
-
-        PluginPolicyInfo pluginPolicy = PolicyUtil.getPluginPolicyInfo(tmpRoot);
+        PluginPolicyInfo pluginPolicy = PolicyUtil.getPluginPolicyInfo(tmpRoot, env.tmpFile());
         if (pluginPolicy != null) {
             Set<String> permissions = PluginSecurity.getPermissionDescriptions(pluginPolicy, env.tmpFile());
             PluginSecurity.confirmPolicyExceptions(terminal, permissions, isBatch);

distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java

@@ -1477,7 +1477,7 @@ public class InstallPluginCommandTests extends ESTestCase {
     public void testPolicyConfirmation() throws Exception {
         Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
-        writePluginSecurityPolicy(pluginDir, "setAccessible", "setFactory");
+        writePluginSecurityPolicy(pluginDir, "createClassLoader", "setFactory");
         String pluginZip = createPluginUrl("fake", pluginDir);
 
         assertPolicyConfirmation(env, pluginZip, "plugin requires additional permissions");

plugins/repository-hdfs/src/main/plugin-metadata/plugin-security.policy

@@ -68,7 +68,7 @@ grant {
   permission java.security.SecurityPermission "putProviderProperty.SaslPlainServer";
 
   // org.apache.hadoop.security.SaslPlainServer.SecurityProvider.SecurityProvider init
-  permission java.security.SecurityPermission "insertProvider.SaslPlainServer";
+  permission java.security.SecurityPermission "insertProvider";
 
   // org.apache.hadoop.security.SaslRpcClient.getServerPrincipal -> KerberosPrincipal init
   permission javax.security.auth.kerberos.ServicePermission "*", "initiate";

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

@@ -50,9 +50,7 @@ public class PolicyUtilTests extends ESTestCase {
 
     @Before
     public void assumeSecurityManagerDisabled() {
-        assumeTrue(
-            "test cannot run with security manager enabled",
-            System.getSecurityManager() == null);
+        assumeTrue("test cannot run with security manager enabled", System.getSecurityManager() == null);
     }
 
     URL makeUrl(String s) {
@@ -100,12 +98,12 @@ public class PolicyUtilTests extends ESTestCase {
     }
 
     public void testPluginPolicyInfoEmpty() throws Exception {
-        assertThat(PolicyUtil.getPluginPolicyInfo(createTempDir()), is(nullValue()));
+        assertThat(PolicyUtil.readPolicyInfo(createTempDir()), is(nullValue()));
     }
 
     public void testPluginPolicyInfoNoJars() throws Exception {
         Path noJarsPlugin = makeDummyPlugin("dummy.policy");
-        PluginPolicyInfo info = PolicyUtil.getPluginPolicyInfo(noJarsPlugin);
+        PluginPolicyInfo info = PolicyUtil.readPolicyInfo(noJarsPlugin);
         assertThat(info.policy, is(not(nullValue())));
         assertThat(info.jars, emptyIterable());
     }
@@ -113,7 +111,7 @@ public class PolicyUtilTests extends ESTestCase {
     public void testPluginPolicyInfo() throws Exception {
         Path plugin = makeDummyPlugin("dummy.policy",
             "foo.jar", "foo.txt", "bar.jar");
-        PluginPolicyInfo info = PolicyUtil.getPluginPolicyInfo(plugin);
+        PluginPolicyInfo info = PolicyUtil.readPolicyInfo(plugin);
         assertThat(info.policy, is(not(nullValue())));
         assertThat(info.jars, containsInAnyOrder(
             plugin.resolve("foo.jar").toUri().toURL(),
@@ -146,4 +144,240 @@ public class PolicyUtilTests extends ESTestCase {
             clearProperty("jarUrl");
         }
     }
+
+    private Path makeSinglePermissionPlugin(String jarUrl, String clazz, String name, String actions) throws IOException {
+        Path plugin = createTempDir();
+        StringBuilder policyString = new StringBuilder("grant");
+        if (jarUrl != null) {
+            Path jar = plugin.resolve(jarUrl);
+            Files.createFile(jar);
+            policyString.append(" codeBase \"" + jar.toUri().toURL().toString() + "\"");
+        }
+        policyString.append(" {\n  permission ");
+        policyString.append(clazz);
+        // wildcard
+        policyString.append(" \"" + name + "\"");
+        if (actions != null) {
+            policyString.append(", \"" + actions + "\"");
+        }
+        policyString.append(";\n};");
+
+        logger.info(policyString.toString());
+        Files.writeString(plugin.resolve(PluginInfo.ES_PLUGIN_POLICY), policyString.toString());
+
+        return plugin;
+    }
+
+    interface PolicyParser {
+        PluginPolicyInfo parse(Path pluginRoot, Path tmpDir) throws IOException;
+    }
+
+    void assertAllowedPermission(String clazz, String name, String actions, Path tmpDir, PolicyParser parser) throws Exception {
+        // global policy
+        Path plugin = makeSinglePermissionPlugin(null, clazz, name, actions);
+        assertNotNull(parser.parse(plugin, tmpDir)); // no error
+
+        // specific jar policy
+        plugin = makeSinglePermissionPlugin("foobar.jar", clazz, name, actions);
+        assertNotNull(parser.parse(plugin, tmpDir)); // no error
+    }
+
+    void assertAllowedPermissions(List<String> allowedPermissions, PolicyParser parser) throws Exception {
+        Path tmpDir = createTempDir();
+        for (String rawPermission : allowedPermissions) {
+            String[] elements = rawPermission.split(" ");
+            assert elements.length <= 3;
+            assert elements.length >= 2;
+
+            assertAllowedPermission(elements[0], elements[1], elements.length == 3 ? elements[2] : null, tmpDir, parser);
+        }
+    }
+
+    void assertIllegalPermission(String clazz, String name, String actions, Path tmpDir, PolicyParser parser) throws Exception {
+        // global policy
+        final Path globalPlugin = makeSinglePermissionPlugin(null, clazz, name, actions);
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            "Permission (" + clazz + " " + name + (actions == null ? "" : (" " + actions)) + ") should be illegal",
+            () -> parser.parse(globalPlugin, tmpDir)); // no error
+        assertThat(e.getMessage(), containsString("contains illegal permission"));
+        assertThat(e.getMessage(), containsString("in global grant"));
+
+        // specific jar policy
+        final Path jarPlugin = makeSinglePermissionPlugin("foobar.jar", clazz, name, actions);
+        e = expectThrows(IllegalArgumentException.class, () -> parser.parse(jarPlugin, tmpDir)); // no error
+        assertThat(e.getMessage(), containsString("contains illegal permission"));
+        assertThat(e.getMessage(), containsString("for jar"));
+    }
+
+    void assertIllegalPermissions(List<String> illegalPermissions, PolicyParser parser) throws Exception {
+        Path tmpDir = createTempDir();
+        for (String rawPermission : illegalPermissions) {
+            String[] elements = rawPermission.split(" ");
+            assert elements.length <= 3;
+            assert elements.length >= 2;
+
+            assertIllegalPermission(elements[0], elements[1], elements.length == 3 ? elements[2] : null, tmpDir, parser);
+        }
+    }
+
+    static final List<String> PLUGIN_TEST_PERMISSIONS = List.of(
+        "java.lang.reflect.ReflectPermission suppressAccessChecks",
+        "java.lang.RuntimePermission createClassLoader",
+        "java.lang.RuntimePermission getClassLoader",
+        "java.lang.RuntimePermission setContextClassLoader",
+        "java.lang.RuntimePermission setFactory",
+        "java.lang.RuntimePermission loadLibrary.*",
+        "java.lang.RuntimePermission accessClassInPackage.*",
+        "java.lang.RuntimePermission accessDeclaredMembers",
+        "java.net.NetPermission requestPasswordAuthentication",
+        "java.net.NetPermission getProxySelector",
+        "java.net.NetPermission getCookieHandler",
+        "java.net.NetPermission getResponseCache",
+        "java.net.SocketPermission * accept,connect,listen,resolve",
+        "java.net.SocketPermission www.elastic.co accept,connect,listen,resolve",
+        "java.net.URLPermission https://elastic.co",
+        "java.net.URLPermission http://elastic.co",
+        "java.security.SecurityPermission createAccessControlContext",
+        "java.security.SecurityPermission insertProvider",
+        "java.security.SecurityPermission putProviderProperty.*",
+        "java.security.SecurityPermission putProviderProperty.foo",
+        "java.sql.SQLPermission callAbort",
+        "java.sql.SQLPermission setNetworkTimeout",
+        "java.util.PropertyPermission * read",
+        "java.util.PropertyPermission someProperty read",
+        "java.util.PropertyPermission * write",
+        "java.util.PropertyPermission foo.bar write",
+        "javax.security.auth.AuthPermission doAs",
+        "javax.security.auth.AuthPermission doAsPrivileged",
+        "javax.security.auth.AuthPermission getSubject",
+        "javax.security.auth.AuthPermission getSubjectFromDomainCombiner",
+        "javax.security.auth.AuthPermission setReadOnly",
+        "javax.security.auth.AuthPermission modifyPrincipals",
+        "javax.security.auth.AuthPermission modifyPublicCredentials",
+        "javax.security.auth.AuthPermission modifyPrivateCredentials",
+        "javax.security.auth.AuthPermission refreshCredential",
+        "javax.security.auth.AuthPermission destroyCredential",
+        "javax.security.auth.AuthPermission createLoginContext.*",
+        "javax.security.auth.AuthPermission getLoginConfiguration",
+        "javax.security.auth.AuthPermission setLoginConfiguration",
+        "javax.security.auth.AuthPermission createLoginConfiguration.*",
+        "javax.security.auth.AuthPermission refreshLoginConfiguration",
+        "javax.security.auth.kerberos.DelegationPermission host/www.elastic.co@ELASTIC.CO krbtgt/ELASTIC.CO@ELASTIC.CO",
+        "javax.security.auth.kerberos.ServicePermission host/www.elastic.co@ELASTIC.CO accept"
+    );
+
+    public void testPluginPolicyAllowedPermissions() throws Exception {
+        assertAllowedPermissions(PLUGIN_TEST_PERMISSIONS, PolicyUtil::getPluginPolicyInfo);
+        assertIllegalPermissions(MODULE_TEST_PERMISSIONS, PolicyUtil::getPluginPolicyInfo);
+    }
+
+    public void testPrivateCredentialPermissionAllowed() throws Exception {
+        // the test permission list relies on name values not containing spaces, so this
+        // exists to also check PrivateCredentialPermission which requires a space in the name
+        String clazz = "javax.security.auth.PrivateCredentialPermission";
+        String name = "com.sun.PrivateCredential com.sun.Principal \\\"duke\\\"";
+
+        assertAllowedPermission(clazz, name, "read", createTempDir(), PolicyUtil::getPluginPolicyInfo);
+    }
+
+    static final List<String> MODULE_TEST_PERMISSIONS = List.of(
+        "java.io.FilePermission /foo/bar read",
+        "java.io.FilePermission /foo/bar write",
+        "java.lang.RuntimePermission getFileStoreAttributes",
+        "java.lang.RuntimePermission accessUserInformation"
+    );
+
+    public void testModulePolicyAllowedPermissions() throws Exception {
+        assertAllowedPermissions(MODULE_TEST_PERMISSIONS, PolicyUtil::getModulePolicyInfo);
+    }
+
+    static final List<String> ILLEGAL_TEST_PERMISSIONS = List.of(
+        "java.awt.AWTPermission *",
+        "java.io.FilePermission /foo/bar execute",
+        "java.io.FilePermission /foo/bar delete",
+        "java.io.FilePermission /foo/bar readLink",
+        "java.io.SerializablePermission enableSubclassImplementation",
+        "java.io.SerializablePermission enableSubstitution",
+        "java.lang.management.ManagementPermission control",
+        "java.lang.management.ManagementPermission monitor",
+        "java.lang.reflect.ReflectPermission newProxyInPackage.*",
+        "java.lang.RuntimePermission enableContextClassLoaderOverride",
+        "java.lang.RuntimePermission closeClassLoader",
+        "java.lang.RuntimePermission setSecurityManager",
+        "java.lang.RuntimePermission createSecurityManager",
+        "java.lang.RuntimePermission getenv.*",
+        "java.lang.RuntimePermission getenv.FOOBAR",
+        "java.lang.RuntimePermission shutdownHooks",
+        "java.lang.RuntimePermission setIO",
+        "java.lang.RuntimePermission modifyThread",
+        "java.lang.RuntimePermission stopThread",
+        "java.lang.RuntimePermission modifyThreadGroup",
+        "java.lang.RuntimePermission getProtectionDomain",
+        "java.lang.RuntimePermission readFileDescriptor",
+        "java.lang.RuntimePermission writeFileDescriptor",
+        "java.lang.RuntimePermission defineClassInPackage.*",
+        "java.lang.RuntimePermission defineClassInPackage.foobar",
+        "java.lang.RuntimePermission queuePrintJob",
+        "java.lang.RuntimePermission getStackTrace",
+        "java.lang.RuntimePermission setDefaultUncaughtExceptionHandler",
+        "java.lang.RuntimePermission preferences",
+        "java.lang.RuntimePermission usePolicy",
+        "java.net.NetPermission setDefaultAuthenticator",
+        "java.net.NetPermission specifyStreamHandler",
+        "java.net.NetPermission setProxySelector",
+        "java.net.NetPermission setCookieHandler",
+        "java.net.NetPermission setResponseCache",
+        "java.nio.file.LinkPermission hard",
+        "java.nio.file.LinkPermission symbolic",
+        "java.security.SecurityPermission getDomainCombiner",
+        "java.security.SecurityPermission getPolicy",
+        "java.security.SecurityPermission setPolicy",
+        "java.security.SecurityPermission getProperty.*",
+        "java.security.SecurityPermission getProperty.foobar",
+        "java.security.SecurityPermission setProperty.*",
+        "java.security.SecurityPermission setProperty.foobar",
+        "java.security.SecurityPermission removeProvider.*",
+        "java.security.SecurityPermission removeProvider.foobar",
+        "java.security.SecurityPermission clearProviderProperties.*",
+        "java.security.SecurityPermission clearProviderProperties.foobar",
+        "java.security.SecurityPermission removeProviderProperty.*",
+        "java.security.SecurityPermission removeProviderProperty.foobar",
+        "java.security.SecurityPermission insertProvider.*",
+        "java.security.SecurityPermission insertProvider.foobar",
+        "java.security.SecurityPermission setSystemScope",
+        "java.security.SecurityPermission setIdentityPublicKey",
+        "java.security.SecurityPermission setIdentityInfo",
+        "java.security.SecurityPermission addIdentityCertificate",
+        "java.security.SecurityPermission removeIdentityCertificate",
+        "java.security.SecurityPermission printIdentity",
+        "java.security.SecurityPermission getSignerPrivateKey",
+        "java.security.SecurityPermission getSignerKeyPair",
+        "java.sql.SQLPermission setLog",
+        "java.sql.SQLPermission setSyncFactory",
+        "java.sql.SQLPermission deregisterDriver",
+        "java.util.logging.LoggingPermission control",
+        "javax.management.MBeanPermission * *",
+        "javax.management.MBeanServerPermission *",
+        "javax.management.MBeanTrustPermission *",
+        "javax.management.remote.SubjectDelegationPermission *",
+        "javax.net.ssl.SSLPermission setHostnameVerifier",
+        "javax.net.ssl.SSLPermission getSSLSessionContext",
+        "javax.net.ssl.SSLPermission setDefaultSSLContext",
+        "javax.sound.sampled.AudioPermission play",
+        "javax.sound.sampled.AudioPermission record",
+        "javax.xml.bind.JAXBPermission setDatatypeConverter",
+        "javax.xml.ws.WebServicePermission publishEndpoint"
+    );
+
+    public void testIllegalPermissions() throws Exception {
+        assertIllegalPermissions(ILLEGAL_TEST_PERMISSIONS, PolicyUtil::getPluginPolicyInfo);
+        assertIllegalPermissions(ILLEGAL_TEST_PERMISSIONS, PolicyUtil::getModulePolicyInfo);
+    }
+
+    public void testAllPermission() throws Exception {
+        // AllPermission has no name element, so doesn't work with the format above
+        Path tmpDir = createTempDir();
+        assertIllegalPermission("java.security.AllPermission", null, null, tmpDir, PolicyUtil::getPluginPolicyInfo);
+        assertIllegalPermission("java.security.AllPermission", null, null, tmpDir, PolicyUtil::getModulePolicyInfo);
+    }
 }

qa/evil-tests/src/test/java/org/elasticsearch/plugins/PluginSecurityTests.java

@@ -26,6 +26,7 @@ import org.elasticsearch.test.ESTestCase;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.PropertyPermission;
 import java.util.Set;
 
 import static org.hamcrest.Matchers.contains;
@@ -40,7 +41,7 @@ public class PluginSecurityTests extends ESTestCase {
         for (String file : files) {
             Files.createFile(plugin.resolve(file));
         }
-        return PolicyUtil.getPluginPolicyInfo(plugin);
+        return PolicyUtil.getPluginPolicyInfo(plugin, createTempDir());
     }
 
     /** Test that we can parse the set of permissions correctly for a simple policy */
@@ -51,7 +52,7 @@ public class PluginSecurityTests extends ESTestCase {
         Path scratch = createTempDir();
         PluginPolicyInfo info = makeDummyPlugin("security/simple-plugin-security.policy");
         Set<String> actual = PluginSecurity.getPermissionDescriptions(info, scratch);
-        assertThat(actual, contains(PluginSecurity.formatPermission(new RuntimePermission("queuePrintJob"))));
+        assertThat(actual, contains(PluginSecurity.formatPermission(new PropertyPermission("someProperty", "read"))));
     }
 
     /** Test that we can parse the set of permissions correctly for a complex policy */
@@ -64,24 +65,13 @@ public class PluginSecurityTests extends ESTestCase {
         Set<String> actual = PluginSecurity.getPermissionDescriptions(info, scratch);
         assertThat(actual, containsInAnyOrder(
             PluginSecurity.formatPermission(new RuntimePermission("getClassLoader")),
-            PluginSecurity.formatPermission(new RuntimePermission("closeClassLoader"))));
+            PluginSecurity.formatPermission(new RuntimePermission("createClassLoader"))));
     }
 
     /** Test that we can format some simple permissions properly */
     public void testFormatSimplePermission() throws Exception {
         assertEquals(
-                "java.lang.RuntimePermission queuePrintJob",
-                PluginSecurity.formatPermission(new RuntimePermission("queuePrintJob")));
-    }
-
-    /** Test that we can format an unresolved permission properly */
-    public void testFormatUnresolvedPermission() throws Exception {
-        assumeTrue(
-                "test cannot run with security manager enabled",
-                System.getSecurityManager() == null);
-        Path scratch = createTempDir();
-        PluginPolicyInfo info = makeDummyPlugin("security/unresolved-plugin-security.policy");
-        Set<String> permissions = PluginSecurity.getPermissionDescriptions(info, scratch);
-        assertThat(permissions, contains("org.fake.FakePermission fakeName"));
+                "java.lang.RuntimePermission accessDeclaredMembers",
+                PluginSecurity.formatPermission(new RuntimePermission("accessDeclaredMembers")));
     }
 }

qa/evil-tests/src/test/resources/org/elasticsearch/plugins/security/complex-plugin-security.policy

@@ -20,5 +20,5 @@
 grant {
   // needed to cause problems
   permission java.lang.RuntimePermission "getClassLoader";
-  permission java.lang.RuntimePermission "closeClassLoader";
+  permission java.lang.RuntimePermission "createClassLoader";
 };

qa/evil-tests/src/test/resources/org/elasticsearch/plugins/security/simple-plugin-security.policy

@@ -18,6 +18,5 @@
  */
 
 grant {
-  // needed to waste paper
-  permission java.lang.RuntimePermission "queuePrintJob";
+  permission java.util.PropertyPermission "someProperty", "read";
 };

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

@@ -20,14 +20,17 @@
 package org.elasticsearch.bootstrap;
 
 import java.net.URL;
+import java.nio.file.Path;
 import java.security.Policy;
 import java.util.Set;
 
 public class PluginPolicyInfo {
+    public final Path file;
     public final Set<URL> jars;
     public final Policy policy;
 
-    PluginPolicyInfo(Set<URL> jars, Policy policy) {
+    PluginPolicyInfo(Path file, Set<URL> jars, Policy policy) {
+        this.file = file;
         this.jars = jars;
         this.policy = policy;
     }

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

@@ -23,10 +23,20 @@ import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.plugins.PluginInfo;
+import org.elasticsearch.script.ClassPermission;
 
+import javax.security.auth.AuthPermission;
+import javax.security.auth.PrivateCredentialPermission;
+import javax.security.auth.kerberos.DelegationPermission;
+import javax.security.auth.kerberos.ServicePermission;
+import java.io.FilePermission;
 import java.io.IOException;
+import java.lang.reflect.ReflectPermission;
+import java.net.NetPermission;
+import java.net.SocketPermission;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.net.URLPermission;
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -34,20 +44,149 @@ import java.security.CodeSource;
 import java.security.NoSuchAlgorithmException;
 import java.security.Permission;
 import java.security.PermissionCollection;
+import java.security.Permissions;
 import java.security.Policy;
 import java.security.ProtectionDomain;
+import java.security.SecurityPermission;
 import java.security.URIParameter;
+import java.security.UnresolvedPermission;
 import java.security.cert.Certificate;
+import java.sql.SQLPermission;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
+import java.util.PropertyPermission;
 import java.util.Properties;
 import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 public class PolicyUtil {
+
+    // this object is checked by reference, so the value in the list does not matter
+    static final List<String> ALLOW_ALL_NAMES = List.of("ALLOW ALL NAMES SENTINEL");
+
+    static class PermissionMatcher implements Predicate<Permission> {
+
+        PermissionCollection namedPermissions;
+        Map<String, List<String>> classPermissions;
+
+        PermissionMatcher(PermissionCollection namedPermissions,
+                          Map<String, List<String>> classPermissions) {
+            this.namedPermissions = namedPermissions;
+            this.classPermissions = classPermissions;
+        }
+
+        @Override
+        public boolean test(Permission permission) {
+            if (namedPermissions.implies(permission)) {
+                return true;
+            }
+            String clazz = permission.getClass().getCanonicalName();
+            String name = permission.getName();
+            if (permission.getClass().equals(UnresolvedPermission.class)) {
+                UnresolvedPermission up = (UnresolvedPermission) permission;
+                clazz = up.getUnresolvedType();
+                name = up.getUnresolvedName();
+            }
+            List<String> allowedNames = classPermissions.get(clazz);
+            return allowedNames != null && (allowedNames == ALLOW_ALL_NAMES || allowedNames.contains(name));
+        }
+    }
+
+    private static final PermissionMatcher ALLOWED_PLUGIN_PERMISSIONS;
+    private static final PermissionMatcher ALLOWED_MODULE_PERMISSIONS;
+    static {
+        List<Permission> namedPermissions = List.of(
+            new ReflectPermission("suppressAccessChecks"),
+            new RuntimePermission("createClassLoader"),
+            new RuntimePermission("getClassLoader"),
+            new RuntimePermission("setContextClassLoader"),
+            new RuntimePermission("setFactory"),
+            new RuntimePermission("loadLibrary.*"),
+            new RuntimePermission("accessClassInPackage.*"),
+            new RuntimePermission("accessDeclaredMembers"),
+            new NetPermission("requestPasswordAuthentication"),
+            new NetPermission("getProxySelector"),
+            new NetPermission("getCookieHandler"),
+            new NetPermission("getResponseCache"),
+            new SocketPermission("*", "accept,connect,listen,resolve"),
+            new SecurityPermission("createAccessControlContext"),
+            new SecurityPermission("insertProvider"),
+            new SecurityPermission("putProviderProperty.*"),
+            // apache abuses the SecurityPermission class for it's own purposes
+            new SecurityPermission("org.apache.*"),
+            // write is needed because of HdfsPlugin
+            new PropertyPermission("*", "read,write"),
+            new AuthPermission("doAs"),
+            new AuthPermission("doAsPrivileged"),
+            new AuthPermission("getSubject"),
+            new AuthPermission("getSubjectFromDomainCombiner"),
+            new AuthPermission("setReadOnly"),
+            new AuthPermission("modifyPrincipals"),
+            new AuthPermission("modifyPublicCredentials"),
+            new AuthPermission("modifyPrivateCredentials"),
+            new AuthPermission("refreshCredential"),
+            new AuthPermission("destroyCredential"),
+            new AuthPermission("createLoginContext.*"),
+            new AuthPermission("getLoginConfiguration"),
+            new AuthPermission("setLoginConfiguration"),
+            new AuthPermission("createLoginConfiguration.*"),
+            new AuthPermission("refreshLoginConfiguration")
+        );
+        // While it would be ideal to represent all allowed permissions with concrete instances so that we can
+        // use the builtin implies method to match them against the parsed policy, this does not work in all
+        // cases for two reasons:
+        // (1) Some permissions classes do not have a name argument that can represent all possible variants.
+        //     For example, FilePermission has "<< ALL FILES >>" so all paths can be matched, but DelegationPermission
+        //     does not have anything to represent all principals.
+        // (2) Some permissions classes are in java modules that are not accessible from the classloader used by
+        //     the policy parser. This results in those permissions being in UnresolvedPermission instances. Those
+        //     are normally resolved at runtime when that permission is checked by SecurityManager. But there is
+        //     no general purpose utility to resolve those permissions, so we must be able to match those
+        //     unresolved permissions in the policy by class and name values.
+        // Given the above, the below map is from permission class to the list of allowed name values. A sentinel value
+        // 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));
+        PermissionCollection pluginPermissionCollection = new Permissions();
+        namedPermissions.forEach(pluginPermissionCollection::add);
+        pluginPermissionCollection.setReadOnly();
+        ALLOWED_PLUGIN_PERMISSIONS = new PermissionMatcher(pluginPermissionCollection, classPermissions);
+
+        // Modules are allowed a few extra permissions. While we should strive to keep this list small, modules
+        // are essentially part of core, so these are permissions we need for various reasons in core functionality,
+        // but that we do not think plugins in general should need.
+        List<Permission> modulePermissions = List.of(
+            createFilePermission("<<ALL FILES>>", "read,write"),
+            new RuntimePermission("getFileStoreAttributes"),
+            new RuntimePermission("accessUserInformation"),
+            new AuthPermission("modifyPrivateCredentials")
+        );
+        PermissionCollection modulePermissionCollection = new Permissions();
+        namedPermissions.forEach(modulePermissionCollection::add);
+        modulePermissions.forEach(modulePermissionCollection::add);
+        modulePermissionCollection.setReadOnly();
+        ALLOWED_MODULE_PERMISSIONS = new PermissionMatcher(modulePermissionCollection, classPermissions);
+    }
+
+    @SuppressForbidden(reason = "create permission for test")
+    private static FilePermission createFilePermission(String path, String actions) {
+        return new FilePermission(path, actions);
+    }
+
     /**
      * Return a map from codebase name to codebase url of jar codebases used by ES core.
      */
@@ -138,10 +277,8 @@ public class PolicyUtil {
         }
     }
 
-    /**
-     * Return info about the security policy for a plugin.
-     */
-    public static PluginPolicyInfo getPluginPolicyInfo(Path pluginRoot) throws IOException {
+    // pakcage private for tests
+    static PluginPolicyInfo readPolicyInfo(Path pluginRoot) throws IOException {
         Path policyFile = pluginRoot.resolve(PluginInfo.ES_PLUGIN_POLICY);
         if (Files.exists(policyFile) == false) {
             return null;
@@ -162,7 +299,47 @@ public class PolicyUtil {
         // parse the plugin's policy file into a set of permissions
         Policy policy = readPolicy(policyFile.toUri().toURL(), getCodebaseJarMap(jars));
 
-        return new PluginPolicyInfo(jars, policy);
+        return new PluginPolicyInfo(policyFile, jars, policy);
+    }
+
+    private static void validatePolicyPermissionsForJar(String type, Path file, URL jar, Policy policy,
+                                                        PermissionMatcher allowedPermissions, Path tmpDir) throws IOException {
+        Set<Permission> jarPermissions = getPolicyPermissions(jar, policy, tmpDir);
+        for (Permission permission : jarPermissions) {
+            if (allowedPermissions.test(permission) == false) {
+                String scope = jar == null ? " in global grant" : " for jar " + jar;
+                throw new IllegalArgumentException(type + " policy [" + file + "] contains illegal permission " + permission + scope);
+            }
+        }
+    }
+
+    private static void validatePolicyPermissions(String type, PluginPolicyInfo info, PermissionMatcher allowedPermissions,
+                                                  Path tmpDir) throws IOException {
+        if (info == null) {
+            return;
+        }
+        validatePolicyPermissionsForJar(type, info.file, null, info.policy, allowedPermissions, tmpDir);
+        for (URL jar : info.jars) {
+            validatePolicyPermissionsForJar(type, info.file, jar, info.policy, allowedPermissions, tmpDir);
+        }
+    }
+
+    /**
+     * Return info about the security policy for a plugin.
+     */
+    public static PluginPolicyInfo getPluginPolicyInfo(Path pluginRoot, Path tmpDir) throws IOException {
+        PluginPolicyInfo info = readPolicyInfo(pluginRoot);
+        validatePolicyPermissions("plugin", info, ALLOWED_PLUGIN_PERMISSIONS, tmpDir);
+        return info;
+    }
+
+    /**
+     * Return info about the security policy for a module.
+     */
+    public static PluginPolicyInfo getModulePolicyInfo(Path moduleRoot, Path tmpDir) throws IOException {
+        PluginPolicyInfo info = readPolicyInfo(moduleRoot);
+        validatePolicyPermissions("module", info, ALLOWED_MODULE_PERMISSIONS, tmpDir);
+        return info;
     }
 
     /**
@@ -200,7 +377,6 @@ public class PolicyUtil {
             throw new UnsupportedOperationException("JavaPolicy implementation does not support retrieving permissions");
         }
 
-
         Set<Permission> actualPermissions = new HashSet<>();
         for (Permission permission : Collections.list(permissions.elements())) {
             if (emptyPolicy.implies(protectionDomain, permission) == false) {

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

@@ -44,9 +44,9 @@ import java.security.Policy;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Consumer;
 
 import static org.elasticsearch.bootstrap.FilePermissionUtils.addDirectoryPath;
 import static org.elasticsearch.bootstrap.FilePermissionUtils.addSingleFilePath;
@@ -134,15 +134,9 @@ final class Security {
     @SuppressForbidden(reason = "proper use of URL")
     static Map<String,Policy> getPluginAndModulePermissions(Environment environment) throws IOException {
         Map<String,Policy> map = new HashMap<>();
-        // collect up set of plugins and modules by listing directories.
-        Set<Path> pluginsAndModules = new LinkedHashSet<>(PluginsService.findPluginDirs(environment.pluginsFile()));
-        pluginsAndModules.addAll(PluginsService.findPluginDirs(environment.modulesFile()));
-
-        // now process each one
-        for (Path plugin : pluginsAndModules) {
-            PluginPolicyInfo pluginPolicy = PolicyUtil.getPluginPolicyInfo(plugin);
+        Consumer<PluginPolicyInfo> addPolicy = pluginPolicy -> {
             if (pluginPolicy == null) {
-                continue;
+                return;
             }
 
             // consult this policy for each of the plugin's jars:
@@ -152,6 +146,13 @@ final class Security {
                     throw new IllegalStateException("per-plugin permissions already granted for jar file: " + jar);
                 }
             }
+        };
+
+        for (Path plugin : PluginsService.findPluginDirs(environment.pluginsFile())) {
+            addPolicy.accept(PolicyUtil.getPluginPolicyInfo(plugin, environment.tmpFile()));
+        }
+        for (Path plugin : PluginsService.findPluginDirs(environment.modulesFile())) {
+            addPolicy.accept(PolicyUtil.getModulePolicyInfo(plugin, environment.tmpFile()));
         }
 
         return Collections.unmodifiableMap(map);

x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java

@@ -36,7 +36,7 @@ public class ExampleSecurityExtension implements SecurityExtension {
     static {
         // check that the extension's policy works.
         AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
-            System.getSecurityManager().checkPrintJobAccess();
+            System.getSecurityManager().checkCreateClassLoader();
             return null;
         });
     }

x-pack/qa/security-example-spi-extension/src/main/plugin-metadata/plugin-security.policy

@@ -1,3 +1,4 @@
 grant {
-  permission java.lang.RuntimePermission "queuePrintJob";
+  // example security manager permission
+  permission java.lang.RuntimePermission "createClassLoader";
 };