Restrict adding invalid (from ConfigVariableExpander point of view) key names to the keystore through keystore CLI. Keysotre now warns on invalid keys if they were already added.

Author: mashhurs

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 characters. It can only accept digits, letters and optional `.` (dots) and/or `_` (underscores) symbols included between or after.
 ::::
 
 

logstash-core/src/main/java/org/logstash/secret/KeyValidator.java

@@ -20,6 +20,10 @@
 
 package org.logstash.secret;
 
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.util.Strings;
+
 import java.util.Locale;
 import java.util.regex.Pattern;
 
@@ -29,17 +33,22 @@
  */
 public class SecretIdentifier {
 
+    // aligns with ConfigVariableExpander substitution pattern
+    public final static Pattern KEY_PATTERN = Pattern.compile("[a-zA-Z_.][a-zA-Z0-9_.]*");
+
     private final static Pattern colonPattern = Pattern.compile(":");
     private final static String VERSION = "v1";
     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 +58,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);
         }
@@ -64,8 +73,14 @@ public static SecretIdentifier fromExternalForm(String urn) {
      * @param partName The name of the part used for logging.
      * @return The validated and transformed part.
      */
-    private static String validateWithTransform(String key, String partName) {
-        KeyValidator.validateKey(key, partName);
+    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 (!KEY_PATTERN.matcher(key).matches()) {
+            logger.warn(String.format("Invalid %s key appeared in keystore. Please remove it as it cannot be used in the pipelines", key));
+        }
         return key.toLowerCase(Locale.US);
     }
 

logstash-core/src/main/java/org/logstash/secret/SecretIdentifier.java

@@ -26,7 +26,13 @@
 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 +184,7 @@ Set<CommandOptions> getValidOptions() {
         }
     }
 
-    public SecretStoreCli(Terminal terminal){
+    public SecretStoreCli(Terminal terminal) {
         this(terminal, SecretStoreFactory.fromEnvironment());
     }
 
@@ -189,9 +195,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 +219,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 +234,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 +246,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 +258,13 @@ 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 (!SecretIdentifier.KEY_PATTERN.matcher(argument).matches()) {
+                            final String errorMessage = String.format(
+                                    "Invalid secret key %s provided. " +
+                                            "It can only accept digits, letters with optional `.` " +
+                                            "and/or `_` symbols included between or after.", argument);
+                            throw new IllegalArgumentException(errorMessage);
+                        }
                         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]");