diff --git a/docs/reference/keystore.md b/docs/reference/keystore.md index 9aae6fd7482..1d57209d8c6 100644 --- a/docs/reference/keystore.md +++ b/docs/reference/keystore.md @@ -136,7 +136,7 @@ bin/logstash-keystore add ES_USER ES_PWD 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. +Key values are limited to ASCII letters (`a`-`z`, `A`-`Z`), numbers (`0`-`9`), underscores (`_`), and dots (`.`); they must be at least one character long and cannot begin with a number. :::: diff --git a/logstash-core/src/main/java/org/logstash/plugins/ConfigVariableExpander.java b/logstash-core/src/main/java/org/logstash/plugins/ConfigVariableExpander.java index 008ddc599d6..def0a7a1620 100644 --- a/logstash-core/src/main/java/org/logstash/plugins/ConfigVariableExpander.java +++ b/logstash-core/src/main/java/org/logstash/plugins/ConfigVariableExpander.java @@ -35,11 +35,14 @@ * */ public class ConfigVariableExpander implements AutoCloseable { - private static String SUBSTITUTION_PLACEHOLDER_REGEX = "\\$\\{(?[a-zA-Z_.][a-zA-Z0-9_.]*)(:(?[^}]*))?}"; - - private Pattern substitutionPattern = Pattern.compile(SUBSTITUTION_PLACEHOLDER_REGEX); - private SecretStore secretStore; - private EnvironmentVariableProvider envVarProvider; + public static final Pattern KEY_PATTERN = Pattern.compile("[a-zA-Z_.][a-zA-Z0-9_.]*"); + public static final String KEY_PATTERN_DESCRIPTION = "Key names are limited to ASCII letters (`a`-`z`, `A`-`Z`), numbers (`0`-`9`), " + + "underscores (`_`), and dots (`.`); they must be at least one character long and cannot begin with a number"; + private static final String SUBSTITUTION_PLACEHOLDER_REGEX = "\\$\\{(?" + KEY_PATTERN + ")(:(?[^}]*))?}"; + + private static final Pattern substitutionPattern = Pattern.compile(SUBSTITUTION_PLACEHOLDER_REGEX); + private final SecretStore secretStore; + private final EnvironmentVariableProvider envVarProvider; /** * Creates a ConfigVariableExpander that doesn't lookup any secreted placeholder. diff --git a/logstash-core/src/main/java/org/logstash/secret/SecretIdentifier.java b/logstash-core/src/main/java/org/logstash/secret/SecretIdentifier.java index 08f43abb1ee..2e5bf353e12 100644 --- a/logstash-core/src/main/java/org/logstash/secret/SecretIdentifier.java +++ b/logstash-core/src/main/java/org/logstash/secret/SecretIdentifier.java @@ -20,6 +20,11 @@ package org.logstash.secret; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.util.Strings; +import org.logstash.plugins.ConfigVariableExpander; + import java.util.Locale; import java.util.regex.Pattern; @@ -34,12 +39,14 @@ public class SecretIdentifier { private final static Pattern urnPattern = Pattern.compile("urn:logstash:secret:"+ VERSION + ":.*$"); private final String key; + private static final Logger logger = LogManager.getLogger(SecretIdentifier.class); + /** * Constructor * * @param key The unique part of the identifier. This is the key to reference the secret, and the key itself should not be sensitive. For example: {@code db.pass} */ - public SecretIdentifier(String key) { + public SecretIdentifier(final String key) { this.key = validateWithTransform(key, "key"); } @@ -49,7 +56,7 @@ public SecretIdentifier(String key) { * @param urn The {@link String} formatted identifier obtained originally from {@link SecretIdentifier#toExternalForm()} * @return The {@link SecretIdentifier} object used to identify secrets, null if not valid external form. */ - public static SecretIdentifier fromExternalForm(String urn) { + public static SecretIdentifier fromExternalForm(final String urn) { if (urn == null || !urnPattern.matcher(urn).matches()) { throw new IllegalArgumentException("Invalid external form " + urn); } @@ -57,20 +64,6 @@ public static SecretIdentifier fromExternalForm(String urn) { return new SecretIdentifier(validateWithTransform(parts[4], "key")); } - /** - * Minor validation and downcases the parts - * - * @param part The part of the URN to validate - * @param partName The name of the part used for logging. - * @return The validated and transformed part. - */ - private static String validateWithTransform(String part, String partName) { - if (part == null || part.isEmpty()) { - throw new IllegalArgumentException(String.format("%s may not be null or empty", partName)); - } - return part.toLowerCase(Locale.US); - } - @Override public boolean equals(Object o) { if (this == o) return true; @@ -118,4 +111,22 @@ public String toExternalForm() { public String toString() { return toExternalForm(); } + + /** + * Minor validation and downcases the parts + * + * @param key The key, a part of the URN to validate. + * @param partName The name of the part used for logging. + * @return The validated and transformed part. + */ + private static String validateWithTransform(final String key, final String partName) { + if (key == null || key.isEmpty() || Strings.isBlank(key)) { + throw new IllegalArgumentException(String.format("%s may not be null or empty", partName)); + } + + if (!ConfigVariableExpander.KEY_PATTERN.matcher(key).matches()) { + logger.warn(String.format("Invalid secret key name `%s` provided. %s", key, ConfigVariableExpander.KEY_PATTERN_DESCRIPTION)); + } + return key.toLowerCase(Locale.US); + } } 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 0cf277aa03a..f83fac87e04 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 @@ -20,13 +20,23 @@ package org.logstash.secret.cli; +import org.logstash.plugins.ConfigVariableExpander; import org.logstash.secret.SecretIdentifier; -import org.logstash.secret.store.*; +import org.logstash.secret.store.SecretStore; +import org.logstash.secret.store.SecretStoreFactory; +import org.logstash.secret.store.SecretStoreUtil; +import org.logstash.secret.store.SecureConfig; import java.nio.CharBuffer; import java.nio.charset.CharsetEncoder; import java.nio.charset.StandardCharsets; -import java.util.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import static org.logstash.secret.store.SecretStoreFactory.LOGSTASH_MARKER; @@ -178,7 +188,7 @@ Set getValidOptions() { } } - public SecretStoreCli(Terminal terminal){ + public SecretStoreCli(Terminal terminal) { this(terminal, SecretStoreFactory.fromEnvironment()); } @@ -189,9 +199,10 @@ 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 allArguments This can be either identifiers for a secret, or a sub command like --help. May be null. + * @param config The configuration needed to work a secret store. May be null for help. + * @param allArguments 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... allArguments) { terminal.writeLine(""); @@ -212,7 +223,7 @@ public void command(String primaryCommand, SecureConfig config, String... allArg final CommandLine commandLine = commandParseResult.get(); switch (commandLine.getCommand()) { case CREATE: { - if (commandLine.hasOption(CommandOptions.HELP)){ + if (commandLine.hasOption(CommandOptions.HELP)) { terminal.writeLine("Creates a new keystore. For example: 'bin/logstash-keystore create'"); return; } @@ -227,7 +238,7 @@ public void command(String primaryCommand, SecureConfig config, String... allArg break; } case LIST: { - if (commandLine.hasOption(CommandOptions.HELP)){ + if (commandLine.hasOption(CommandOptions.HELP)) { terminal.writeLine("List all secret identifiers from the keystore. For example: " + "`bin/logstash-keystore list`. Note - only the identifiers will be listed, not the secrets."); return; @@ -239,7 +250,7 @@ public void command(String primaryCommand, SecureConfig config, String... allArg break; } case ADD: { - if (commandLine.hasOption(CommandOptions.HELP)){ + if (commandLine.hasOption(CommandOptions.HELP)) { 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; @@ -251,6 +262,9 @@ public void command(String primaryCommand, SecureConfig config, String... allArg if (secretStoreFactory.exists(config.clone())) { final SecretStore secretStore = secretStoreFactory.load(config); for (String argument : commandLine.getArguments()) { + if (!ConfigVariableExpander.KEY_PATTERN.matcher(argument).matches()) { + throw new IllegalArgumentException(String.format("Invalid secret key name `%s` provided. %s", argument, ConfigVariableExpander.KEY_PATTERN_DESCRIPTION)); + } final SecretIdentifier id = new SecretIdentifier(argument); final byte[] existingValue = secretStore.retrieveSecret(id); if (existingValue != null) { @@ -263,7 +277,7 @@ public void command(String primaryCommand, SecureConfig config, String... allArg final String enterValueMessage = String.format("Enter value for %s: ", argument); char[] secret = null; - while(secret == null) { + while (secret == null) { terminal.write(enterValueMessage); final char[] readSecret = terminal.readSecret(); @@ -288,7 +302,7 @@ public void command(String primaryCommand, SecureConfig config, String... allArg break; } case REMOVE: { - if (commandLine.hasOption(CommandOptions.HELP)){ + if (commandLine.hasOption(CommandOptions.HELP)) { terminal.writeLine("Remove secrets from the keystore. For example: " + "`bin/logstash-keystore remove my-secret`"); return; @@ -314,7 +328,7 @@ public void command(String primaryCommand, SecureConfig config, String... allArg } } - private void printHelp(){ + private void printHelp() { terminal.writeLine("Usage:"); terminal.writeLine("--------"); terminal.writeLine("bin/logstash-keystore [option] command [argument]"); 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 7416a7ec988..2aee8e13832 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 @@ -35,7 +35,7 @@ import java.util.Map; import java.util.Optional; import java.util.Queue; -import java.util.UUID; +import java.util.Random; import static junit.framework.TestCase.assertTrue; import static org.assertj.core.api.Assertions.assertThat; @@ -147,8 +147,8 @@ public void testAddEmptyValue() { terminal.in.add(""); // sets the empty value terminal.in.add("value"); - String id = UUID.randomUUID().toString(); - cli.command("add", newStoreConfig.clone(), id); + final String keyName = getRandomKeyName(); + cli.command("add", newStoreConfig.clone(), keyName); assertThat(terminal.out).containsIgnoringCase("ERROR: Value cannot be empty"); } @@ -159,7 +159,7 @@ public void testAddNonAsciiValue() { terminal.in.add("€€€€€"); // sets non-ascii value value terminal.in.add("value"); - String id = UUID.randomUUID().toString(); + final String id = getRandomKeyName(); cli.command("add", newStoreConfig.clone(), id); assertThat(terminal.out).containsIgnoringCase("ERROR: Value must contain only ASCII characters"); } @@ -168,8 +168,8 @@ public void testAddNonAsciiValue() { public void testAdd() { createKeyStore(); - terminal.in.add(UUID.randomUUID().toString()); // sets the value - String id = UUID.randomUUID().toString(); + terminal.in.add(getRandomKeyName()); // sets the value + final String id = getRandomKeyName(); cli.command("add", newStoreConfig.clone(), id); terminal.reset(); @@ -197,11 +197,11 @@ public void testAddWithNoIdentifiers() { public void testAddMultipleKeys() { createKeyStore(); - terminal.in.add(UUID.randomUUID().toString()); - terminal.in.add(UUID.randomUUID().toString()); + terminal.in.add(getRandomKeyName()); + terminal.in.add(getRandomKeyName()); - final String keyOne = UUID.randomUUID().toString(); - final String keyTwo = UUID.randomUUID().toString(); + final String keyOne = getRandomKeyName(); + final String keyTwo = getRandomKeyName(); cli.command("add", newStoreConfig.clone(), keyOne, keyTwo); terminal.reset(); @@ -211,7 +211,7 @@ public void testAddMultipleKeys() { @Test public void testAddWithoutCreatedKeystore() { - cli.command("add", newStoreConfig.clone(), UUID.randomUUID().toString()); + cli.command("add", newStoreConfig.clone(), getRandomKeyName()); assertThat(terminal.out).containsIgnoringCase("ERROR: Logstash keystore not found. Use 'create' command to create one."); } @@ -219,10 +219,10 @@ public void testAddWithoutCreatedKeystore() { public void testAddWithStdinOption() { createKeyStore(); - terminal.in.add(UUID.randomUUID().toString()); // sets the value - terminal.in.add(UUID.randomUUID().toString()); // sets the value + terminal.in.add(getRandomKeyName()); // sets the value + terminal.in.add(getRandomKeyName()); // sets the value - String id = UUID.randomUUID().toString(); + final String id = getRandomKeyName(); cli.command("add", newStoreConfig.clone(), id, SecretStoreCli.CommandOptions.STDIN.getOption()); terminal.reset(); @@ -235,8 +235,8 @@ public void testAddWithStdinOption() { public void testRemove() { createKeyStore(); - terminal.in.add(UUID.randomUUID().toString()); // sets the value - String id = UUID.randomUUID().toString(); + terminal.in.add(getRandomKeyName()); // sets the value + final String id = getRandomKeyName(); cli.command("add", newStoreConfig.clone(), id); System.out.println(terminal.out); terminal.reset(); @@ -256,11 +256,11 @@ public void testRemove() { public void testRemoveMultipleKeys() { createKeyStore(); - terminal.in.add(UUID.randomUUID().toString()); - terminal.in.add(UUID.randomUUID().toString()); + terminal.in.add(getRandomKeyName()); + terminal.in.add(getRandomKeyName()); - final String keyOne = UUID.randomUUID().toString(); - final String keyTwo = UUID.randomUUID().toString(); + final String keyOne = getRandomKeyName(); + final String keyTwo = getRandomKeyName(); cli.command("add", newStoreConfig.clone(), keyOne, keyTwo); terminal.reset(); @@ -281,8 +281,8 @@ public void testRemoveMultipleKeys() { public void testRemoveMissing() { createKeyStore(); - terminal.in.add(UUID.randomUUID().toString()); // sets the value - String id = UUID.randomUUID().toString(); + terminal.in.add(getRandomKeyName()); // sets the value + final String id = getRandomKeyName(); cli.command("add", newStoreConfig.clone(), id); System.out.println(terminal.out); terminal.reset(); @@ -318,7 +318,7 @@ public void testCommandWithUnrecognizedOption() { terminal.in.add("foo"); final String invalidOption = "--invalid-option"; - cli.command("add", newStoreConfig.clone(), UUID.randomUUID().toString(), invalidOption); + cli.command("add", newStoreConfig.clone(), getRandomKeyName(), invalidOption); assertThat(terminal.out).contains(String.format("Unrecognized option '%s' for command 'add'", invalidOption)); terminal.reset(); @@ -387,6 +387,23 @@ private void assertNotListed(String... expected) { assertTrue(Arrays.stream(expected).noneMatch(terminal.out::contains)); } + private final static String KEY_FIRST_CHAR_CANDIDATES = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_."; + private final static String KEY_REST_CHARS = KEY_FIRST_CHAR_CANDIDATES.concat("01234567890"); + private String getRandomKeyName() { + final Random random = new Random(); + final StringBuilder sb = new StringBuilder(); + + // add a random legal first-character + sb.append(KEY_FIRST_CHAR_CANDIDATES.charAt(random.nextInt(KEY_FIRST_CHAR_CANDIDATES.length()))); + + // add between 0 and 40 random legal rest-characters + final int more = 40; + random.ints(more, 0, KEY_REST_CHARS.length()) + .mapToObj(KEY_REST_CHARS::charAt) + .forEach(sb::append); + return sb.toString().toLowerCase(); + } + private void assertPrimaryHelped() { assertThat(terminal.out). containsIgnoringCase("Commands").