diff --git a/java/org/apache/catalina/filters/LocalStrings.properties b/java/org/apache/catalina/filters/LocalStrings.properties index 63a5c1485daf..2b405e397a7d 100644 --- a/java/org/apache/catalina/filters/LocalStrings.properties +++ b/java/org/apache/catalina/filters/LocalStrings.properties @@ -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 diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java index 6e116047a3b2..e21b2d2bd42d 100644 --- a/java/org/apache/catalina/filters/RemoteIpFilter.java +++ b/java/org/apache/catalina/filters/RemoteIpFilter.java @@ -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; @@ -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); @@ -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) */ @@ -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 proxiesHeaderValue = new ArrayDeque<>(); StringBuilder concatRemoteIpHeaderValue = new StringBuilder(); @@ -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 @@ -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}. */ @@ -951,6 +1006,14 @@ public Pattern getInternalProxies() { return internalProxies; } + public NetMaskSet getInternalProxiesCidr() { + return internalProxiesCidr; + } + + public NetMaskSet getTrustedProxiesCidr() { + return trustedProxiesCidr; + } + public String getProtocolHeader() { return protocolHeader; } @@ -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)); } @@ -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))); @@ -1133,6 +1204,58 @@ public void setInternalProxies(String internalProxies) { } } + /** + *

+ * CIDR notation that defines the internal proxies. + *

+ *

+ * If this is set, it will be used instead of the regex pattern for internal proxies. + *

+ * + * @param internalProxiesCidr The CIDR notation + */ + public void setInternalProxiesCidr(String internalProxiesCidr) { + if (internalProxiesCidr == null || internalProxiesCidr.isEmpty()) { + this.internalProxiesCidr = null; + } else { + this.internalProxiesCidr = new NetMaskSet(); + List 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())); + } + } + } + + /** + *

+ * CIDR notation defining proxies that are trusted when they appear in the {@link #remoteIpHeader} header. + *

+ *

+ * If this is set, it will be used instead of the regex pattern for trusted proxies. + *

+ * + * @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 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())); + } + } + } + /** *

* Header that holds the incoming host, usually named X-Forwarded-Host. diff --git a/test/org/apache/catalina/filters/TestRemoteIpFilter.java b/test/org/apache/catalina/filters/TestRemoteIpFilter.java index f69e255ee19c..7e3479156ef8 100644 --- a/test/org/apache/catalina/filters/TestRemoteIpFilter.java +++ b/test/org/apache/catalina/filters/TestRemoteIpFilter.java @@ -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; @@ -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; @@ -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)); + } }