storm-commits mailing list archives

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


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/fc942ee1
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/fc942ee1
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/fc942ee1

Branch: refs/heads/1.x-branch
Commit: fc942ee10d86649db9e9b8ce3dc0a04ea23439ce
Parents: 7c4ae00
Author: Arun Mahadevan <arunm@apache.org>
Authored: Tue Aug 7 12:13:54 2018 -0700
Committer: Arun Mahadevan <arunm@apache.org>
Committed: Wed Aug 8 11:52:02 2018 -0700

----------------------------------------------------------------------
 .../apache/storm/common/AbstractAutoCreds.java  |  4 ++-
 .../src/clj/org/apache/storm/daemon/nimbus.clj  |  6 ++--
 .../src/clj/org/apache/storm/daemon/worker.clj  |  6 ++--
 storm-core/src/jvm/org/apache/storm/Config.java |  9 ++++++
 .../storm/daemon/supervisor/Supervisor.java     |  2 +-
 .../jvm/org/apache/storm/utils/ConfigUtils.java | 31 ++++++++++++++++++++
 .../validation/ConfigValidationAnnotations.java |  7 +++++
 .../org/apache/storm/utils/ConfigUtilsTest.java | 12 ++++++++
 8 files changed, 69 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/storm/blob/fc942ee1/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractAutoCreds.java
----------------------------------------------------------------------
diff --git a/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractAutoCreds.java
b/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractAutoCreds.java
index 7b2fc2d..2ce99aa 100644
--- a/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractAutoCreds.java
+++ b/external/storm-autocreds/src/main/java/org/apache/storm/common/AbstractAutoCreds.java
@@ -28,6 +28,7 @@ import org.apache.hadoop.security.token.TokenIdentifier;
 import org.apache.storm.security.INimbusCredentialPlugin;
 import org.apache.storm.security.auth.IAutoCredentials;
 import org.apache.storm.security.auth.ICredentialsRenewer;
