From cbd1ed55b4ac50cb3b1f058e394b7f734e8046f1 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Thu, 25 Jul 2019 13:11:42 +0300 Subject: [PATCH 1/2] Restore behavior for force paramater 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 | 14 +++++++++++--- .../settings/AddFileKeyStoreCommandTests.java | 9 ++++++++- .../settings/AddStringKeyStoreCommandTests.java | 8 +++++++- 6 files changed, 30 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..fa4b3c85a556c 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); } /** @@ -112,4 +113,11 @@ static SecureString readPassword(Terminal terminal, boolean withVerification) th * respectively. */ protected abstract void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception; + + public abstract class ForceableBaseKeyStoreCommand extends BaseKeyStoreCommand { + + public ForceableBaseKeyStoreCommand(String description, boolean keyStoreMustExist) { + super(description, keyStoreMustExist); + } + } } 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 7978940412d10..edb3e64caae71 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 @@ -59,13 +59,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"); From f95bacc0d7f9130f4c5f55c937847efe7f4310d9 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Thu, 25 Jul 2019 13:37:22 +0300 Subject: [PATCH 2/2] cleanup --- .../elasticsearch/common/settings/BaseKeyStoreCommand.java | 7 ------- 1 file changed, 7 deletions(-) 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 fa4b3c85a556c..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 @@ -113,11 +113,4 @@ static SecureString readPassword(Terminal terminal, boolean withVerification) th * respectively. */ protected abstract void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception; - - public abstract class ForceableBaseKeyStoreCommand extends BaseKeyStoreCommand { - - public ForceableBaseKeyStoreCommand(String description, boolean keyStoreMustExist) { - super(description, keyStoreMustExist); - } - } }