This is an automated email from the ASF dual-hosted git repository. andor pushed a commit to branch branch-3.8.6 in repository https://gitbox.apache.org/repos/asf/zookeeper.git
commit 06996fe213fb2f5e02954ebaa9e3a9c61aa964ac 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 | 18 +++++- .../org/apache/zookeeper/common/ZKConfigTest.java | 71 +++++++++++++++++++++- .../org/apache/zookeeper/test/LoggerTestTool.java | 29 ++++++++- .../src/test/resources/zookeeper-client.config | 19 ++++++ 4 files changed, 132 insertions(+), 5 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 7aa68cc85..7d7e3519c 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 @@ -22,6 +22,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.util.HashMap; +import java.util.Locale; import java.util.Map; import java.util.Map.Entry; import java.util.Properties; @@ -82,7 +83,11 @@ public ZKConfig(String configPath) throws ConfigException { public ZKConfig(File configFile) throws ConfigException { this(); addConfiguration(configFile); - LOG.info("ZK Config {}", this.properties); + 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); } private void init() { @@ -182,7 +187,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)); } } @@ -283,4 +288,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..c88b59bd5 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,34 @@ 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 ch.qos.logback.classic.Level; +import java.io.File; +import java.io.IOException; +import org.apache.zookeeper.server.quorum.QuorumPeerConfig; +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; 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 +111,52 @@ public void testBooleanRetrievalFromPropertyWithWhitespacesAtBeginningAndEnd() { boolean result = conf.getBoolean(x509Util.getSslProtocolProperty(), defaultValue); assertEquals(value, result); } + + @Test + public void testLogRedactorFromConfigFile() throws IOException, QuorumPeerConfig.ConfigException { + // Arrange + File configFile = new File("./src/test/resources/zookeeper-client.config"); + + // Act + new ZKConfig(configFile); + + // 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..5633c08b0 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,26 @@ 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 +60,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 +70,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 +78,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 +90,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..4718f6d9c --- /dev/null +++ b/zookeeper-server/src/test/resources/zookeeper-client.config @@ -0,0 +1,19 @@ +# 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. + +# Sample secrets +zookeeper.ssl.keyStore.password=FileSecret456! +zookeeper.ssl.keyStore.passwordPath=/home/zookeeper/top_secret.txt +ssl.quorum.keyStore.password=AnotherFileSecret789!
