Skip to content

Commit

Permalink
Restore behavior for force parameter (elastic#44847)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jkakavas authored and williamrandolph committed Nov 18, 2019
1 parent 1e802e1 commit 0aac95d
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 11 deletions.
1 change: 0 additions & 1 deletion distribution/docker/docker-test-entrypoint.sh
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@
*/
class AddFileKeyStoreCommand extends BaseKeyStoreCommand {

private final OptionSpec<Void> forceOption;
private final OptionSpec<String> 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
Expand All @@ -52,7 +52,6 @@ class AddFileKeyStoreCommand extends BaseKeyStoreCommand {

@Override
protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception {

List<String> argumentValues = arguments.values(options);
if (argumentValues.size() == 0) {
throw new UserException(ExitCodes.USAGE, "Missing setting name");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@
class AddStringKeyStoreCommand extends BaseKeyStoreCommand {

private final OptionSpec<Void> stdinOption;
private final OptionSpec<Void> forceOption;
private final OptionSpec<String> 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");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,6 +35,7 @@ public abstract class BaseKeyStoreCommand extends EnvironmentAwareCommand {
private KeyStoreWrapper keyStore;
private SecureString keyStorePassword;
private final boolean keyStoreMustExist;
OptionSpec<Void> forceOption;

public BaseKeyStoreCommand(String description, boolean keyStoreMustExist) {
super(description);
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,21 @@ 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");
execute("foo", file1.toString());
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 0aac95d

Please sign in to comment.