Skip to content

Commit f1322bb

Browse files
committed
ZOOKEEPER-5058: Remove special characters from ensemble name before logging in EnsembleAuthenticationProvider
Reviewers: PDavid, PDavid, phunt Author: anmolnar Closes #2409 from anmolnar/ZOOKEEPER-5058
1 parent e1cbeb5 commit f1322bb

2 files changed

Lines changed: 65 additions & 1 deletion

File tree

zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ public KeeperException.Code handleAuthentication(ServerCnxn cnxn, byte[] authDat
9292
long currentTime = System.currentTimeMillis();
9393
if (lastFailureLogged + MIN_LOGGING_INTERVAL_MS < currentTime) {
9494
String id = cnxn.getRemoteSocketAddress().getAddress().getHostAddress();
95-
LOG.warn("Unexpected ensemble name: ensemble name: {} client ip: {}", receivedEnsembleName, id);
95+
String logEnsembleName = receivedEnsembleName.replaceAll("[\\x00-\\x1F]", "");
96+
LOG.warn("Unexpected ensemble name: ensemble name: {} client ip: {}", logEnsembleName, id);
9697
lastFailureLogged = currentTime;
9798
}
9899
/*
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.zookeeper.server.auth;
19+
20+
import static org.mockito.Mockito.doReturn;
21+
import static org.mockito.Mockito.mock;
22+
import ch.qos.logback.classic.Level;
23+
import java.io.IOException;
24+
import java.net.InetSocketAddress;
25+
import java.nio.charset.StandardCharsets;
26+
import org.apache.zookeeper.KeeperException;
27+
import org.apache.zookeeper.server.ServerCnxn;
28+
import org.apache.zookeeper.test.LoggerTestTool;
29+
import org.junit.jupiter.api.AfterAll;
30+
import org.junit.jupiter.api.Assertions;
31+
import org.junit.jupiter.api.BeforeAll;
32+
import org.junit.jupiter.api.Test;
33+
34+
public class EnsembleAuthenticationProviderTest {
35+
private static LoggerTestTool loggerTestTool;
36+
37+
@BeforeAll
38+
public static void setupBeforeClass() {
39+
loggerTestTool = new LoggerTestTool(EnsembleAuthenticationProvider.class, Level.INFO);
40+
}
41+
42+
@AfterAll
43+
public static void tearDownAfterClass() throws Exception {
44+
loggerTestTool.close();
45+
}
46+
47+
@Test
48+
public void testLogForgeryWithSpecialCharacters() throws IOException {
49+
ServerCnxn mockServerCnxn = mock(ServerCnxn.class);
50+
InetSocketAddress mockAddress = new InetSocketAddress("127.0.0.1", 1234);
51+
doReturn(mockAddress).when(mockServerCnxn).getRemoteSocketAddress();
52+
53+
EnsembleAuthenticationProvider provider = new EnsembleAuthenticationProvider();
54+
provider.setEnsembleNames("test-ensemble");
55+
56+
byte[] authData = "andor-ensemble\nTHIS SHOULD\t NOT\r BE HERE".getBytes(StandardCharsets.UTF_8);
57+
KeeperException.Code err = provider.handleAuthentication(mockServerCnxn, authData);
58+
String logLine = loggerTestTool.readLogLine("andor-ensemble");
59+
Assertions.assertTrue(logLine.contains("THIS SHOULD NOT BE HERE"), "Log line doesn't contain the entire ensemble name. Forged?");
60+
61+
Assertions.assertEquals(KeeperException.Code.BADARGUMENTS, err);
62+
}
63+
}

0 commit comments

Comments
 (0)