Settings: Move keystore creation to plugin installation (#26329)

This commit removes the keystore creation on elasticsearch startup, and
instead adds a plugin property which indicates the plugin needs the
keystore to exist. It does still make sure the keystore.seed exists on
ES startup, but through an "upgrade" method that loading the keystore in
Bootstrap calls.

closes #26309

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

@@ -46,6 +46,10 @@ class PluginPropertiesExtension {
     @Input
     boolean hasClientJar = false
 
+    /** True if the plugin requires the elasticsearch keystore to exist, false otherwise. */
+    @Input
+    boolean requiresKeystore = false
+
     /** A license file that should be included in the built plugin zip. */
     @Input
     File licenseFile = null

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

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

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

@@ -42,3 +42,6 @@ elasticsearch.version=${elasticsearchVersion}
 #
 # 'has.native.controller': whether or not the plugin has a native controller
 has.native.controller=${hasNativeController}
+#
+# 'requires.keystore': whether or not the plugin needs the elasticsearch keystore be created
+requires.keystore=${requiresKeystore}

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

@@ -226,16 +226,13 @@ static SecureSettings loadSecureSettings(Environment initialEnv) throws Bootstra
         } catch (IOException e) {
             throw new BootstrapException(e);
         }
+        if (keystore == null) {
+            return null; // no keystore
+        }
 
         try {
-            if (keystore == null) {
-                // create it, we always want one! we use an empty passphrase, but a user can change this later if they want.
-                KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create(new char[0]);
-                keyStoreWrapper.save(initialEnv.configFile());
-                return keyStoreWrapper;
-            } else {
-                keystore.decrypt(new char[0] /* TODO: read password from stdin */);
-            }
+            keystore.decrypt(new char[0] /* TODO: read password from stdin */);
+            KeyStoreWrapper.upgrade(keystore, initialEnv.configFile());
         } catch (Exception e) {
             throw new BootstrapException(e);
         }

core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java

@@ -154,7 +154,7 @@ private KeyStoreWrapper(int formatVersion, boolean hasPassword, String type,
     }
 
     /** Returns a path representing the ES keystore in the given config dir. */
-    static Path keystorePath(Path configDir) {
+    public static Path keystorePath(Path configDir) {
         return configDir.resolve(KEYSTORE_FILENAME);
     }
 
@@ -171,7 +171,8 @@ public static KeyStoreWrapper create(char[] password) throws Exception {
     }
 
     /** Add the bootstrap seed setting, which may be used as a unique, secure, random value by the node */
-    private static void addBootstrapSeed(KeyStoreWrapper wrapper) throws GeneralSecurityException {
+    public static void addBootstrapSeed(KeyStoreWrapper wrapper) throws GeneralSecurityException {
+        assert wrapper.getSettingNames().contains(SEED_SETTING.getKey()) == false;
         SecureRandom random = Randomness.createSecure();
         int passwordLength = 20; // Generate 20 character passwords
         char[] characters = new char[passwordLength];
@@ -227,6 +228,16 @@ public static KeyStoreWrapper load(Path configDir) throws IOException {
         }
     }
 
+    /** Upgrades the format of the keystore, if necessary. */
+    public static void upgrade(KeyStoreWrapper wrapper, Path configDir) throws Exception {
+        // ensure keystore.seed exists
+        if (wrapper.getSettingNames().contains(SEED_SETTING.getKey())) {
+            return;
+        }
+        addBootstrapSeed(wrapper);
+        wrapper.save(configDir);
+    }
+
     @Override
     public boolean isLoaded() {
         return keystore.get() != null;
@@ -252,11 +263,9 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio
         } finally {
             Arrays.fill(keystoreBytes, (byte)0);
         }
-
         keystorePassword.set(new KeyStore.PasswordProtection(password));
         Arrays.fill(password, '\0');
 
-
         Enumeration<String> aliases = keystore.get().aliases();
         if (formatVersion == 1) {
             while (aliases.hasMoreElements()) {
@@ -279,6 +288,7 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio
 
     /** Write the keystore to the given config directory. */
     public void save(Path configDir) throws Exception {
+        assert isLoaded();
         char[] password = this.keystorePassword.get().getPassword();
 
         SimpleFSDirectory directory = new SimpleFSDirectory(configDir);
@@ -332,6 +342,7 @@ public Set<String> getSettingNames() {
     // TODO: make settings accessible only to code that registered the setting
     @Override
     public SecureString getString(String setting) throws GeneralSecurityException {
+        assert isLoaded();
         KeyStore.Entry entry = keystore.get().getEntry(setting, keystorePassword.get());
         if (settingTypes.get(setting) != KeyType.STRING ||
             entry instanceof KeyStore.SecretKeyEntry == false) {
@@ -347,6 +358,7 @@ public SecureString getString(String setting) throws GeneralSecurityException {
 
     @Override
     public InputStream getFile(String setting) throws GeneralSecurityException {
+        assert isLoaded();
         KeyStore.Entry entry = keystore.get().getEntry(setting, keystorePassword.get());
         if (settingTypes.get(setting) != KeyType.FILE ||
             entry instanceof KeyStore.SecretKeyEntry == false) {
@@ -377,6 +389,7 @@ public void close() throws IOException {
      * @throws IllegalArgumentException if the value is not ASCII
      */
     void setString(String setting, char[] value) throws GeneralSecurityException {
+        assert isLoaded();
         if (ASCII_ENCODER.canEncode(CharBuffer.wrap(value)) == false) {
             throw new IllegalArgumentException("Value must be ascii");
         }
@@ -387,6 +400,7 @@ void setString(String setting, char[] value) throws GeneralSecurityException {
 
     /** Set a file setting. */
     void setFile(String setting, byte[] bytes) throws GeneralSecurityException {
+        assert isLoaded();
         bytes = Base64.getEncoder().encode(bytes);
         char[] chars = new char[bytes.length];
         for (int i = 0; i < chars.length; ++i) {
@@ -399,6 +413,7 @@ void setFile(String setting, byte[] bytes) throws GeneralSecurityException {
 
     /** Remove the given setting from the keystore. */
     void remove(String setting) throws KeyStoreException {
+        assert isLoaded();
         keystore.get().deleteEntry(setting);
         settingTypes.remove(setting);
     }

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

@@ -21,7 +21,7 @@
 public class DummyPluginInfo extends PluginInfo {
 
     private DummyPluginInfo(String name, String description, String version, String classname) {
-        super(name, description, version, classname, false);
+        super(name, description, version, classname, false, false);
     }
 
     public static final DummyPluginInfo INSTANCE =

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

@@ -21,6 +21,7 @@
 
 import org.elasticsearch.Version;
 import org.elasticsearch.bootstrap.JarHell;
+import org.elasticsearch.common.Booleans;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -48,6 +49,7 @@ public class PluginInfo implements Writeable, ToXContentObject {
     private final String version;
     private final String classname;
     private final boolean hasNativeController;
+    private final boolean requiresKeystore;
 
     /**
      * Construct plugin info.
@@ -57,18 +59,16 @@ public class PluginInfo implements Writeable, ToXContentObject {
      * @param version             the version of Elasticsearch the plugin is built for
      * @param classname           the entry point to the plugin
      * @param hasNativeController whether or not the plugin has a native controller
+     * @param requiresKeystore    whether or not the plugin requires the elasticsearch keystore to be created
      */
-    public PluginInfo(
-            final String name,
-            final String description,
-            final String version,
-            final String classname,
-            final boolean hasNativeController) {
+    public PluginInfo(String name, String description, String version, String classname,
+                      boolean hasNativeController, boolean requiresKeystore) {
         this.name = name;
         this.description = description;
         this.version = version;
         this.classname = classname;
         this.hasNativeController = hasNativeController;
+        this.requiresKeystore = requiresKeystore;
     }
 
     /**
@@ -87,6 +87,11 @@ public PluginInfo(final StreamInput in) throws IOException {
         } else {
             hasNativeController = false;
         }
+        if (in.getVersion().onOrAfter(Version.V_6_0_0_beta2)) {
+            requiresKeystore = in.readBoolean();
+        } else {
+            requiresKeystore = false;
+        }
     }
 
     @Override
@@ -98,6 +103,9 @@ public void writeTo(final StreamOutput out) throws IOException {
         if (out.getVersion().onOrAfter(Version.V_5_4_0)) {
             out.writeBoolean(hasNativeController);
         }
+        if (out.getVersion().onOrAfter(Version.V_6_0_0_beta2)) {
+            out.writeBoolean(requiresKeystore);
+        }
     }
 
     /** reads (and validates) plugin metadata descriptor file */
@@ -173,17 +181,26 @@ public static PluginInfo readFromProperties(final Path path) throws IOException
                     break;
                 default:
                     final String message = String.format(
-                            Locale.ROOT,
-                            "property [%s] must be [%s], [%s], or unspecified but was [%s]",
-                            "has_native_controller",
-                            "true",
-                            "false",
-                            hasNativeControllerValue);
+                        Locale.ROOT,
+                        "property [%s] must be [%s], [%s], or unspecified but was [%s]",
+                        "has_native_controller",
+                        "true",
+                        "false",
+                        hasNativeControllerValue);
                     throw new IllegalArgumentException(message);
             }
         }
 
-        return new PluginInfo(name, description, version, classname, hasNativeController);
+        final String requiresKeystoreValue = props.getProperty("requires.keystore", "false");
+        final boolean requiresKeystore;
+        try {
+            requiresKeystore = Booleans.parseBoolean(requiresKeystoreValue);
+        } catch (IllegalArgumentException e) {
+            throw new IllegalArgumentException("property [requires.keystore] must be [true] or [false]," +
+                                               " but was [" + requiresKeystoreValue + "]", e);
+        }
+
+        return new PluginInfo(name, description, version, classname, hasNativeController, requiresKeystore);
     }
 
     /**
@@ -231,6 +248,15 @@ public boolean hasNativeController() {
         return hasNativeController;
     }
 
+    /**
+     * Whether or not the plugin requires the elasticsearch keystore to exist.
+     *
+     * @return {@code true} if the plugin requires a keystore, {@code false} otherwise
+     */
+    public boolean requiresKeystore() {
+        return requiresKeystore;
+    }
+
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
@@ -240,6 +266,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             builder.field("description", description);
             builder.field("classname", classname);
             builder.field("has_native_controller", hasNativeController);
+            builder.field("requires_keystore", requiresKeystore);
         }
         builder.endObject();
 
@@ -272,6 +299,7 @@ public String toString() {
                 .append("Description: ").append(description).append("\n")
                 .append("Version: ").append(version).append("\n")
                 .append("Native Controller: ").append(hasNativeController).append("\n")
+                .append("Requires Keystore: ").append(requiresKeystore).append("\n")
                 .append(" * Classname: ").append(classname);
         return information.toString();
     }

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

@@ -102,7 +102,7 @@ public PluginsService(Settings settings, Path configPath, Path modulesDirectory,
         // first we load plugins that are on the classpath. this is for tests and transport clients
         for (Class<? extends Plugin> pluginClass : classpathPlugins) {
             Plugin plugin = loadPlugin(pluginClass, settings, configPath);
-            PluginInfo pluginInfo = new PluginInfo(pluginClass.getName(), "classpath plugin", "NA", pluginClass.getName(), false);
+            PluginInfo pluginInfo = new PluginInfo(pluginClass.getName(), "classpath plugin", "NA", pluginClass.getName(), false, false);
             if (logger.isTraceEnabled()) {
                 logger.trace("plugin loaded from classpath [{}]", pluginInfo);
             }

core/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java

@@ -50,13 +50,6 @@ public void setupEnv() throws IOException {
         env = KeyStoreCommandTestCase.setupEnv(true, fileSystems);
     }
 
-    public void testLoadSecureSettingsCreatesKeystore() throws BootstrapException {
-        final Path configPath = env.configFile();
-        assertFalse(Files.exists(configPath.resolve("elasticsearch.keystore")));
-        Bootstrap.loadSecureSettings(env);
-        assertTrue(Files.exists(configPath.resolve("elasticsearch.keystore")));
-    }
-
     public void testLoadSecureSettings() throws Exception {
         final Path configPath = env.configFile();
         final SecureString seed;

core/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java

@@ -69,8 +69,32 @@ public void testFileSettingExhaustiveBytes() throws Exception {
         }
     }
 
-    public void testKeystoreSeed() throws Exception {
+    public void testCreate() throws Exception {
         KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]);
         assertTrue(keystore.getSettingNames().contains(KeyStoreWrapper.SEED_SETTING.getKey()));
     }
+
+    public void testUpgradeNoop() throws Exception {
+        KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]);
+        SecureString seed = keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey());
+        keystore.save(env.configFile());
+        // upgrade does not overwrite seed
+        KeyStoreWrapper.upgrade(keystore, env.configFile());
+        assertEquals(seed.toString(), keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()).toString());
+        keystore = KeyStoreWrapper.load(env.configFile());
+        keystore.decrypt(new char[0]);
+        assertEquals(seed.toString(), keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()).toString());
+    }
+
+    public void testUpgradeAddsSeed() throws Exception {
+        KeyStoreWrapper keystore = KeyStoreWrapper.create(new char[0]);
+        keystore.remove(KeyStoreWrapper.SEED_SETTING.getKey());
+        keystore.save(env.configFile());
+        KeyStoreWrapper.upgrade(keystore, env.configFile());
+        SecureString seed = keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey());
+        assertNotNull(seed);
+        keystore = KeyStoreWrapper.load(env.configFile());
+        keystore.decrypt(new char[0]);
+        assertEquals(seed.toString(), keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()).toString());
+    }
 }

core/src/test/java/org/elasticsearch/nodesinfo/NodeInfoStreamingTests.java

@@ -143,13 +143,13 @@ private static NodeInfo createNodeInfo() {
             List<PluginInfo> plugins = new ArrayList<>();
             for (int i = 0; i < numPlugins; i++) {
                 plugins.add(new PluginInfo(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10),
-                        randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomBoolean()));
+                        randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomBoolean(), randomBoolean()));
             }
             int numModules = randomIntBetween(0, 5);
             List<PluginInfo> modules = new ArrayList<>();
             for (int i = 0; i < numModules; i++) {
                 modules.add(new PluginInfo(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10),
-                        randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomBoolean()));
+                        randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10), randomBoolean(), randomBoolean()));
             }
             pluginsAndModules = new PluginsAndModules(plugins, modules);
         }

core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java

@@ -209,11 +209,11 @@ public void testReadFromPropertiesJvmMissingClassname() throws Exception {
 
     public void testPluginListSorted() {
         List<PluginInfo> plugins = new ArrayList<>();
-        plugins.add(new PluginInfo("c", "foo", "dummy", "dummyclass", randomBoolean()));
-        plugins.add(new PluginInfo("b", "foo", "dummy", "dummyclass", randomBoolean()));
-        plugins.add(new PluginInfo("e", "foo", "dummy", "dummyclass", randomBoolean()));
-        plugins.add(new PluginInfo("a", "foo", "dummy", "dummyclass", randomBoolean()));
-        plugins.add(new PluginInfo("d", "foo", "dummy", "dummyclass", randomBoolean()));
+        plugins.add(new PluginInfo("c", "foo", "dummy", "dummyclass", randomBoolean(), randomBoolean()));
+        plugins.add(new PluginInfo("b", "foo", "dummy", "dummyclass", randomBoolean(), randomBoolean()));
+        plugins.add(new PluginInfo("e", "foo", "dummy", "dummyclass", randomBoolean(), randomBoolean()));
+        plugins.add(new PluginInfo("a", "foo", "dummy", "dummyclass", randomBoolean(), randomBoolean()));
+        plugins.add(new PluginInfo("d", "foo", "dummy", "dummyclass", randomBoolean(), randomBoolean()));
         PluginsAndModules pluginsInfo = new PluginsAndModules(plugins, Collections.emptyList());
 
 

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

@@ -34,6 +34,7 @@
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.hash.MessageDigests;
 import org.elasticsearch.common.io.FileSystemUtils;
+import org.elasticsearch.common.settings.KeyStoreWrapper;
 import org.elasticsearch.env.Environment;
 
 import java.io.BufferedReader;
@@ -572,6 +573,15 @@ public FileVisitResult postVisitDirectory(final Path dir, final IOException exc)
                 }
             });
 
+            if (info.requiresKeystore()) {
+                KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile());
+                if (keystore == null) {
+                    terminal.println("Elasticsearch keystore is required by plugin [" + info.getName() + "], creating...");
+                    keystore = KeyStoreWrapper.create(new char[0]);
+                    keystore.save(env.configFile());
+                }
+            }
+
             terminal.println("-> Installed " + info.getName());
 
         } catch (Exception installProblem) {