Require keystore file and classname when generating a valid secure config, safely resolve keystore file and classname from the settings if available. If they both not available, it might be that user intentionally turned off the keystore otherwise require both.

Author: mashhurs

logstash-core/src/main/java/org/logstash/execution/AbstractPipelineExt.java

@@ -281,7 +281,8 @@ private AbstractPipelineExt initialize(final ThreadContext context,
             }
         }
         boolean supportEscapes = getSetting(context, "config.support_escapes").isTrue();
-        try (ConfigVariableExpander cve = new ConfigVariableExpander(getSecretStore(context), EnvironmentVariableProvider.defaultProvider())) {
+        try (ConfigVariableExpander cve = new ConfigVariableExpander(getSecretStore(context),
+                             EnvironmentVariableProvider.defaultProvider())) {
             lir = ConfigCompiler.configToPipelineIR(configParts, supportEscapes, cve);
         } catch (InvalidIRException iirex) {
             throw new IllegalArgumentException(iirex);
@@ -842,15 +843,28 @@ protected final boolean hasSetting(final ThreadContext context, final String nam
     }
 
     protected SecretStore getSecretStore(final ThreadContext context) {
-        String keystoreFile = hasSetting(context, "keystore.file")
-                ? getSetting(context, "keystore.file").asJavaString()
-                : null;
-        String keystoreClassname = hasSetting(context, "keystore.classname")
-                ? getSetting(context, "keystore.classname").asJavaString()
-                : null;
-        return (keystoreFile != null && keystoreClassname != null)
-                ? SecretStoreExt.getIfExists(keystoreFile, keystoreClassname)
-                : null;
+        final String keystoreFile = safelyGetSettingValueAsString(context, "keystore.file");
+        final String keystoreClassname = safelyGetSettingValueAsString(context, "keystore.classname");
+        if (keystoreFile == null && keystoreClassname == null) {
+            // explicitly set keystore and classname null
+            return null;
+        }
+
+        if (keystoreFile == null | keystoreClassname == null) {
+            throw new IllegalStateException("Setting `keystore.file` requires `keystore.classname`, or vice versa");
+        }
+        return SecretStoreExt.getIfExists(keystoreFile, keystoreClassname);
+    }
+
+    private String safelyGetSettingValueAsString(final ThreadContext context, final String settingName) {
+        final boolean hasKeystoreFileSetting = hasSetting(context, settingName);
+        if (hasKeystoreFileSetting) {
+            final IRubyObject keystoreFileSettingValue = getSetting(context, settingName);
+            if (!keystoreFileSettingValue.isNil()) {
+                return keystoreFileSettingValue.asJavaString();
+            }
+        }
+        return null;
     }
 
     private AbstractNamespacedMetricExt getDlqMetric(final ThreadContext context) {

logstash-core/src/main/java/org/logstash/secret/store/SecretStoreExt.java

@@ -31,16 +31,20 @@ public class SecretStoreExt {
 
     private static final SecretStoreFactory SECRET_STORE_FACTORY = SecretStoreFactory.fromEnvironment();
 
-    public static SecureConfig getConfig(String keystoreFile, String keystoreClassname) {
+    public static SecureConfig getConfig(final String keystoreFile, final String keystoreClassname) {
         return getSecureConfig(RubyUtil.RUBY.getENV(), keystoreFile, keystoreClassname);
     }
 
-    private static SecureConfig getSecureConfig(RubyHash env, String file, String classname) {
+    private static SecureConfig getSecureConfig(final RubyHash env, final String file, final String classname) {
         String keystorePass = (String) env.get("LOGSTASH_KEYSTORE_PASS");
         return getSecureConfig(file, keystorePass, classname);
     }
 
-    private static SecureConfig getSecureConfig(String keystoreFile, String keystorePass, String keystoreClassname) {
+    private static SecureConfig getSecureConfig(final String keystoreFile, final String keystorePass, final String keystoreClassname) {
+        if (keystoreFile == null || keystoreClassname == null) {
+            throw new IllegalArgumentException("`keystore.file` and `keystore.classname` cannot be null");
+        }
+
         SecureConfig sc = new SecureConfig();
         sc.add("keystore.file", keystoreFile.toCharArray());
         if (keystorePass != null) {
@@ -50,18 +54,18 @@ private static SecureConfig getSecureConfig(String keystoreFile, String keystore
         return sc;
     }
 
-    public static boolean exists(String keystoreFile, String keystoreClassname) {
+    public static boolean exists(final String keystoreFile, final String keystoreClassname) {
         return SECRET_STORE_FACTORY.exists(getConfig(keystoreFile, keystoreClassname));
     }
 
-    public static SecretStore getIfExists(String keystoreFile, String keystoreClassname) {
+    public static SecretStore getIfExists(final String keystoreFile, final String keystoreClassname) {
         SecureConfig sc = getConfig(keystoreFile, keystoreClassname);
         return SECRET_STORE_FACTORY.exists(sc)
                 ? SECRET_STORE_FACTORY.load(sc)
                 : null;
     }
 
-    public static SecretIdentifier getStoreId(String id) {
+    public static SecretIdentifier getStoreId(final String id) {
         return new SecretIdentifier(id);
     }
 }