This is an automated email from the ASF dual-hosted git repository.
anmolnar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 34a949208 ZOOKEEPER-5049: Redact passwords from
PrometheusMetricsProvider configuration logging
34a949208 is described below
commit 34a9492085b3ecc18c9eb83414ebe5e17ede287a
Author: Dávid Paksy <[email protected]>
AuthorDate: Tue May 19 16:40:51 2026 +0200
ZOOKEEPER-5049: Redact passwords from PrometheusMetricsProvider
configuration logging
Reviewers: anmolnar, meszibalu
Author: PDavid
Closes #2387 from PDavid/ZOOKEEPER-5049-PrometheusMetricsProvider-log-redact
---
.../prometheus/PrometheusMetricsProvider.java | 3 +-
.../PrometheusHttpsMetricsProviderTest.java | 41 +++++++++
.../org/apache/zookeeper/common/LogRedactor.java | 62 +++++++++++++
.../java/org/apache/zookeeper/common/ZKConfig.java | 21 +----
.../apache/zookeeper/common/LogRedactorTest.java | 100 +++++++++++++++++++++
5 files changed, 209 insertions(+), 18 deletions(-)
diff --git
a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
index 85cacb686..001b7f25d 100644
---
a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java
+++
b/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.metrics.core.metrics.GaugeWithCallback;
import io.prometheus.metrics.exporter.servlet.javax.PrometheusMetricsServlet;
import io.prometheus.metrics.instrumentation.jvm.JvmMetrics;
@@ -136,7 +137,7 @@ protected void doTrace(HttpServletRequest req,
HttpServletResponse resp) throws
@Override
public void configure(Properties configuration) throws
MetricsProviderLifeCycleException {
- LOG.info("Initializing Prometheus metrics with Jetty, configuration:
{}", configuration);
+ LOG.info("Initializing Prometheus metrics with Jetty, configuration:
{}", redactSensitiveValues(configuration));
this.host = configuration.getProperty(HTTP_HOST, "0.0.0.0");
this.httpPort = Integer.parseInt(configuration.getProperty(HTTP_PORT,
"-1"));
diff --git
a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusHttpsMetricsProviderTest.java
b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusHttpsMetricsProviderTest.java
index f730b4dbe..53332e24f 100644
---
a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusHttpsMetricsProviderTest.java
+++
b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusHttpsMetricsProviderTest.java
@@ -21,6 +21,11 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
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 ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
import java.io.BufferedReader;
import java.io.FileInputStream;
import java.io.IOException;
@@ -36,6 +41,7 @@
import org.apache.zookeeper.metrics.Counter;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
+import org.slf4j.LoggerFactory;
/**
* Tests about Prometheus Metrics Provider. Please note that we are not
testing Prometheus but only our integration.
@@ -176,4 +182,39 @@ private void validateMetricResponse(String response)
throws IOException {
assertThat(response, containsString("# TYPE cc_total counter"));
assertThat(response, containsString("cc_total 10.0"));
}
+
+ @Test
+ void testLogRedactorRedactsPasswords() throws Exception {
+ Logger logger = (Logger)
LoggerFactory.getLogger(PrometheusMetricsProvider.class);
+ ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
+ listAppender.start();
+ logger.addAppender(listAppender);
+
+ try {
+ provider = new PrometheusMetricsProvider();
+ Properties configuration = new Properties();
+ configuration.setProperty("httpPort", String.valueOf(httpPort));
+ configuration.setProperty("httpsPort", String.valueOf(httpsPort));
+ configuration.setProperty("ssl.keyStore.location", testDataPath +
"/ssl/server_keystore.jks");
+ configuration.setProperty("ssl.keyStore.password",
"SuperSecret123!");
+ configuration.setProperty("ssl.trustStore.location", testDataPath
+ "/ssl/server_truststore.jks");
+ configuration.setProperty("ssl.trustStore.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 keyStore password");
+ assertFalse(logOutput.contains("AnotherSecret456!"),
+ "Logs should not contain trustStore password");
+ assertTrue(logOutput.contains(testDataPath +
"/ssl/server_keystore.jks"),
+ "Logs should still contain non-sensitive config like
keyStore location");
+ } finally {
+ logger.detachAppender(listAppender);
+ }
+ }
}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/LogRedactor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/LogRedactor.java
new file mode 100644
index 000000000..7a9cf1a85
--- /dev/null
+++
b/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;
+ }
+}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
index 2c9e63199..442d8e722 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
@@ -18,6 +18,8 @@
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;
@@ -25,7 +27,6 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.HashMap;
-import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
@@ -105,11 +106,7 @@ public ZKConfig(File configFile) throws ConfigException {
public ZKConfig(Path configPath) throws ConfigException {
this();
addConfiguration(configPath);
- 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));
}
private void init() {
@@ -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));
}
}
@@ -341,14 +338,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;
- }
}
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/LogRedactorTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/LogRedactorTest.java
new file mode 100644
index 000000000..a15696f02
--- /dev/null
+++
b/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());
+ }
+}
