storm-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kabh...@apache.org
Subject [1/5] storm git commit: STORM-3184: Mask the plaintext passwords from the logs
Date Tue, 14 Aug 2018 14:24:56 GMT
Repository: storm
Updated Branches:
  refs/heads/master c2931dab7 -> 4c42ee3d2


STORM-3184: Mask the plaintext passwords from the logs

Introduce a `Password` config annotation and use it to mark configs that are
sensitive and mask the values while logging.


Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/9d36724e
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/9d36724e
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/9d36724e

Branch: refs/heads/master
Commit: 9d36724e88f54ae48fd892b001eeb76313adc9da
Parents: 154173a
Author: Arun Mahadevan <arunm@apache.org>
Authored: Fri Aug 10 17:39:49 2018 -0700
Committer: Arun Mahadevan <arunm@apache.org>
Committed: Fri Aug 10 17:41:19 2018 -0700

----------------------------------------------------------------------
 .../AbstractHadoopNimbusPluginAutoCreds.java    |  4 ++-
 storm-client/pom.xml                            |  6 ++++
 .../org/apache/storm/daemon/worker/Worker.java  |  5 +--
 .../jvm/org/apache/storm/utils/ConfigUtils.java | 37 ++++++++++++++++++++
 .../storm/validation/ConfigValidation.java      |  2 +-
 .../validation/ConfigValidationAnnotations.java |  6 ++++
 .../java/org/apache/storm/DaemonConfig.java     | 10 ++++++
 .../org/apache/storm/daemon/nimbus/Nimbus.java  |  4 +--
 .../storm/daemon/supervisor/Supervisor.java     |  2 +-
 .../java/org/apache/storm/DaemonConfigTest.java | 12 +++++++
 10 files changed, 81 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractHadoopNimbusPluginAutoCreds.java
----------------------------------------------------------------------
diff --git a/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractHadoopNimbusPluginAutoCreds.java
b/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractHadoopNimbusPluginAutoCreds.java
index dee337e..0ddf381 100644
--- a/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractHadoopNimbusPluginAutoCreds.java
+++ b/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractHadoopNimbusPluginAutoCreds.java
@@ -25,6 +25,7 @@ import org.apache.hadoop.security.Credentials;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.storm.security.INimbusCredentialPlugin;
 import org.apache.storm.security.auth.ICredentialsRenewer;
