Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions java/org/apache/catalina/filters/LocalStrings.properties
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ remoteCidrFilter.noRemoteIp=Client does not have an IP address. Request denied.

remoteIpFilter.invalidHostHeader=Invalid value [{0}] found for Host in HTTP header [{1}]
remoteIpFilter.invalidHostWithPort=Host value [{0}] in HTTP header [{1}] included a port number which will be ignored
remoteIpFilter.invalidNetMask=One or more netmasks provided are invalid: {0}
remoteIpFilter.invalidNumber=Illegal number for parameter [{0}]: [{1}]
remoteIpFilter.invalidPort=Port [{0}] in HTTP header [{1}] included a port number which will be ignored
remoteIpFilter.invalidRemoteAddress=Unable to determine the remote host because the reported remote address [{0}] is not valid
Expand Down
133 changes: 128 additions & 5 deletions java/org/apache/catalina/filters/RemoteIpFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

import org.apache.catalina.AccessLog;
import org.apache.catalina.Globals;
import org.apache.catalina.util.NetMaskSet;
import org.apache.catalina.util.RequestUtil;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
Expand Down Expand Up @@ -656,6 +657,10 @@ public StringBuffer getRequestURL() {

protected static final String INTERNAL_PROXIES_PARAMETER = "internalProxies";

protected static final String INTERNAL_PROXIES_CIDR_PARAMETER = "internalProxiesCidr";

protected static final String TRUSTED_PROXIES_CIDR_PARAMETER = "trustedProxiesCidr";

// Log must be non-static as loggers are created per class-loader and this
// Filter may be used in multiple class loaders
private transient Log log = LogFactory.getLog(RemoteIpFilter.class);
Expand Down Expand Up @@ -702,6 +707,16 @@ public StringBuffer getRequestURL() {
"172\\.2[0-9]{1}\\.\\d{1,3}\\.\\d{1,3}|" + "172\\.3[0-1]{1}\\.\\d{1,3}\\.\\d{1,3}|" +
"0:0:0:0:0:0:0:1|::1|" + "fe[89ab]\\p{XDigit}:.*|" + "f[cd]\\p{XDigit}{2}+:.*");

/**
* CIDR notation for internal proxies
*/
private NetMaskSet internalProxiesCidr = null;

/**
* CIDR notation for trusted proxies
*/
private NetMaskSet trustedProxiesCidr = null;

/**
* @see #setProtocolHeader(String)
*/
Expand Down Expand Up @@ -742,9 +757,9 @@ public StringBuffer getRequestURL() {
public void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
throws IOException, ServletException {

boolean isInternal = internalProxies != null && internalProxies.matcher(request.getRemoteAddr()).matches();
boolean isInternal = isInternalProxy(request.getRemoteAddr());

if (isInternal || (trustedProxies != null && trustedProxies.matcher(request.getRemoteAddr()).matches())) {
if (isInternal || isTrustedProxy(request.getRemoteAddr())) {
String remoteIp = null;
Deque<String> proxiesHeaderValue = new ArrayDeque<>();
StringBuilder concatRemoteIpHeaderValue = new StringBuilder();
Expand All @@ -766,9 +781,10 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F
for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
String currentRemoteIp = remoteIpHeaderValue[idx];
remoteIp = currentRemoteIp;
if (internalProxies != null && internalProxies.matcher(currentRemoteIp).matches()) {

if (isInternalProxy(currentRemoteIp)) {
// do nothing, internalProxies IPs are not appended to the
} else if (trustedProxies != null && trustedProxies.matcher(currentRemoteIp).matches()) {
} else if (isTrustedProxy(currentRemoteIp)) {
proxiesHeaderValue.addFirst(currentRemoteIp);
} else {
idx--; // decrement idx because break statement doesn't do it
Expand Down Expand Up @@ -877,13 +893,52 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F
} else {
if (log.isTraceEnabled()) {
log.trace("Skip RemoteIpFilter for request " + request.getRequestURI() + " with originalRemoteAddr '" +
request.getRemoteAddr() + "'");
request.getRemoteAddr() + "'");
}
chain.doFilter(request, response);
}

}

/**
* Checks if the given IP address is from an internal proxy.
*
* @param remoteIp The IP address to check
* @return {@code true} if the IP address is from an internal proxy, otherwise {@code false}
*/
private boolean isInternalProxy(String remoteIp) {
if (internalProxies != null && internalProxies.matcher(remoteIp).matches()) {
return true;
}
return checkIsCidr(internalProxiesCidr, remoteIp);
}

/**
* Checks if the given IP address is from a trusted proxy.
*
* @param remoteIp The IP address to check
* @return {@code true} if the IP address is from a trusted proxy, otherwise {@code false}
*/
private boolean isTrustedProxy(String remoteIp) {
if (trustedProxies != null && trustedProxies.matcher(remoteIp).matches()) {
return true;
}

return checkIsCidr(trustedProxiesCidr, remoteIp);
}

private boolean checkIsCidr(NetMaskSet netMaskSet, String remoteIp) {
if (netMaskSet == null) {
return false;
}
try {
return netMaskSet.contains(remoteIp);
} catch (UnknownHostException uhe) {
log.debug(sm.getString("remoteIpFilter.invalidRemoteAddress", remoteIp), uhe);
}
return false;
}

/*
* Considers the value to be secure if it exclusively holds forwards for {@link #protocolHeaderHttpsValue}.
*/
Expand Down Expand Up @@ -951,6 +1006,14 @@ public Pattern getInternalProxies() {
return internalProxies;
}

public NetMaskSet getInternalProxiesCidr() {
return internalProxiesCidr;
}

public NetMaskSet getTrustedProxiesCidr() {
return trustedProxiesCidr;
}

public String getProtocolHeader() {
return protocolHeader;
}
Expand Down Expand Up @@ -994,6 +1057,10 @@ public void init() throws ServletException {
setInternalProxies(getInitParameter(INTERNAL_PROXIES_PARAMETER));
}

if (getInitParameter(INTERNAL_PROXIES_CIDR_PARAMETER) != null) {
setInternalProxiesCidr(getInitParameter(INTERNAL_PROXIES_CIDR_PARAMETER));
}

if (getInitParameter(PROTOCOL_HEADER_PARAMETER) != null) {
setProtocolHeader(getInitParameter(PROTOCOL_HEADER_PARAMETER));
}
Expand Down Expand Up @@ -1030,6 +1097,10 @@ public void init() throws ServletException {
setTrustedProxies(getInitParameter(TRUSTED_PROXIES_PARAMETER));
}

if (getInitParameter(TRUSTED_PROXIES_CIDR_PARAMETER) != null) {
setTrustedProxiesCidr(getInitParameter(TRUSTED_PROXIES_CIDR_PARAMETER));
}

if (getInitParameter(HTTP_SERVER_PORT_PARAMETER) != null) {
try {
setHttpServerPort(Integer.parseInt(getInitParameter(HTTP_SERVER_PORT_PARAMETER)));
Expand Down Expand Up @@ -1133,6 +1204,58 @@ public void setInternalProxies(String internalProxies) {
}
}

/**
* <p>
* CIDR notation that defines the internal proxies.
* </p>
* <p>
* If this is set, it will be used instead of the regex pattern for internal proxies.
* </p>
*
* @param internalProxiesCidr The CIDR notation
*/
public void setInternalProxiesCidr(String internalProxiesCidr) {
if (internalProxiesCidr == null || internalProxiesCidr.isEmpty()) {
this.internalProxiesCidr = null;
} else {
this.internalProxiesCidr = new NetMaskSet();
List<String> errors = this.internalProxiesCidr.addAll(internalProxiesCidr);
if (!errors.isEmpty()) {
StringBuilder sb = new StringBuilder();
for (String error : errors) {
sb.append(error).append("; ");
}
throw new IllegalArgumentException(sm.getString("remoteIpFilter.invalidNetMask", sb.toString()));
}
}
}

/**
* <p>
* CIDR notation defining proxies that are trusted when they appear in the {@link #remoteIpHeader} header.
* </p>
* <p>
* If this is set, it will be used instead of the regex pattern for trusted proxies.
* </p>
*
* @param trustedProxiesCidr The trusted proxies in CIDR notation
*/
public void setTrustedProxiesCidr(String trustedProxiesCidr) {
if (trustedProxiesCidr == null || trustedProxiesCidr.isEmpty()) {
this.trustedProxiesCidr = null;
} else {
this.trustedProxiesCidr = new NetMaskSet();
List<String> errors = this.trustedProxiesCidr.addAll(trustedProxiesCidr);
if (!errors.isEmpty()) {
StringBuilder sb = new StringBuilder();
for (String error : errors) {
sb.append(error).append("; ");
}
throw new IllegalArgumentException(sm.getString("remoteIpFilter.invalidNetMask", sb.toString()));
}
}
}

/**
* <p>
* Header that holds the incoming host, usually named <code>X-Forwarded-Host</code>.
Expand Down
65 changes: 65 additions & 0 deletions test/org/apache/catalina/filters/TestRemoteIpFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.io.PrintWriter;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -37,6 +38,7 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

import org.apache.catalina.util.NetMaskSet;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -887,4 +889,67 @@ private void doTestPattern(Pattern pattern, String input, boolean expectedMatch)
boolean match = pattern.matcher(input).matches();
Assert.assertEquals(input, Boolean.valueOf(expectedMatch), Boolean.valueOf(match));
}

@Test
public void testInvokeAllowedRemoteAddrWithNullRemoteIpHeaderCidr() throws Exception {
// PREPARE
FilterDef filterDef = new FilterDef();
filterDef.addInitParameter("internalProxies", "");
filterDef.addInitParameter("trustedProxies", "");
filterDef.addInitParameter("internalProxiesCidr", "192.168.0.0/24");
filterDef.addInitParameter("trustedProxiesCidr", "203.0.113.0/24");
filterDef.addInitParameter("remoteIpHeader", "x-forwarded-for");
filterDef.addInitParameter("proxiesHeader", "x-forwarded-by");

MockHttpServletRequest request = new MockHttpServletRequest();
request.setRemoteAddr("192.168.0.10");
request.setRemoteHost("remote-host-original-value");

// TEST
HttpServletRequest actualRequest = testRemoteIpFilter(filterDef, request).getRequest();

// VERIFY
String actualXForwardedFor = request.getHeader("x-forwarded-for");
Assert.assertNull("x-forwarded-for must be null", actualXForwardedFor);

String actualXForwardedBy = request.getHeader("x-forwarded-by");
Assert.assertNull("x-forwarded-by must be null", actualXForwardedBy);

String actualRemoteAddr = actualRequest.getRemoteAddr();
Assert.assertEquals("remoteAddr", "192.168.0.10", actualRemoteAddr);

String actualRemoteHost = actualRequest.getRemoteHost();
Assert.assertEquals("remoteHost", "remote-host-original-value", actualRemoteHost);
}

@Test
public void testInternalProxiesCidr() throws Exception {
RemoteIpFilter remoteIpFilter = new RemoteIpFilter();
remoteIpFilter.setInternalProxiesCidr("192.168.0.0/24,223.168.0.0/24");
NetMaskSet internalProxiesCidr = remoteIpFilter.getInternalProxiesCidr();

doTestNetMaskSet(internalProxiesCidr, "192.168.0.0", true);
doTestNetMaskSet(internalProxiesCidr, "192.168.0.1", true);
doTestNetMaskSet(internalProxiesCidr, "193.168.1.0", false);
doTestNetMaskSet(internalProxiesCidr, "223.168.0.0", true);
doTestNetMaskSet(internalProxiesCidr, "223.168.0.123", true);
doTestNetMaskSet(internalProxiesCidr, "223.168.1.123", false);
}

@Test
public void testInternalProxiesCidrWithNull() throws Exception {
RemoteIpFilter remoteIpFilter = new RemoteIpFilter();
try {
// Multiple invalid CIDR formats
remoteIpFilter.setInternalProxiesCidr("192.168.0.0/33,2001:db8::/129");
Assert.fail("Expected IllegalArgumentException was not thrown");
} catch (IllegalArgumentException e) {
// Test passes - exception was thrown as expected
Assert.assertNotNull("Exception message should not be null", e.getMessage());
} }

private void doTestNetMaskSet(NetMaskSet netMaskSet, String remoteIp, boolean expectedMatch) throws UnknownHostException {
boolean match = netMaskSet.contains(remoteIp);
Assert.assertEquals(remoteIp, Boolean.valueOf(expectedMatch), Boolean.valueOf(match));
}
}