+import org.apache.storm.utils.ConfigUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -149,7 +150,8 @@ public abstract class AbstractAutoCreds implements IAutoCredentials, ICredential
 
     protected void fillHadoopConfiguration(Map topoConf, String configKey, Configuration
configuration) {
         Map<String, Object> config = (Map<String, Object>) topoConf.get(configKey);
-        LOG.info("TopoConf {}, got config {}, for configKey {}", topoConf, config, configKey);
+        LOG.info("TopoConf {}, got config {}, for configKey {}", ConfigUtils.maskPasswords(topoConf),
+                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/fc942ee1/storm-core/src/clj/org/apache/storm/daemon/nimbus.clj
----------------------------------------------------------------------
diff --git a/storm-core/src/clj/org/apache/storm/daemon/nimbus.clj b/storm-core/src/clj/org/apache/storm/daemon/nimbus.clj
index 850867e..fc89ac4 100644
--- a/storm-core/src/clj/org/apache/storm/daemon/nimbus.clj
+++ b/storm-core/src/clj/org/apache/storm/daemon/nimbus.clj
@@ -61,7 +61,7 @@
   (:use [org.apache.storm.daemon common])
   (:use [org.apache.storm config])
   (:import [org.apache.zookeeper data.ACL ZooDefs$Ids ZooDefs$Perms])
-  (:import [org.apache.storm.utils VersionInfo Time]
+  (:import [org.apache.storm.utils VersionInfo Time ConfigUtils]
            (org.apache.storm.metric ClusterMetricsConsumerExecutor)
            (org.apache.storm.metric.api IClusterMetricsConsumer$ClusterInfo DataPoint IClusterMetricsConsumer$SupervisorInfo)
            (org.apache.storm Config)
@@ -1748,7 +1748,7 @@
                        " (storm-" (.get_storm_version topology)
                        " JDK-" (.get_jdk_version topology)
                        ") with conf "
-                       (redact-value storm-conf STORM-ZOOKEEPER-TOPOLOGY-AUTH-PAYLOAD))
+                       (redact-value (ConfigUtils/maskPasswords storm-conf) 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
           (locking (:submit-lock nimbus)
@@ -2463,7 +2463,7 @@
 
 (defserverfn service-handler [conf inimbus]
   (.prepare inimbus conf (master-inimbus-dir conf))
-  (log-message "Starting Nimbus with conf " conf)
+  (log-message "Starting Nimbus with conf " (ConfigUtils/maskPasswords conf))
   (let [nimbus (nimbus-data conf inimbus)
         blob-store (:blob-store nimbus)]
     (.prepare ^org.apache.storm.nimbus.ITopologyValidator (:validator nimbus) conf)

http://git-wip-us.apache.org/repos/asf/storm/blob/fc942ee1/storm-core/src/clj/org/apache/storm/daemon/worker.clj
----------------------------------------------------------------------
diff --git a/storm-core/src/clj/org/apache/storm/daemon/worker.clj b/storm-core/src/clj/org/apache/storm/daemon/worker.clj
index 859a735..13daa10 100644
--- a/storm-core/src/clj/org/apache/storm/daemon/worker.clj
+++ b/storm-core/src/clj/org/apache/storm/daemon/worker.clj
@@ -25,7 +25,7 @@
   (:import [java.util.concurrent Executors]
            [org.apache.storm.hooks IWorkerHook BaseWorkerHook])
   (:import [java.util ArrayList HashMap])
-  (:import [org.apache.storm.utils Utils TransferDrainer ThriftTopologyUtils WorkerBackpressureThread
DisruptorQueue])
+  (:import [org.apache.storm.utils Utils TransferDrainer ThriftTopologyUtils WorkerBackpressureThread
DisruptorQueue ConfigUtils])
   (:import [org.apache.storm.grouping LoadMapping])
   (:import [org.apache.storm.messaging TransportFactory])
   (:import [org.apache.storm.messaging TaskMessage IContext IConnection ConnectionWithStatus
ConnectionWithStatus$Status])
@@ -604,7 +604,7 @@
 ;; should guarantee this consistency
 (defserverfn mk-worker [conf shared-mq-context storm-id assignment-id port worker-id]
   (log-message "Launching worker for " storm-id " on " assignment-id ":" port " with id "
worker-id
-               " and conf " conf)
+               " and conf " (ConfigUtils/maskPasswords conf))
   ;; create an empty list to store deserialized hooks
   (def deserialized-hooks (java.util.ArrayList.))
   (if-not (local-mode? conf)
@@ -778,7 +778,7 @@
     (schedule-recurring (:reset-log-levels-timer worker) 0 (conf WORKER-LOG-LEVEL-RESET-POLL-SECS)
(fn [] (reset-log-levels latest-log-config)))
     (schedule-recurring (:refresh-active-timer worker) 0 (conf TASK-REFRESH-POLL-SECS) (partial
refresh-storm-active worker))
 
-    (log-message "Worker has topology config " (redact-value (:storm-conf worker) STORM-ZOOKEEPER-TOPOLOGY-AUTH-PAYLOAD))
+    (log-message "Worker has topology config " (redact-value (ConfigUtils/maskPasswords (:storm-conf
worker)) STORM-ZOOKEEPER-TOPOLOGY-AUTH-PAYLOAD))
     (log-message "Worker " worker-id " for storm " storm-id " on " assignment-id ":" port
" has finished loading")
     ret
     ))))))

http://git-wip-us.apache.org/repos/asf/storm/blob/fc942ee1/storm-core/src/jvm/org/apache/storm/Config.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/Config.java b/storm-core/src/jvm/org/apache/storm/Config.java
index 628c9ff..fc9fb55 100644
--- a/storm-core/src/jvm/org/apache/storm/Config.java
+++ b/storm-core/src/jvm/org/apache/storm/Config.java
@@ -812,6 +812,7 @@ public class Config extends HashMap<String, Object> {
      * Password for the keystore for HTTPS for Storm Logviewer
      */
     @isString
+    @Password
     public static final String LOGVIEWER_HTTPS_KEYSTORE_PASSWORD = "logviewer.https.keystore.password";
 
     /**
@@ -825,6 +826,7 @@ public class Config extends HashMap<String, Object> {
      * 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";
 
     /**
@@ -837,6 +839,7 @@ public class Config extends HashMap<String, Object> {
      * Password for the truststore for HTTPS for Storm Logviewer
      */
     @isString
+    @Password
     public static final String LOGVIEWER_HTTPS_TRUSTSTORE_PASSWORD = "logviewer.https.truststore.password";
 
     /**
@@ -915,6 +918,7 @@ public class Config extends HashMap<String, Object> {
      * 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";
 
     /**
@@ -928,6 +932,7 @@ public class Config extends HashMap<String, Object> {
      * 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";
 
     /**
@@ -940,6 +945,7 @@ public class Config extends HashMap<String, Object> {
      * 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";
 
     /**
@@ -1041,6 +1047,7 @@ public class Config extends HashMap<String, Object> {
      * 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";
 
     /**
@@ -1054,6 +1061,7 @@ public class Config extends HashMap<String, Object> {
      * 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";
 
     /**
@@ -1066,6 +1074,7 @@ public class Config extends HashMap<String, Object> {
      * 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/fc942ee1/storm-core/src/jvm/org/apache/storm/daemon/supervisor/Supervisor.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/daemon/supervisor/Supervisor.java b/storm-core/src/jvm/org/apache/storm/daemon/supervisor/Supervisor.java
index c305a72..2b10da9 100644
--- a/storm-core/src/jvm/org/apache/storm/daemon/supervisor/Supervisor.java
+++ b/storm-core/src/jvm/org/apache/storm/daemon/supervisor/Supervisor.java
@@ -192,7 +192,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 = ConfigUtils.supervisorTmpDir(conf);
         FileUtils.cleanDirectory(new File(path));
 

http://git-wip-us.apache.org/repos/asf/storm/blob/fc942ee1/storm-core/src/jvm/org/apache/storm/utils/ConfigUtils.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/utils/ConfigUtils.java b/storm-core/src/jvm/org/apache/storm/utils/ConfigUtils.java
index 76176b4..a7c32c1 100644
--- a/storm-core/src/jvm/org/apache/storm/utils/ConfigUtils.java
+++ b/storm-core/src/jvm/org/apache/storm/utils/ConfigUtils.java
@@ -18,16 +18,19 @@
 
 package org.apache.storm.utils;
 
+import com.google.common.collect.Maps;
 import org.apache.commons.io.FileUtils;
 import org.apache.storm.Config;
 import org.apache.storm.daemon.supervisor.AdvancedFSOps;
 import org.apache.storm.generated.StormTopology;
 import org.apache.storm.validation.ConfigValidation;
+import org.apache.storm.validation.ConfigValidationAnnotations;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 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.ArrayList;
@@ -48,6 +51,34 @@ public class ConfigUtils {
     public final static String NIMBUS_DO_NOT_REASSIGN = "NIMBUS-DO-NOT-REASSIGN";
     public static final String FILE_SEPARATOR = File.separator;
 
+    private static final Set<String> passwordConfigKeys = new HashSet<>();
+
+    static {
+        for (Field field : Config.class.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
+                    }
+                }
+            }
+        }
+    }
+
+    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);
+    }
+
     // A singleton instance allows us to mock delegated static methods in our
     // tests by subclassing.
     private static ConfigUtils _instance = new ConfigUtils();

http://git-wip-us.apache.org/repos/asf/storm/blob/fc942ee1/storm-core/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
b/storm-core/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
index b770f34..8a54a94 100644
--- a/storm-core/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
+++ b/storm-core/src/jvm/org/apache/storm/validation/ConfigValidationAnnotations.java
@@ -214,5 +214,12 @@ public class ConfigValidationAnnotations {
     public @interface CustomValidator {
         Class validatorClass();
     }
+
+    @Retention(RetentionPolicy.RUNTIME)
+    @Target(ElementType.FIELD)
+    public @interface Password {
+        Class validatorClass() default ConfigValidation.NotNullValidator.class;
+    }
+
 }
 

http://git-wip-us.apache.org/repos/asf/storm/blob/fc942ee1/storm-core/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java
----------------------------------------------------------------------
diff --git a/storm-core/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java b/storm-core/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java
index 6f5caf2..ccc17c9 100644
--- a/storm-core/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java
+++ b/storm-core/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java
@@ -95,4 +95,16 @@ public class ConfigUtilsTest {
         Map map = mockMap(key, values);
         Assert.assertEquals(expectedValue, ConfigUtils.getValueAsList(key, map));
     }
+
+    @Test
+    public void testMaskPasswords() {
+        Map<String, Object> conf = new HashMap<>();
+        conf.put(Config.LOGVIEWER_HTTPS_KEY_PASSWORD, "pass1");
+        conf.put(Config.TOPOLOGY_MESSAGE_TIMEOUT_SECS, 100);
+        Map result = ConfigUtils.maskPasswords(conf);
+        Assert.assertEquals("*****", result.get(Config.LOGVIEWER_HTTPS_KEY_PASSWORD));
+        Assert.assertEquals(100, result.get(Config.TOPOLOGY_MESSAGE_TIMEOUT_SECS));
+    }
+
+
 }
\ No newline at end of file


Mime
View raw message