Modify permissions dialog for plugins

This commit modifies the handling of plugins that require special
permissions to cover a case that was not previously covered.

Relates #23742

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesExtension.groovy

@@ -39,6 +39,9 @@ class PluginPropertiesExtension {
     @Input
     String classname
 
+    @Input
+    boolean hasNativeController = false
+
     /** Indicates whether the plugin jar should be made available for the transport client. */
     @Input
     boolean hasClientJar = false

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy

@@ -79,7 +79,8 @@ class PluginPropertiesTask extends Copy {
             'version': stringSnap(extension.version),
             'elasticsearchVersion': stringSnap(VersionProperties.elasticsearch),
             'javaVersion': project.targetCompatibility as String,
-            'classname': extension.classname
+            'classname': extension.classname,
+            'hasNativeController': extension.hasNativeController
         ]
     }
 }

buildSrc/src/main/resources/checkstyle_suppressions.xml

@@ -605,7 +605,6 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]JavaVersion.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]Natives.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]Security.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]Spawner.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]StartupException.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]SystemCallFilter.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]cli[/\\]Command.java" checks="LineLength" />
@@ -1565,14 +1564,12 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]AnalysisPlugin.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]ClusterPlugin.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]DiscoveryPlugin.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]DummyPluginInfo.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]IngestPlugin.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]InstallPluginCommand.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]ListPluginsCommand.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]MetaDataUpgrader.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]NetworkPlugin.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]Plugin.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]PluginInfo.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]PluginSecurity.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]PluginsService.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]ProgressInputStream.java" checks="LineLength" />
@@ -2388,7 +2385,6 @@
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]JNANativesTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]JarHellTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]MaxMapCountCheckTests.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]SpawnerTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]broadcast[/\\]BroadcastActionsIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]bwcompat[/\\]OldIndexBackwardsCompatibilityIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]bwcompat[/\\]RecoveryWithUnsupportedIndicesIT.java" checks="LineLength" />
@@ -3009,7 +3005,6 @@
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]nodesinfo[/\\]NodeInfoStreamingTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]nodesinfo[/\\]SimpleNodesInfoIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]operateAllIndices[/\\]DestructiveOperationsIT.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]PluginInfoTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]PluginsServiceTests.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]recovery[/\\]FullRollingRestartIT.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]recovery[/\\]RecoveriesCollectionTests.java" checks="LineLength" />
@@ -3949,11 +3944,8 @@
   <suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]logging[/\\]EvilLoggerTests.java" checks="LineLength" />
   <suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]env[/\\]NodeEnvironmentEvilTests.java" checks="LineLength" />
   <suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]InstallPluginCommandTests.java" checks="LineLength" />
-  <suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]ListPluginsCommandTests.java" checks="LineLength" />
-  <suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]PluginSecurityTests.java" checks="LineLength" />
   <suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]RemovePluginCommandTests.java" checks="LineLength" />
   <suppress files="qa[/\\]evil-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]tribe[/\\]TribeUnitTests.java" checks="LineLength" />
-  <suppress files="qa[/\\]no-bootstrap-tests[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]SpawnerNoBootstrapTests.java" checks="LineLength" />
   <suppress files="qa[/\\]smoke-test-client[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]smoketest[/\\]ESSmokeClientTestCase.java" checks="LineLength" />
   <suppress files="qa[/\\]smoke-test-client[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]smoketest[/\\]SmokeTestClientIT.java" checks="LineLength" />
   <suppress files="qa[/\\]smoke-test-http[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]http[/\\]ContextAndHeaderTransportIT.java" checks="LineLength" />

buildSrc/src/main/resources/plugin-descriptor.properties

@@ -30,11 +30,15 @@ name=${name}
 # 'classname': the name of the class to load, fully-qualified.
 classname=${classname}
 #
-# 'java.version' version of java the code is built against
+# 'java.version': version of java the code is built against
 # use the system property java.specification.version
 # version string must be a sequence of nonnegative decimal integers
 # separated by "."'s and may have leading zeros
 java.version=${javaVersion}
 #
-# 'elasticsearch.version' version of elasticsearch compiled against
+# 'elasticsearch.version': version of elasticsearch compiled against
 elasticsearch.version=${elasticsearchVersion}
+### optional elements for plugins:
+#
+# 'has.native.controller': whether or not the plugin has a native controller
+has.native.controller=${hasNativeController}

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

@@ -19,9 +19,10 @@
 
 package org.elasticsearch.bootstrap;
 
-import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.IOUtils;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.plugins.PluginInfo;
+import org.elasticsearch.plugins.Platforms;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -32,97 +33,89 @@
 import java.util.Collections;
 import java.util.List;
 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 filter being installed.
