From 9398aac4394c4a3ba2b85af145ce55d2b70aca2f Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 23 Jul 2019 20:09:19 +0300 Subject: [PATCH] Add passphrase support to elasticsearch-keystore (#38498) This change adds support for keystore passphrases to all subcommands of the elasticsearch-keystore cli tool and adds a subcommand for changing the passphrase of an existing keystore. The work to read the passphrase in Elasticsearch when loading, which will be addressed in a different PR. Subcommands of elasticsearch-keystore can handle (open and create) passphrase protected keystores When reading a keystore, a user is only prompted for a passphrase only if the keystore is passphrase protected. When creating a keystore, a user is allowed (default behavior) to create one with an empty passphrase Passphrase can be set to be empty when changing/setting it for an existing keystore Relates to: #32691 Supersedes: #37472 --- distribution/docker/docker-test-entrypoint.sh | 3 +- .../settings/AddFileKeyStoreCommand.java | 29 ++-- .../settings/AddStringKeyStoreCommand.java | 45 ++---- .../common/settings/BaseKeyStoreCommand.java | 115 +++++++++++++++ .../ChangeKeyStorePasswordCommand.java | 47 +++++++ .../settings/CreateKeyStoreCommand.java | 39 +++--- .../common/settings/KeyStoreCli.java | 1 + .../common/settings/ListKeyStoreCommand.java | 19 +-- .../RemoveSettingKeyStoreCommand.java | 22 +-- .../settings/UpgradeKeyStoreCommand.java | 18 +-- .../settings/AddFileKeyStoreCommandTests.java | 103 +++++++++----- .../AddStringKeyStoreCommandTests.java | 132 ++++++++---------- .../ChangeKeyStorePasswordCommandTests.java | 95 +++++++++++++ .../settings/CreateKeyStoreCommandTests.java | 27 ++++ .../settings/KeyStoreCommandTestCase.java | 8 +- .../common/settings/KeyStoreWrapperTests.java | 6 +- .../settings/ListKeyStoreCommandTests.java | 28 +++- .../RemoveSettingKeyStoreCommandTests.java | 41 +++++- .../settings/UpgradeKeyStoreCommandTests.java | 2 +- .../common/settings/KeyStoreWrapper.java | 22 ++- 20 files changed, 557 insertions(+), 245 deletions(-) create mode 100644 distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java create mode 100644 distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java create mode 100644 distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java diff --git a/distribution/docker/docker-test-entrypoint.sh b/distribution/docker/docker-test-entrypoint.sh index a1e5dd0ffda2f..68160cffd1cee 100755 --- a/distribution/docker/docker-test-entrypoint.sh +++ b/distribution/docker/docker-test-entrypoint.sh @@ -1,6 +1,7 @@ #!/bin/bash cd /usr/share/elasticsearch/bin/ -./elasticsearch-users useradd x_pack_rest_user -p x-pack-test-password -r superuser || true +./elasticsearch-users useradd x_pack_rest_user -p x-pack-test-password -r superuser || true +./elasticsearch-keystore create echo "testnode" > /tmp/password cat /tmp/password | ./elasticsearch-keystore add -x -f -v 'xpack.security.transport.ssl.keystore.secure_password' cat /tmp/password | ./elasticsearch-keystore add -x -f -v 'xpack.security.http.ssl.keystore.secure_password' diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java index f5b3cb9cf7104..db65250e571d2 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java @@ -26,7 +26,6 @@ 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; @@ -37,13 +36,13 @@ /** * A subcommand for the keystore cli which adds a file setting. */ -class AddFileKeyStoreCommand extends EnvironmentAwareCommand { +class AddFileKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec forceOption; private final OptionSpec arguments; AddFileKeyStoreCommand() { - super("Add a file setting to the keystore"); + super("Add a file setting to the keystore", false); this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting"); // jopt simple has issue with multiple non options, so we just get one set of them here // and convert to File when necessary @@ -52,27 +51,15 @@ class AddFileKeyStoreCommand extends EnvironmentAwareCommand { } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - if (options.has(forceOption) == false && - terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; - } - keystore = KeyStoreWrapper.create(); - keystore.save(env.configFile(), new char[0] /* always use empty passphrase for auto created keystore */); - terminal.println("Created elasticsearch keystore in " + env.configFile()); - } else { - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - } + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { List argumentValues = arguments.values(options); if (argumentValues.size() == 0) { throw new UserException(ExitCodes.USAGE, "Missing setting name"); } String setting = argumentValues.get(0); - if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { + final KeyStoreWrapper keyStore = getKeyStore(); + if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) { if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { terminal.println("Exiting without modifying keystore."); return; @@ -90,11 +77,11 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th throw new UserException(ExitCodes.USAGE, "Unrecognized extra arguments [" + String.join(", ", argumentValues.subList(2, argumentValues.size())) + "] after filepath"); } - keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), new char[0]); + keyStore.setFile(setting, Files.readAllBytes(file)); + keyStore.save(env.configFile(), getKeyStorePassword().getChars()); } - @SuppressForbidden(reason="file arg for cli") + @SuppressForbidden(reason = "file arg for cli") private Path getPath(String file) { return PathUtils.get(file); } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java index ba006cd36f372..45284fd8ee2c2 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java @@ -20,7 +20,6 @@ package org.elasticsearch.common.settings; import java.io.BufferedReader; -import java.io.CharArrayWriter; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; @@ -28,7 +27,6 @@ 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; @@ -37,14 +35,14 @@ /** * A subcommand for the keystore cli which adds a string setting. */ -class AddStringKeyStoreCommand extends EnvironmentAwareCommand { +class AddStringKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec stdinOption; private final OptionSpec forceOption; private final OptionSpec arguments; AddStringKeyStoreCommand() { - super("Add a string setting to the keystore"); + super("Add a string setting to the keystore", false); 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"); @@ -56,26 +54,13 @@ InputStream getStdin() { } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - KeyStoreWrapper keystore = KeyStoreWrapper.load(env.configFile()); - if (keystore == null) { - if (options.has(forceOption) == false && - terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; - } - keystore = KeyStoreWrapper.create(); - keystore.save(env.configFile(), new char[0] /* always use empty passphrase for auto created keystore */); - terminal.println("Created elasticsearch keystore in " + env.configFile()); - } else { - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - } - + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { String setting = arguments.value(options); if (setting == null) { throw new UserException(ExitCodes.USAGE, "The setting name can not be null"); } - if (keystore.getSettingNames().contains(setting) && options.has(forceOption) == false) { + final KeyStoreWrapper keyStore = getKeyStore(); + if (keyStore.getSettingNames().contains(setting) && options.has(forceOption) == false) { if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) { terminal.println("Exiting without modifying keystore."); return; @@ -84,26 +69,18 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th final char[] value; if (options.has(stdinOption)) { - try (BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8)); - CharArrayWriter writer = new CharArrayWriter()) { - int charInt; - while ((charInt = stdinReader.read()) != -1) { - if ((char) charInt == '\r' || (char) charInt == '\n') { - break; - } - writer.write((char) charInt); - } - value = writer.toCharArray(); - } + BufferedReader stdinReader = new BufferedReader(new InputStreamReader(getStdin(), StandardCharsets.UTF_8)); + value = stdinReader.readLine().toCharArray(); } else { value = terminal.readSecret("Enter value for " + setting + ": "); } try { - keystore.setString(setting, value); - } catch (final IllegalArgumentException e) { + keyStore.setString(setting, value); + } catch (IllegalArgumentException e) { throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); } - keystore.save(env.configFile(), new char[0]); + keyStore.save(env.configFile(), getKeyStorePassword().getChars()); + } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java new file mode 100644 index 0000000000000..624e88717ad4b --- /dev/null +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java @@ -0,0 +1,115 @@ +/* + * 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 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; + +import java.nio.file.Path; +import java.util.Arrays; + +public abstract class BaseKeyStoreCommand extends EnvironmentAwareCommand { + + private KeyStoreWrapper keyStore; + private SecureString keyStorePassword; + private final boolean keyStoreMustExist; + + public BaseKeyStoreCommand(String description, boolean keyStoreMustExist) { + super(description); + this.keyStoreMustExist = keyStoreMustExist; + } + + @Override + protected final void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + try { + final Path configFile = env.configFile(); + keyStore = KeyStoreWrapper.load(configFile); + if (keyStore == null) { + if (keyStoreMustExist) { + throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found at [" + + KeyStoreWrapper.keystorePath(env.configFile()) + "]. Use 'create' command to create one."); + } else { + if (terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { + terminal.println("Exiting without creating keystore."); + return; + } + } + keyStorePassword = new SecureString(new char[0]); + keyStore = KeyStoreWrapper.create(); + keyStore.save(configFile, keyStorePassword.getChars()); + } else { + keyStorePassword = keyStore.hasPassword() ? readPassword(terminal, false) : new SecureString(new char[0]); + keyStore.decrypt(keyStorePassword.getChars()); + } + executeCommand(terminal, options, env); + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); + } finally { + if (keyStorePassword != null) { + keyStorePassword.close(); + } + } + } + + protected KeyStoreWrapper getKeyStore() { + return keyStore; + } + + protected SecureString getKeyStorePassword() { + return keyStorePassword; + } + + /** + * Reads the keystore password from the {@link Terminal}, prompting for verification where applicable and returns it as a + * {@link SecureString}. + * + * @param terminal the terminal to use for user inputs + * @param withVerification whether the user should be prompted for password verification + * @return a SecureString with the password the user entered + * @throws UserException If the user is prompted for verification and enters a different password + */ + static SecureString readPassword(Terminal terminal, boolean withVerification) throws UserException { + final char[] passwordArray; + if (withVerification) { + passwordArray = terminal.readSecret("Enter new password for the elasticsearch keystore (empty for no password): "); + char[] passwordVerification = terminal.readSecret("Enter same password again: "); + if (Arrays.equals(passwordArray, passwordVerification) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passwords are not equal, exiting."); + } + Arrays.fill(passwordVerification, '\u0000'); + } else { + passwordArray = terminal.readSecret("Enter password for the elasticsearch keystore : "); + } + final SecureString password = new SecureString(passwordArray); + return password; + } + + /** + * This is called after the keystore password has been read from the stdin and the keystore is decrypted and + * loaded. The keystore and keystore passwords are available to classes extending {@link BaseKeyStoreCommand} + * using {@link BaseKeyStoreCommand#getKeyStore()} and {@link BaseKeyStoreCommand#getKeyStorePassword()} + * respectively. + */ + protected abstract void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception; +} diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java new file mode 100644 index 0000000000000..526201ede8f66 --- /dev/null +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommand.java @@ -0,0 +1,47 @@ +/* + * 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 joptsimple.OptionSet; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.Terminal; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +/** + * A sub-command for the keystore cli which changes the password. + */ +class ChangeKeyStorePasswordCommand extends BaseKeyStoreCommand { + + ChangeKeyStorePasswordCommand() { + super("Changes the password of a keystore", true); + } + + @Override + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { + try (SecureString newPassword = readPassword(terminal, true)) { + final KeyStoreWrapper keyStore = getKeyStore(); + keyStore.save(env.configFile(), newPassword.getChars()); + terminal.println("Elasticsearch keystore password changed successfully."); + } catch (SecurityException e) { + throw new UserException(ExitCodes.DATA_ERROR, e.getMessage()); + } + } +} diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 3529d7f6810bd..379af6f3a47f3 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -21,41 +21,44 @@ import java.nio.file.Files; import java.nio.file.Path; +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 to create a new keystore. + * A sub-command for the keystore cli to create a new keystore. */ class CreateKeyStoreCommand extends EnvironmentAwareCommand { + private final OptionSpec passwordOption; + CreateKeyStoreCommand() { super("Creates a new elasticsearch keystore"); + this.passwordOption = parser.acceptsAll(Arrays.asList("p", "password"), "Prompt for password to encrypt the keystore"); } @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); - if (Files.exists(keystoreFile)) { - if (terminal.promptYesNo("An elasticsearch keystore already exists. Overwrite?", false) == false) { - terminal.println("Exiting without creating keystore."); - return; + try (SecureString password = options.has(passwordOption) ? + BaseKeyStoreCommand.readPassword(terminal, true) : new SecureString(new char[0])) { + Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); + if (Files.exists(keystoreFile)) { + if (terminal.promptYesNo("An elasticsearch keystore already exists. Overwrite?", false) == false) { + terminal.println("Exiting without creating keystore."); + return; + } } + KeyStoreWrapper keystore = KeyStoreWrapper.create(); + keystore.save(env.configFile(), password.getChars()); + terminal.println("Created elasticsearch keystore in " + KeyStoreWrapper.keystorePath(env.configFile())); + } catch (SecurityException e) { + throw new UserException(ExitCodes.IO_ERROR, "Error creating the elasticsearch keystore."); } - - - 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(); - keystore.save(env.configFile(), password); - terminal.println("Created elasticsearch keystore in " + env.configFile()); } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java index 19a453f7e90fd..151c13ad68ebe 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/KeyStoreCli.java @@ -35,6 +35,7 @@ private KeyStoreCli() { subcommands.put("add-file", new AddFileKeyStoreCommand()); subcommands.put("remove", new RemoveSettingKeyStoreCommand()); subcommands.put("upgrade", new UpgradeKeyStoreCommand()); + subcommands.put("passwd", new ChangeKeyStorePasswordCommand()); } public static void main(String[] args) throws Exception { diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java index 8eef02f213189..edd8a68cc6f9f 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/ListKeyStoreCommand.java @@ -25,31 +25,22 @@ 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 { +class ListKeyStoreCommand extends BaseKeyStoreCommand { ListKeyStoreCommand() { - super("List entries in the keystore"); + super("List entries in the keystore", true); } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { - 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.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - - List sortedEntries = new ArrayList<>(keystore.getSettingNames()); + protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception { + final KeyStoreWrapper keyStore = getKeyStore(); + List sortedEntries = new ArrayList<>(keyStore.getSettingNames()); Collections.sort(sortedEntries); for (String entry : sortedEntries) { terminal.println(entry); diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java index 9a83375e6e01a..6e839d4f331ba 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommand.java @@ -23,7 +23,6 @@ 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; @@ -32,35 +31,28 @@ /** * A subcommand for the keystore cli to remove a setting. */ -class RemoveSettingKeyStoreCommand extends EnvironmentAwareCommand { +class RemoveSettingKeyStoreCommand extends BaseKeyStoreCommand { private final OptionSpec arguments; RemoveSettingKeyStoreCommand() { - super("Remove a setting from the keystore"); + super("Remove a setting from the keystore", true); arguments = parser.nonOptions("setting names"); } @Override - protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { + protected void executeCommand(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.load(env.configFile()); - if (keystore == null) { - throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found. Use 'create' command to create one."); - } - - keystore.decrypt(new char[0] /* TODO: prompt for password when they are supported */); - + final KeyStoreWrapper keyStore = getKeyStore(); for (String setting : arguments.values(options)) { - if (keystore.getSettingNames().contains(setting) == false) { + if (keyStore.getSettingNames().contains(setting) == false) { throw new UserException(ExitCodes.CONFIG, "Setting [" + setting + "] does not exist in the keystore."); } - keystore.remove(setting); + keyStore.remove(setting); } - keystore.save(env.configFile(), new char[0]); + keyStore.save(env.configFile(), getKeyStorePassword().getChars()); } } diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java index 6338f40ea05fa..640a76432d3b4 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommand.java @@ -20,31 +20,21 @@ package org.elasticsearch.common.settings; 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 sub-command for the keystore CLI that enables upgrading the keystore format. */ -public class UpgradeKeyStoreCommand extends EnvironmentAwareCommand { +public class UpgradeKeyStoreCommand extends BaseKeyStoreCommand { UpgradeKeyStoreCommand() { - super("Upgrade the keystore format"); + super("Upgrade the keystore format", true); } @Override - protected void execute(final Terminal terminal, final OptionSet options, final Environment env) throws Exception { - final KeyStoreWrapper wrapper = KeyStoreWrapper.load(env.configFile()); - if (wrapper == null) { - throw new UserException( - ExitCodes.CONFIG, - "keystore does not exist at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"); - } - wrapper.decrypt(new char[0]); - KeyStoreWrapper.upgrade(wrapper, env.configFile(), new char[0]); + protected void executeCommand(final Terminal terminal, final OptionSet options, final Environment env) throws Exception { + KeyStoreWrapper.upgrade(getKeyStore(), env.configFile(), getKeyStorePassword().getChars()); } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java index 6cfa2c1fdf255..7252ea0e99ca5 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java @@ -53,110 +53,149 @@ private Path createRandomFile() throws IOException { return file; } - private void addFile(KeyStoreWrapper keystore, String setting, Path file) throws Exception { + private void addFile(KeyStoreWrapper keystore, String setting, Path file, String password) throws Exception { keystore.setFile(setting, Files.readAllBytes(file)); - keystore.save(env.configFile(), new char[0]); + keystore.save(env.configFile(), password.toCharArray()); } - public void testMissingPromptCreate() throws Exception { + public void testMissingCreateWithEmptyPassword() throws Exception { + String password = ""; Path file1 = createRandomFile(); terminal.addTextInput("y"); execute("foo", file1.toString()); - assertSecureFile("foo", file1); - } - - public void testMissingForceCreate() throws Exception { - Path file1 = createRandomFile(); - terminal.addSecretInput("bar"); - execute("-f", "foo", file1.toString()); - assertSecureFile("foo", file1); + assertSecureFile("foo", file1, password); } public void testMissingNoCreate() throws Exception { + terminal.addSecretInput(randomFrom("", "keystorepassword")); terminal.addTextInput("n"); // explicit no execute("foo"); assertNull(KeyStoreWrapper.load(env.configFile())); } public void testOverwritePromptDefault() throws Exception { + String password = "keystorepassword"; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addTextInput(""); execute("foo", "path/dne"); - assertSecureFile("foo", file); + assertSecureFile("foo", file, password); } public void testOverwritePromptExplicitNo() throws Exception { + String password = "keystorepassword"; Path file = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); + terminal.addSecretInput(password); terminal.addTextInput("n"); // explicit no execute("foo", "path/dne"); - assertSecureFile("foo", file); + assertSecureFile("foo", file, password); } public void testOverwritePromptExplicitYes() throws Exception { + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file1, password); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addTextInput("y"); Path file2 = createRandomFile(); execute("foo", file2.toString()); - assertSecureFile("foo", file2); + assertSecureFile("foo", file2, password); } public void testOverwriteForceShort() throws Exception { + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file1, password); Path file2 = createRandomFile(); + terminal.addSecretInput(password); + terminal.addSecretInput(password); execute("-f", "foo", file2.toString()); - assertSecureFile("foo", file2); + assertSecureFile("foo", file2, password); } public void testOverwriteForceLong() throws Exception { + String password = "keystorepassword"; Path file1 = createRandomFile(); - KeyStoreWrapper keystore = createKeystore(""); - addFile(keystore, "foo", file1); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file1, password); Path file2 = createRandomFile(); + terminal.addSecretInput(password); execute("--force", "foo", file2.toString()); - assertSecureFile("foo", file2); + assertSecureFile("foo", file2, password); } public void testForceNonExistent() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); Path file = createRandomFile(); + terminal.addSecretInput(password); execute("--force", "foo", file.toString()); - assertSecureFile("foo", file); + assertSecureFile("foo", file, password); } public void testMissingSettingName() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, this::execute); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Missing setting name")); } public void testMissingFileName() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Missing file name")); } public void testFileDNE() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo", "path/dne")); assertEquals(ExitCodes.IO_ERROR, e.exitCode); assertThat(e.getMessage(), containsString("File [path/dne] does not exist")); } public void testExtraArguments() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); Path file = createRandomFile(); + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString(), "bar")); assertEquals(e.getMessage(), ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("Unrecognized extra arguments [bar]")); } + + public void testIncorrectPassword() throws Exception { + String password = "keystorepassword"; + createKeystore(password); + Path file = createRandomFile(); + terminal.addSecretInput("thewrongkeystorepassword"); + UserException e = expectThrows(UserException.class, () -> execute("foo", file.toString())); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); + } + + public void testAddToUnprotectedKeystore() throws Exception { + String password = ""; + Path file = createRandomFile(); + KeyStoreWrapper keystore = createKeystore(password); + addFile(keystore, "foo", file, password); + terminal.addTextInput(""); + // will not be prompted for a password + execute("foo", "path/dne"); + assertSecureFile("foo", file, password); + } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java index be4fb90fc8278..7978940412d10 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java @@ -20,10 +20,8 @@ package org.elasticsearch.common.settings; import java.io.ByteArrayInputStream; -import java.io.CharArrayWriter; import java.io.InputStream; import java.nio.charset.StandardCharsets; -import java.util.Locale; import java.util.Map; import org.elasticsearch.cli.Command; @@ -32,7 +30,6 @@ import org.elasticsearch.env.Environment; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasToString; public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase { InputStream input; @@ -44,6 +41,7 @@ protected Command newCommand() { protected Environment createEnv(Map settings) throws UserException { return env; } + @Override InputStream getStdin() { return input; @@ -51,17 +49,21 @@ InputStream getStdin() { }; } - public void testMissingPromptCreate() throws Exception { - terminal.addTextInput("y"); - terminal.addSecretInput("bar"); - execute("foo"); - assertSecureString("foo", "bar"); + public void testInvalidPassphrease() throws Exception { + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput("thewrongpassword"); + UserException e = expectThrows(UserException.class, () -> execute("foo2")); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); + } - public void testMissingForceCreate() throws Exception { + public void testMissingPromptCreateWithoutPassword() throws Exception { + terminal.addTextInput("y"); terminal.addSecretInput("bar"); - execute("-f", "foo"); - assertSecureString("foo", "bar"); + execute("foo"); + assertSecureString("foo", "bar", ""); } public void testMissingNoCreate() throws Exception { @@ -71,119 +73,105 @@ public void testMissingNoCreate() throws Exception { } public void testOverwritePromptDefault() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addTextInput(""); execute("foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", password); } public void testOverwritePromptExplicitNo() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addTextInput("n"); // explicit no execute("foo"); - assertSecureString("foo", "bar"); + assertSecureString("foo", "bar", password); } public void testOverwritePromptExplicitYes() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); terminal.addTextInput("y"); + terminal.addSecretInput(password); terminal.addSecretInput("newvalue"); execute("foo"); - assertSecureString("foo", "newvalue"); + assertSecureString("foo", "newvalue", password); } public void testOverwriteForceShort() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addSecretInput("newvalue"); execute("-f", "foo"); // force - assertSecureString("foo", "newvalue"); + assertSecureString("foo", "newvalue", password); } public void testOverwriteForceLong() throws Exception { - createKeystore("", "foo", "bar"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); terminal.addSecretInput("and yet another secret value"); execute("--force", "foo"); // force - assertSecureString("foo", "and yet another secret value"); + assertSecureString("foo", "and yet another secret value", password); } public void testForceNonExistent() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); terminal.addSecretInput("value"); execute("--force", "foo"); // force - assertSecureString("foo", "value"); + assertSecureString("foo", "value", password); } public void testPromptForValue() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); terminal.addSecretInput("secret value"); execute("foo"); - assertSecureString("foo", "secret value"); + assertSecureString("foo", "secret value", password); } public void testStdinShort() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); setInput("secret value 1"); execute("-x", "foo"); - assertSecureString("foo", "secret value 1"); + assertSecureString("foo", "secret value 1", password); } public void testStdinLong() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); + String password = "keystorepassword"; + KeyStoreWrapper.create().save(env.configFile(), password.toCharArray()); + terminal.addSecretInput(password); setInput("secret value 2"); execute("--stdin", "foo"); - assertSecureString("foo", "secret value 2"); - } - - public void testStdinNoInput() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); - setInput(""); - execute("-x", "foo"); - assertSecureString("foo", ""); - } - - public void testStdinInputWithLineBreaks() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); - setInput("Typedthisandhitenter\n"); - execute("-x", "foo"); - assertSecureString("foo", "Typedthisandhitenter"); - } - - public void testStdinInputWithCarriageReturn() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); - setInput("Typedthisandhitenter\r"); - execute("-x", "foo"); - assertSecureString("foo", "Typedthisandhitenter"); - } - - public void testAddUtf8String() throws Exception { - KeyStoreWrapper.create().save(env.configFile(), new char[0]); - final int stringSize = randomIntBetween(8, 16); - try (CharArrayWriter secretChars = new CharArrayWriter(stringSize)) { - for (int i = 0; i < stringSize; i++) { - secretChars.write((char) randomIntBetween(129, 2048)); - } - setInput(secretChars.toString()); - execute("-x", "foo"); - assertSecureString("foo", secretChars.toString()); - } + assertSecureString("foo", "secret value 2", password); } public void testMissingSettingName() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); + terminal.addSecretInput(password); terminal.addTextInput(""); UserException e = expectThrows(UserException.class, this::execute); assertEquals(ExitCodes.USAGE, e.exitCode); assertThat(e.getMessage(), containsString("The setting name can not be null")); } - public void testUpperCaseInName() throws Exception { - createKeystore(""); - terminal.addSecretInput("value"); - final String key = randomAlphaOfLength(4) + randomAlphaOfLength(1).toUpperCase(Locale.ROOT) + randomAlphaOfLength(4); - final UserException e = expectThrows(UserException.class, () -> execute(key)); - assertThat( - e, - hasToString(containsString("Setting name [" + key + "] does not match the allowed setting name pattern [[a-z0-9_\\-.]+]"))); + public void testAddToUnprotectedKeystore() throws Exception { + String password = ""; + createKeystore(password, "foo", "bar"); + terminal.addTextInput(""); + // will not be prompted for a password + execute("foo"); + assertSecureString("foo", "bar", password); } void setInput(String inputStr) { diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java new file mode 100644 index 0000000000000..ca0b5fa363351 --- /dev/null +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ChangeKeyStorePasswordCommandTests.java @@ -0,0 +1,95 @@ +/* + * 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.Command; +import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.UserException; +import org.elasticsearch.env.Environment; + +import java.util.Map; + +import static org.hamcrest.Matchers.containsString; + +public class ChangeKeyStorePasswordCommandTests extends KeyStoreCommandTestCase { + @Override + protected Command newCommand() { + return new ChangeKeyStorePasswordCommand() { + @Override + protected Environment createEnv(Map settings) throws UserException { + return env; + } + }; + } + + public void testSetKeyStorePassword() throws Exception { + createKeystore(""); + loadKeystore(""); + terminal.addSecretInput("thepassword"); + terminal.addSecretInput("thepassword"); + // Prompted twice for the new password, since we didn't have an existing password + execute(); + loadKeystore("thepassword"); + } + + public void testChangeKeyStorePassword() throws Exception { + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldpassword"); + terminal.addSecretInput("thepassword"); + terminal.addSecretInput("thepassword"); + // Prompted thrice: Once for the existing and twice for the new password + execute(); + loadKeystore("thepassword"); + } + + public void testChangeKeyStorePasswordToEmpty() throws Exception { + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldpassword"); + terminal.addSecretInput(""); + terminal.addSecretInput(""); + // Prompted thrice: Once for the existing and twice for the new password + execute(); + loadKeystore(""); + } + + public void testChangeKeyStorePasswordWrongVerification() throws Exception { + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldpassword"); + terminal.addSecretInput("thepassword"); + terminal.addSecretInput("themisspelledpassword"); + // Prompted thrice: Once for the existing and twice for the new password + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); + } + + public void testChangeKeyStorePasswordWrongExistingPassword() throws Exception { + createKeystore("theoldpassword"); + loadKeystore("theoldpassword"); + terminal.addSecretInput("theoldmisspelledpassword"); + // We'll only be prompted once (for the old password) + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); + } +} diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java index aefedf86e7761..4fd21a9b61a7f 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/CreateKeyStoreCommandTests.java @@ -25,9 +25,12 @@ import java.util.Map; import org.elasticsearch.cli.Command; +import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; +import static org.hamcrest.Matchers.containsString; + public class CreateKeyStoreCommandTests extends KeyStoreCommandTestCase { @Override @@ -40,13 +43,34 @@ protected Environment createEnv(Map settings) throws UserExcepti }; } + public void testNotMatchingPasswords() throws Exception { + String password = randomFrom("", "keystorepassword"); + terminal.addSecretInput(password); + terminal.addSecretInput("notthekeystorepasswordyouarelookingfor"); + UserException e = expectThrows(UserException.class, () -> execute(randomFrom("-p", "--password"))); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Passwords are not equal, exiting")); + } + + public void testDefaultNotPromptForPassword() throws Exception { + execute(); + Path configDir = env.configFile(); + assertNotNull(KeyStoreWrapper.load(configDir)); + } + public void testPosix() throws Exception { + String password = randomFrom("", "keystorepassword"); + terminal.addSecretInput(password); + terminal.addSecretInput(password); execute(); Path configDir = env.configFile(); assertNotNull(KeyStoreWrapper.load(configDir)); } public void testNotPosix() throws Exception { + String password = randomFrom("", "keystorepassword"); + terminal.addSecretInput(password); + terminal.addSecretInput(password); env = setupEnv(false, fileSystems); execute(); Path configDir = env.configFile(); @@ -54,6 +78,7 @@ public void testNotPosix() throws Exception { } public void testOverwrite() throws Exception { + String password = randomFrom("", "keystorepassword"); Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); byte[] content = "not a keystore".getBytes(StandardCharsets.UTF_8); Files.write(keystoreFile, content); @@ -67,6 +92,8 @@ public void testOverwrite() throws Exception { assertArrayEquals(content, Files.readAllBytes(keystoreFile)); terminal.addTextInput("y"); + terminal.addSecretInput(password); + terminal.addSecretInput(password); execute(); assertNotNull(KeyStoreWrapper.load(env.configFile())); } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java index 7f8c71889e038..1e5527a1e245b 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreCommandTestCase.java @@ -89,16 +89,16 @@ KeyStoreWrapper loadKeystore(String password) throws Exception { return keystore; } - void assertSecureString(String setting, String value) throws Exception { - assertSecureString(loadKeystore(""), setting, value); + void assertSecureString(String setting, String value, String password) throws Exception { + assertSecureString(loadKeystore(password), setting, value); } void assertSecureString(KeyStoreWrapper keystore, String setting, String value) throws Exception { assertEquals(value, keystore.getString(setting).toString()); } - void assertSecureFile(String setting, Path file) throws Exception { - assertSecureFile(loadKeystore(""), setting, file); + void assertSecureFile(String setting, Path file, String password) throws Exception { + assertSecureFile(loadKeystore(password), setting, file); } void assertSecureFile(KeyStoreWrapper keystore, String setting, Path file) throws Exception { diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java index 568ddfe97df16..b440822501ef7 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java @@ -83,7 +83,7 @@ public void testFileSettingExhaustiveBytes() throws Exception { KeyStoreWrapper keystore = KeyStoreWrapper.create(); byte[] bytes = new byte[256]; for (int i = 0; i < 256; ++i) { - bytes[i] = (byte)i; + bytes[i] = (byte) i; } keystore.setFile("foo", bytes); keystore.save(env.configFile(), new char[0]); @@ -112,7 +112,7 @@ public void testDecryptKeyStoreWithWrongPassword() throws Exception { final KeyStoreWrapper loadedkeystore = KeyStoreWrapper.load(env.configFile()); final SecurityException exception = expectThrows(SecurityException.class, () -> loadedkeystore.decrypt(new char[]{'i', 'n', 'v', 'a', 'l', 'i', 'd'})); - assertThat(exception.getMessage(), containsString("Keystore has been corrupted or tampered with")); + assertThat(exception.getMessage(), containsString("Provided keystore password was incorrect")); } public void testCannotReadStringFromClosedKeystore() throws Exception { @@ -387,7 +387,7 @@ public void testBackcompatV2() throws Exception { byte[] base64Bytes = Base64.getEncoder().encode(fileBytes); char[] chars = new char[base64Bytes.length]; for (int i = 0; i < chars.length; ++i) { - chars[i] = (char)base64Bytes[i]; // PBE only stores the lower 8 bits, so this narrowing is ok + chars[i] = (char) base64Bytes[i]; // PBE only stores the lower 8 bits, so this narrowing is ok } secretKey = secretFactory.generateSecret(new PBEKeySpec(chars)); keystore.setEntry("file_setting", new KeyStore.SecretKeyEntry(secretKey), protectionParameter); diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java index 27c30d3aa8f58..f79fd751465ec 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/ListKeyStoreCommandTests.java @@ -47,20 +47,42 @@ public void testMissing() throws Exception { } public void testEmpty() throws Exception { - createKeystore(""); + String password = randomFrom("", "keystorepassword"); + createKeystore(password); + terminal.addSecretInput(password); execute(); assertEquals("keystore.seed\n", terminal.getOutput()); } public void testOne() throws Exception { - createKeystore("", "foo", "bar"); + String password = randomFrom("", "keystorepassword"); + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); execute(); assertEquals("foo\nkeystore.seed\n", terminal.getOutput()); } public void testMultiple() throws Exception { - createKeystore("", "foo", "1", "baz", "2", "bar", "3"); + String password = randomFrom("", "keystorepassword"); + createKeystore(password, "foo", "1", "baz", "2", "bar", "3"); + terminal.addSecretInput(password); execute(); assertEquals("bar\nbaz\nfoo\nkeystore.seed\n", terminal.getOutput()); // sorted } + + public void testListWithIncorrectPassword() throws Exception { + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput("thewrongkeystorepassword"); + UserException e = expectThrows(UserException.class, this::execute); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); + } + + public void testListWithUnprotectedKeystore() throws Exception { + createKeystore("", "foo", "bar"); + execute(); + // Not prompted for a password + assertEquals("foo\nkeystore.seed\n", terminal.getOutput()); + } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java index 2259dee31a8cb..b4cc08c846513 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/RemoveSettingKeyStoreCommandTests.java @@ -41,39 +41,66 @@ protected Environment createEnv(Map settings) throws UserExcepti }; } - public void testMissing() throws Exception { + public void testMissing() { + String password = "keystorepassword"; + terminal.addSecretInput(password); UserException e = expectThrows(UserException.class, () -> execute("foo")); assertEquals(ExitCodes.DATA_ERROR, e.exitCode); assertThat(e.getMessage(), containsString("keystore not found")); } public void testNoSettings() throws Exception { - createKeystore(""); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); 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(""); + String password = "keystorepassword"; + createKeystore(password); + terminal.addSecretInput(password); 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"); + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput(password); execute("foo"); - assertFalse(loadKeystore("").getSettingNames().contains("foo")); + assertFalse(loadKeystore(password).getSettingNames().contains("foo")); } public void testMany() throws Exception { - createKeystore("", "foo", "1", "bar", "2", "baz", "3"); + String password = "keystorepassword"; + createKeystore(password, "foo", "1", "bar", "2", "baz", "3"); + terminal.addSecretInput(password); execute("foo", "baz"); - Set settings = loadKeystore("").getSettingNames(); + Set settings = loadKeystore(password).getSettingNames(); assertFalse(settings.contains("foo")); assertFalse(settings.contains("baz")); assertTrue(settings.contains("bar")); assertEquals(2, settings.size()); // account for keystore.seed too } + + public void testRemoveWithIncorrectPassword() throws Exception { + String password = "keystorepassword"; + createKeystore(password, "foo", "bar"); + terminal.addSecretInput("thewrongpassword"); + UserException e = expectThrows(UserException.class, () -> execute("foo")); + assertEquals(e.getMessage(), ExitCodes.DATA_ERROR, e.exitCode); + assertThat(e.getMessage(), containsString("Provided keystore password was incorrect")); + } + + public void testRemoveFromUnprotectedKeystore() throws Exception { + String password = ""; + createKeystore(password, "foo", "bar"); + // will not be prompted for a password + execute("foo"); + assertFalse(loadKeystore(password).getSettingNames().contains("foo")); + } } diff --git a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java index 7f670ae77a27b..9744d73569c60 100644 --- a/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java +++ b/distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/UpgradeKeyStoreCommandTests.java @@ -69,7 +69,7 @@ public void testKeystoreUpgrade() throws Exception { public void testKeystoreDoesNotExist() { final UserException e = expectThrows(UserException.class, this::execute); - assertThat(e, hasToString(containsString("keystore does not exist at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"))); + assertThat(e, hasToString(containsString("keystore not found at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]"))); } } diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index 7ad69c1eebe0c..e6b4d61e6871c 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -32,6 +32,7 @@ import org.elasticsearch.common.Randomness; import org.elasticsearch.common.hash.MessageDigests; +import javax.crypto.AEADBadTagException; import javax.crypto.Cipher; import javax.crypto.CipherInputStream; import javax.crypto.CipherOutputStream; @@ -368,6 +369,9 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio throw new SecurityException("Keystore has been corrupted or tampered with"); } } catch (IOException e) { + if (e.getCause() instanceof AEADBadTagException) { + throw new SecurityException("Provided keystore password was incorrect", e); + } throw new SecurityException("Keystore has been corrupted or tampered with", e); } } @@ -502,9 +506,9 @@ public synchronized void save(Path configDir, char[] password) throws Exception } catch (final AccessDeniedException e) { final String message = String.format( - Locale.ROOT, - "unable to create temporary keystore at [%s], please check filesystem permissions", - configDir.resolve(tmpFile)); + Locale.ROOT, + "unable to create temporary keystore at [%s], please check filesystem permissions", + configDir.resolve(tmpFile)); throw new UserException(ExitCodes.CONFIG, message, e); } @@ -569,7 +573,9 @@ public static void validateSettingName(String setting) { } } - /** Set a string setting. */ + /** + * Set a string setting. + */ synchronized void setString(String setting, char[] value) { ensureOpen(); validateSettingName(setting); @@ -582,7 +588,9 @@ synchronized void setString(String setting, char[] value) { } } - /** Set a file setting. */ + /** + * Set a file setting. + */ synchronized void setFile(String setting, byte[] bytes) { ensureOpen(); validateSettingName(setting); @@ -593,7 +601,9 @@ synchronized void setFile(String setting, byte[] bytes) { } } - /** Remove the given setting from the keystore. */ + /** + * Remove the given setting from the keystore. + */ void remove(String setting) { ensureOpen(); Entry oldEntry = entries.get().remove(setting);