ZOOKEEPER-5049: Redact passwords from PrometheusMetricsProvider configuration logging

zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml

@@ -84,6 +84,11 @@
       <artifactId>junit-platform-runner</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>ch.qos.logback</groupId>
+      <artifactId>logback-classic</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java

@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.metrics.prometheus;
 
+import static org.apache.zookeeper.common.LogRedactor.redactSensitiveValues;
 import io.prometheus.client.Collector;
 import io.prometheus.client.CollectorRegistry;
 import io.prometheus.client.exporter.MetricsServlet;
@@ -113,7 +114,7 @@ public class PrometheusMetricsProvider implements MetricsProvider {
 
     @Override
     public void configure(Properties configuration) throws MetricsProviderLifeCycleException {
-        LOG.info("Initializing metrics, configuration: {}", configuration);
+        LOG.info("Initializing metrics, configuration: {}", redactSensitiveValues(configuration));
         this.host = configuration.getProperty("httpHost", "0.0.0.0");
         this.port = Integer.parseInt(configuration.getProperty("httpPort", "7000"));
         this.exportJvmInfo = Boolean.parseBoolean(configuration.getProperty("exportJvmInfo", "true"));

zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java

@@ -18,11 +18,17 @@
 
 package org.apache.zookeeper.metrics.prometheus;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import io.prometheus.client.CollectorRegistry;
 import java.util.Properties;
 import org.apache.zookeeper.metrics.MetricsProviderLifeCycleException;
 import org.junit.Assert;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
 import org.junit.Test;
+import org.slf4j.LoggerFactory;
 
 
 public class PrometheusMetricsProviderConfigTest {
@@ -63,4 +69,36 @@ public void testValidConfig() throws MetricsProviderLifeCycleException {
         provider.start();
     }
 
+    @Test
+    public void testLogRedactorRedactsPasswords() throws Exception {
+        Logger logger = (Logger) LoggerFactory.getLogger(PrometheusMetricsProvider.class);
+        ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
+        listAppender.start();
+        logger.addAppender(listAppender);
+
+        try {
+            PrometheusMetricsProvider provider = new PrometheusMetricsProvider();
+            Properties configuration = new Properties();
+            configuration.setProperty("httpHost", "0.0.0.0");
+            configuration.setProperty("httpPort", "65536");
+            configuration.setProperty("some.password", "SuperSecret123!");
+            configuration.setProperty("some-other.password", "AnotherSecret456!");
+            provider.configure(configuration);
+
+            String logOutput = listAppender.list.stream()
+                    .map(ILoggingEvent::getFormattedMessage)
+                    .filter(msg -> msg.contains("configuration"))
+                    .findFirst()
+                    .orElse("");
+
+            assertFalse(logOutput.contains("SuperSecret123!"),
+                    "Logs should not contain some password");
+            assertFalse(logOutput.contains("AnotherSecret456!"),
+                    "Logs should not contain other password");
+            assertTrue(logOutput.contains("0.0.0.0"),
+                    "Logs should still contain non-sensitive config like host");
+        } finally {
+            logger.detachAppender(listAppender);
+        }
+    }
 }

zookeeper-server/src/main/java/org/apache/zookeeper/common/LogRedactor.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+import java.util.Locale;
+import java.util.Map;
+import java.util.Properties;
+
+public class LogRedactor {
+
+    private LogRedactor() {}
+
+    /**
+     * Redacts all values which ends with "password" from the given properties / map.
+     * Other values are not changed.
+     *
+     * @param properties the properties in which all passwords should be redacted.
+     * @return new Properties object containing all values, passwords redacted.
+     */
+    public static Properties redactSensitiveValues(Map<?, ?> properties) {
+        Properties redactedConfig = new Properties();
+        properties.forEach((k, v) -> {
+            if (k != null && v != null) {
+                redactedConfig.put(k, redactValue((String) k, (String) v));
+            }
+        });
+        return redactedConfig;
+    }
+
+    /**
+     * Returns redacted value when the key ends with "password".
+     * Otherwise, just returns the value.
+     *
+     * @param key the key to check if it ends with "password".
+     * @return redacted value when the key ends with "password". Otherwise, the value.
+     */
+    public static String redactValue(String key, String value) {
+        if (key == null) {
+            return value;
+        }
+        if (key.toLowerCase(Locale.ROOT).endsWith("password")) {
+            return "***";
+        }
+        return value;
+    }
+}

zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java

@@ -18,12 +18,13 @@
 
 package org.apache.zookeeper.common;
 
+import static org.apache.zookeeper.common.LogRedactor.redactSensitiveValues;
+import static org.apache.zookeeper.common.LogRedactor.redactValue;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.HashMap;
-import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Properties;
@@ -94,11 +95,7 @@ public ZKConfig(String configPath) throws QuorumPeerConfig.ConfigException {
     public ZKConfig(File configFile) throws QuorumPeerConfig.ConfigException {
         this();
         addConfiguration(configFile);
-        Map<String, String> p = new HashMap<>();
-        for (Entry<String, String> entry : properties.entrySet()) {
-            p.put(entry.getKey(), logRedactor(entry.getKey(), entry.getValue()));
-        }
-        LOG.info("ZK Config {}", p);
+        LOG.info("ZK Config {}", redactSensitiveValues(properties));
     }
 
     /**
@@ -211,7 +208,7 @@ public void setProperty(String key, String value) {
         }
         String oldValue = properties.put(key, value);
         if (null != oldValue && !oldValue.equals(value)) {
-            LOG.debug("key {}'s value {} is replaced with new value {}", key, logRedactor(key, oldValue), logRedactor(key, value));
+            LOG.debug("key {}'s value {} is replaced with new value {}", key, redactValue(key, oldValue), redactValue(key, value));
         }
     }
 
@@ -334,14 +331,4 @@ public int getInt(String key, int defaultValue) {
         }
         return defaultValue;
     }
-
-    private String logRedactor(String key, String value) {
-        if (key == null) {
-            return value;
-        }
-        if (key.toLowerCase(Locale.ROOT).endsWith("password")) {
-            return "***";
-        }
-        return value;
-    }
 }

zookeeper-server/src/test/java/org/apache/zookeeper/common/LogRedactorTest.java

@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import org.junit.jupiter.api.Test;
+
+class LogRedactorTest {
+
+    @Test
+    void testRedactValueWithPasswordKey() {
+        assertEquals("***", LogRedactor.redactValue("ssl.keyStore.password", "secret"));
+        assertEquals("***", LogRedactor.redactValue("ssl.trustStore.password", "secret"));
+        assertEquals("***", LogRedactor.redactValue("somePassword", "secret"));
+    }
+
+    @Test
+    void testRedactValueCaseInsensitive() {
+        assertEquals("***", LogRedactor.redactValue("ssl.keyStore.PASSWORD", "secret"));
+        assertEquals("***", LogRedactor.redactValue("ssl.keyStore.Password", "secret"));
+        assertEquals("***", LogRedactor.redactValue("ssl.keyStore.passWORD", "secret"));
+    }
+
+    @Test
+    void testRedactValueNonSensitiveKey() {
+        assertEquals("localhost", LogRedactor.redactValue("server.host", "localhost"));
+        assertEquals("8080", LogRedactor.redactValue("httpPort", "8080"));
+        assertEquals("/path/to/keystore", LogRedactor.redactValue("ssl.keyStore.location", "/path/to/keystore"));
+    }
+
+    @Test
+    void testRedactValueNullKey() {
+        assertEquals("someValue", LogRedactor.redactValue(null, "someValue"));
+    }
+
+    @Test
+    void testRedactSensitiveValuesWithStringMap() {
+        Map<String, String> config = new HashMap<>();
+        config.put("ssl.keyStore.location", "/path/to/keystore.jks");
+        config.put("ssl.keyStore.password", "SuperSecret");
+        config.put("httpPort", "9141");
+
+        Properties redacted = LogRedactor.redactSensitiveValues(config);
+
+        assertEquals("/path/to/keystore.jks", redacted.get("ssl.keyStore.location"));
+        assertEquals("***", redacted.get("ssl.keyStore.password"));
+        assertEquals("9141", redacted.get("httpPort"));
+    }
+
+    @Test
+    void testRedactSensitiveValuesWithProperties() {
+        Properties config = new Properties();
+        config.setProperty("ssl.trustStore.password", "TrustSecret");
+        config.setProperty("ssl.trustStore.location", "/path/to/truststore.jks");
+
+        Properties redacted = LogRedactor.redactSensitiveValues(config);
+
+        assertEquals("***", redacted.get("ssl.trustStore.password"));
+        assertEquals("/path/to/truststore.jks", redacted.get("ssl.trustStore.location"));
+    }
+
+    @Test
+    void testRedactSensitiveValuesSkipsNullValues() {
+        Map<String, String> config = new HashMap<>();
+        config.put("ssl.keyStore.location", null);
+        config.put("httpPort", "9141");
+
+        Properties redacted = LogRedactor.redactSensitiveValues(config);
+
+        assertFalse(redacted.containsKey("ssl.keyStore.location"));
+        assertEquals("9141", redacted.get("httpPort"));
+    }
+
+    @Test
+    void testRedactSensitiveValuesEmptyMap() {
+        Properties redacted = LogRedactor.redactSensitiveValues(new HashMap<>());
+        assertTrue(redacted.isEmpty());
+    }
+}