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 67037ad90 ZOOKEEPER-4860: Disable X-Forwarded-For by default
67037ad90 is described below
commit 67037ad9087b4e554f77427e88d40df1eb19d6d3
Author: Andor Molnár <[email protected]>
AuthorDate: Tue Sep 17 10:34:39 2024 -0500
ZOOKEEPER-4860: Disable X-Forwarded-For by default
Reviewers: eolivelli, purushah
Author: anmolnar
Closes #2187 from anmolnar/ZOOKEEPER-4860
(cherry picked from commit 978d431f6e241dd7f58c5b105afcf4443f5bcbcb)
Signed-off-by: Andor Molnar <[email protected]>
---
.../src/main/resources/markdown/zookeeperAdmin.md | 15 +++++++------
.../server/auth/IPAuthenticationProvider.java | 4 ++--
.../server/auth/IPAuthenticationProviderTest.java | 26 +++++++++++++++++-----
.../java/org/apache/zookeeper/test/IPAuthTest.java | 17 +++++++++++---
4 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 9c4408d2e..377c2f30a 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -1572,14 +1572,15 @@ and [SASL authentication for
ZooKeeper](https://cwiki.apache.org/confluence/disp
- 1. Regenerate `superDigest` when migrating to new algorithm.
- 2. `SetAcl` for a znode which already had a digest auth of old
algorithm.
-* *IPAuthenticationProvider.skipxforwardedfor* :
- (Java system property:
**zookeeper.IPAuthenticationProvider.skipxforwardedfor**)
+* *IPAuthenticationProvider.usexforwardedfor* :
+ (Java system property:
**zookeeper.IPAuthenticationProvider.usexforwardedfor**)
**New in 3.9.3:**
- IPAuthenticationProvider needs the client IP address to authenticate the
user.
- By default, it tries to read **X-Forwarded-For** HTTP header first and if
it's not
- found, reads the **Host** header. Some proxy configuration requires this to
- properly identify the client IP, but we can disable it relying only on the
**Host**
- header by setting this config option to **true**.
+ IPAuthenticationProvider uses the client IP address to authenticate the
user. By
+ default it reads the **Host** HTTP header to detect client IP address. In
some
+ proxy configurations the proxy server adds the **X-Forwarded-For** header
to
+ the request in order to provide the IP address of the original client
request.
+ By enabling **usexforwardedfor** ZooKeeper setting, **X-Forwarded-For**
will be preferred
+ over the standard **Host** header.
Default value is **false**.
* *X509AuthenticationProvider.superUser* :
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java
index c73d12f78..26c14a4e8 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java
@@ -30,7 +30,7 @@ import org.apache.zookeeper.server.ServerCnxn;
public class IPAuthenticationProvider implements AuthenticationProvider {
public static final String X_FORWARDED_FOR_HEADER_NAME = "X-Forwarded-For";
- static final String SKIP_X_FORWARDED_FOR_KEY =
"zookeeper.IPAuthenticationProvider.skipxforwardedfor";
+ public static final String USE_X_FORWARDED_FOR_KEY =
"zookeeper.IPAuthenticationProvider.usexforwardedfor";
public String getScheme() {
return "ip";
@@ -152,7 +152,7 @@ public class IPAuthenticationProvider implements
AuthenticationProvider {
* @return IP address
*/
public static String getClientIPAddress(final HttpServletRequest request) {
- if (Boolean.getBoolean(SKIP_X_FORWARDED_FOR_KEY)) {
+ if (!Boolean.getBoolean(USE_X_FORWARDED_FOR_KEY)) {
return request.getRemoteAddr();
}
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/IPAuthenticationProviderTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/IPAuthenticationProviderTest.java
index fc814f7ea..c1a1d1f52 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/IPAuthenticationProviderTest.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/auth/IPAuthenticationProviderTest.java
@@ -17,7 +17,7 @@
*/
package org.apache.zookeeper.server.auth;
-import static
org.apache.zookeeper.server.auth.IPAuthenticationProvider.SKIP_X_FORWARDED_FOR_KEY;
+import static
org.apache.zookeeper.server.auth.IPAuthenticationProvider.USE_X_FORWARDED_FOR_KEY;
import static
org.apache.zookeeper.server.auth.IPAuthenticationProvider.X_FORWARDED_FOR_HEADER_NAME;
import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.doReturn;
@@ -33,19 +33,33 @@ public class IPAuthenticationProviderTest {
@Before
public void setUp() throws Exception {
- System.clearProperty(SKIP_X_FORWARDED_FOR_KEY);
+ System.clearProperty(USE_X_FORWARDED_FOR_KEY);
request = mock(HttpServletRequest.class);
}
@After
public void tearDown() {
- System.clearProperty(SKIP_X_FORWARDED_FOR_KEY);
+ System.clearProperty(USE_X_FORWARDED_FOR_KEY);
}
@Test
public void testGetClientIPAddressSkipXForwardedFor() {
// Arrange
- System.setProperty(SKIP_X_FORWARDED_FOR_KEY, "true");
+ System.setProperty(USE_X_FORWARDED_FOR_KEY, "false");
+ doReturn("192.168.1.1").when(request).getRemoteAddr();
+
doReturn("192.168.1.2,192.168.1.3,192.168.1.4").when(request).getHeader(X_FORWARDED_FOR_HEADER_NAME);
+
+ // Act
+ String clientIp = IPAuthenticationProvider.getClientIPAddress(request);
+
+ // Assert
+ assertEquals("192.168.1.1", clientIp);
+ }
+
+ @Test
+ public void testGetClientIPAddressDefaultBehaviour() {
+ // Arrange
+ System.clearProperty(USE_X_FORWARDED_FOR_KEY);
doReturn("192.168.1.1").when(request).getRemoteAddr();
doReturn("192.168.1.2,192.168.1.3,192.168.1.4").when(request).getHeader(X_FORWARDED_FOR_HEADER_NAME);
@@ -59,7 +73,7 @@ public class IPAuthenticationProviderTest {
@Test
public void testGetClientIPAddressWithXForwardedFor() {
// Arrange
- System.setProperty(SKIP_X_FORWARDED_FOR_KEY, "false");
+ System.setProperty(USE_X_FORWARDED_FOR_KEY, "true");
doReturn("192.168.1.1").when(request).getRemoteAddr();
doReturn("192.168.1.2,192.168.1.3,192.168.1.4").when(request).getHeader(X_FORWARDED_FOR_HEADER_NAME);
@@ -73,7 +87,7 @@ public class IPAuthenticationProviderTest {
@Test
public void testGetClientIPAddressMissingXForwardedFor() {
// Arrange
- System.setProperty(SKIP_X_FORWARDED_FOR_KEY, "false");
+ System.setProperty(USE_X_FORWARDED_FOR_KEY, "false");
doReturn("192.168.1.1").when(request).getRemoteAddr();
// Act
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/IPAuthTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/IPAuthTest.java
index 45ce447fe..b50b8ed76 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/IPAuthTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/IPAuthTest.java
@@ -23,13 +23,24 @@ import static org.mockito.Mockito.mock;
import java.util.Arrays;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
-import org.apache.zookeeper.ZKTestCase;
import org.apache.zookeeper.data.Id;
import org.apache.zookeeper.server.auth.IPAuthenticationProvider;
-import org.junit.jupiter.api.Test;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
import org.mockito.Mockito;
-public class IPAuthTest extends ZKTestCase {
+public class IPAuthTest {
+ @Before
+ public void setUp() {
+ System.setProperty(IPAuthenticationProvider.USE_X_FORWARDED_FOR_KEY,
"true");
+ }
+
+ @After
+ public void tearDown() {
+ System.clearProperty(IPAuthenticationProvider.USE_X_FORWARDED_FOR_KEY);
+ }
+
@Test
public void testHandleAuthentication_Forwarded() {
final IPAuthenticationProvider provider = new
IPAuthenticationProvider();
