This is an automated email from the ASF dual-hosted git repository.
andor pushed a commit to branch branch-3.9
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/branch-3.9 by this push:
new f61fdb1d1 Add log redactor method when logging ZK config properties
f61fdb1d1 is described below
commit f61fdb1d1cfa67b5233a193844362bb8dfdaa9c0
Author: Andor Molnar <[email protected]>
AuthorDate: Fri Dec 5 16:16:39 2025 -0600
Add log redactor method when logging ZK config properties
---
.../java/org/apache/zookeeper/common/ZKConfig.java | 17 ++++-
.../org/apache/zookeeper/common/ZKConfigTest.java | 72 +++++++++++++++++++++-
.../org/apache/zookeeper/test/LoggerTestTool.java | 30 ++++++++-
.../src/test/resources/zookeeper-client.config | 5 ++
4 files changed, 120 insertions(+), 4 deletions(-)
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 f6221eeb7..fa8cd5459 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
@@ -23,9 +23,11 @@
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;
+
import org.apache.zookeeper.Environment;
import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
import org.apache.zookeeper.server.util.VerifyingFileFactory;
@@ -93,6 +95,10 @@ 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 {}", this.properties);
}
@@ -206,7 +212,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,
oldValue, value);
+ LOG.debug("key {}'s value {} is replaced with new value {}", key,
logRedactor(key, oldValue), logRedactor(key, value));
}
}
@@ -330,4 +336,13 @@ 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/ZKConfigTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKConfigTest.java
index c64d5e694..7b87e2e3e 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKConfigTest.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKConfigTest.java
@@ -19,13 +19,35 @@
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.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.zookeeper.test.LoggerTestTool;
+import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
+import ch.qos.logback.classic.Level;
+
+import java.io.File;
+import java.io.IOException;
public class ZKConfigTest {
- X509Util x509Util = new ClientX509Util();
+ private final X509Util x509Util = new ClientX509Util();
+ private static LoggerTestTool loggerTestTool;
+
+ @BeforeAll
+ public static void setUpBeforeClass() {
+ loggerTestTool = new LoggerTestTool(ZKConfig.class, Level.DEBUG);
+ }
+
+ @AfterAll
+ public static void tearDownAfterClass() throws Exception {
+ loggerTestTool.close();
+ }
@AfterEach
public void tearDown() throws Exception {
@@ -90,4 +112,52 @@ public void
testBooleanRetrievalFromPropertyWithWhitespacesAtBeginningAndEnd() {
boolean result = conf.getBoolean(x509Util.getSslProtocolProperty(),
defaultValue);
assertEquals(value, result);
}
+
+ @Test
+ public void testLogRedactorFromConfigFile() throws ConfigException,
IOException {
+ // Arrange
+ File configFile = new
File("./src/test/resources/zookeeper-client.config");
+
+ // Act
+ new ZKConfig(configFile.toPath());
+
+ // Assert
+ String logLine = loggerTestTool.readLogLine("ZK Config");
+ assertNotNull(logLine, "Unable to find ZK Config line in the logs");
+ assertFalse(logLine.contains("FileSecret456!"), "Logs should not
contain any secrets");
+ assertFalse(logLine.contains("AnotherFileSecret789!"), "Logs should
not contain any secrets");
+ assertTrue(logLine.contains("/home/zookeeper/top_secret.txt"));
// what we shouldn't redact
+ }
+
+ @Test
+ public void testLogRedactorInDebugLogs() throws IOException {
+ // Arrange
+ ZKConfig conf = new ZKConfig();
+
+ // Act
+ conf.setProperty("zookeeper.some.secret.password", "0ldP4ssw0rd");
+ conf.setProperty("zookeeper.some.secret.password", "N3Ws3cr3t");
+
+ // Assert
+ String logLine = loggerTestTool.readLogLine("replaced with new value");
+ assertNotNull(logLine, "Unable to find relevant line in the logs");
+ assertFalse(logLine.contains("0ldP4ssw0rd"), "Logs should not contain
any secrets");
+ assertFalse(logLine.contains("N3Ws3cr3t"), "Logs should not contain
any secrets");
+ }
+
+ @Test
+ public void testDontRedactorInDebugLogs() throws IOException {
+ // Arrange
+ ZKConfig conf = new ZKConfig();
+
+ // Act
+ conf.setProperty("zookeeper.some.secret.passwordPath",
"/home/zookeeper/old_secret.txt"); // what we shouldn't redact
+ conf.setProperty("zookeeper.some.secret.passwordPath",
"/home/zookeeper/new_secret.txt"); // what we shouldn't redact
+
+ // Assert
+ String logLine = loggerTestTool.readLogLine("replaced with new value");
+ assertNotNull(logLine, "Unable to find relevant line in the logs");
+ assertTrue(logLine.contains("/home/zookeeper/new_secret.txt"));
+ }
+
}
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/LoggerTestTool.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/LoggerTestTool.java
index c95a0d32d..5f225ee18 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/LoggerTestTool.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/LoggerTestTool.java
@@ -27,17 +27,27 @@
import ch.qos.logback.core.OutputStreamAppender;
import ch.qos.logback.core.encoder.LayoutWrappingEncoder;
import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.LineNumberReader;
+import java.io.StringReader;
+
import org.slf4j.LoggerFactory;
public class LoggerTestTool implements AutoCloseable {
private final ByteArrayOutputStream os;
private Appender<ILoggingEvent> appender;
private Logger qlogger;
+ private Level logLevel = Level.INFO;
public LoggerTestTool(Class<?> cls) {
os = createLoggingStream(cls);
}
+ public LoggerTestTool(Class<?> cls, Level logLevel) {
+ this.logLevel = logLevel;
+ this.os = createLoggingStream(cls);
+ }
+
public LoggerTestTool(String cls) {
os = createLoggingStream(cls);
}
@@ -51,7 +61,7 @@ private ByteArrayOutputStream createLoggingStream(Class<?>
cls) {
appender = getConsoleAppender(os);
qlogger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(cls);
qlogger.addAppender(appender);
- qlogger.setLevel(Level.INFO);
+ qlogger.setLevel(logLevel);
appender.start();
return os;
}
@@ -61,7 +71,7 @@ private ByteArrayOutputStream createLoggingStream(String cls)
{
appender = getConsoleAppender(os);
qlogger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(cls);
qlogger.addAppender(appender);
- qlogger.setLevel(Level.INFO);
+ qlogger.setLevel(logLevel);
appender.start();
return os;
}
@@ -69,6 +79,7 @@ private ByteArrayOutputStream createLoggingStream(String cls)
{
private OutputStreamAppender<ILoggingEvent>
getConsoleAppender(ByteArrayOutputStream os) {
Logger rootLogger =
(ch.qos.logback.classic.Logger)
LoggerFactory.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME);
+ rootLogger.setLevel(logLevel);
Layout<ILoggingEvent> layout = ((LayoutWrappingEncoder<ILoggingEvent>)
((OutputStreamAppender<ILoggingEvent>)
rootLogger.getAppender("CONSOLE")).getEncoder()).getLayout();
@@ -80,6 +91,21 @@ private OutputStreamAppender<ILoggingEvent>
getConsoleAppender(ByteArrayOutputSt
return appender;
}
+ public String readLogLine(String search) throws IOException {
+ try {
+ LineNumberReader r = new LineNumberReader(new
StringReader(os.toString()));
+ String line;
+ while ((line = r.readLine()) != null) {
+ if (line.contains(search)) {
+ return line;
+ }
+ }
+ return null;
+ } finally {
+ os.reset();
+ }
+ }
+
@Override
public void close() throws Exception {
qlogger.detachAppender(appender);
diff --git a/zookeeper-server/src/test/resources/zookeeper-client.config
b/zookeeper-server/src/test/resources/zookeeper-client.config
new file mode 100644
index 000000000..096105ce9
--- /dev/null
+++ b/zookeeper-server/src/test/resources/zookeeper-client.config
@@ -0,0 +1,5 @@
+# Sample secrets
+zookeeper.ssl.keyStore.password=FileSecret456!
+zookeeper.ssl.keyStore.passwordPath=/home/zookeeper/top_secret.txt
+ssl.quorum.keyStore.password=AnotherFileSecret789!
+