+ * Spawns native plugin controller processes if present. Will only work prior to a system call
+ * filter being installed.
  */
 final class Spawner implements Closeable {
 
-    private static final String PROGRAM_NAME = Constants.WINDOWS ? "controller.exe" : "controller";
-    private static final String PLATFORM_NAME = makePlatformName(Constants.OS_NAME, Constants.OS_ARCH);
-    private static final String TMP_ENVVAR = "TMPDIR";
-
-    /**
+    /*
      * References to the processes that have been spawned, so that we can destroy them.
      */
     private final List<Process> processes = new ArrayList<>();
+    private AtomicBoolean spawned = new AtomicBoolean();
 
     @Override
     public void close() throws IOException {
-        try {
-            IOUtils.close(() -> processes.stream().map(s -> (Closeable)s::destroy).iterator());
-        } finally {
-            processes.clear();
-        }
+        IOUtils.close(() -> processes.stream().map(s -> (Closeable) s::destroy).iterator());
     }
 
     /**
-     * For each plugin, attempt to spawn the controller daemon.  Silently ignore any plugins
-     * that don't include a controller for the correct platform.
+     * Spawns the native controllers for each plugin
+     *
+     * @param environment the node environment
+     * @throws IOException if an I/O error occurs reading the plugins or spawning a native process
      */
-    void spawnNativePluginControllers(Environment environment) throws IOException {
-        if (Files.exists(environment.pluginsFile())) {
-            try (DirectoryStream<Path> stream = Files.newDirectoryStream(environment.pluginsFile())) {
-                for (Path plugin : stream) {
-                    Path spawnPath = makeSpawnPath(plugin);
-                    if (Files.isRegularFile(spawnPath)) {
-                        spawnNativePluginController(spawnPath, environment.tmpFile());
-                    }
+    void spawnNativePluginControllers(final Environment environment) throws IOException {
+        if (!spawned.compareAndSet(false, true)) {
+            throw new IllegalStateException("native controllers already spawned");
+        }
+        final Path pluginsFile = environment.pluginsFile();
+        if (!Files.exists(pluginsFile)) {
+            throw new IllegalStateException("plugins directory [" + pluginsFile + "] not found");
+        }
+        /*
+         * For each plugin, attempt to spawn the controller daemon. Silently ignore any plugin that
+         * don't include a controller for the correct platform.
+         */
+        try (DirectoryStream<Path> stream = Files.newDirectoryStream(pluginsFile)) {
+            for (final Path plugin : stream) {
+                final PluginInfo info = PluginInfo.readFromProperties(plugin);
+                final Path spawnPath = Platforms.nativeControllerPath(plugin);
+                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",
+                            plugin.getFileName());
+                    throw new IllegalArgumentException(message);
                 }
+                final Process process =
+                        spawnNativePluginController(spawnPath, environment.tmpFile());
+                processes.add(process);
             }
         }
     }
 
     /**
-     * Attempt to spawn the controller daemon for a given plugin.  The spawned process
-     * will remain connected to this JVM via its stdin, stdout and stderr, but the
-     * references to these streams are not available to code outside this package.
+     * Attempt to spawn the controller daemon for a given plugin. The spawned process will remain
+     * connected to this JVM via its stdin, stdout, and stderr streams, but the references to these
+     * streams are not available to code outside this package.
      */
-    private void spawnNativePluginController(Path spawnPath, Path tmpPath) throws IOException {
-        ProcessBuilder pb = new ProcessBuilder(spawnPath.toString());
+    private Process spawnNativePluginController(
+            final Path spawnPath,
+            final Path tmpPath) throws IOException {
+        final ProcessBuilder pb = new ProcessBuilder(spawnPath.toString());
 
-        // The only environment variable passes on the path to the temporary directory
+        // the only environment variable passes on the path to the temporary directory
         pb.environment().clear();
-        pb.environment().put(TMP_ENVVAR, tmpPath.toString());
+        pb.environment().put("TMPDIR", tmpPath.toString());
 
-        // The output stream of the Process object corresponds to the daemon's stdin
-        processes.add(pb.start());
-    }
-
-    List<Process> getProcesses() {
-        return Collections.unmodifiableList(processes);
+        // the output stream of the process object corresponds to the daemon's stdin
+        return pb.start();
     }
 
     /**
-     * Make the full path to the program to be spawned.
+     * The collection of processes representing spawned native controllers.
+     *
+     * @return the processes
      */
-    static Path makeSpawnPath(Path plugin) {
-        return plugin.resolve("platform").resolve(PLATFORM_NAME).resolve("bin").resolve(PROGRAM_NAME);
+    List<Process> getProcesses() {
+        return Collections.unmodifiableList(processes);
     }
 
-    /**
-     * Make the platform name in the format used in Kibana downloads, for example:
-     * - darwin-x86_64
-     * - linux-x86-64
-     * - windows-x86_64
-     * For *nix platforms this is more-or-less `uname -s`-`uname -m` converted to lower case.
-     * However, for consistency between different operating systems on the same architecture
-     * "amd64" is replaced with "x86_64" and "i386" with "x86".
-     * For Windows it's "windows-" followed by either "x86" or "x86_64".
-     */
-    static String makePlatformName(String osName, String osArch) {
-        String os = osName.toLowerCase(Locale.ROOT);
-        if (os.startsWith("windows")) {
-            os = "windows";
-        } else if (os.equals("mac os x")) {
-            os = "darwin";
-        }
-        String cpu = osArch.toLowerCase(Locale.ROOT);
-        if (cpu.equals("amd64")) {
-            cpu = "x86_64";
-        } else if (cpu.equals("i386")) {
-            cpu = "x86";
-        }
-        return os + "-" + cpu;
-    }
 }

core/src/main/java/org/elasticsearch/plugins/DummyPluginInfo.java

@@ -21,9 +21,13 @@
 public class DummyPluginInfo extends PluginInfo {
 
     private DummyPluginInfo(String name, String description, String version, String classname) {
-        super(name, description, version, classname);
+        super(name, description, version, classname, false);
     }
 
-    public static final DummyPluginInfo INSTANCE = new DummyPluginInfo(
-            "dummy_plugin_name", "dummy plugin description", "dummy_plugin_version", "DummyPluginName");
+    public static final DummyPluginInfo INSTANCE =
+            new DummyPluginInfo(
+                    "dummy_plugin_name",
+                    "dummy plugin description",
+                    "dummy_plugin_version",
+                    "DummyPluginName");
 }

core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

@@ -458,7 +458,7 @@ private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch, E
         // if it exists, confirm or warn the user
         Path policy = pluginRoot.resolve(PluginInfo.ES_PLUGIN_POLICY);
         if (Files.exists(policy)) {
-            PluginSecurity.readPolicy(policy, terminal, env, isBatch);
+            PluginSecurity.readPolicy(info, policy, terminal, env::tmpFile, isBatch);
         }
 
         return info;

core/src/main/java/org/elasticsearch/plugins/ListPluginsCommand.java

@@ -61,7 +61,7 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th
                 PluginInfo info = PluginInfo.readFromProperties(env.pluginsFile().resolve(plugin.toAbsolutePath()));
                 terminal.println(Terminal.Verbosity.VERBOSE, info.toString());
             } catch (IllegalArgumentException e) {
-                if (e.getMessage().contains("incompatible with Elasticsearch")) {
+                if (e.getMessage().contains("incompatible with version")) {
                     terminal.println("WARNING: " + e.getMessage());
                 } else {
                     throw e;

core/src/main/java/org/elasticsearch/plugins/Platforms.java

@@ -0,0 +1,83 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.plugins;
+
+import org.apache.lucene.util.Constants;
+
+import java.nio.file.Path;
+import java.util.Locale;
+
+/**
+ * Encapsulates platform-dependent methods for handling native components of plugins.
+ */
+public class Platforms {
+
+    private static final String PROGRAM_NAME = Constants.WINDOWS ? "controller.exe" : "controller";
+    private static final String PLATFORM_NAME =
+            Platforms.platformName(Constants.OS_NAME, Constants.OS_ARCH);
+
+    private Platforms() {}
+
+    /**
+     * The path to the native controller for a plugin with native components.
+     */
+    public static Path nativeControllerPath(Path plugin) {
+        return plugin
+                .resolve("platform")
+                .resolve(PLATFORM_NAME)
+                .resolve("bin")
+                .resolve(PROGRAM_NAME);
+    }
+
+    /**
+     * Return the platform name based on the OS name and
+     * - darwin-x86_64
+     * - linux-x86-64
+     * - windows-x86_64
+     * For *nix platforms this is more-or-less `uname -s`-`uname -m` converted to lower case.
+     * However, for consistency between different operating systems on the same architecture
+     * "amd64" is replaced with "x86_64" and "i386" with "x86".
+     * For Windows it's "windows-" followed by either "x86" or "x86_64".
+     */
+    public static String platformName(final String osName, final String osArch) {
+        final String lowerCaseOs = osName.toLowerCase(Locale.ROOT);
+        final String normalizedOs;
+        if (lowerCaseOs.startsWith("windows")) {
+            normalizedOs = "windows";
+        } else if (lowerCaseOs.equals("mac os x")) {
+            normalizedOs = "darwin";
+        } else {
+            normalizedOs = lowerCaseOs;
+        }
+
+        final String lowerCaseArch = osArch.toLowerCase(Locale.ROOT);
+        final String normalizedArch;
+        if (lowerCaseArch.equals("amd64")) {
+            normalizedArch = "x86_64";
+        } else if (lowerCaseArch.equals("i386")) {
+            normalizedArch = "x86";
+        } else {
+            normalizedArch = lowerCaseArch;
+        }
+
+        return normalizedOs + "-" + normalizedArch;
+    }
+
+}