You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@storm.apache.org by ka...@apache.org on 2018/08/14 14:24:49 UTC
[1/2] storm git commit: STORM-3184: Mask the plaintext passwords from
the logs
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 <ar...@apache.org>
Authored: Tue Aug 7 12:13:54 2018 -0700
Committer: Arun Mahadevan <ar...@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
[2/2] storm git commit: Merge branch 'STORM-3184' of
https://github.com/arunmahadevan/storm into STORM-3184-1.x-merge
Posted by ka...@apache.org.
Merge branch 'STORM-3184' of https://github.com/arunmahadevan/storm into STORM-3184-1.x-merge
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/4f8b0e39
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/4f8b0e39
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/4f8b0e39
Branch: refs/heads/1.x-branch
Commit: 4f8b0e39baf12b7e6993d6785822a2a01745a9cb
Parents: 7c4ae00 fc942ee
Author: Jungtaek Lim <ka...@gmail.com>
Authored: Tue Aug 14 23:24:39 2018 +0900
Committer: Jungtaek Lim <ka...@gmail.com>
Committed: Tue Aug 14 23:24:39 2018 +0900
----------------------------------------------------------------------
.../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(-)
----------------------------------------------------------------------