+import org.apache.storm.utils.ConfigUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -80,7 +81,8 @@ public abstract class AbstractHadoopNimbusPluginAutoCreds
 
     protected void fillHadoopConfiguration(Map topologyConf, String configKey, Configuration
configuration) {
         Map<String, Object> config = (Map<String, Object>) topologyConf.get(configKey);
-        LOG.info("TopoConf {}, got config {}, for configKey {}", topologyConf, config, configKey);
+        LOG.info("TopoConf {}, got config {}, for configKey {}", ConfigUtils.maskPasswords(topologyConf),
+                ConfigUtils.maskPasswords(config), configKey);
         if (config != null) {
             List<String> resourcesToLoad = new ArrayList<>();
             for (Map.Entry<String, Object> entry : config.entrySet()) {

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-client/pom.xml
----------------------------------------------------------------------
diff --git a/storm-client/pom.xml b/storm-client/pom.xml
index 7c28cce..c925ffd 100644
--- a/storm-client/pom.xml
+++ b/storm-client/pom.xml
@@ -97,6 +97,12 @@
 
         <!-- end of transitive dependency management -->
 
+        <dependency>
+            <groupId>com.google.guava</groupId>
+            <artifactId>guava</artifactId>
+            <version>${guava.version}</version>
+        </dependency>
+
         <!-- test -->
         <dependency>
             <groupId>org.mockito</groupId>

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java b/storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java
index 233dfac..9f8428b 100644
--- a/storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java
+++ b/storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java
@@ -132,7 +132,7 @@ public class Worker implements Shutdownable, DaemonCommon {
 
     public void start() throws Exception {
         LOG.info("Launching worker for {} on {}:{} with id {} and conf {}", topologyId, assignmentId,
port, workerId,
-                 conf);
+                 ConfigUtils.maskPasswords(conf));
         // because in local mode, its not a separate
         // process. supervisor will register it in this case
         // if ConfigUtils.isLocalMode(conf) returns false then it is in distributed mode.
@@ -278,7 +278,8 @@ public class Worker implements Shutdownable, DaemonCommon {
         setupFlushTupleTimer(topologyConf, newExecutors);
         setupBackPressureCheckTimer(topologyConf);
 
-        LOG.info("Worker has topology config {}", Utils.redactValue(topologyConf, Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD));
+        LOG.info("Worker has topology config {}", Utils.redactValue(ConfigUtils.maskPasswords(topologyConf),
+                Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD));
         LOG.info("Worker {} for storm {} on {}:{}  has finished loading", workerId, topologyId,
assignmentId, port);
         return this;
     }

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java b/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java
index 2a87a34..5fefcea 100644
--- a/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java
+++ b/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java
@@ -14,6 +14,8 @@ package org.apache.storm.utils;
 
 import java.io.File;
 import java.io.IOException;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
 import java.net.URLEncoder;
 import java.util.Arrays;
 import java.util.Collection;
@@ -22,19 +24,44 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
+import java.util.Set;
 import java.util.function.BooleanSupplier;
 import java.util.stream.Collectors;
+
+import com.google.common.collect.Maps;
 import org.apache.storm.Config;
 import org.apache.storm.daemon.supervisor.AdvancedFSOps;
 import org.apache.storm.generated.StormTopology;
 import org.apache.storm.shade.org.apache.commons.io.FileUtils;
 import org.apache.storm.validation.ConfigValidation;
+import org.apache.storm.validation.ConfigValidationAnnotations;
 
 public class ConfigUtils {
     public static final String FILE_SEPARATOR = File.separator;
     public static final String STORM_HOME = "storm.home";
     public final static String RESOURCES_SUBDIR = "resources";
 
+    private static final Set<String> passwordConfigKeys = new HashSet<>();
+
+    static {
+        for (Class<?> clazz: ConfigValidation.getConfigClasses()) {
+            for (Field field : clazz.getFields()) {
+                for (Annotation annotation : field.getAnnotations()) {
+                    boolean isPassword = annotation.annotationType().getName().equals(
+                            ConfigValidationAnnotations.Password.class.getName());
+                    if (isPassword) {
+                        try {
+                            passwordConfigKeys.add((String) field.get(null));
+                        } catch (IllegalAccessException e) {
+                            // ignore
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+
     // A singleton instance allows us to mock delegated static methods in our
     // tests by subclassing.
     private static ConfigUtils _instance = new ConfigUtils();
@@ -52,6 +79,16 @@ public class ConfigUtils {
         return oldInstance;
     }
 
+    public static Map<String, Object> maskPasswords(final Map<String, Object>
conf) {
+        Maps.EntryTransformer<String, Object, Object> maskPasswords =
+                new Maps.EntryTransformer<String, Object, Object>() {
+                    public Object transformEntry(String key, Object value) {
+                        return passwordConfigKeys.contains(key) ? "*****" : value;
+                    }
+                };
+        return Maps.transformEntries(conf, maskPasswords);
+    }
+
     public static boolean isLocalMode(Map<String, Object> conf) {
         String mode = (String) conf.get(Config.STORM_CLUSTER_MODE);
         if (mode != null) {

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-client/src/jvm/org/apache/storm/validation/ConfigValidation.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/validation/ConfigValidation.java b/storm-client/src/jvm/org/apache/storm/validation/ConfigValidation.java
index f0bc41e..4b13dd8 100644
--- a/storm-client/src/jvm/org/apache/storm/validation/ConfigValidation.java
+++ b/storm-client/src/jvm/org/apache/storm/validation/ConfigValidation.java
@@ -62,7 +62,7 @@ public class ConfigValidation {
         validateField(fieldName, conf, getConfigClasses());
     }
 
-    private static synchronized List<Class<?>> getConfigClasses() {
+    public static synchronized List<Class<?>> getConfigClasses() {
         if (configClasses == null) {
             List<Class<?>> ret = new ArrayList<>();
             Set<String> classesToScan = new HashSet<>();

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-client/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
b/storm-client/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
index e0ce479..6f00441 100644
--- a/storm-client/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
+++ b/storm-client/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
@@ -197,6 +197,12 @@ public class ConfigValidationAnnotations {
         Class<?> validatorClass();
     }
 
+    @Retention(RetentionPolicy.RUNTIME)
+    @Target(ElementType.FIELD)
+    public @interface Password {
+        Class validatorClass() default ConfigValidation.NotNullValidator.class;
+    }
+
     /**
      * Field names for annotations
      */

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-server/src/main/java/org/apache/storm/DaemonConfig.java
----------------------------------------------------------------------
diff --git a/storm-server/src/main/java/org/apache/storm/DaemonConfig.java b/storm-server/src/main/java/org/apache/storm/DaemonConfig.java
index 5985593..d9869d4 100644
--- a/storm-server/src/main/java/org/apache/storm/DaemonConfig.java
+++ b/storm-server/src/main/java/org/apache/storm/DaemonConfig.java
@@ -44,6 +44,7 @@ import static org.apache.storm.validation.ConfigValidationAnnotations.isPositive
 import static org.apache.storm.validation.ConfigValidationAnnotations.isString;
 import static org.apache.storm.validation.ConfigValidationAnnotations.isStringList;
 import static org.apache.storm.validation.ConfigValidationAnnotations.isStringOrStringList;
+import static org.apache.storm.validation.ConfigValidationAnnotations.Password;
 
 /**
  * Storm configs are specified as a plain old map. This class provides constants for all
the configurations possible on a Storm cluster.
@@ -374,6 +375,7 @@ public class DaemonConfig implements Validated {
      * Password for the keystore for HTTPS for Storm Logviewer.
      */
     @isString
+    @Password
     public static final String LOGVIEWER_HTTPS_KEYSTORE_PASSWORD = "logviewer.https.keystore.password";
 
     /**
@@ -387,6 +389,7 @@ public class DaemonConfig implements Validated {
      * Password to the private key in the keystore for setting up HTTPS (SSL).
      */
     @isString
+    @Password
     public static final String LOGVIEWER_HTTPS_KEY_PASSWORD = "logviewer.https.key.password";
 
     /**
@@ -399,6 +402,7 @@ public class DaemonConfig implements Validated {
      * Password for the truststore for HTTPS for Storm Logviewer.
      */
     @isString
+    @Password
     public static final String LOGVIEWER_HTTPS_TRUSTSTORE_PASSWORD = "logviewer.https.truststore.password";
 
     /**
@@ -477,6 +481,7 @@ public class DaemonConfig implements Validated {
      * Password to the keystore used by Storm UI for setting up HTTPS (SSL).
      */
     @isString
+    @Password
     public static final String UI_HTTPS_KEYSTORE_PASSWORD = "ui.https.keystore.password";
 
     /**
@@ -491,6 +496,7 @@ public class DaemonConfig implements Validated {
      * Password to the private key in the keystore for setting up HTTPS (SSL).
      */
     @isString
+    @Password
     public static final String UI_HTTPS_KEY_PASSWORD = "ui.https.key.password";
 
     /**
@@ -503,6 +509,7 @@ public class DaemonConfig implements Validated {
      * Password to the truststore used by Storm UI setting up HTTPS (SSL).
      */
     @isString
+    @Password
     public static final String UI_HTTPS_TRUSTSTORE_PASSWORD = "ui.https.truststore.password";
 
     /**
@@ -559,6 +566,7 @@ public class DaemonConfig implements Validated {
      * Password to the keystore used by Storm DRPC for setting up HTTPS (SSL).
      */
     @isString
+    @Password
     public static final String DRPC_HTTPS_KEYSTORE_PASSWORD = "drpc.https.keystore.password";
 
     /**
@@ -573,6 +581,7 @@ public class DaemonConfig implements Validated {
      * Password to the private key in the keystore for setting up HTTPS (SSL).
      */
     @isString
+    @Password
     public static final String DRPC_HTTPS_KEY_PASSWORD = "drpc.https.key.password";
 
     /**
@@ -585,6 +594,7 @@ public class DaemonConfig implements Validated {
      * Password to the truststore used by Storm DRPC setting up HTTPS (SSL).
      */
     @isString
+    @Password
     public static final String DRPC_HTTPS_TRUSTSTORE_PASSWORD = "drpc.https.truststore.password";
 
     /**

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
----------------------------------------------------------------------
diff --git a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
index a096217..b66153b 100644
--- a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
+++ b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
@@ -2798,7 +2798,7 @@ public class Nimbus implements Iface, Shutdownable, DaemonCommon {
             IStormClusterState state = stormClusterState;
             NimbusInfo hpi = nimbusHostPortInfo;
 
-            LOG.info("Starting Nimbus with conf {}", conf);
+            LOG.info("Starting Nimbus with conf {}", ConfigUtils.maskPasswords(conf));
             validator.prepare(conf);
 
             //add to nimbuses
@@ -3075,7 +3075,7 @@ public class Nimbus implements Iface, Shutdownable, DaemonCommon {
             }
             LOG.info("Received topology submission for {} (storm-{} JDK-{}) with conf {}",
topoName,
                      topoVersionString, topology.get_jdk_version(),
-                     Utils.redactValue(topoConf, Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD));
+                     Utils.redactValue(ConfigUtils.maskPasswords(topoConf), Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD));
 
             // lock protects against multiple topologies being submitted at once and
             // cleanup thread killing topology in b/w assignment and starting the topology

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-server/src/main/java/org/apache/storm/daemon/supervisor/Supervisor.java
----------------------------------------------------------------------
diff --git a/storm-server/src/main/java/org/apache/storm/daemon/supervisor/Supervisor.java
b/storm-server/src/main/java/org/apache/storm/daemon/supervisor/Supervisor.java
index a2cf66f..d13bd63 100644
--- a/storm-server/src/main/java/org/apache/storm/daemon/supervisor/Supervisor.java
+++ b/storm-server/src/main/java/org/apache/storm/daemon/supervisor/Supervisor.java
@@ -268,7 +268,7 @@ public class Supervisor implements DaemonCommon, AutoCloseable {
      * Launch the supervisor.
      */
     public void launch() throws Exception {
-        LOG.info("Starting Supervisor with conf {}", conf);
+        LOG.info("Starting Supervisor with conf {}", ConfigUtils.maskPasswords(conf));
         String path = ServerConfigUtils.supervisorTmpDir(conf);
         FileUtils.cleanDirectory(new File(path));
 

http://git-wip-us.apache.org/repos/asf/storm/blob/9d36724e/storm-server/src/test/java/org/apache/storm/DaemonConfigTest.java
----------------------------------------------------------------------
diff --git a/storm-server/src/test/java/org/apache/storm/DaemonConfigTest.java b/storm-server/src/test/java/org/apache/storm/DaemonConfigTest.java
index 67e210d..2304f77 100644
--- a/storm-server/src/test/java/org/apache/storm/DaemonConfigTest.java
+++ b/storm-server/src/test/java/org/apache/storm/DaemonConfigTest.java
@@ -18,6 +18,8 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.Map;
+
+import org.apache.storm.utils.ConfigUtils;
 import org.apache.storm.validation.ConfigValidation;
 import org.junit.Assert;
 import org.junit.Test;
@@ -89,4 +91,14 @@ public class DaemonConfigTest {
         InstantiationException, IllegalAccessException {
         stringOrStringListTest(DaemonConfig.SUPERVISOR_CHILDOPTS);
     }
+
+    @Test
+    public void testMaskPasswords() {
+        Map<String, Object> conf = new HashMap<>();
+        conf.put(DaemonConfig.LOGVIEWER_HTTPS_KEY_PASSWORD, "pass1");
+        conf.put(Config.TOPOLOGY_MESSAGE_TIMEOUT_SECS, 100);
+        Map result = ConfigUtils.maskPasswords(conf);
+        Assert.assertEquals("*****", result.get(DaemonConfig.LOGVIEWER_HTTPS_KEY_PASSWORD));
+        Assert.assertEquals(100, result.get(Config.TOPOLOGY_MESSAGE_TIMEOUT_SECS));
+    }
 }


Mime
View raw message