Only enable modules to have native controllers

This commit removes the ability for a plugin to have a native controller
as leaves it as only modules can have a native controller.

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

@@ -590,6 +590,9 @@ class InstallPluginCommand extends EnvironmentAwareCommand {
     /** Load information about the plugin, and verify it can be installed with no errors. */
     private PluginInfo loadPluginInfo(Terminal terminal, Path pluginRoot, boolean isBatch, Environment env) throws Exception {
         final PluginInfo info = PluginInfo.readFromProperties(pluginRoot);
+        if (info.hasNativeController()) {
+            throw new IllegalStateException("plugins can not have native controllers");
+        }
         PluginsService.verifyCompatibility(info);
 
         // checking for existing version of the plugin
@@ -678,19 +681,16 @@ class InstallPluginCommand extends EnvironmentAwareCommand {
 
         Set<String> permissions = new HashSet<>();
         final List<PluginInfo> pluginInfos = new ArrayList<>();
-        boolean hasNativeController = false;
         for (Path plugin : pluginPaths) {
             final PluginInfo info = loadPluginInfo(terminal, plugin, isBatch, env);
             pluginInfos.add(info);
 
-            hasNativeController |= info.hasNativeController();
-
             Path policy = plugin.resolve(PluginInfo.ES_PLUGIN_POLICY);
             if (Files.exists(policy)) {
                 permissions.addAll(PluginSecurity.parsePermissions(policy, env.tmpFile()));
             }
         }
-        PluginSecurity.confirmPolicyExceptions(terminal, permissions, hasNativeController, isBatch);
+        PluginSecurity.confirmPolicyExceptions(terminal, permissions, isBatch);
 
         // move support files and rename as needed to prepare the exploded plugin for its final location
         for (int i = 0; i < pluginPaths.size(); ++i) {
@@ -723,7 +723,7 @@ class InstallPluginCommand extends EnvironmentAwareCommand {
         } else {
             permissions = Collections.emptySet();
         }
-        PluginSecurity.confirmPolicyExceptions(terminal, permissions, info.hasNativeController(), isBatch);
+        PluginSecurity.confirmPolicyExceptions(terminal, permissions, isBatch);
 
         final Path destination = env.pluginsFile().resolve(info.getName());
         deleteOnFailure.add(destination);

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

@@ -1242,42 +1242,16 @@ public class InstallPluginCommandTests extends ESTestCase {
         assertMetaPlugin("meta-plugin", "fake2", metaDir, env.v2());
     }
 
-    public void testNativeControllerConfirmation() throws Exception {
+    public void testPluginWithNativeController() throws Exception {
         Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
         String pluginZip = createPluginUrl("fake", pluginDir, "has.native.controller", "true");
 
-        assertPolicyConfirmation(env, pluginZip, "plugin forks a native controller");
+        final IllegalStateException e = expectThrows(IllegalStateException.class, () -> installPlugin(pluginZip, env.v1()));
-        assertPlugin("fake", pluginDir, env.v2());
+        assertThat(e, hasToString(containsString("plugins can not have native controllers")));
     }
 
-    public void testMetaPluginNativeControllerConfirmation() throws Exception {
+    public void testMetaPluginWithNativeController() throws Exception {
-        Tuple<Path, Environment> env = createEnv(fs, temp);
-        Path metaDir = createPluginDir(temp);
-        Path fake1Dir = metaDir.resolve("fake1");
-        Files.createDirectory(fake1Dir);
-        writePlugin("fake1", fake1Dir, "has.native.controller", "true");
-        Path fake2Dir = metaDir.resolve("fake2");
-        Files.createDirectory(fake2Dir);
-        writePlugin("fake2", fake2Dir);
-        String pluginZip = createMetaPluginUrl("meta-plugin", metaDir);
-
-        assertPolicyConfirmation(env, pluginZip, "plugin forks a native controller");
-        assertMetaPlugin("meta-plugin", "fake1", metaDir, env.v2());
-        assertMetaPlugin("meta-plugin", "fake2", metaDir, env.v2());
-    }
-
-    public void testNativeControllerAndPolicyConfirmation() throws Exception {
-        Tuple<Path, Environment> env = createEnv(fs, temp);
-        Path pluginDir = createPluginDir(temp);
-        writePluginSecurityPolicy(pluginDir, "setAccessible", "setFactory");
-        String pluginZip = createPluginUrl("fake", pluginDir, "has.native.controller", "true");
-
-        assertPolicyConfirmation(env, pluginZip, "plugin requires additional permissions", "plugin forks a native controller");
-        assertPlugin("fake", pluginDir, env.v2());
-    }
-
-    public void testMetaPluginNativeControllerAndPolicyConfirmation() throws Exception {
         Tuple<Path, Environment> env = createEnv(fs, temp);
         Path metaDir = createPluginDir(temp);
         Path fake1Dir = metaDir.resolve("fake1");
@@ -1289,8 +1263,8 @@ public class InstallPluginCommandTests extends ESTestCase {
         writePlugin("fake2", fake2Dir, "has.native.controller", "true");
         String pluginZip = createMetaPluginUrl("meta-plugin", metaDir);
 
-        assertPolicyConfirmation(env, pluginZip, "plugin requires additional permissions", "plugin forks a native controller");
+        final IllegalStateException e = expectThrows(IllegalStateException.class, () -> installPlugin(pluginZip, env.v1()));
-        assertMetaPlugin("meta-plugin", "fake1", metaDir, env.v2());
+        assertThat(e, hasToString(containsString("plugins can not have native controllers")));
-        assertMetaPlugin("meta-plugin", "fake2", metaDir, env.v2());
     }
+
 }

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

@@ -19,62 +19,17 @@
 
 package org.elasticsearch.plugins;
 
-import org.elasticsearch.cli.MockTerminal;
-import org.elasticsearch.cli.UserException;
 import org.elasticsearch.test.ESTestCase;
 
-import java.io.IOException;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Set;
 
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.hasToString;
-import static org.hamcrest.Matchers.not;
 
 /** Tests plugin manager security check */
 public class PluginSecurityTests extends ESTestCase {
 
-    public void testHasNativeController() throws Exception {
-        assumeTrue(
-                "test cannot run with security manager enabled",
-                System.getSecurityManager() == null);
-        final MockTerminal terminal = new MockTerminal();
-        terminal.addTextInput("y");
-        terminal.addTextInput("y");
-        final Path policyFile = this.getDataPath("security/simple-plugin-security.policy");
-        Set<String> permissions = PluginSecurity.parsePermissions(policyFile, createTempDir());
-        PluginSecurity.confirmPolicyExceptions(terminal, permissions, true, false);
-        final String output = terminal.getOutput();
-        assertThat(output, containsString("plugin forks a native controller"));
-    }
-
-    public void testDeclineNativeController() throws IOException {
-        assumeTrue("test cannot run with security manager enabled", System.getSecurityManager() == null);
-        final MockTerminal terminal = new MockTerminal();
-        terminal.addTextInput("y");
-        terminal.addTextInput("n");
-        final Path policyFile = this.getDataPath("security/simple-plugin-security.policy");
-        Set<String> permissions = PluginSecurity.parsePermissions(policyFile, createTempDir());
-        UserException e = expectThrows(UserException.class,
-                () -> PluginSecurity.confirmPolicyExceptions(terminal, permissions, true, false));
-        assertThat(e, hasToString(containsString("installation aborted by user")));
-    }
-
-    public void testDoesNotHaveNativeController() throws Exception {
-        assumeTrue("test cannot run with security manager enabled", System.getSecurityManager() == null);
-        final MockTerminal terminal = new MockTerminal();
-        terminal.addTextInput("y");
-        final Path policyFile = this.getDataPath("security/simple-plugin-security.policy");
-        Set<String> permissions = PluginSecurity.parsePermissions(policyFile, createTempDir());
-        PluginSecurity.confirmPolicyExceptions(terminal, permissions, false, false);
-        final String output = terminal.getOutput();
-        assertThat(output, not(containsString("plugin forks a native controller")));
-    }
-
     /** Test that we can parse the set of permissions correctly for a simple policy */
     public void testParsePermissions() throws Exception {
         assumeTrue(

qa/no-bootstrap-tests/src/test/java/org/elasticsearch/bootstrap/SpawnerNoBootstrapTests.java

@@ -66,7 +66,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
             + "read SOMETHING\n";
 
     /**
-     * Simplest case: a plugin with no controller daemon.
+     * Simplest case: a module with no controller daemon.
      */
     public void testNoControllerSpawn() throws IOException, InterruptedException {
         Path esHome = createTempDir().resolve("esHome");
@@ -77,7 +77,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
         Environment environment = TestEnvironment.newEnvironment(settings);
 
         // This plugin will NOT have a controller daemon
-        Path plugin = environment.pluginsFile().resolve("a_plugin");
+        Path plugin = environment.modulesFile().resolve("a_plugin");
         Files.createDirectories(environment.modulesFile());
         Files.createDirectories(plugin);
         PluginTestUtil.writePluginProperties(
@@ -100,11 +100,11 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
      * Two plugins - one with a controller daemon and one without.
      */
     public void testControllerSpawn() throws Exception {
-        assertControllerSpawns(Environment::pluginsFile);
+        assertControllerSpawns(Environment::pluginsFile, false);
-        assertControllerSpawns(Environment::modulesFile);
+        assertControllerSpawns(Environment::modulesFile, true);
     }
 
-    private void assertControllerSpawns(Function<Environment, Path> pluginsDirFinder) throws Exception {
+    private void assertControllerSpawns(final Function<Environment, Path> pluginsDirFinder, boolean expectSpawn) throws Exception {
         /*
          * On Windows you can not directly run a batch file - you have to run cmd.exe with the batch
          * file as an argument and that's out of the remit of the controller daemon process spawner.
@@ -152,30 +152,35 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
         spawner.spawnNativeControllers(environment);
 
         List<Process> processes = spawner.getProcesses();
-        /*
+
-         * As there should only be a reference in the list for the plugin that had the controller
+        if (expectSpawn) {
-         * daemon, we expect one here.
+             // as there should only be a reference in the list for the module that had the controller daemon, we expect one here
-         */
             assertThat(processes, hasSize(1));
             Process process = processes.get(0);
-        final InputStreamReader in =
+            final InputStreamReader in = new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8);
-            new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8);
             try (BufferedReader stdoutReader = new BufferedReader(in)) {
                 String line = stdoutReader.readLine();
                 assertEquals("I am alive", line);
                 spawner.close();
-            /*
+                // fail if the process does not die within one second; usually it will be even quicker but it depends on OS scheduling
-             * Fail if the process does not die within one second; usually it will be even quicker
-             * but it depends on OS scheduling.
-             */
                 assertTrue(process.waitFor(1, TimeUnit.SECONDS));
             }
+        } else {
+            assertThat(processes, hasSize(0));
+        }
     }
 
     /**
-     * Two plugins in a meta plugin - one with a controller daemon and one without.
+     * Two plugins in a meta module - one with a controller daemon and one without.
      */
-    public void testControllerSpawnMetaPlugin() throws IOException, InterruptedException {
+    public void testControllerSpawnMeta() throws Exception {
+        runTestControllerSpawnMeta(Environment::pluginsFile, false);
+        runTestControllerSpawnMeta(Environment::modulesFile, true);
+    }
+
+
+    private void runTestControllerSpawnMeta(
+            final Function<Environment, Path> pluginsDirFinder, final boolean expectSpawn) throws Exception {
         /*
          * On Windows you can not directly run a batch file - you have to run cmd.exe with the batch
          * file as an argument and that's out of the remit of the controller daemon process spawner.
@@ -189,17 +194,17 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
 
         Environment environment = TestEnvironment.newEnvironment(settings);
 
-        Path metaPlugin = environment.pluginsFile().resolve("meta_plugin");
+        Path metaModule = pluginsDirFinder.apply(environment).resolve("meta_module");
         Files.createDirectories(environment.modulesFile());
-        Files.createDirectories(metaPlugin);
+        Files.createDirectories(metaModule);
         PluginTestUtil.writeMetaPluginProperties(
-            metaPlugin,
+                metaModule,
                 "description", "test_plugin",
                 "name", "meta_plugin",
                 "plugins", "test_plugin,other_plugin");
 
         // this plugin will have a controller daemon
-        Path plugin = metaPlugin.resolve("test_plugin");
+        Path plugin = metaModule.resolve("test_plugin");
 
         Files.createDirectories(plugin);
         PluginTestUtil.writePluginProperties(
@@ -215,7 +220,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
         createControllerProgram(controllerProgram);
 
         // this plugin will not have a controller daemon
-        Path otherPlugin = metaPlugin.resolve("other_plugin");
+        Path otherPlugin = metaModule.resolve("other_plugin");
         Files.createDirectories(otherPlugin);
         PluginTestUtil.writePluginProperties(
                 otherPlugin,
@@ -231,10 +236,9 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
         spawner.spawnNativeControllers(environment);
 
         List<Process> processes = spawner.getProcesses();
-        /*
+
-         * As there should only be a reference in the list for the plugin that had the controller
+        if (expectSpawn) {
-         * daemon, we expect one here.
+             // as there should only be a reference in the list for the plugin that had the controller daemon, we expect one here
-         */
             assertThat(processes, hasSize(1));
             Process process = processes.get(0);
             final InputStreamReader in =
@@ -243,12 +247,12 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
                 String line = stdoutReader.readLine();
                 assertEquals("I am alive", line);
                 spawner.close();
-            /*
+                // fail if the process does not die within one second; usually it will be even quicker but it depends on OS scheduling
-             * Fail if the process does not die within one second; usually it will be even quicker
-             * but it depends on OS scheduling.
-             */
                 assertTrue(process.waitFor(1, TimeUnit.SECONDS));
             }
+        } else {
+            assertThat(processes, hasSize(0));
+        }
     }
 
     public void testControllerSpawnWithIncorrectDescriptor() throws IOException {
@@ -260,7 +264,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
 
         Environment environment = TestEnvironment.newEnvironment(settings);
 
-        Path plugin = environment.pluginsFile().resolve("test_plugin");
+        Path plugin = environment.modulesFile().resolve("test_plugin");
         Files.createDirectories(plugin);
         PluginTestUtil.writePluginProperties(
                 plugin,
@@ -280,7 +284,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
                 () -> spawner.spawnNativeControllers(environment));
         assertThat(
                 e.getMessage(),
-                equalTo("plugin [test_plugin] does not have permission to fork native controller"));
+                equalTo("module [test_plugin] does not have permission to fork native controller"));
     }
 
     public void testSpawnerHandlingOfDesktopServicesStoreFiles() throws IOException {
@@ -292,7 +296,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
         Files.createDirectories(environment.modulesFile());
         Files.createDirectories(environment.pluginsFile());
 
-        final Path desktopServicesStore = environment.pluginsFile().resolve(".DS_Store");
+        final Path desktopServicesStore = environment.modulesFile().resolve(".DS_Store");
         Files.createFile(desktopServicesStore);
 
         final Spawner spawner = new Spawner();
@@ -301,8 +305,7 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
             spawner.spawnNativeControllers(environment);
         } else {
             // we do not ignore these files on non-macOS systems
-            final FileSystemException e =
+            final FileSystemException e = expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment));
-                    expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment));
             if (Constants.WINDOWS) {
                 assertThat(e, instanceOf(NoSuchFileException.class));
             } else {

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

@@ -37,8 +37,7 @@ import java.util.Locale;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
- * Spawns native plugin controller processes if present. Will only work prior to a system call
+ * Spawns native module controller processes if present. Will only work prior to a system call filter being installed.
- * filter being installed.
  */
 final class Spawner implements Closeable {
 
@@ -54,55 +53,46 @@ final class Spawner implements Closeable {
     }
 
     /**
-     * Spawns the native controllers for each plugin/module.
+     * Spawns the native controllers for each module.
      *
      * @param environment the node environment
-     * @throws IOException if an I/O error occurs reading the plugins or spawning a native process
+     * @throws IOException if an I/O error occurs reading the module or spawning a native process
      */
     void spawnNativeControllers(final Environment environment) throws IOException {
         if (!spawned.compareAndSet(false, true)) {
             throw new IllegalStateException("native controllers already spawned");
         }
-        spawnControllers(environment.pluginsFile(), "plugins", environment.tmpFile());
+        if (!Files.exists(environment.modulesFile())) {
-        spawnControllers(environment.modulesFile(), "modules", environment.tmpFile());
+            throw new IllegalStateException("modules directory [" + environment.modulesFile() + "] not found");
-    }
-
-    /** Spawn controllers in plugins found within the given directory. */
-    private void spawnControllers(Path pluginsDir, String type, Path tmpDir) throws IOException {
-        if (!Files.exists(pluginsDir)) {
-            throw new IllegalStateException(type + " directory [" + pluginsDir + "] not found");
         }
         /*
-         * For each plugin, attempt to spawn the controller daemon. Silently ignore any plugin that
+         * For each module, attempt to spawn the controller daemon. Silently ignore any module that doesn't include a controller for the
-         * don't include a controller for the correct platform.
+         * correct platform.
          */
-        List<Path> paths = PluginsService.findPluginDirs(pluginsDir);
+        List<Path> paths = PluginsService.findPluginDirs(environment.modulesFile());
-        for (Path plugin : paths) {
+        for (final Path modules : paths) {
-            final PluginInfo info = PluginInfo.readFromProperties(plugin);
+            final PluginInfo info = PluginInfo.readFromProperties(modules);
-            final Path spawnPath = Platforms.nativeControllerPath(plugin);
+            final Path spawnPath = Platforms.nativeControllerPath(modules);
             if (!Files.isRegularFile(spawnPath)) {
                 continue;
             }
             if (!info.hasNativeController()) {
                 final String message = String.format(
                     Locale.ROOT,
-                    "plugin [%s] does not have permission to fork native controller",
+                    "module [%s] does not have permission to fork native controller",
-                    plugin.getFileName());
+                    modules.getFileName());
                 throw new IllegalArgumentException(message);
             }
-            final Process process = spawnNativePluginController(spawnPath, tmpDir);
+            final Process process = spawnNativeController(spawnPath, environment.tmpFile());
             processes.add(process);
         }
     }
 
     /**
-     * Attempt to spawn the controller daemon for a given plugin. The spawned process will remain
+     * Attempt to spawn the controller daemon for a given module. The spawned process will remain connected to this JVM via its stdin,
-     * connected to this JVM via its stdin, stdout, and stderr streams, but the references to these
+     * stdout, and stderr streams, but the references to these streams are not available to code outside this package.
-     * streams are not available to code outside this package.
      */
-    private Process spawnNativePluginController(
+    private Process spawnNativeController(final Path spawnPath, final Path tmpPath) throws IOException {
-            final Path spawnPath,
-            final Path tmpPath) throws IOException {
         final String command;
         if (Constants.WINDOWS) {
             /*

server/src/main/java/org/elasticsearch/plugins/PluginSecurity.java

@@ -19,11 +19,11 @@
 
 package org.elasticsearch.plugins;
 
-import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.cli.ExitCodes;
 import org.elasticsearch.cli.Terminal;
 import org.elasticsearch.cli.Terminal.Verbosity;
 import org.elasticsearch.cli.UserException;
+import org.elasticsearch.core.internal.io.IOUtils;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -37,10 +37,8 @@ import java.security.URIParameter;
 import java.security.UnresolvedPermission;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.List;
 import java.util.Set;
-import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 class PluginSecurity {
@@ -48,8 +46,7 @@ class PluginSecurity {
     /**
      * prints/confirms policy exceptions with the user
      */
-    static void confirmPolicyExceptions(Terminal terminal, Set<String> permissions,
+    static void confirmPolicyExceptions(Terminal terminal, Set<String> permissions, boolean batch) throws UserException {
-                                        boolean needsNativeController, boolean batch) throws UserException {
         List<String> requested = new ArrayList<>(permissions);
         if (requested.isEmpty()) {
             terminal.println(Verbosity.VERBOSE, "plugin has a policy file with no additional permissions");
@@ -69,15 +66,6 @@ class PluginSecurity {
             terminal.println(Verbosity.NORMAL, "for descriptions of what these permissions allow and the associated risks.");
             prompt(terminal, batch);
         }
-
-        if (needsNativeController) {
-            terminal.println(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
-            terminal.println(Verbosity.NORMAL, "@        WARNING: plugin forks a native controller        @");
-            terminal.println(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
-            terminal.println(Verbosity.NORMAL, "This plugin launches a native controller that is not subject to the Java");
-            terminal.println(Verbosity.NORMAL, "security manager nor to system call filters.");
-            prompt(terminal, batch);
-        }
     }
 
     private static void prompt(final Terminal terminal, final boolean batch) throws UserException {