diff --git a/docs/static/keystore.asciidoc b/docs/static/keystore.asciidoc index ce6a9f0b128..2dc1dbe3268 100644 --- a/docs/static/keystore.asciidoc +++ b/docs/static/keystore.asciidoc @@ -90,7 +90,7 @@ bin/logstash-keystore create This setup requires the user running Logstash to have the environment variable `LOGSTASH_KEYSTORE_PASS=mypassword` defined. If the environment variable is not defined, -Logstash cannot access the the keystore. +Logstash cannot access the keystore. When you run Logstash from an RPM or DEB package installation, the environment variables are sourced from `/etc/sysconfig/logstash`. @@ -154,10 +154,12 @@ use the `add` command: ["source","sh",subs="attributes"] ---------------------------------------------------------------- -bin/logstash-keystore add ES_PWD +bin/logstash-keystore add ES_USER ES_PWD ---------------------------------------------------------------- -When prompted, enter a value for the key. +When prompted, enter a value for each key. + +NOTE: Key values are limited to ASCII characters. It includes digits, letters, and a few special symbols. [discrete] [[list-settings]] @@ -174,9 +176,9 @@ bin/logstash-keystore list [[remove-settings]] === Remove keys -To remove a key from the keystore, use: +To remove keys from the keystore, use: ["source","sh",subs="attributes"] ---------------------------------------------------------------- -bin/logstash-keystore remove ES_PWD +bin/logstash-keystore remove ES_USER ES_PWD ---------------------------------------------------------------- diff --git a/lib/secretstore/cli.rb b/lib/secretstore/cli.rb index 3caee9c8f2a..08612d4ddad 100644 --- a/lib/secretstore/cli.rb +++ b/lib/secretstore/cli.rb @@ -49,7 +49,7 @@ class LogStash::SecretStoreCli LogStash::Util::SettingsHelper.post_process secure_config = SecretStoreExt.getConfig(LogStash::SETTINGS.get_setting("keystore.file").value, LogStash::SETTINGS.get_setting("keystore.classname").value) cli = SecretStoreCli.new(Terminal.new) - cli.command(ARGV[0], secure_config, ARGV[1]) + cli.command(ARGV[0], secure_config, *ARGV[1, ARGV.length]) exit 0 rescue => e logger.error(e.message, :cause => e.cause, :backtrace => e.backtrace) diff --git a/logstash-core/src/main/java/org/logstash/secret/cli/SecretStoreCli.java b/logstash-core/src/main/java/org/logstash/secret/cli/SecretStoreCli.java index d3e30f8910e..5d91456538a 100644 --- a/logstash-core/src/main/java/org/logstash/secret/cli/SecretStoreCli.java +++ b/logstash-core/src/main/java/org/logstash/secret/cli/SecretStoreCli.java @@ -23,9 +23,13 @@ import org.logstash.secret.SecretIdentifier; import org.logstash.secret.store.*; +import java.nio.CharBuffer; +import java.nio.charset.CharsetEncoder; +import java.nio.charset.StandardCharsets; import java.util.*; import java.util.stream.Collectors; +import static java.util.Objects.nonNull; import static org.logstash.secret.store.SecretStoreFactory.LOGSTASH_MARKER; /** @@ -34,6 +38,7 @@ */ public class SecretStoreCli { + private static final CharsetEncoder ASCII_ENCODER = StandardCharsets.US_ASCII.newEncoder(); private final Terminal terminal; private final SecretStoreFactory secretStoreFactory; @@ -50,6 +55,10 @@ static Optional fromString(final String input) { Optional command = EnumSet.allOf(Command.class).stream().filter(c -> c.option.equals(input)).findFirst(); return command; } + + String getOption() { + return option; + } } public SecretStoreCli(Terminal terminal){ @@ -65,13 +74,13 @@ public SecretStoreCli(Terminal terminal){ * Entry point to issue a command line command. * @param primaryCommand The string representation of a {@link SecretStoreCli.Command}, if the String does not map to a {@link SecretStoreCli.Command}, then it will show the help menu. * @param config The configuration needed to work a secret store. May be null for help. - * @param argument This can be either the identifier for a secret, or a sub command like --help. May be null. + * @param arguments This can be either identifiers for a secret, or a sub command like --help. May be null. */ - public void command(String primaryCommand, SecureConfig config, String argument) { + public void command(String primaryCommand, SecureConfig config, String... arguments) { terminal.writeLine(""); final Command command = Command.fromString(primaryCommand).orElse(Command.HELP); - final Optional sub = Command.fromString(argument); - boolean help = Command.HELP.equals(sub.orElse(null)); + boolean help = nonNull(arguments) && Arrays.asList(arguments).contains(Command.HELP.getOption()); + switch (command) { case CREATE: { if (help){ @@ -102,59 +111,75 @@ public void command(String primaryCommand, SecureConfig config, String argument) } case ADD: { if (help){ - terminal.writeLine("Adds a new secret to the keystore. For example: " + + terminal.writeLine("Add secrets to the keystore. For example: " + "`bin/logstash-keystore add my-secret`, at the prompt enter your secret. You will use the identifier ${my-secret} in your Logstash configuration."); return; } - if (argument == null || argument.isEmpty()) { - terminal.writeLine("ERROR: You must supply a identifier to add. (e.g. bin/logstash-keystore add my-secret)"); + if (arguments == null || arguments.length == 0) { + terminal.writeLine("ERROR: You must supply an identifier to add. (e.g. bin/logstash-keystore add my-secret)"); return; } if (secretStoreFactory.exists(config.clone())) { - SecretIdentifier id = new SecretIdentifier(argument); - SecretStore secretStore = secretStoreFactory.load(config); - byte[] s = secretStore.retrieveSecret(id); - if (s == null) { - terminal.write(String.format("Enter value for %s: ", argument)); - char[] secret = terminal.readSecret(); - if(secret == null || secret.length == 0){ - terminal.writeLine("ERROR: You must supply a identifier to add. (e.g. bin/logstash-keystore add my-secret)"); - return; + final SecretStore secretStore = secretStoreFactory.load(config); + for (String argument : arguments) { + final SecretIdentifier id = new SecretIdentifier(argument); + final byte[] existingValue = secretStore.retrieveSecret(id); + if (existingValue != null) { + SecretStoreUtil.clearBytes(existingValue); + terminal.write(String.format("%s already exists. Overwrite ? [y/N] ", argument)); + if (!isYes(terminal.readLine())) { + continue; + } } - add(secretStore, id, SecretStoreUtil.asciiCharToBytes(secret)); - } else { - SecretStoreUtil.clearBytes(s); - terminal.write(String.format("%s already exists. Overwrite ? [y/N] ", argument)); - if (isYes(terminal.readLine())) { - terminal.write(String.format("Enter value for %s: ", argument)); - char[] secret = terminal.readSecret(); - add(secretStore, id, SecretStoreUtil.asciiCharToBytes(secret)); + + final String enterValueMessage = String.format("Enter value for %s: ", argument); + char[] secret = null; + while(secret == null) { + terminal.write(enterValueMessage); + final char[] readSecret = terminal.readSecret(); + + if (readSecret == null || readSecret.length == 0) { + terminal.writeLine("ERROR: Value cannot be empty"); + continue; + } + + if (!ASCII_ENCODER.canEncode(CharBuffer.wrap(readSecret))) { + terminal.writeLine("ERROR: Value must contain only ASCII characters"); + continue; + } + + secret = readSecret; } + + add(secretStore, id, SecretStoreUtil.asciiCharToBytes(secret)); } } else { - terminal.writeLine(String.format("ERROR: Logstash keystore not found. Use 'create' command to create one.")); + terminal.writeLine("ERROR: Logstash keystore not found. Use 'create' command to create one."); } break; } case REMOVE: { if (help){ - terminal.writeLine("Removes a secret from the keystore. For example: " + + terminal.writeLine("Remove secrets from the keystore. For example: " + "`bin/logstash-keystore remove my-secret`"); return; } - if (argument == null || argument.isEmpty()) { + if (arguments == null || arguments.length == 0) { terminal.writeLine("ERROR: You must supply a value to remove. (e.g. bin/logstash-keystore remove my-secret)"); return; } - SecretIdentifier id = new SecretIdentifier(argument); - SecretStore secretStore = secretStoreFactory.load(config); - if (secretStore.containsSecret(id)) { - secretStore.purgeSecret(id); - terminal.writeLine(String.format("Removed '%s' from the Logstash keystore.", id.getKey())); - } else { - terminal.writeLine(String.format("ERROR: '%s' does not exist in the Logstash keystore.", argument)); + final SecretStore secretStore = secretStoreFactory.load(config); + for (String argument : arguments) { + SecretIdentifier id = new SecretIdentifier(argument); + if (secretStore.containsSecret(id)) { + secretStore.purgeSecret(id); + terminal.writeLine(String.format("Removed '%s' from the Logstash keystore.", id.getKey())); + } else { + terminal.writeLine(String.format("ERROR: '%s' does not exist in the Logstash keystore.", argument)); + } } + break; } case HELP: { @@ -166,8 +191,8 @@ public void command(String primaryCommand, SecureConfig config, String argument) terminal.writeLine("--------"); terminal.writeLine("create - Creates a new Logstash keystore (e.g. bin/logstash-keystore create)"); terminal.writeLine("list - List entries in the keystore (e.g. bin/logstash-keystore list)"); - terminal.writeLine("add - Add a value to the keystore (e.g. bin/logstash-keystore add my-secret)"); - terminal.writeLine("remove - Remove a value from the keystore (e.g. bin/logstash-keystore remove my-secret)"); + terminal.writeLine("add - Add values to the keystore (e.g. bin/logstash-keystore add secret-one secret-two)"); + terminal.writeLine("remove - Remove values from the keystore (e.g. bin/logstash-keystore remove secret-one secret-two)"); terminal.writeLine(""); terminal.writeLine("Argument:"); terminal.writeLine("--------"); diff --git a/logstash-core/src/test/java/org/logstash/secret/cli/SecretStoreCliTest.java b/logstash-core/src/test/java/org/logstash/secret/cli/SecretStoreCliTest.java index 24d06ea2f7e..7bd6ed557fb 100644 --- a/logstash-core/src/test/java/org/logstash/secret/cli/SecretStoreCliTest.java +++ b/logstash-core/src/test/java/org/logstash/secret/cli/SecretStoreCliTest.java @@ -28,11 +28,15 @@ import org.logstash.secret.store.SecureConfig; import java.nio.file.Paths; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedList; import java.util.Map; +import java.util.Queue; import java.util.UUID; +import static junit.framework.TestCase.assertTrue; import static org.assertj.core.api.Assertions.assertThat; import static org.logstash.secret.store.SecretStoreFactory.ENVIRONMENT_PASS_KEY; @@ -54,6 +58,7 @@ public void _setup() throws Exception { final SecretStoreFactory secretStoreFactory = SecretStoreFactory.withEnvironment(environment); cli = new SecretStoreCli(terminal, secretStoreFactory); + existingStoreConfig = new SecureConfig(); existingStoreConfig.add("keystore.file", Paths.get(this.getClass().getClassLoader().getResource("logstash.keystore.with.default.pass").toURI()).toString().toCharArray()); @@ -64,14 +69,14 @@ public void _setup() throws Exception { @Test public void testBadCommand() { - cli.command("nonsense", null, null); + cli.command("nonsense", null); assertPrimaryHelped(); } @Test public void testHelpAdd() { cli.command("add", null, "--help"); - assertThat(terminal.out).containsIgnoringCase("Adds a new secret to the keystore"); + assertThat(terminal.out).containsIgnoringCase("Add secrets to the keystore"); } @Test @@ -89,12 +94,12 @@ public void testHelpList() { @Test public void testHelpRemove() { cli.command("remove", null, "--help"); - assertThat(terminal.out).containsIgnoringCase("Removes a secret from the keystore"); + assertThat(terminal.out).containsIgnoringCase("Remove secrets from the keystore"); } @Test public void testList() { - cli.command("list", existingStoreConfig, null); + cli.command("list", existingStoreConfig); // contents of the existing store is a-z for both the key and value for (int i = 65; i <= 90; i++) { @@ -106,104 +111,166 @@ public void testList() { @Test public void testCreateNewAllYes() { - terminal.in = "y"; - cli.command("create", newStoreConfig, null); + terminal.in.add("y"); + cli.command("create", newStoreConfig); assertCreated(); } @Test public void testCreateNewAllNo() { - terminal.in = "n"; - cli.command("create", newStoreConfig, null); + terminal.in.add("n"); + cli.command("create", newStoreConfig); assertNotCreated(); } @Test public void testCreateNoEnvironmentWarning() { - cli.command("create", newStoreConfig, null); + cli.command("create", newStoreConfig); assertThat(terminal.out).contains("Please set the environment variable `LOGSTASH_KEYSTORE_PASS`. Failure to do so will result in reduced security."); } @Test public void testDoubleCreateWarning() { - terminal.in = "y"; - cli.command("create", newStoreConfig, null); - assertCreated(); - terminal.reset(); + createKeyStore(); - cli.command("create", newStoreConfig, null); + cli.command("create", newStoreConfig); assertThat(terminal.out).contains("Overwrite"); assertNotCreated(); } @Test public void testAddEmptyValue() { - terminal.in = "y"; - cli.command("create", newStoreConfig, null); - assertCreated(); - terminal.reset(); + createKeyStore(); + + terminal.in.add(""); // sets the empty value + terminal.in.add("value"); + + String id = UUID.randomUUID().toString(); + cli.command("add", newStoreConfig.clone(), id); + assertThat(terminal.out).containsIgnoringCase("ERROR: Value cannot be empty"); + } + + @Test + public void testAddNonAsciiValue() { + createKeyStore(); + + terminal.in.add("€€€€€"); // sets non-ascii value value + terminal.in.add("value"); - terminal.in = ""; // sets the value String id = UUID.randomUUID().toString(); cli.command("add", newStoreConfig.clone(), id); - assertThat(terminal.out).containsIgnoringCase("ERROR"); + assertThat(terminal.out).containsIgnoringCase("ERROR: Value must contain only ASCII characters"); } @Test public void testAdd() { - terminal.in = "y"; - cli.command("create", newStoreConfig, null); - assertCreated(); - terminal.reset(); + createKeyStore(); - terminal.in = UUID.randomUUID().toString(); // sets the value + terminal.in.add(UUID.randomUUID().toString()); // sets the value String id = UUID.randomUUID().toString(); cli.command("add", newStoreConfig.clone(), id); terminal.reset(); - cli.command("list", newStoreConfig, null); + cli.command("list", newStoreConfig); assertListed(id); } @Test - public void testRemove() { - terminal.in = "y"; - cli.command("create", newStoreConfig, null); - assertCreated(); + public void testAddWithNoIdentifiers() { + final String expectedMessage = "ERROR: You must supply an identifier to add"; + + createKeyStore(); + + String[] nullArguments = null; + cli.command("add", newStoreConfig.clone(), nullArguments); + assertThat(terminal.out).containsIgnoringCase(expectedMessage); + terminal.reset(); - terminal.in = UUID.randomUUID().toString(); // sets the value + cli.command("add", newStoreConfig.clone()); + assertThat(terminal.out).containsIgnoringCase(expectedMessage); + } + + @Test + public void testAddMultipleKeys() { + createKeyStore(); + + terminal.in.add(UUID.randomUUID().toString()); + terminal.in.add(UUID.randomUUID().toString()); + + final String keyOne = UUID.randomUUID().toString(); + final String keyTwo = UUID.randomUUID().toString(); + cli.command("add", newStoreConfig.clone(), keyOne, keyTwo); + terminal.reset(); + + cli.command("list", newStoreConfig); + assertListed(keyOne, keyTwo); + } + + @Test + public void testAddWithoutCreatedKeystore() { + cli.command("add", newStoreConfig.clone(), UUID.randomUUID().toString()); + assertThat(terminal.out).containsIgnoringCase("ERROR: Logstash keystore not found. Use 'create' command to create one."); + } + + @Test + public void testRemove() { + createKeyStore(); + + terminal.in.add(UUID.randomUUID().toString()); // sets the value String id = UUID.randomUUID().toString(); cli.command("add", newStoreConfig.clone(), id); System.out.println(terminal.out); terminal.reset(); - cli.command("list", newStoreConfig.clone(), null); + cli.command("list", newStoreConfig.clone()); assertListed(id); terminal.reset(); cli.command("remove", newStoreConfig.clone(), id); terminal.reset(); - cli.command("list", newStoreConfig, null); + cli.command("list", newStoreConfig); assertThat(terminal.out).doesNotContain(id); } @Test - public void testRemoveMissing() { - terminal.in = "y"; - cli.command("create", newStoreConfig, null); - assertCreated(); + public void testRemoveMultipleKeys() { + createKeyStore(); + + terminal.in.add(UUID.randomUUID().toString()); + terminal.in.add(UUID.randomUUID().toString()); + + final String keyOne = UUID.randomUUID().toString(); + final String keyTwo = UUID.randomUUID().toString(); + + cli.command("add", newStoreConfig.clone(), keyOne, keyTwo); + terminal.reset(); + + cli.command("list", newStoreConfig.clone()); + assertListed(keyOne, keyTwo); terminal.reset(); - terminal.in = UUID.randomUUID().toString(); // sets the value + cli.command("remove", newStoreConfig.clone(), keyOne, keyTwo); + terminal.reset(); + + cli.command("list", newStoreConfig); + assertThat(terminal.out).doesNotContain(keyOne); + assertThat(terminal.out).doesNotContain(keyTwo); + } + + @Test + public void testRemoveMissing() { + createKeyStore(); + + terminal.in.add(UUID.randomUUID().toString()); // sets the value String id = UUID.randomUUID().toString(); cli.command("add", newStoreConfig.clone(), id); System.out.println(terminal.out); terminal.reset(); - cli.command("list", newStoreConfig.clone(), null); + cli.command("list", newStoreConfig.clone()); assertListed(id); terminal.reset(); @@ -211,6 +278,29 @@ public void testRemoveMissing() { assertThat(terminal.out).containsIgnoringCase("error"); } + @Test + public void testRemoveWithNoIdentifiers() { + final String expectedMessage = "ERROR: You must supply a value to remove."; + + createKeyStore(); + + String[] nullArguments = null; + cli.command("remove", newStoreConfig.clone(), nullArguments); + assertThat(terminal.out).containsIgnoringCase(expectedMessage); + + terminal.reset(); + + cli.command("remove", newStoreConfig.clone()); + assertThat(terminal.out).containsIgnoringCase(expectedMessage); + } + + private void createKeyStore() { + terminal.reset(); + terminal.in.add("y"); + cli.command("create", newStoreConfig); + assertCreated(); + terminal.reset(); + } private void assertNotCreated() { assertThat(terminal.out).doesNotContain("Created Logstash keystore"); @@ -220,8 +310,8 @@ private void assertCreated() { assertThat(terminal.out).contains("Created Logstash keystore"); } - private void assertListed(String expected) { - assertThat(terminal.out).contains(expected); + private void assertListed(String... expected) { + assertTrue(Arrays.stream(expected).allMatch(terminal.out::contains)); } private void assertPrimaryHelped() { @@ -243,7 +333,7 @@ private Map environmentWithout(final String key) { class TestTerminal extends Terminal { public String out = ""; - public String in = ""; + public final Queue in = new LinkedList<>(); @Override public void writeLine(String text) { @@ -257,16 +347,16 @@ public void write(String text) { @Override public String readLine() { - return in; + return in.poll(); } @Override public char[] readSecret() { - return in.toCharArray(); + return in.poll().toCharArray(); } public void reset() { - in = ""; + in.clear(); out = ""; } }