From 0aac95d8af460e2230cf1991f2a29ccfd54c7b4d Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Fri, 26 Jul 2019 16:52:15 +0300 Subject: [PATCH] Restore behavior for force parameter (#44847) Turns out that the behavior of `-f` for the add and add-file sub commands where it would also forcibly create the keystore if it didn't exist, was by design - although undocumented. This change restores that behavior auto-creating a keystore that is not password protected if the force flag is used. The force OptionSpec is moved to the BaseKeyStoreCommand as we will presumably want to maintain the same behavior in any other command that takes a force option. --- distribution/docker/docker-test-entrypoint.sh | 1 - .../common/settings/AddFileKeyStoreCommand.java | 5 ++--- .../common/settings/AddStringKeyStoreCommand.java | 4 ++-- .../common/settings/BaseKeyStoreCommand.java | 7 ++++--- .../common/settings/AddFileKeyStoreCommandTests.java | 9 ++++++++- .../common/settings/AddStringKeyStoreCommandTests.java | 8 +++++++- 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/distribution/docker/docker-test-entrypoint.sh b/distribution/docker/docker-test-entrypoint.sh index 68160cffd1cee..1dca4b6a35e73 100755 --- a/distribution/docker/docker-test-entrypoint.sh +++ b/distribution/docker/docker-test-entrypoint.sh @@ -1,7 +1,6 @@ #!/bin/bash cd /usr/share/elasticsearch/bin/ ./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 db65250e571d2..544c58e038866 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 @@ -38,12 +38,12 @@ */ class AddFileKeyStoreCommand extends BaseKeyStoreCommand { - private final OptionSpec forceOption; private final OptionSpec arguments; AddFileKeyStoreCommand() { super("Add a file setting to the keystore", false); - this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting"); + this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), + "Overwrite existing setting without prompting, creating keystore if necessary"); // jopt simple has issue with multiple non options, so we just get one set of them here // and convert to File when necessary // see https://github.com/jopt-simple/jopt-simple/issues/103 @@ -52,7 +52,6 @@ class AddFileKeyStoreCommand extends BaseKeyStoreCommand { @Override 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"); 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 45284fd8ee2c2..b480051d410b6 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 @@ -38,13 +38,13 @@ 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", 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.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), + "Overwrite existing setting without prompting, creating keystore if necessary"); this.arguments = parser.nonOptions("setting name"); } 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 index 624e88717ad4b..75eec8d985b62 100644 --- 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 @@ -20,6 +20,7 @@ package org.elasticsearch.common.settings; import joptsimple.OptionSet; +import joptsimple.OptionSpec; import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; import org.elasticsearch.cli.Terminal; @@ -34,6 +35,7 @@ public abstract class BaseKeyStoreCommand extends EnvironmentAwareCommand { private KeyStoreWrapper keyStore; private SecureString keyStorePassword; private final boolean keyStoreMustExist; + OptionSpec forceOption; public BaseKeyStoreCommand(String description, boolean keyStoreMustExist) { super(description); @@ -49,7 +51,7 @@ protected final void execute(Terminal terminal, OptionSet options, Environment e if (keyStoreMustExist) { throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found at [" + KeyStoreWrapper.keystorePath(env.configFile()) + "]. Use 'create' command to create one."); - } else { + } else if (options.has(forceOption) == false) { if (terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) { terminal.println("Exiting without creating keystore."); return; @@ -101,8 +103,7 @@ static SecureString readPassword(Terminal terminal, boolean withVerification) th } else { passwordArray = terminal.readSecret("Enter password for the elasticsearch keystore : "); } - final SecureString password = new SecureString(passwordArray); - return password; + return new SecureString(passwordArray); } /** 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 7252ea0e99ca5..83768ca619bf5 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 @@ -58,7 +58,7 @@ private void addFile(KeyStoreWrapper keystore, String setting, Path file, String keystore.save(env.configFile(), password.toCharArray()); } - public void testMissingCreateWithEmptyPassword() throws Exception { + public void testMissingCreateWithEmptyPasswordWhenPrompted() throws Exception { String password = ""; Path file1 = createRandomFile(); terminal.addTextInput("y"); @@ -66,6 +66,13 @@ public void testMissingCreateWithEmptyPassword() throws Exception { assertSecureFile("foo", file1, password); } + public void testMissingCreateWithEmptyPasswordWithoutPromptIfForced() throws Exception { + String password = ""; + Path file1 = createRandomFile(); + execute("-f", "foo", file1.toString()); + assertSecureFile("foo", file1, password); + } + public void testMissingNoCreate() throws Exception { terminal.addSecretInput(randomFrom("", "keystorepassword")); terminal.addTextInput("n"); // explicit no 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 ddafcb09fcdd8..84d85694c3e72 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 @@ -60,13 +60,19 @@ public void testInvalidPassphrease() throws Exception { } - public void testMissingPromptCreateWithoutPassword() throws Exception { + public void testMissingPromptCreateWithoutPasswordWhenPrompted() throws Exception { terminal.addTextInput("y"); terminal.addSecretInput("bar"); execute("foo"); assertSecureString("foo", "bar", ""); } + public void testMissingPromptCreateWithoutPasswordWithoutPromptIfForced() throws Exception { + terminal.addSecretInput("bar"); + execute("-f", "foo"); + assertSecureString("foo", "bar", ""); + } + public void testMissingNoCreate() throws Exception { terminal.addTextInput("n"); // explicit no execute("foo");