From 6493e94069e90f0ccd4a5017333090b27c303e2b Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Tue, 23 Jun 2026 17:25:51 -0500 Subject: [PATCH 1/2] ZOOKEEPER-5058. Remove special characters from ensemble name before logging --- .../auth/EnsembleAuthenticationProvider.java | 3 +- .../EnsembleAuthenticationProviderTest.java | 65 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProviderTest.java diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java index 1e24c928353..331db2c5b31 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java @@ -92,7 +92,8 @@ public KeeperException.Code handleAuthentication(ServerCnxn cnxn, byte[] authDat long currentTime = System.currentTimeMillis(); if (lastFailureLogged + MIN_LOGGING_INTERVAL_MS < currentTime) { String id = cnxn.getRemoteSocketAddress().getAddress().getHostAddress(); - LOG.warn("Unexpected ensemble name: ensemble name: {} client ip: {}", receivedEnsembleName, id); + String logEnsembleName = receivedEnsembleName.replaceAll("[\\x00-\\x1F]", ""); + LOG.warn("Unexpected ensemble name: ensemble name: {} client ip: {}", logEnsembleName, id); lastFailureLogged = currentTime; } /* diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProviderTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProviderTest.java new file mode 100644 index 00000000000..002200691f8 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProviderTest.java @@ -0,0 +1,65 @@ +/* + * 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.server.auth; + +import ch.qos.logback.classic.Level; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.server.ServerCnxn; +import org.apache.zookeeper.test.LoggerTestTool; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; + +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +public class EnsembleAuthenticationProviderTest { + private static LoggerTestTool loggerTestTool; + + @BeforeAll + public static void setupBeforeClass() { + loggerTestTool = new LoggerTestTool(EnsembleAuthenticationProvider.class, Level.INFO); + } + + @AfterAll + public static void tearDownAfterClass() throws Exception { + loggerTestTool.close(); + } + + @Test + public void testLogForgeryWithSpecialCharacters() throws IOException { + ServerCnxn mockServerCnxn = mock(ServerCnxn.class); + InetSocketAddress mockAddress = new InetSocketAddress("127.0.0.1", 1234); + doReturn(mockAddress).when(mockServerCnxn).getRemoteSocketAddress(); + + EnsembleAuthenticationProvider provider = new EnsembleAuthenticationProvider(); + provider.setEnsembleNames("test-ensemble"); + + byte[] authData = "andor-ensemble\nTHIS SHOULD\t NOT\r BE HERE".getBytes(StandardCharsets.UTF_8); + KeeperException.Code err = provider.handleAuthentication(mockServerCnxn, authData); + String logLine = loggerTestTool.readLogLine("andor-ensemble"); + Assertions.assertTrue(logLine.contains("THIS SHOULD NOT BE HERE"), "Log line doesn't contain the entire ensemble name. Forged?"); + + Assertions.assertEquals(KeeperException.Code.BADARGUMENTS, err); + } +} From 66266bda02c53d7d0d87fc77abfc6015dee96ec8 Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Tue, 23 Jun 2026 17:32:50 -0500 Subject: [PATCH 2/2] ZOOKEEPER-5058. Checkstyle fix --- .../auth/EnsembleAuthenticationProviderTest.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProviderTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProviderTest.java index 002200691f8..23f914d2fe1 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProviderTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProviderTest.java @@ -17,7 +17,12 @@ */ package org.apache.zookeeper.server.auth; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; import ch.qos.logback.classic.Level; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.server.ServerCnxn; import org.apache.zookeeper.test.LoggerTestTool; @@ -26,13 +31,6 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import java.io.IOException; -import java.net.InetSocketAddress; -import java.nio.charset.StandardCharsets; - -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; - public class EnsembleAuthenticationProviderTest { private static LoggerTestTool loggerTestTool;