From fb690ef748977d6bc2d01fab76b14d223d32eb91 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 22 Dec 2016 16:28:34 -0800 Subject: [PATCH 1/6] Settings: Add infrastructure for elasticsearch keystore This change is the first towards providing the ability to store sensitive settings in elasticsearch. It adds the `elasticsearch-keystore` tool, which allows managing a java keystore. The keystore is loaded upon node startup in Elasticsearch, and used by the Setting infrastructure when a setting is configured as secure. There are a lot of caveats to this PR. The most important is it only provides the tool and setting infrastructure for secure strings. It does not yet provide for keystore passwords, keypairs, certificates, or even convert any existing string settings to secure string settings. Those will all come in follow up PRs. But this PR was already too big, so this at least gets a basic version of the infrastructure in. The two main things to look at. The first is the `SecureSetting` class, which extends `Setting`, but removes the assumption for the raw value of the setting to be a string. SecureSetting provides, for now, a single helper, `stringSetting()` to create a SecureSetting which will return a SecureString (which is like String, but is closeable, so that the underlying character array can be cleared). The second is the `KeyStoreWrapper` class, which wraps the java `KeyStore` to provide a simpler api (we do not need the entire keystore api) and also extend the serialized format to add metadata needed for loading the keystore with no assumptions about keystore type (so that we can change this in the future) as well as whether the keystore has a password (so that we can know whether prompting is necessary when we add support for keystore passwords). --- core/build.gradle | 2 + .../elasticsearch/bootstrap/Bootstrap.java | 38 ++- .../bootstrap/BootstrapException.java | 2 +- .../bootstrap/Elasticsearch.java | 6 +- .../settings/AddStringKeyStoreCommand.java | 86 +++++++ .../settings/CreateKeyStoreCommand.java | 62 +++++ .../common/settings/KeyStoreCli.java | 41 ++++ .../common/settings/KeyStoreWrapper.java | 218 ++++++++++++++++++ .../common/settings/ListKeyStoreCommand.java | 63 +++++ .../RemoveSettingKeyStoreCommand.java | 66 ++++++ .../common/settings/SecureSetting.java | 112 +++++++++ .../common/settings/SecureString.java | 105 +++++++++ .../common/settings/Setting.java | 15 +- .../common/settings/Settings.java | 42 +++- .../internal/InternalSettingsPreparer.java | 16 +- .../bootstrap/ElasticsearchCliTests.java | 4 +- .../AddStringKeyStoreCommandTests.java | 125 ++++++++++ .../settings/CreateKeyStoreCommandTests.java | 73 ++++++ .../settings/KeyStoreCommandTestCase.java | 98 ++++++++ .../settings/ListKeyStoreCommandTests.java | 67 ++++++ .../RemoveSettingKeyStoreCommandTests.java | 80 +++++++ .../main/resources/bin/elasticsearch-keystore | 91 ++++++++ .../resources/bin/elasticsearch-keystore.bat | 30 +++ docs/java-rest/configuration.asciidoc | 4 +- .../bootstrap/EvilElasticsearchCliTests.java | 5 +- .../bootstrap/ESElasticsearchCliTestCase.java | 6 +- 26 files changed, 1419 insertions(+), 38 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java create mode 100644 core/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java create mode 100644 core/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java create mode 100644 core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java create mode 100644 core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java create mode 100644 core/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java create mode 100644 core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java create mode 100644 core/src/main/java/org/elasticsearch/common/settings/SecureString.java create mode 100644 core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java create mode 100644 core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java create mode 100644 core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java create mode 100644 core/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java create mode 100644 core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java create mode 100755 distribution/src/main/resources/bin/elasticsearch-keystore create mode 100644 distribution/src/main/resources/bin/elasticsearch-keystore.bat diff --git a/core/build.gradle b/core/build.gradle index 7a580335571..6e0b94dd6f9 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -94,6 +94,8 @@ dependencies { exclude group: 'org.elasticsearch', module: 'elasticsearch' } } + testCompile 'com.google.jimfs:jimfs:1.1' + testCompile 'com.google.guava:guava:18.0' } if (isEclipse) { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 2ee2d69baf3..b4a95e01cc4 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -30,16 +30,15 @@ import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.StringHelper; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; -import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; import org.elasticsearch.common.PidFile; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.inject.CreationException; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.logging.LogConfigurator; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.KeyStoreWrapper; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.env.Environment; @@ -224,13 +223,34 @@ final class Bootstrap { }; } - private static Environment initialEnvironment(boolean foreground, Path pidFile, Settings initialSettings) { + private static KeyStoreWrapper loadKeyStore(Environment env0) throws BootstrapException { + final KeyStoreWrapper keystore; + try { + keystore = KeyStoreWrapper.loadMetadata(env0.configFile()); + } catch (IOException e) { + throw new BootstrapException(e); + } + if (keystore == null) { + return null; // no keystore + } + + try { + keystore.loadKeystore(new char[0] /* TODO: read password from stdin */); + } catch (Exception e) { + throw new BootstrapException(e); + } + return keystore; + } + + private static Environment initialEnvironment(boolean foreground, Path pidFile, + KeyStoreWrapper keystore, Settings initialSettings) { Terminal terminal = foreground ? Terminal.DEFAULT : null; Settings.Builder builder = Settings.builder(); if (pidFile != null) { builder.put(Environment.PIDFILE_SETTING.getKey(), pidFile); } builder.put(initialSettings); + builder.setKeyStore(keystore); return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal, Collections.emptyMap()); } @@ -261,7 +281,7 @@ final class Bootstrap { final boolean foreground, final Path pidFile, final boolean quiet, - final Settings initialSettings) throws BootstrapException, NodeValidationException, UserException { + final Environment env0) throws BootstrapException, NodeValidationException, UserException { // Set the system property before anything has a chance to trigger its use initLoggerPrefix(); @@ -271,7 +291,8 @@ final class Bootstrap { INSTANCE = new Bootstrap(); - Environment environment = initialEnvironment(foreground, pidFile, initialSettings); + final KeyStoreWrapper keystore = loadKeyStore(env0); + Environment environment = initialEnvironment(foreground, pidFile, keystore, env0.settings()); try { LogConfigurator.configure(environment); } catch (IOException e) { @@ -309,6 +330,13 @@ final class Bootstrap { INSTANCE.setup(true, environment); + try { + // any secure settings must be read during node construction + IOUtils.close(keystore); + } catch (IOException e) { + throw new BootstrapException(e); + } + INSTANCE.start(); if (closeStandardStreams) { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java index 07ae0f9033f..635afaf9599 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java @@ -26,7 +26,7 @@ import java.nio.file.Path; * during bootstrap should explicitly declare the checked exceptions that they can throw, rather * than declaring the top-level checked exception {@link Exception}. This exception exists to wrap * these checked exceptions so that - * {@link Bootstrap#init(boolean, Path, boolean, org.elasticsearch.common.settings.Settings)} + * {@link Bootstrap#init(boolean, Path, boolean, org.elasticsearch.env.Environment)} * does not have to declare all of these checked exceptions. */ class BootstrapException extends Exception { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 1e530184734..36e695f591d 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -111,16 +111,16 @@ class Elasticsearch extends EnvironmentAwareCommand { final boolean quiet = options.has(quietOption); try { - init(daemonize, pidFile, quiet, env.settings()); + init(daemonize, pidFile, quiet, env); } catch (NodeValidationException e) { throw new UserException(ExitCodes.CONFIG, e.getMessage()); } } - void init(final boolean daemonize, final Path pidFile, final boolean quiet, Settings initialSettings) + void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment env0) throws NodeValidationException, UserException { try { - Bootstrap.init(!daemonize, pidFile, quiet, initialSettings); + Bootstrap.init(!daemonize, pidFile, quiet, env0); } catch (BootstrapException | RuntimeException e) { // format exceptions to the console in a special way // to avoid 2MB stacktraces from guice, etc. diff --git a/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java new file mode 100644 index 00000000000..289c02a1781 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -0,0 +1,86 @@ +/* + * 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.common.settings; + +import java.io.BufferedReader; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +import joptsimple.OptionSet; +import joptsimple.OptionSpec; +import org.elasticsearch.cli.EnvironmentAwareCommand; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +/** + * A subcommand for the keystore cli which adds a string setting. + */ +class AddStringKeyStoreCommand extends EnvironmentAwareCommand { + + private final OptionSpec stdinOption; + private final OptionSpec forceOption; + private final OptionSpec arguments; + + AddStringKeyStoreCommand() { + super("Add a string setting to the keystore"); + this.stdinOption = parser.acceptsAll(Arrays.asList("x", "stdin"), "Read setting value from stdin"); + this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting"); + this.arguments = parser.nonOptions("setting name"); + } + + // pkg private so tests can manipulate + InputStream getStdin() { + return System.in; + } + + @Override + protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + KeyStoreWrapper keystore = KeyStoreWrapper.loadMetadata(env.configFile()); + if (keystore == null) { + throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); + } + + keystore.loadKeystore(new char[0] /* TODO: prompt for password when they are supported */); + + String setting = arguments.value(options); + if (keystore.getSettings().contains(setting) && options.has(forceOption) == false) { + String answer = terminal.readText("Setting " + setting + " already exists. Overwrite? [y/N]"); + if (answer.equals("y") == false) { + terminal.println("Exiting without modifying keystore."); + return; + } + } + + final char[] value; + if (options.has(stdinOption)) { + BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8)); + value = stdinReader.readLine().toCharArray(); + } else { + value = terminal.readSecret("Enter value for " + setting + ": "); + } + + keystore.setStringSetting(setting, value); + keystore.save(env.configFile()); + } +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java new file mode 100644 index 00000000000..7c2f8aec19a --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -0,0 +1,62 @@ +/* + * 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.common.settings; + +import java.nio.file.Files; +import java.nio.file.Path; + +import joptsimple.OptionSet; +import org.elasticsearch.cli.EnvironmentAwareCommand; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.env.Environment; + +/** + * A subcommand for the keystore cli to create a new keystore. + */ +class CreateKeyStoreCommand extends EnvironmentAwareCommand { + + CreateKeyStoreCommand() { + super("Creates a new elasticsearch keystore"); + } + + @Override + protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); + if (Files.exists(keystoreFile)) { + String answer = terminal.readText("An elasticsearch keystore already exists. Overwrite? [y/N] "); + if (answer.equals("y") == false) { + terminal.println("Exiting without creating keystore."); + return; + } + } + + + char[] password = new char[0];// terminal.readSecret("Enter passphrase (empty for no passphrase): "); + /* TODO: uncomment when entering passwords on startup is supported + char[] passwordRepeat = terminal.readSecret("Enter same passphrase again: "); + if (Arrays.equals(password, passwordRepeat) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passphrases are not equal, exiting."); + }*/ + + KeyStoreWrapper keystore = KeyStoreWrapper.create(password); + keystore.save(env.configFile()); + terminal.println("Created elasticsearch keystore in " + env.configFile()); + } +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java new file mode 100644 index 00000000000..5bded392fdb --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java @@ -0,0 +1,41 @@ +/* + * 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.common.settings; + +import org.elasticsearch.cli.MultiCommand; +import org.elasticsearch.cli.Terminal; + +/** + * A cli tool for managing secrets in the elasticsearch keystore. + */ +public class KeyStoreCli extends MultiCommand { + + private KeyStoreCli() { + super("A tool for managing settings stored in the elasticsearch keystore"); + subcommands.put("create", new CreateKeyStoreCommand()); + subcommands.put("list", new ListKeyStoreCommand()); + subcommands.put("add", new AddStringKeyStoreCommand()); + subcommands.put("remove", new RemoveSettingKeyStoreCommand()); + } + + public static void main(String[] args) throws Exception { + exit(new KeyStoreCli().main(args, Terminal.DEFAULT)); + } +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java new file mode 100644 index 00000000000..d1a605fff92 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -0,0 +1,218 @@ +/* + * 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.common.settings; + +import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import javax.security.auth.DestroyFailedException; +import java.io.Closeable; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermissions; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.util.Arrays; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Set; + +import org.apache.lucene.util.SetOnce; + +/** + * A wrapper around a Java KeyStore which provides supplements the keystore with extra metadata. + * + * Loading a keystore has 2 phases. First, call {@link #loadMetadata(Path)}. Then call + * {@link #loadKeystore(char[])} with the keystore password, or an empty char array if + * {@link #hasPassword()} is {@code false}. + */ +public class KeyStoreWrapper implements Closeable { + + /** The version of the metadata written before the keystore data. */ + private static final int FORMAT_VERSION = 1; + + /** The keystore type for a newly created keystore. */ + private static final String NEW_KEYSTORE_TYPE = "PKCS12"; + + /** A factory necessary for constructing instances of secrets in a {@link KeyStore}. */ + private static final SecretKeyFactory passwordFactory; + static { + try { + passwordFactory = SecretKeyFactory.getInstance("PBE"); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } + } + + /** True iff the keystore has a password needed to read. */ + private final boolean hasPassword; + + /** The type of the keystore, as passed to {@link java.security.KeyStore#getInstance(String)} */ + private final String type; + + /** A stream of the actual keystore data. */ + private final InputStream input; + + /** The loaded keystore. See {@link #loadKeystore(char[])}. */ + private final SetOnce keystore = new SetOnce<>(); + + /** The password for the keystore. See {@link #loadKeystore(char[])}. */ + private final SetOnce keystorePassword = new SetOnce<>(); + + /** The setting names contained in the loaded keystore. */ + private final Set settingNames = new HashSet<>(); + + private KeyStoreWrapper(boolean hasPassword, String type, InputStream input) { + this.hasPassword = hasPassword; + this.type = type; + this.input = input; + } + + /** Returns a path representing the ES keystore in the given config dir. */ + static Path keystorePath(Path configDir) { + return configDir.resolve("elasticsearch.keystore"); + } + + /** Constructs a new keystore with the given password. */ + static KeyStoreWrapper create(char[] password) throws Exception { + KeyStoreWrapper wrapper = new KeyStoreWrapper(password.length != 0, NEW_KEYSTORE_TYPE, null); + KeyStore keyStore = KeyStore.getInstance(NEW_KEYSTORE_TYPE); + keyStore.load(null, null); + wrapper.keystore.set(keyStore); + wrapper.keystorePassword.set(new KeyStore.PasswordProtection(password)); + return wrapper; + } + + /** + * Loads information about the Elasticsearch keystore from the provided config directory. + * + * {@link #loadKeystore(char[])} must be called before reading or writing any entries. + * Returns {@code null} if no keystore exists. + */ + public static KeyStoreWrapper loadMetadata(Path configDir) throws IOException { + Path keystoreFile = keystorePath(configDir); + if (Files.exists(keystoreFile) == false) { + return null; + } + DataInputStream inputStream = new DataInputStream(Files.newInputStream(keystoreFile)); + int format = inputStream.readInt(); + if (format != FORMAT_VERSION) { + throw new IllegalStateException("Unknown keystore metadata format [" + format + "]"); + } + boolean hasPassword = inputStream.readBoolean(); + String type = inputStream.readUTF(); + return new KeyStoreWrapper(hasPassword, type, inputStream); + } + + /** Returns true iff {@link #loadKeystore(char[])} has been called. */ + public boolean isLoaded() { + return keystore.get() != null; + } + + /** Return true iff calling {@link #loadKeystore(char[])} requires a non-empty password. */ + public boolean hasPassword() { + return hasPassword; + } + + /** Loads the keystore this metadata wraps. This may only be called once. */ + public void loadKeystore(char[] password) throws GeneralSecurityException, IOException { + this.keystore.set(KeyStore.getInstance(type)); + try (InputStream in = input) { + keystore.get().load(in, password); + } + + this.keystorePassword.set(new KeyStore.PasswordProtection(password)); + Arrays.fill(password, '\0'); + + // convert keystore aliases enum into a set for easy lookup + Enumeration aliases = keystore.get().aliases(); + while (aliases.hasMoreElements()) { + settingNames.add(aliases.nextElement()); + } + } + + /** Write the keystore to the given config directory. */ + void save(Path configDir) throws Exception { + Path keystoreFile = keystorePath(configDir); + try (DataOutputStream outputStream = new DataOutputStream(Files.newOutputStream(keystoreFile))) { + outputStream.writeInt(FORMAT_VERSION); + char[] password = this.keystorePassword.get().getPassword(); + outputStream.writeBoolean(password.length != 0); + outputStream.writeUTF(type); + keystore.get().store(outputStream, password); + } + PosixFileAttributeView attrs = Files.getFileAttributeView(keystoreFile, PosixFileAttributeView.class); + if (attrs != null) { + // don't rely on umask: ensure the keystore has minimal permissions + attrs.setPermissions(PosixFilePermissions.fromString("rw-------")); + } + } + + /** Returns the names of all settings in this keystore. */ + public Set getSettings() { + return settingNames; + } + + /** Retrieve a string setting. The {@link SecureString} should be closed once it is used. */ + SecureString getStringSetting(String setting) throws GeneralSecurityException { + KeyStore.Entry entry = keystore.get().getEntry(setting, keystorePassword.get()); + if (entry instanceof KeyStore.SecretKeyEntry == false) { + throw new IllegalStateException("Secret setting " + setting + " is not a string"); + } + // TODO: only allow getting a setting once? + KeyStore.SecretKeyEntry secretKeyEntry = (KeyStore.SecretKeyEntry)entry; + PBEKeySpec keySpec = (PBEKeySpec)passwordFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class); + SecureString value = new SecureString(keySpec.getPassword()); + keySpec.clearPassword(); + return value; + } + + /** Set a string setting. */ + void setStringSetting(String setting, char[] value) throws GeneralSecurityException { + SecretKey secretKey = passwordFactory.generateSecret(new PBEKeySpec(value)); + keystore.get().setEntry(setting, new KeyStore.SecretKeyEntry(secretKey), keystorePassword.get()); + settingNames.add(setting); + } + + /** Remove the given setting from the keystore. */ + void remove(String setting) throws KeyStoreException { + keystore.get().deleteEntry(setting); + settingNames.remove(setting); + } + + @Override + public void close() throws IOException { + try { + if (keystorePassword.get() != null) { + keystorePassword.get().destroy(); + } + } catch (DestroyFailedException e) { + throw new IOException(e); + } + } +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java new file mode 100644 index 00000000000..07bf4263d19 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -0,0 +1,63 @@ +/* + * 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.common.settings; + + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import joptsimple.OptionSet; +import org.elasticsearch.cli.EnvironmentAwareCommand; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +/** + * A subcommand for the keystore cli to list all settings in the keystore. + */ +class ListKeyStoreCommand extends EnvironmentAwareCommand { + + ListKeyStoreCommand() { + super("List entries in the keystore"); + } + + @Override + protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + KeyStoreWrapper keystore = KeyStoreWrapper.loadMetadata(env.configFile()); + if (keystore == null) { + throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); + } + + keystore.loadKeystore(new char[0] /* TODO: prompt for password when they are supported */); + + List sortedEntries = new ArrayList<>(keystore.getSettings()); + Collections.sort(sortedEntries); + for (String entry : sortedEntries) { + terminal.println(entry); + } + + // TODO: + // 2. delete command + // 3. tests for delete + // 4. shell script + } +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java new file mode 100644 index 00000000000..fb35640b727 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -0,0 +1,66 @@ +/* + * 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.common.settings; + +import java.util.List; + +import joptsimple.OptionSet; +import joptsimple.OptionSpec; +import org.elasticsearch.cli.EnvironmentAwareCommand; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +/** + * A subcommand for the keystore cli to remove a setting. + */ +class RemoveSettingKeyStoreCommand extends EnvironmentAwareCommand { + + private final OptionSpec arguments; + + RemoveSettingKeyStoreCommand() { + super("Remove a setting from the keystore"); + arguments = parser.nonOptions("setting names"); + } + + @Override + protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + List settings = arguments.values(options); + if (settings.isEmpty()) { + throw new UserException(ExitCodes.USAGE, "Must supply at least one setting to remove"); + } + + KeyStoreWrapper keystore = KeyStoreWrapper.loadMetadata(env.configFile()); + if (keystore == null) { + throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); + } + + keystore.loadKeystore(new char[0] /* TODO: prompt for password when they are supported */); + + for (String setting : arguments.values(options)) { + if (keystore.getSettings().contains(setting) == false) { + throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); + } + keystore.remove(setting); + } + keystore.save(env.configFile()); + } +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java b/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java new file mode 100644 index 00000000000..fb4073482f3 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/SecureSetting.java @@ -0,0 +1,112 @@ +/* + * 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.common.settings; + +import java.security.GeneralSecurityException; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Objects; +import java.util.Set; + + +/** + * A secure setting. + * + * This class allows access to settings from the Elasticsearch keystore. + */ +public abstract class SecureSetting extends Setting { + private static final Set ALLOWED_PROPERTIES = new HashSet<>( + Arrays.asList(Property.Deprecated, Property.Shared) + ); + + private SecureSetting(String key, Setting.Property... properties) { + super(key, (String)null, null, properties); + assert assertAllowedProperties(properties); + } + + private boolean assertAllowedProperties(Setting.Property... properties) { + for (Setting.Property property : properties) { + if (ALLOWED_PROPERTIES.contains(property) == false) { + return false; + } + } + return true; + } + + @Override + public String getDefaultRaw(Settings settings) { + throw new UnsupportedOperationException("secure settings are not strings"); + } + + @Override + public T getDefault(Settings settings) { + throw new UnsupportedOperationException("secure settings are not strings"); + } + + @Override + public String getRaw(Settings settings) { + throw new UnsupportedOperationException("secure settings are not strings"); + } + + @Override + public T get(Settings settings) { + checkDeprecation(settings); + final KeyStoreWrapper keystore = Objects.requireNonNull(settings.getKeyStore()); + if (keystore.getSettings().contains(getKey()) == false) { + return getFallback(settings); + } + try { + return getSecret(keystore); + } catch (GeneralSecurityException e) { + throw new RuntimeException("failed to read secure setting " + getKey(), e); + } + } + + /** Returns the secret setting from the keyStoreReader store. */ + abstract T getSecret(KeyStoreWrapper keystore) throws GeneralSecurityException; + + /** Returns the value from a fallback setting. Returns null if no fallback exists. */ + abstract T getFallback(Settings settings); + + // TODO: override toXContent + + /** + * A setting which contains a sensitive string. + * + * This may be any sensitive string, e.g. a username, a password, an auth token, etc. + */ + public static SecureSetting stringSetting(String name, Setting fallback, Property... properties) { + return new SecureSetting(name, properties) { + @Override + protected SecureString getSecret(KeyStoreWrapper keystore) throws GeneralSecurityException { + return keystore.getStringSetting(getKey()); + } + @Override + SecureString getFallback(Settings settings) { + if (fallback != null) { + return new SecureString(fallback.get(settings).toCharArray()); + } + return null; + } + }; + } + + +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/SecureString.java b/core/src/main/java/org/elasticsearch/common/settings/SecureString.java new file mode 100644 index 00000000000..7d209fc6171 --- /dev/null +++ b/core/src/main/java/org/elasticsearch/common/settings/SecureString.java @@ -0,0 +1,105 @@ +/* + * 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.common.settings; + +import java.util.Arrays; +import java.util.Objects; + +/** + * A String implementations which allows clearing the underlying char array. + */ +public final class SecureString implements CharSequence, AutoCloseable { + + private char[] chars; + + /** + * Constructs a new SecureString which controls the passed in char array. + * + * Note: When this instance is closed, the array will be zeroed out. + */ + public SecureString(char[] chars) { + this.chars = Objects.requireNonNull(chars); + } + + /** Constant time equality to avoid potential timing attacks. */ + @Override + public boolean equals(Object o) { + ensureNotClosed(); + if (this == o) return true; + if (o == null || o instanceof CharSequence == false) return false; + CharSequence that = (CharSequence) o; + if (chars.length != that.length()) { + return false; + } + + int equals = 0; + for (int i = 0; i < chars.length; i++) { + equals |= chars[i] ^ that.charAt(i); + } + + return equals == 0; + } + + @Override + public int hashCode() { + return Arrays.hashCode(chars); + } + + @Override + public int length() { + ensureNotClosed(); + return chars.length; + } + + @Override + public char charAt(int index) { + ensureNotClosed(); + return chars[index]; + } + + @Override + public SecureString subSequence(int start, int end) { + throw new UnsupportedOperationException("Cannot get subsequence of SecureString"); + } + + /** + * Convert to a {@link String}. This only be used with APIs that do not take {@link CharSequence}. + */ + @Override + public String toString() { + return new String(chars); + } + + /** + * Closes the string by clearing the underlying char array. + */ + @Override + public void close() { + Arrays.fill(chars, '\0'); + chars = null; + } + + /** Throw an exception if this string has been closed, indicating something is trying to access the data after being closed. */ + private void ensureNotClosed() { + if (chars == null) { + throw new IllegalStateException("SecureString has already been closed"); + } + } +} diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java index 6f52ea16097..4ed64b77c4a 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -274,7 +274,7 @@ public class Setting extends ToXContentToBytes { * Returns the default value string representation for this setting. * @param settings a settings object for settings that has a default value depending on another setting if available */ - public final String getDefaultRaw(Settings settings) { + public String getDefaultRaw(Settings settings) { return defaultValue.apply(settings); } @@ -282,7 +282,7 @@ public class Setting extends ToXContentToBytes { * Returns the default value for this setting. * @param settings a settings object for settings that has a default value depending on another setting if available */ - public final T getDefault(Settings settings) { + public T getDefault(Settings settings) { return parser.apply(getDefaultRaw(settings)); } @@ -290,7 +290,7 @@ public class Setting extends ToXContentToBytes { * Returns true iff this setting is present in the given settings object. Otherwise false */ public boolean exists(Settings settings) { - return settings.get(getKey()) != null; + return settings.contains(getKey()); } /** @@ -330,14 +330,19 @@ public class Setting extends ToXContentToBytes { * instead. This is useful if the value can't be parsed due to an invalid value to access the actual value. */ public String getRaw(Settings settings) { + checkDeprecation(settings); + return settings.get(getKey(), defaultValue.apply(settings)); + } + + /** Logs a deprecation warning if the setting is deprecated and used. */ + protected void checkDeprecation(Settings settings) { // They're using the setting, so we need to tell them to stop if (this.isDeprecated() && this.exists(settings)) { // It would be convenient to show its replacement key, but replacement is often not so simple final DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(getClass())); deprecationLogger.deprecated("[{}] setting was deprecated in Elasticsearch and it will be removed in a future release! " + - "See the breaking changes lists in the documentation for details", getKey()); + "See the breaking changes lists in the documentation for details", getKey()); } - return settings.get(getKey(), defaultValue.apply(settings)); } /** diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index 67bf8081479..2bf93fb8e0e 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -76,10 +76,29 @@ public final class Settings implements ToXContent { public static final Settings EMPTY = new Builder().build(); private static final Pattern ARRAY_PATTERN = Pattern.compile("(.*)\\.\\d+$"); + /** The raw settings from the full key to raw string value. */ private Map settings; - Settings(Map settings) { - this.settings = Collections.unmodifiableMap(settings); + /** The keystore storage associated with these settings. */ + private KeyStoreWrapper keystore; + + Settings(Map settings, KeyStoreWrapper keystore) { + // we use a sorted map for consistent serialization when using getAsMap() + this.settings = Collections.unmodifiableSortedMap(new TreeMap<>(settings)); + this.keystore = keystore; + } + + /** + * Retrieve the keystore that contains secure settings. + */ + KeyStoreWrapper getKeyStore() { + // pkg private so it can only be accessed by local subclasses of SecureSetting + return keystore; + } + + /** Returns true if the setting exists, false otherwise. */ + public boolean contains(String key) { + return settings.containsKey(key) || keystore != null && keystore.getSettings().contains(key); } /** @@ -185,16 +204,18 @@ public final class Settings implements ToXContent { /** * A settings that are filtered (and key is removed) with the specified prefix. + * Secure settings may not be access through the prefixed settings. */ public Settings getByPrefix(String prefix) { - return new Settings(new FilteredMap(this.settings, (k) -> k.startsWith(prefix), prefix)); + return new Settings(new FilteredMap(this.settings, (k) -> k.startsWith(prefix), prefix), null); } /** * Returns a new settings object that contains all setting of the current one filtered by the given settings key predicate. + * Secure settings may not be accessed through a filter. */ public Settings filter(Predicate predicate) { - return new Settings(new FilteredMap(this.settings, predicate, null)); + return new Settings(new FilteredMap(this.settings, predicate, null), null); } /** @@ -456,7 +477,7 @@ public final class Settings implements ToXContent { } Map retVal = new LinkedHashMap<>(); for (Map.Entry> entry : map.entrySet()) { - retVal.put(entry.getKey(), new Settings(Collections.unmodifiableMap(entry.getValue()))); + retVal.put(entry.getKey(), new Settings(Collections.unmodifiableMap(entry.getValue()), keystore)); } return Collections.unmodifiableMap(retVal); } @@ -591,6 +612,8 @@ public final class Settings implements ToXContent { // we use a sorted map for consistent serialization when using getAsMap() private final Map map = new TreeMap<>(); + private KeyStoreWrapper keystore; + private Builder() { } @@ -613,6 +636,13 @@ public final class Settings implements ToXContent { return map.get(key); } + /** Sets the secret store for these settings. */ + public void setKeyStore(KeyStoreWrapper keystore) { + assert this.keystore == null; // only set once! + assert keystore.isLoaded(); + this.keystore = Objects.requireNonNull(keystore); + } + /** * Puts tuples of key value pairs of settings. Simplified version instead of repeating calling * put for each one. @@ -1019,7 +1049,7 @@ public final class Settings implements ToXContent { * set on this builder. */ public Settings build() { - return new Settings(map); + return new Settings(map, keystore); } } diff --git a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java index 8fb86ebdac0..840378ffd08 100644 --- a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java +++ b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java @@ -19,14 +19,6 @@ package org.elasticsearch.node.internal; -import org.elasticsearch.cli.Terminal; -import org.elasticsearch.cluster.ClusterName; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.settings.SettingsException; -import org.elasticsearch.env.Environment; - import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -39,6 +31,14 @@ import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsException; +import org.elasticsearch.env.Environment; + import static org.elasticsearch.common.Strings.cleanPath; public class InternalSettingsPreparer { diff --git a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java index c31bbe8c74f..8a00a430dbc 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/ElasticsearchCliTests.java @@ -152,8 +152,8 @@ public class ElasticsearchCliTests extends ESElasticsearchCliTestCase { ExitCodes.OK, true, output -> {}, - (foreground, pidFile, quiet, esSettings) -> { - Map settings = esSettings.getAsMap(); + (foreground, pidFile, quiet, env) -> { + Map settings = env.settings().getAsMap(); assertThat(settings, hasEntry("foo", "bar")); assertThat(settings, hasEntry("baz", "qux")); }, diff --git a/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java new file mode 100644 index 00000000000..8ac07aedcfc --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -0,0 +1,125 @@ +/* + * 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.common.settings; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.Map; + +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import static org.hamcrest.Matchers.containsString; + +public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase { + InputStream input; + + @Override + protected Command newCommand() { + return new AddStringKeyStoreCommand() { + @Override + protected Environment createEnv(Terminal terminal, Map settings) { + return env; + } + @Override + InputStream getStdin() { + return input; + } + }; + } + + public void testMissing() throws Exception { + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("keystore not found")); + } + + public void testOverwritePromptDefault() throws Exception { + createKeystore("", "foo", "bar"); + terminal.addTextInput(""); + execute("foo"); + assertSecureString("foo", "bar"); + } + + public void testOverwritePromptExplicitNo() throws Exception { + createKeystore("", "foo", "bar"); + terminal.addTextInput("n"); // explicit no + execute("foo"); + assertSecureString("foo", "bar"); + } + + public void testOverwritePromptExplicitYes() throws Exception { + createKeystore("", "foo", "bar"); + terminal.addTextInput("y"); + terminal.addSecretInput("newvalue"); + execute("foo"); + assertSecureString("foo", "newvalue"); + } + + public void testOverwriteForceShort() throws Exception { + createKeystore("", "foo", "bar"); + terminal.addSecretInput("newvalue"); + execute("-f", "foo"); // force + assertSecureString("foo", "newvalue"); + } + + public void testOverwriteForceLong() throws Exception { + createKeystore("", "foo", "bar"); + terminal.addSecretInput("and yet another secret value"); + execute("--force", "foo"); // force + assertSecureString("foo", "and yet another secret value"); + } + + public void testForceNonExistent() throws Exception { + createKeystore(""); + terminal.addSecretInput("value"); + execute("--force", "foo"); // force + assertSecureString("foo", "value"); + } + + public void testPromptForValue() throws Exception { + KeyStoreWrapper.create(new char[0]).save(env.configFile()); + terminal.addSecretInput("secret value"); + execute("foo"); + assertSecureString("foo", "secret value"); + } + + public void testStdinShort() throws Exception { + KeyStoreWrapper.create(new char[0]).save(env.configFile()); + setInput("secret value 1"); + execute("-x", "foo"); + assertSecureString("foo", "secret value 1"); + } + + public void testStdinLong() throws Exception { + KeyStoreWrapper.create(new char[0]).save(env.configFile()); + setInput("secret value 2"); + execute("--stdin", "foo"); + assertSecureString("foo", "secret value 2"); + } + + void setInput(String inputStr) { + input = new ByteArrayInputStream(inputStr.getBytes(StandardCharsets.UTF_8)); + } +} diff --git a/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java new file mode 100644 index 00000000000..7f1f29753a1 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -0,0 +1,73 @@ +/* + * 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.common.settings; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.MockTerminal; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.env.Environment; + +public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { + + @Override + protected Command newCommand() { + return new CreateKeyStoreCommand() { + @Override + protected Environment createEnv(Terminal terminal, Map settings) { + return env; + } + }; + } + + public void testPosix() throws Exception { + execute(); + Path configDir = env.configFile(); + assertNotNull(KeyStoreWrapper.loadMetadata(configDir)); + } + + public void testNotPosix() throws Exception { + setupEnv(false); + execute(); + Path configDir = env.configFile(); + assertNotNull(KeyStoreWrapper.loadMetadata(configDir)); + } + + public void testOverwrite() throws Exception { + Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); + byte[] content = "not a keystore".getBytes(); + Files.write(keystoreFile, content); + + terminal.addTextInput(""); // default is no + execute(); + assertArrayEquals(content, Files.readAllBytes(keystoreFile)); + + terminal.addTextInput("n"); // explicit no + execute(); + assertArrayEquals(content, Files.readAllBytes(keystoreFile)); + + terminal.addTextInput("y"); + execute(); + assertNotNull(KeyStoreWrapper.loadMetadata(env.configFile())); + } +} diff --git a/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java b/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java new file mode 100644 index 00000000000..cb8f84f9d2b --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java @@ -0,0 +1,98 @@ +/* + * 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.common.settings; + +import java.io.IOException; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import com.google.common.jimfs.Configuration; +import com.google.common.jimfs.Jimfs; +import org.apache.lucene.util.IOUtils; +import org.apache.lucene.util.LuceneTestCase; +import org.elasticsearch.cli.CommandTestCase; +import org.elasticsearch.common.io.PathUtilsForTesting; +import org.elasticsearch.env.Environment; +import org.elasticsearch.test.ESTestCase; +import org.junit.After; +import org.junit.Before; + +/** + * Base test case for manipulating the ES keystore. + */ +@LuceneTestCase.SuppressFileSystems("*") // we do our own mocking +public abstract class KeyStoreCommandTestCase extends CommandTestCase { + + Environment env; + + List fileSystems = new ArrayList<>(); + + @After + public void closeMockFileSystems() throws IOException { + IOUtils.close(fileSystems); + } + + @Before + public void setupEnv() throws IOException { + setupEnv(true); // default to posix, but tests may call setupEnv(false) to overwrite + } + + void setupEnv(boolean posix) throws IOException { + final Configuration configuration; + if (posix) { + configuration = Configuration.unix().toBuilder().setAttributeViews("basic", "owner", "posix", "unix").build(); + } else { + configuration = Configuration.unix(); + } + FileSystem fs = Jimfs.newFileSystem(configuration); + fileSystems.add(fs); + PathUtilsForTesting.installMock(fs); // restored by restoreFileSystem in ESTestCase + Path home = fs.getPath("/", "test-home"); + Files.createDirectories(home.resolve("config")); + env = new Environment(Settings.builder().put("path.home", home).build()); + } + + KeyStoreWrapper createKeystore(String password, String... settings) throws Exception { + KeyStoreWrapper keystore = KeyStoreWrapper.create(password.toCharArray()); + assertEquals(0, settings.length % 2); + for (int i = 0; i < settings.length; i += 2) { + keystore.setStringSetting(settings[i], settings[i + 1].toCharArray()); + } + keystore.save(env.configFile()); + return keystore; + } + + KeyStoreWrapper loadKeystore(String password) throws Exception { + KeyStoreWrapper keystore = KeyStoreWrapper.loadMetadata(env.configFile()); + keystore.loadKeystore(password.toCharArray()); + return keystore; + } + + void assertSecureString(String setting, String value) throws Exception { + assertSecureString(loadKeystore(""), setting, value); + } + + void assertSecureString(KeyStoreWrapper keystore, String setting, String value) throws Exception { + assertEquals(value, keystore.getStringSetting(setting).toString()); + } +} diff --git a/core/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java new file mode 100644 index 00000000000..1a8bdfc077d --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java @@ -0,0 +1,67 @@ +/* + * 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.common.settings; + +import java.util.Map; + +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import static org.hamcrest.Matchers.containsString; + +public class ListKeyStoreCommandTests extends KeyStoreCommandTestCase { + + @Override + protected Command newCommand() { + return new ListKeyStoreCommand() { + @Override + protected Environment createEnv(Terminal terminal, Map settings) { + return env; + } + }; + } + + public void testMissing() throws Exception { + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("keystore not found")); + } + + public void testEmpty() throws Exception { + createKeystore(""); + execute(); + assertTrue(terminal.getOutput(), terminal.getOutput().isEmpty()); + } + + public void testOne() throws Exception { + createKeystore("", "foo", "bar"); + execute(); + assertEquals("foo\n", terminal.getOutput()); + } + + public void testMultiple() throws Exception { + createKeystore("", "foo", "1", "baz", "2", "bar", "3"); + execute(); + assertEquals("bar\nbaz\nfoo\n", terminal.getOutput()); // sorted + } +} diff --git a/core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java new file mode 100644 index 00000000000..e83ed873116 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java @@ -0,0 +1,80 @@ +/* + * 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.common.settings; + +import java.util.Map; +import java.util.Set; + +import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import static org.hamcrest.Matchers.containsString; + +public class RemoveSettingKeyStoreCommandTests extends KeyStoreCommandTestCase { + + @Override + protected Command newCommand() { + return new RemoveSettingKeyStoreCommand() { + @Override + protected Environment createEnv(Terminal terminal, Map settings) { + return env; + } + }; + } + + public void testMissing() throws Exception { + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("keystore not found")); + } + + public void testNoSettings() throws Exception { + createKeystore(""); + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(ExitCodes.USAGE, e.exitCode); + assertThat(e.getMessage(), containsString("Must supply at least one setting")); + } + + public void testNonExistentSetting() throws Exception { + createKeystore(""); + UserException e = expectThrows(UserException.class, () -> execute("foo")); + assertEquals(ExitCodes.CONFIG, e.exitCode); + assertThat(e.getMessage(), containsString("[foo] does not exist")); + } + + public void testOne() throws Exception { + createKeystore("", "foo", "bar"); + execute("foo"); + assertFalse(loadKeystore("").getSettings().contains("foo")); + } + + public void testMany() throws Exception { + createKeystore("", "foo", "1", "bar", "2", "baz", "3"); + execute("foo", "baz"); + Set settings = loadKeystore("").getSettings(); + assertFalse(settings.contains("foo")); + assertFalse(settings.contains("baz")); + assertTrue(settings.contains("bar")); + assertEquals(1, settings.size()); + } +} diff --git a/distribution/src/main/resources/bin/elasticsearch-keystore b/distribution/src/main/resources/bin/elasticsearch-keystore new file mode 100755 index 00000000000..d99d11aed49 --- /dev/null +++ b/distribution/src/main/resources/bin/elasticsearch-keystore @@ -0,0 +1,91 @@ +#!/bin/bash + +CDPATH="" +SCRIPT="$0" + +# SCRIPT may be an arbitrarily deep series of symlinks. Loop until we have the concrete path. +while [ -h "$SCRIPT" ] ; do + ls=`ls -ld "$SCRIPT"` + # Drop everything prior to -> + link=`expr "$ls" : '.*-> \(.*\)$'` + if expr "$link" : '/.*' > /dev/null; then + SCRIPT="$link" + else + SCRIPT=`dirname "$SCRIPT"`/"$link" + fi +done + +# determine elasticsearch home +ES_HOME=`dirname "$SCRIPT"`/.. + +# make ELASTICSEARCH_HOME absolute +ES_HOME=`cd "$ES_HOME"; pwd` + + +# Sets the default values for elasticsearch variables used in this script +if [ -z "$CONF_DIR" ]; then + CONF_DIR="${path.conf}" +fi + +# The default env file is defined at building/packaging time. +# For a ${project.name} package, the value is "${path.env}". +ES_ENV_FILE="${path.env}" + +# If an include is specified with the ES_INCLUDE environment variable, use it +if [ -n "$ES_INCLUDE" ]; then + ES_ENV_FILE="$ES_INCLUDE" +fi + +# Source the environment file +if [ -n "$ES_ENV_FILE" ]; then + + # If the ES_ENV_FILE is not found, try to resolve the path + # against the ES_HOME directory + if [ ! -f "$ES_ENV_FILE" ]; then + ES_ENV_FILE="$ELASTIC_HOME/$ES_ENV_FILE" + fi + + . "$ES_ENV_FILE" + if [ $? -ne 0 ]; then + echo "Unable to source environment file: $ES_ENV_FILE" >&2 + exit 1 + fi +fi + +# don't let JAVA_TOOL_OPTIONS slip in (e.g. crazy agents in ubuntu) +# works around https://bugs.launchpad.net/ubuntu/+source/jayatana/+bug/1441487 +if [ "x$JAVA_TOOL_OPTIONS" != "x" ]; then + echo "Warning: Ignoring JAVA_TOOL_OPTIONS=$JAVA_TOOL_OPTIONS" + unset JAVA_TOOL_OPTIONS +fi + +# CONF_FILE setting was removed +if [ ! -z "$CONF_FILE" ]; then + echo "CONF_FILE setting is no longer supported. elasticsearch.yml must be placed in the config directory and cannot be renamed." + exit 1 +fi + +if [ -x "$JAVA_HOME/bin/java" ]; then + JAVA=$JAVA_HOME/bin/java +else + JAVA=`which java` +fi + +if [ ! -x "$JAVA" ]; then + echo "Could not find any executable java binary. Please install java in your PATH or set JAVA_HOME" + exit 1 +fi + +# full hostname passed through cut for portability on systems that do not support hostname -s +# export on separate line for shells that do not support combining definition and export +HOSTNAME=`hostname | cut -d. -f1` +export HOSTNAME + +declare -a args=("$@") +path_props=(-Des.path.home="$ES_HOME") + +if [ -e "$CONF_DIR" ]; then + path_props=("${path_props[@]}" -Des.path.conf="$CONF_DIR") +fi + +exec "$JAVA" $ES_JAVA_OPTS -Delasticsearch "${path_props[@]}" -cp "$ES_HOME/lib/*" org.elasticsearch.common.settings.KeyStoreCli "${args[@]}" diff --git a/distribution/src/main/resources/bin/elasticsearch-keystore.bat b/distribution/src/main/resources/bin/elasticsearch-keystore.bat new file mode 100644 index 00000000000..ceb7e96665d --- /dev/null +++ b/distribution/src/main/resources/bin/elasticsearch-keystore.bat @@ -0,0 +1,30 @@ +@echo off + +SETLOCAL enabledelayedexpansion + +IF DEFINED JAVA_HOME ( + set JAVA=%JAVA_HOME%\bin\java.exe +) ELSE ( + FOR %%I IN (java.exe) DO set JAVA=%%~$PATH:I +) +IF NOT EXIST "%JAVA%" ( + ECHO Could not find any executable java binary. Please install java in your PATH or set JAVA_HOME 1>&2 + EXIT /B 1 +) + +set SCRIPT_DIR=%~dp0 +for %%I in ("%SCRIPT_DIR%..") do set ES_HOME=%%~dpfI + +TITLE Elasticsearch Plugin Manager ${project.version} + +SET path_props=-Des.path.home="%ES_HOME%" +IF DEFINED CONF_DIR ( + SET path_props=!path_props! -Des.path.conf="%CONF_DIR%" +) + +SET args=%* +SET HOSTNAME=%COMPUTERNAME% + +"%JAVA%" %ES_JAVA_OPTS% !path_props! -cp "%ES_HOME%/lib/*;" "org.elasticsearch.common.settings.KeyStoreCli" !args! + +ENDLOCAL diff --git a/docs/java-rest/configuration.asciidoc b/docs/java-rest/configuration.asciidoc index ad5a57dd94e..268e71bc3a2 100644 --- a/docs/java-rest/configuration.asciidoc +++ b/docs/java-rest/configuration.asciidoc @@ -93,9 +93,9 @@ https://hc.apache.org/httpcomponents-asyncclient-dev/httpasyncclient/apidocs/org [source,java] -------------------------------------------------- -KeyStore keyStore = KeyStore.getInstance("jks"); +KeyStore keystore = KeyStore.getInstance("jks"); try (InputStream is = Files.newInputStream(keyStorePath)) { - keyStore.load(is, keyStorePass.toCharArray()); + keystore.load(is, keyStorePass.toCharArray()); } RestClient restClient = RestClient.builder(new HttpHost("localhost", 9200)) .setHttpClientConfigCallback(new RestClientBuilder.HttpClientConfigCallback() { diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java index fa04b51ff59..2c00030973b 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilElasticsearchCliTests.java @@ -41,8 +41,7 @@ public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase { true, output -> {}, (foreground, pidFile, quiet, esSettings) -> { - Map settings = esSettings.getAsMap(); - settings.keySet().forEach(System.out::println); + Map settings = esSettings.settings().getAsMap(); assertThat(settings.size(), equalTo(2)); assertThat(settings, hasEntry("path.home", value)); assertThat(settings, hasKey("path.logs")); // added by env initialization @@ -55,7 +54,7 @@ public class EvilElasticsearchCliTests extends ESElasticsearchCliTestCase { true, output -> {}, (foreground, pidFile, quiet, esSettings) -> { - Map settings = esSettings.getAsMap(); + Map settings = esSettings.settings().getAsMap(); assertThat(settings.size(), equalTo(2)); assertThat(settings, hasEntry("path.home", commandLineValue)); assertThat(settings, hasKey("path.logs")); // added by env initialization diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java index f01011b5f61..6ff6068bd49 100644 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java @@ -35,7 +35,7 @@ import static org.hamcrest.CoreMatchers.equalTo; abstract class ESElasticsearchCliTestCase extends ESTestCase { interface InitConsumer { - void accept(final boolean foreground, final Path pidFile, final boolean quiet, final Settings initialSettings); + void accept(final boolean foreground, final Path pidFile, final boolean quiet, final Environment env0); } void runTest( @@ -57,9 +57,9 @@ abstract class ESElasticsearchCliTestCase extends ESTestCase { return new Environment(realSettings); } @Override - void init(final boolean daemonize, final Path pidFile, final boolean quiet, Settings initialSettings) { + void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment env0) { init.set(true); - initConsumer.accept(!daemonize, pidFile, quiet, initialSettings); + initConsumer.accept(!daemonize, pidFile, quiet, env0); } @Override From d4288cce79232b155f9fe87bda25edc261f6a74d Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 23 Dec 2016 10:57:02 -0800 Subject: [PATCH 2/6] Fix getBytes invocation to use explicit charset --- .../common/settings/CreateKeyStoreCommandTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java index 7f1f29753a1..5722b9e33d4 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.Map; @@ -55,7 +56,7 @@ public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { public void testOverwrite() throws Exception { Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); - byte[] content = "not a keystore".getBytes(); + byte[] content = "not a keystore".getBytes(StandardCharsets.UTF_8); Files.write(keystoreFile, content); terminal.addTextInput(""); // default is no From 4e4a40df7afe99eb0d6b7026037263ccce15d48b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 3 Jan 2017 15:42:38 -0800 Subject: [PATCH 3/6] feedback --- .../java/org/elasticsearch/cli/Terminal.java | 21 ++++++++++++ .../settings/AddStringKeyStoreCommand.java | 4 +-- .../settings/CreateKeyStoreCommand.java | 3 +- .../common/settings/KeyStoreWrapper.java | 32 ++++++++++--------- .../common/settings/ListKeyStoreCommand.java | 5 --- .../common/settings/SecureString.java | 2 +- .../common/settings/Settings.java | 12 ++++--- .../org/elasticsearch/cli/TerminalTests.java | 27 ++++++++++++++++ .../RemoveSettingKeyStoreCommandTests.java | 5 ++- 9 files changed, 80 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cli/Terminal.java b/core/src/main/java/org/elasticsearch/cli/Terminal.java index 58eb5012d07..cd7fc76e681 100644 --- a/core/src/main/java/org/elasticsearch/cli/Terminal.java +++ b/core/src/main/java/org/elasticsearch/cli/Terminal.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintWriter; import java.nio.charset.Charset; +import java.util.Locale; /** * A Terminal wraps access to reading input and writing output for a cli. @@ -92,6 +93,26 @@ public abstract class Terminal { } } + /** + * Prompt for a yes or no answer from the user. This method will loop until 'y' or 'n' + * (or the default empty value) is entered. + */ + public final boolean promptYesNo(String prompt, boolean defaultYes) { + String answerPrompt = defaultYes ? " [Y/n]" : " [y/N]"; + while (true) { + String answer = readText(prompt + answerPrompt).toLowerCase(Locale.ROOT); + if (answer.isEmpty()) { + return defaultYes; + } + boolean answerYes = answer.equals("y"); + if (answerYes == false && answer.equals("n") == false) { + println("Did not understand answer '" + answer + "'"); + continue; + } + return answerYes; + } + } + private static class ConsoleTerminal extends Terminal { private static final Console CONSOLE = System.console(); diff --git a/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index 289c02a1781..53571dbada0 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -24,6 +24,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.Locale; import joptsimple.OptionSet; import joptsimple.OptionSpec; @@ -65,8 +66,7 @@ class AddStringKeyStoreCommand extends EnvironmentAwareCommand { String setting = arguments.value(options); if (keystore.getSettings().contains(setting) && options.has(forceOption) == false) { - String answer = terminal.readText("Setting " + setting + " already exists. Overwrite? [y/N]"); - if (answer.equals("y") == false) { + if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { terminal.println("Exiting without modifying keystore."); return; } diff --git a/core/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 7c2f8aec19a..08860cb5ea9 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/core/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -40,8 +40,7 @@ class CreateKeyStoreCommand extends EnvironmentAwareCommand { protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); if (Files.exists(keystoreFile)) { - String answer = terminal.readText("An elasticsearch keystore already exists. Overwrite? [y/N] "); - if (answer.equals("y") == false) { + if (terminal.promptYesNo("An elasticsearch keystore already exists. Overwrite?", false) == false) { terminal.println("Exiting without creating keystore."); return; } diff --git a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index d1a605fff92..7c1d3c62585 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -37,7 +37,6 @@ import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.util.Arrays; -import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; import java.util.Set; @@ -59,15 +58,8 @@ public class KeyStoreWrapper implements Closeable { /** The keystore type for a newly created keystore. */ private static final String NEW_KEYSTORE_TYPE = "PKCS12"; - /** A factory necessary for constructing instances of secrets in a {@link KeyStore}. */ - private static final SecretKeyFactory passwordFactory; - static { - try { - passwordFactory = SecretKeyFactory.getInstance("PBE"); - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException(e); - } - } + /** The algorithm used to store password for a newly created keystore. */ + private static final String NEW_KEYSTORE_SECRET_KEY_ALGO = "PBEWithHmacSHA256AndAES_128"; /** True iff the keystore has a password needed to read. */ private final boolean hasPassword; @@ -75,6 +67,9 @@ public class KeyStoreWrapper implements Closeable { /** The type of the keystore, as passed to {@link java.security.KeyStore#getInstance(String)} */ private final String type; + /** A factory necessary for constructing instances of secrets in a {@link KeyStore}. */ + private final SecretKeyFactory secretFactory; + /** A stream of the actual keystore data. */ private final InputStream input; @@ -87,9 +82,14 @@ public class KeyStoreWrapper implements Closeable { /** The setting names contained in the loaded keystore. */ private final Set settingNames = new HashSet<>(); - private KeyStoreWrapper(boolean hasPassword, String type, InputStream input) { + private KeyStoreWrapper(boolean hasPassword, String type, String secretKeyAlgo, InputStream input) { this.hasPassword = hasPassword; this.type = type; + try { + secretFactory = SecretKeyFactory.getInstance(secretKeyAlgo); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } this.input = input; } @@ -100,7 +100,7 @@ public class KeyStoreWrapper implements Closeable { /** Constructs a new keystore with the given password. */ static KeyStoreWrapper create(char[] password) throws Exception { - KeyStoreWrapper wrapper = new KeyStoreWrapper(password.length != 0, NEW_KEYSTORE_TYPE, null); + KeyStoreWrapper wrapper = new KeyStoreWrapper(password.length != 0, NEW_KEYSTORE_TYPE, NEW_KEYSTORE_SECRET_KEY_ALGO, null); KeyStore keyStore = KeyStore.getInstance(NEW_KEYSTORE_TYPE); keyStore.load(null, null); wrapper.keystore.set(keyStore); @@ -126,7 +126,8 @@ public class KeyStoreWrapper implements Closeable { } boolean hasPassword = inputStream.readBoolean(); String type = inputStream.readUTF(); - return new KeyStoreWrapper(hasPassword, type, inputStream); + String secretKeyAlgo = inputStream.readUTF(); + return new KeyStoreWrapper(hasPassword, type, secretKeyAlgo, inputStream); } /** Returns true iff {@link #loadKeystore(char[])} has been called. */ @@ -164,6 +165,7 @@ public class KeyStoreWrapper implements Closeable { char[] password = this.keystorePassword.get().getPassword(); outputStream.writeBoolean(password.length != 0); outputStream.writeUTF(type); + outputStream.writeUTF(secretFactory.getAlgorithm()); keystore.get().store(outputStream, password); } PosixFileAttributeView attrs = Files.getFileAttributeView(keystoreFile, PosixFileAttributeView.class); @@ -186,7 +188,7 @@ public class KeyStoreWrapper implements Closeable { } // TODO: only allow getting a setting once? KeyStore.SecretKeyEntry secretKeyEntry = (KeyStore.SecretKeyEntry)entry; - PBEKeySpec keySpec = (PBEKeySpec)passwordFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class); + PBEKeySpec keySpec = (PBEKeySpec) secretFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class); SecureString value = new SecureString(keySpec.getPassword()); keySpec.clearPassword(); return value; @@ -194,7 +196,7 @@ public class KeyStoreWrapper implements Closeable { /** Set a string setting. */ void setStringSetting(String setting, char[] value) throws GeneralSecurityException { - SecretKey secretKey = passwordFactory.generateSecret(new PBEKeySpec(value)); + SecretKey secretKey = secretFactory.generateSecret(new PBEKeySpec(value)); keystore.get().setEntry(setting, new KeyStore.SecretKeyEntry(secretKey), keystorePassword.get()); settingNames.add(setting); } diff --git a/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index 07bf4263d19..3e8f31cf744 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -54,10 +54,5 @@ class ListKeyStoreCommand extends EnvironmentAwareCommand { for (String entry : sortedEntries) { terminal.println(entry); } - - // TODO: - // 2. delete command - // 3. tests for delete - // 4. shell script } } diff --git a/core/src/main/java/org/elasticsearch/common/settings/SecureString.java b/core/src/main/java/org/elasticsearch/common/settings/SecureString.java index 7d209fc6171..848393dc7c9 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SecureString.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SecureString.java @@ -80,7 +80,7 @@ public final class SecureString implements CharSequence, AutoCloseable { } /** - * Convert to a {@link String}. This only be used with APIs that do not take {@link CharSequence}. + * Convert to a {@link String}. This should only be used with APIs that do not take {@link CharSequence}. */ @Override public String toString() { diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index 2bf93fb8e0e..3543229966d 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.Version; import org.elasticsearch.common.Booleans; import org.elasticsearch.common.Strings; @@ -612,7 +613,7 @@ public final class Settings implements ToXContent { // we use a sorted map for consistent serialization when using getAsMap() private final Map map = new TreeMap<>(); - private KeyStoreWrapper keystore; + private SetOnce keystore = new SetOnce<>(); private Builder() { @@ -638,9 +639,10 @@ public final class Settings implements ToXContent { /** Sets the secret store for these settings. */ public void setKeyStore(KeyStoreWrapper keystore) { - assert this.keystore == null; // only set once! - assert keystore.isLoaded(); - this.keystore = Objects.requireNonNull(keystore); + if (keystore.isLoaded()) { + throw new IllegalStateException("The keystore wrapper must already be loaded"); + } + this.keystore.set(Objects.requireNonNull(keystore)); } /** @@ -1049,7 +1051,7 @@ public final class Settings implements ToXContent { * set on this builder. */ public Settings build() { - return new Settings(map, keystore); + return new Settings(map, keystore.get()); } } diff --git a/core/src/test/java/org/elasticsearch/cli/TerminalTests.java b/core/src/test/java/org/elasticsearch/cli/TerminalTests.java index 6673bdbc858..795780b4890 100644 --- a/core/src/test/java/org/elasticsearch/cli/TerminalTests.java +++ b/core/src/test/java/org/elasticsearch/cli/TerminalTests.java @@ -46,6 +46,33 @@ public class TerminalTests extends ESTestCase { assertPrinted(terminal, Terminal.Verbosity.NORMAL, "This message contains percent like %20n"); } + public void testPromptYesNoDefault() throws Exception { + MockTerminal terminal = new MockTerminal(); + terminal.addTextInput(""); + assertTrue(terminal.promptYesNo("Answer?", true)); + terminal.addTextInput(""); + assertFalse(terminal.promptYesNo("Answer?", false)); + } + + public void testPromptYesNoReprompt() throws Exception { + MockTerminal terminal = new MockTerminal(); + terminal.addTextInput("blah"); + terminal.addTextInput("y"); + assertTrue(terminal.promptYesNo("Answer? [Y/n]\nDid not understand answer 'blah'\nAnswer? [Y/n]", true)); + } + + public void testPromptYesNoCase() throws Exception { + MockTerminal terminal = new MockTerminal(); + terminal.addTextInput("Y"); + assertTrue(terminal.promptYesNo("Answer?", false)); + terminal.addTextInput("y"); + assertTrue(terminal.promptYesNo("Answer?", false)); + terminal.addTextInput("N"); + assertFalse(terminal.promptYesNo("Answer?", true)); + terminal.addTextInput("n"); + assertFalse(terminal.promptYesNo("Answer?", true)); + } + private void assertPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception { logTerminal.println(verbosity, text); String output = logTerminal.getOutput(); diff --git a/core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java index e83ed873116..e1d526c88be 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java @@ -19,6 +19,9 @@ package org.elasticsearch.common.settings; +import javax.crypto.SecretKeyFactory; +import java.security.Provider; +import java.security.Security; import java.util.Map; import java.util.Set; @@ -43,7 +46,7 @@ public class RemoveSettingKeyStoreCommandTests extends KeyStoreCommandTestCase { } public void testMissing() throws Exception { - UserException e = expectThrows(UserException.class, this::execute); + UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.DATA_ERROR, e.exitCode); assertThat(e.getMessage(), containsString("keystore not found")); } From 6e406aed2d44663e557c09e4e9d9b32d8226025b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 4 Jan 2017 12:53:27 -0800 Subject: [PATCH 4/6] addressing more PR comments --- .../elasticsearch/bootstrap/Bootstrap.java | 4 +- .../settings/AddStringKeyStoreCommand.java | 11 +- .../common/settings/KeyStoreWrapper.java | 135 +++++++++++++----- .../common/settings/ListKeyStoreCommand.java | 4 +- .../RemoveSettingKeyStoreCommand.java | 4 +- .../common/settings/SecureString.java | 12 +- .../common/settings/Settings.java | 2 +- .../AddStringKeyStoreCommandTests.java | 8 ++ .../settings/CreateKeyStoreCommandTests.java | 7 +- .../settings/KeyStoreCommandTestCase.java | 5 +- 10 files changed, 129 insertions(+), 63 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index b4a95e01cc4..d1d321fcefa 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -226,7 +226,7 @@ final class Bootstrap { private static KeyStoreWrapper loadKeyStore(Environment env0) throws BootstrapException { final KeyStoreWrapper keystore; try { - keystore = KeyStoreWrapper.loadMetadata(env0.configFile()); + keystore = KeyStoreWrapper.load(env0.configFile()); } catch (IOException e) { throw new BootstrapException(e); } @@ -235,7 +235,7 @@ final class Bootstrap { } try { - keystore.loadKeystore(new char[0] /* TODO: read password from stdin */); + keystore.decrypt(new char[0] /* TODO: read password from stdin */); } catch (Exception e) { throw new BootstrapException(e); } diff --git a/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index 53571dbada0..e684e925057 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -24,7 +24,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.util.Arrays; -import java.util.Locale; import joptsimple.OptionSet; import joptsimple.OptionSpec; @@ -57,12 +56,12 @@ class AddStringKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.loadMetadata(env.configFile()); + KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } - keystore.loadKeystore(new char[0] /* TODO: prompt for password when they are supported */); + keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); String setting = arguments.value(options); if (keystore.getSettings().contains(setting) && options.has(forceOption) == false) { @@ -80,7 +79,11 @@ class AddStringKeyStoreCommand extends EnvironmentAwareCommand { value = terminal.readSecret("Enter value for " + setting + ": "); } - keystore.setStringSetting(setting, value); + try { + keystore.setStringSetting(setting, value); + } catch (IllegalArgumentException e) { + throw new UserException(ExitCodes.DATA_ERROR, "String value must contain only ASCII"); + } keystore.save(env.configFile()); } } diff --git a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index 7c1d3c62585..d209551632d 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -23,13 +23,17 @@ import javax.crypto.SecretKey; import javax.crypto.SecretKeyFactory; import javax.crypto.spec.PBEKeySpec; import javax.security.auth.DestroyFailedException; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.Closeable; -import java.io.DataInputStream; -import java.io.DataOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.CharBuffer; +import java.nio.charset.CharsetEncoder; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermissions; import java.security.GeneralSecurityException; @@ -39,19 +43,32 @@ import java.security.NoSuchAlgorithmException; import java.util.Arrays; import java.util.Enumeration; import java.util.HashSet; +import java.util.Locale; import java.util.Set; +import org.apache.lucene.codecs.CodecUtil; +import org.apache.lucene.store.BufferedChecksumIndexInput; +import org.apache.lucene.store.ChecksumIndexInput; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexInput; +import org.apache.lucene.store.IndexOutput; +import org.apache.lucene.store.NIOFSDirectory; import org.apache.lucene.util.SetOnce; /** * A wrapper around a Java KeyStore which provides supplements the keystore with extra metadata. * - * Loading a keystore has 2 phases. First, call {@link #loadMetadata(Path)}. Then call - * {@link #loadKeystore(char[])} with the keystore password, or an empty char array if - * {@link #hasPassword()} is {@code false}. + * Loading a keystore has 2 phases. First, call {@link #load(Path)}. Then call + * {@link #decrypt(char[])} with the keystore password, or an empty char array if + * {@link #hasPassword()} is {@code false}. Loading and decrypting should happen + * in a single thread. Once decrypted, keys may be read with the wrapper in + * multiple threads. */ public class KeyStoreWrapper implements Closeable { + /** The name of the keystore file to read and write. */ + private static final String KEYSTORE_FILENAME = "elasticsearch.keystore"; + /** The version of the metadata written before the keystore data. */ private static final int FORMAT_VERSION = 1; @@ -59,7 +76,10 @@ public class KeyStoreWrapper implements Closeable { private static final String NEW_KEYSTORE_TYPE = "PKCS12"; /** The algorithm used to store password for a newly created keystore. */ - private static final String NEW_KEYSTORE_SECRET_KEY_ALGO = "PBEWithHmacSHA256AndAES_128"; + private static final String NEW_KEYSTORE_SECRET_KEY_ALGO = "PBE";//"PBEWithHmacSHA256AndAES_128"; + + /** An encoder to check whether string values are ascii. */ + private static final CharsetEncoder ASCII_ENCODER = StandardCharsets.US_ASCII.newEncoder(); /** True iff the keystore has a password needed to read. */ private final boolean hasPassword; @@ -70,19 +90,19 @@ public class KeyStoreWrapper implements Closeable { /** A factory necessary for constructing instances of secrets in a {@link KeyStore}. */ private final SecretKeyFactory secretFactory; - /** A stream of the actual keystore data. */ - private final InputStream input; + /** The raw bytes of the encrypted keystore. */ + private final byte[] keystoreBytes; - /** The loaded keystore. See {@link #loadKeystore(char[])}. */ + /** The loaded keystore. See {@link #decrypt(char[])}. */ private final SetOnce keystore = new SetOnce<>(); - /** The password for the keystore. See {@link #loadKeystore(char[])}. */ + /** The password for the keystore. See {@link #decrypt(char[])}. */ private final SetOnce keystorePassword = new SetOnce<>(); /** The setting names contained in the loaded keystore. */ private final Set settingNames = new HashSet<>(); - private KeyStoreWrapper(boolean hasPassword, String type, String secretKeyAlgo, InputStream input) { + private KeyStoreWrapper(boolean hasPassword, String type, String secretKeyAlgo, byte[] keystoreBytes) { this.hasPassword = hasPassword; this.type = type; try { @@ -90,12 +110,12 @@ public class KeyStoreWrapper implements Closeable { } catch (NoSuchAlgorithmException e) { throw new RuntimeException(e); } - this.input = input; + this.keystoreBytes = keystoreBytes; } /** Returns a path representing the ES keystore in the given config dir. */ static Path keystorePath(Path configDir) { - return configDir.resolve("elasticsearch.keystore"); + return configDir.resolve(KEYSTORE_FILENAME); } /** Constructs a new keystore with the given password. */ @@ -111,43 +131,61 @@ public class KeyStoreWrapper implements Closeable { /** * Loads information about the Elasticsearch keystore from the provided config directory. * - * {@link #loadKeystore(char[])} must be called before reading or writing any entries. + * {@link #decrypt(char[])} must be called before reading or writing any entries. * Returns {@code null} if no keystore exists. */ - public static KeyStoreWrapper loadMetadata(Path configDir) throws IOException { + public static KeyStoreWrapper load(Path configDir) throws IOException { Path keystoreFile = keystorePath(configDir); if (Files.exists(keystoreFile) == false) { return null; } - DataInputStream inputStream = new DataInputStream(Files.newInputStream(keystoreFile)); - int format = inputStream.readInt(); - if (format != FORMAT_VERSION) { - throw new IllegalStateException("Unknown keystore metadata format [" + format + "]"); + + NIOFSDirectory directory = new NIOFSDirectory(configDir); + try (IndexInput indexInput = directory.openInput(KEYSTORE_FILENAME, IOContext.READONCE)) { + ChecksumIndexInput input = new BufferedChecksumIndexInput(indexInput); + CodecUtil.checkHeader(input, KEYSTORE_FILENAME, FORMAT_VERSION, FORMAT_VERSION); + byte hasPasswordByte = input.readByte(); + boolean hasPassword = hasPasswordByte == 1; + if (hasPassword == false && hasPasswordByte != 0) { + throw new IllegalStateException("hasPassword boolean is corrupt: " + + String.format(Locale.ROOT, "%02x", hasPasswordByte)); + } + String type = input.readString(); + String secretKeyAlgo = input.readString(); + byte[] keystoreBytes = new byte[input.readInt()]; + input.readBytes(keystoreBytes, 0, keystoreBytes.length); + CodecUtil.checkFooter(input); + return new KeyStoreWrapper(hasPassword, type, secretKeyAlgo, keystoreBytes); } - boolean hasPassword = inputStream.readBoolean(); - String type = inputStream.readUTF(); - String secretKeyAlgo = inputStream.readUTF(); - return new KeyStoreWrapper(hasPassword, type, secretKeyAlgo, inputStream); } - /** Returns true iff {@link #loadKeystore(char[])} has been called. */ + /** Returns true iff {@link #decrypt(char[])} has been called. */ public boolean isLoaded() { return keystore.get() != null; } - /** Return true iff calling {@link #loadKeystore(char[])} requires a non-empty password. */ + /** Return true iff calling {@link #decrypt(char[])} requires a non-empty password. */ public boolean hasPassword() { return hasPassword; } - /** Loads the keystore this metadata wraps. This may only be called once. */ - public void loadKeystore(char[] password) throws GeneralSecurityException, IOException { - this.keystore.set(KeyStore.getInstance(type)); - try (InputStream in = input) { + /** + * Decrypts the underlying java keystore. + * + * This may only be called once. The provided password will be zeroed out. + */ + public void decrypt(char[] password) throws GeneralSecurityException, IOException { + if (keystore.get() != null) { + throw new IllegalStateException("Keystore has already been decrypted"); + } + keystore.set(KeyStore.getInstance(type)); + try (InputStream in = new ByteArrayInputStream(keystoreBytes)) { keystore.get().load(in, password); + } finally { + Arrays.fill(keystoreBytes, (byte)0); } - this.keystorePassword.set(new KeyStore.PasswordProtection(password)); + keystorePassword.set(new KeyStore.PasswordProtection(password)); Arrays.fill(password, '\0'); // convert keystore aliases enum into a set for easy lookup @@ -159,15 +197,27 @@ public class KeyStoreWrapper implements Closeable { /** Write the keystore to the given config directory. */ void save(Path configDir) throws Exception { - Path keystoreFile = keystorePath(configDir); - try (DataOutputStream outputStream = new DataOutputStream(Files.newOutputStream(keystoreFile))) { - outputStream.writeInt(FORMAT_VERSION); - char[] password = this.keystorePassword.get().getPassword(); - outputStream.writeBoolean(password.length != 0); - outputStream.writeUTF(type); - outputStream.writeUTF(secretFactory.getAlgorithm()); - keystore.get().store(outputStream, password); + char[] password = this.keystorePassword.get().getPassword(); + + NIOFSDirectory directory = new NIOFSDirectory(configDir); + // write to tmp file first, then overwrite + String tmpFile = KEYSTORE_FILENAME + ".tmp"; + try (IndexOutput output = directory.createOutput(tmpFile, IOContext.DEFAULT)) { + CodecUtil.writeHeader(output, KEYSTORE_FILENAME, FORMAT_VERSION); + output.writeByte(password.length == 0 ? (byte)0 : (byte)1); + output.writeString(type); + output.writeString(secretFactory.getAlgorithm()); + + ByteArrayOutputStream keystoreBytesStream = new ByteArrayOutputStream(); + keystore.get().store(keystoreBytesStream, password); + byte[] keystoreBytes = keystoreBytesStream.toByteArray(); + output.writeInt(keystoreBytes.length); + output.writeBytes(keystoreBytes, keystoreBytes.length); + CodecUtil.writeFooter(output); } + + Path keystoreFile = keystorePath(configDir); + Files.move(configDir.resolve(tmpFile), keystoreFile, StandardCopyOption.REPLACE_EXISTING); PosixFileAttributeView attrs = Files.getFileAttributeView(keystoreFile, PosixFileAttributeView.class); if (attrs != null) { // don't rely on umask: ensure the keystore has minimal permissions @@ -194,8 +244,15 @@ public class KeyStoreWrapper implements Closeable { return value; } - /** Set a string setting. */ + /** + * Set a string setting. + * + * @throws IllegalArgumentException if the value is not ASCII + */ void setStringSetting(String setting, char[] value) throws GeneralSecurityException { + if (ASCII_ENCODER.canEncode(CharBuffer.wrap(value)) == false) { + throw new IllegalArgumentException("Value must be ascii"); + } SecretKey secretKey = secretFactory.generateSecret(new PBEKeySpec(value)); keystore.get().setEntry(setting, new KeyStore.SecretKeyEntry(secretKey), keystorePassword.get()); settingNames.add(setting); diff --git a/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index 3e8f31cf744..b0484fb1526 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/core/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -42,12 +42,12 @@ class ListKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.loadMetadata(env.configFile()); + KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } - keystore.loadKeystore(new char[0] /* TODO: prompt for password when they are supported */); + keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); List sortedEntries = new ArrayList<>(keystore.getSettings()); Collections.sort(sortedEntries); diff --git a/core/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/core/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index fb35640b727..e9089b85b56 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/core/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -48,12 +48,12 @@ class RemoveSettingKeyStoreCommand extends EnvironmentAwareCommand { throw new UserException(ExitCodes.USAGE, "Must supply at least one setting to remove"); } - KeyStoreWrapper keystore = KeyStoreWrapper.loadMetadata(env.configFile()); + KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); if (keystore == null) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); } - keystore.loadKeystore(new char[0] /* TODO: prompt for password when they are supported */); + keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); for (String setting : arguments.values(options)) { if (keystore.getSettings().contains(setting) == false) { diff --git a/core/src/main/java/org/elasticsearch/common/settings/SecureString.java b/core/src/main/java/org/elasticsearch/common/settings/SecureString.java index 848393dc7c9..9ce820952e1 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SecureString.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SecureString.java @@ -40,7 +40,7 @@ public final class SecureString implements CharSequence, AutoCloseable { /** Constant time equality to avoid potential timing attacks. */ @Override - public boolean equals(Object o) { + public synchronized boolean equals(Object o) { ensureNotClosed(); if (this == o) return true; if (o == null || o instanceof CharSequence == false) return false; @@ -58,18 +58,18 @@ public final class SecureString implements CharSequence, AutoCloseable { } @Override - public int hashCode() { + public synchronized int hashCode() { return Arrays.hashCode(chars); } @Override - public int length() { + public synchronized int length() { ensureNotClosed(); return chars.length; } @Override - public char charAt(int index) { + public synchronized char charAt(int index) { ensureNotClosed(); return chars[index]; } @@ -83,7 +83,7 @@ public final class SecureString implements CharSequence, AutoCloseable { * Convert to a {@link String}. This should only be used with APIs that do not take {@link CharSequence}. */ @Override - public String toString() { + public synchronized String toString() { return new String(chars); } @@ -91,7 +91,7 @@ public final class SecureString implements CharSequence, AutoCloseable { * Closes the string by clearing the underlying char array. */ @Override - public void close() { + public synchronized void close() { Arrays.fill(chars, '\0'); chars = null; } diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index 3543229966d..dbd305bb890 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -642,7 +642,7 @@ public final class Settings implements ToXContent { if (keystore.isLoaded()) { throw new IllegalStateException("The keystore wrapper must already be loaded"); } - this.keystore.set(Objects.requireNonNull(keystore)); + this.keystore.set(keystore); } /** diff --git a/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index 8ac07aedcfc..d4c98953cd5 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -119,6 +119,14 @@ public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase { assertSecureString("foo", "secret value 2"); } + public void testNonAsciiValue() throws Exception { + KeyStoreWrapper.create(new char[0]).save(env.configFile()); + terminal.addSecretInput("non-äsčîï"); + UserException e = expectThrows(UserException.class, () -> execute("foo")); + assertEquals(ExitCodes.DATA_ERROR, e.exitCode); + assertEquals("String value must contain only ASCII", e.getMessage()); + } + void setInput(String inputStr) { input = new ByteArrayInputStream(inputStr.getBytes(StandardCharsets.UTF_8)); } diff --git a/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java index 5722b9e33d4..8584d4d1555 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -25,7 +25,6 @@ import java.nio.file.Path; import java.util.Map; import org.elasticsearch.cli.Command; -import org.elasticsearch.cli.MockTerminal; import org.elasticsearch.cli.Terminal; import org.elasticsearch.env.Environment; @@ -44,14 +43,14 @@ public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { public void testPosix() throws Exception { execute(); Path configDir = env.configFile(); - assertNotNull(KeyStoreWrapper.loadMetadata(configDir)); + assertNotNull(KeyStoreWrapper.load(configDir)); } public void testNotPosix() throws Exception { setupEnv(false); execute(); Path configDir = env.configFile(); - assertNotNull(KeyStoreWrapper.loadMetadata(configDir)); + assertNotNull(KeyStoreWrapper.load(configDir)); } public void testOverwrite() throws Exception { @@ -69,6 +68,6 @@ public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { terminal.addTextInput("y"); execute(); - assertNotNull(KeyStoreWrapper.loadMetadata(env.configFile())); + assertNotNull(KeyStoreWrapper.load(env.configFile())); } } diff --git a/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java b/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java index cb8f84f9d2b..1e02ae6a202 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java @@ -33,7 +33,6 @@ import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.cli.CommandTestCase; import org.elasticsearch.common.io.PathUtilsForTesting; import org.elasticsearch.env.Environment; -import org.elasticsearch.test.ESTestCase; import org.junit.After; import org.junit.Before; @@ -83,8 +82,8 @@ public abstract class KeyStoreCommandTestCase extends CommandTestCase { } KeyStoreWrapper loadKeystore(String password) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.loadMetadata(env.configFile()); - keystore.loadKeystore(password.toCharArray()); + KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); + keystore.decrypt(password.toCharArray()); return keystore; } From eb596d7270205fc3c585f7b067081946af523fb6 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 6 Jan 2017 01:03:45 -0800 Subject: [PATCH 5/6] more renames --- .../java/org/elasticsearch/bootstrap/Bootstrap.java | 10 +++++----- .../org/elasticsearch/bootstrap/Elasticsearch.java | 4 ++-- .../elasticsearch/common/settings/KeyStoreWrapper.java | 9 +++++---- .../bootstrap/ESElasticsearchCliTestCase.java | 6 +++--- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index d1d321fcefa..2178d10e501 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -223,10 +223,10 @@ final class Bootstrap { }; } - private static KeyStoreWrapper loadKeyStore(Environment env0) throws BootstrapException { + private static KeyStoreWrapper loadKeyStore(Environment initialEnv) throws BootstrapException { final KeyStoreWrapper keystore; try { - keystore = KeyStoreWrapper.load(env0.configFile()); + keystore = KeyStoreWrapper.load(initialEnv.configFile()); } catch (IOException e) { throw new BootstrapException(e); } @@ -281,7 +281,7 @@ final class Bootstrap { final boolean foreground, final Path pidFile, final boolean quiet, - final Environment env0) throws BootstrapException, NodeValidationException, UserException { + final Environment initialEnv) throws BootstrapException, NodeValidationException, UserException { // Set the system property before anything has a chance to trigger its use initLoggerPrefix(); @@ -291,8 +291,8 @@ final class Bootstrap { INSTANCE = new Bootstrap(); - final KeyStoreWrapper keystore = loadKeyStore(env0); - Environment environment = initialEnvironment(foreground, pidFile, keystore, env0.settings()); + final KeyStoreWrapper keystore = loadKeyStore(initialEnv); + Environment environment = initialEnvironment(foreground, pidFile, keystore, initialEnv.settings()); try { LogConfigurator.configure(environment); } catch (IOException e) { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java index 36e695f591d..fda6e6cfdec 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java @@ -117,10 +117,10 @@ class Elasticsearch extends EnvironmentAwareCommand { } } - void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment env0) + void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv) throws NodeValidationException, UserException { try { - Bootstrap.init(!daemonize, pidFile, quiet, env0); + Bootstrap.init(!daemonize, pidFile, quiet, initialEnv); } catch (BootstrapException | RuntimeException e) { // format exceptions to the console in a special way // to avoid 2MB stacktraces from guice, etc. diff --git a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index d209551632d..7d0e5a8212b 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/core/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -52,7 +52,7 @@ import org.apache.lucene.store.ChecksumIndexInput; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; -import org.apache.lucene.store.NIOFSDirectory; +import org.apache.lucene.store.SimpleFSDirectory; import org.apache.lucene.util.SetOnce; /** @@ -140,7 +140,7 @@ public class KeyStoreWrapper implements Closeable { return null; } - NIOFSDirectory directory = new NIOFSDirectory(configDir); + SimpleFSDirectory directory = new SimpleFSDirectory(configDir); try (IndexInput indexInput = directory.openInput(KEYSTORE_FILENAME, IOContext.READONCE)) { ChecksumIndexInput input = new BufferedChecksumIndexInput(indexInput); CodecUtil.checkHeader(input, KEYSTORE_FILENAME, FORMAT_VERSION, FORMAT_VERSION); @@ -199,7 +199,7 @@ public class KeyStoreWrapper implements Closeable { void save(Path configDir) throws Exception { char[] password = this.keystorePassword.get().getPassword(); - NIOFSDirectory directory = new NIOFSDirectory(configDir); + SimpleFSDirectory directory = new SimpleFSDirectory(configDir); // write to tmp file first, then overwrite String tmpFile = KEYSTORE_FILENAME + ".tmp"; try (IndexOutput output = directory.createOutput(tmpFile, IOContext.DEFAULT)) { @@ -217,7 +217,7 @@ public class KeyStoreWrapper implements Closeable { } Path keystoreFile = keystorePath(configDir); - Files.move(configDir.resolve(tmpFile), keystoreFile, StandardCopyOption.REPLACE_EXISTING); + Files.move(configDir.resolve(tmpFile), keystoreFile, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); PosixFileAttributeView attrs = Files.getFileAttributeView(keystoreFile, PosixFileAttributeView.class); if (attrs != null) { // don't rely on umask: ensure the keystore has minimal permissions @@ -230,6 +230,7 @@ public class KeyStoreWrapper implements Closeable { return settingNames; } + // TODO: make settings accessible only to code that registered the setting /** Retrieve a string setting. The {@link SecureString} should be closed once it is used. */ SecureString getStringSetting(String setting) throws GeneralSecurityException { KeyStore.Entry entry = keystore.get().getEntry(setting, keystorePassword.get()); diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java index 6ff6068bd49..d12a73c00c6 100644 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/ESElasticsearchCliTestCase.java @@ -35,7 +35,7 @@ import static org.hamcrest.CoreMatchers.equalTo; abstract class ESElasticsearchCliTestCase extends ESTestCase { interface InitConsumer { - void accept(final boolean foreground, final Path pidFile, final boolean quiet, final Environment env0); + void accept(final boolean foreground, final Path pidFile, final boolean quiet, final Environment initialEnv); } void runTest( @@ -57,9 +57,9 @@ abstract class ESElasticsearchCliTestCase extends ESTestCase { return new Environment(realSettings); } @Override - void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment env0) { + void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv) { init.set(true); - initConsumer.accept(!daemonize, pidFile, quiet, env0); + initConsumer.accept(!daemonize, pidFile, quiet, initialEnv); } @Override From 42ebfe7bdb6e2ad17cc57be85ae8a86fc3d9c2b2 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 6 Jan 2017 09:11:07 -0800 Subject: [PATCH 6/6] fix NPE --- .../java/org/elasticsearch/bootstrap/Bootstrap.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index 2178d10e501..3d1e11869a7 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -242,15 +242,17 @@ final class Bootstrap { return keystore; } - private static Environment initialEnvironment(boolean foreground, Path pidFile, - KeyStoreWrapper keystore, Settings initialSettings) { + private static Environment createEnvironment(boolean foreground, Path pidFile, + KeyStoreWrapper keystore, Settings initialSettings) { Terminal terminal = foreground ? Terminal.DEFAULT : null; Settings.Builder builder = Settings.builder(); if (pidFile != null) { builder.put(Environment.PIDFILE_SETTING.getKey(), pidFile); } builder.put(initialSettings); - builder.setKeyStore(keystore); + if (keystore != null) { + builder.setKeyStore(keystore); + } return InternalSettingsPreparer.prepareEnvironment(builder.build(), terminal, Collections.emptyMap()); } @@ -292,7 +294,7 @@ final class Bootstrap { INSTANCE = new Bootstrap(); final KeyStoreWrapper keystore = loadKeyStore(initialEnv); - Environment environment = initialEnvironment(foreground, pidFile, keystore, initialEnv.settings()); + Environment environment = createEnvironment(foreground, pidFile, keystore, initialEnv.settings()); try { LogConfigurator.configure(environment); } catch (IOException e) {