diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java index 13a0eb7bf9b7..8781a470d298 100644 --- a/java/org/apache/catalina/filters/RemoteIpFilter.java +++ b/java/org/apache/catalina/filters/RemoteIpFilter.java @@ -21,6 +21,7 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayDeque; +import java.util.Arrays; import java.util.Collections; import java.util.Deque; import java.util.Enumeration; @@ -44,6 +45,7 @@ import org.apache.catalina.AccessLog; import org.apache.catalina.Globals; import org.apache.catalina.connector.RequestFacade; +import org.apache.catalina.util.NetMask; import org.apache.catalina.util.RequestUtil; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -667,6 +669,8 @@ public PushBuilder newPushBuilder() { protected static final String INTERNAL_PROXIES_PARAMETER = "internalProxies"; + protected static final String INTERNAL_PROXIES_MASKS_PARAMETER = "internalProxiesMasks"; + // 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); @@ -690,6 +694,8 @@ public PushBuilder newPushBuilder() { protected static final String TRUSTED_PROXIES_PARAMETER = "trustedProxies"; + protected static final String TRUSTED_PROXIES_MASKS_PARAMETER = "trustedProxiesMasks"; + protected static final String ENABLE_LOOKUPS_PARAMETER = "enableLookups"; /** @@ -725,6 +731,11 @@ protected static String[] commaDelimitedListToStringArray(String commaDelimitedS "172\\.1[6-9]{1}\\.\\d{1,3}\\.\\d{1,3}|" + "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"); + /** + * @see #setInternalProxiesMasks(String) + */ + private List internalProxiesMasks = Arrays.asList(new NetMask("10.0.0.0/8"), new NetMask("192.168.0.0/16"), new NetMask("169.254.0.0/16"), new NetMask("127.0.0.0/8"), new NetMask("100.64.0.0/10"), new NetMask("172.16.0.0/12"), new NetMask("::1/128")); + /** * @see #setProtocolHeader(String) */ @@ -760,14 +771,19 @@ protected static String[] commaDelimitedListToStringArray(String commaDelimitedS */ private Pattern trustedProxies = null; + /** + * @see #setTrustedProxiesMasks(String) + */ + private List trustedProxiesMasks = null; + private boolean enableLookups; public void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { - boolean isInternal = internalProxies != null && internalProxies.matcher(request.getRemoteAddr()).matches(); + boolean isInternal = matchesInternalProxies(request.getRemoteAddr()); - if (isInternal || (trustedProxies != null && trustedProxies.matcher(request.getRemoteAddr()).matches())) { + if (isInternal || matchesTrustedProxies(request.getRemoteAddr())) { String remoteIp = null; Deque proxiesHeaderValue = new ArrayDeque<>(); StringBuilder concatRemoteIpHeaderValue = new StringBuilder(); @@ -789,9 +805,9 @@ 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 (matchesInternalProxies(currentRemoteIp)) { // do nothing, internalProxies IPs are not appended to the - } else if (trustedProxies != null && trustedProxies.matcher(currentRemoteIp).matches()) { + } else if (matchesTrustedProxies(currentRemoteIp)) { proxiesHeaderValue.addFirst(currentRemoteIp); } else { idx--; // decrement idx because break statement doesn't do it @@ -907,6 +923,31 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F } + private boolean matchesInternalProxies(String remoteAddr) { + return matchesPatternOrNetMasks(remoteAddr, internalProxies, internalProxiesMasks); + } + + private boolean matchesTrustedProxies(String remoteAddr) { + return matchesPatternOrNetMasks(remoteAddr, trustedProxies, trustedProxiesMasks); + } + + private boolean matchesPatternOrNetMasks(String remoteAddr, Pattern pattern, List masks) { + if (pattern != null) { + return pattern.matcher(remoteAddr).matches(); + } else if (masks != null && !masks.isEmpty()) { + try { + InetAddress address = InetAddress.getByName(remoteAddr); + return masks.stream() + .anyMatch(mask -> mask.matches(address)); + } catch (UnknownHostException e) { + // should never happen, as no lookups are performed with dotted-decimal address format + log.debug("unable to map remote address to InetAddress", e); + return false; + } + } + return false; + } + /* * Considers the value to be secure if it exclusively holds forwards for {@link #protocolHeaderHttpsValue}. */ @@ -975,6 +1016,10 @@ public Pattern getInternalProxies() { return internalProxies; } + public List getInternalProxiesMasks() { + return internalProxiesMasks; + } + public String getProtocolHeader() { return protocolHeader; } @@ -1018,6 +1063,11 @@ public void init() throws ServletException { setInternalProxies(getInitParameter(INTERNAL_PROXIES_PARAMETER)); } + if (getInitParameter(INTERNAL_PROXIES_MASKS_PARAMETER) != null) { + setInternalProxiesMasks(getInitParameter(INTERNAL_PROXIES_MASKS_PARAMETER)); + setInternalProxies(null); + } + if (getInitParameter(PROTOCOL_HEADER_PARAMETER) != null) { setProtocolHeader(getInitParameter(PROTOCOL_HEADER_PARAMETER)); } @@ -1054,6 +1104,11 @@ public void init() throws ServletException { setTrustedProxies(getInitParameter(TRUSTED_PROXIES_PARAMETER)); } + if (getInitParameter(TRUSTED_PROXIES_MASKS_PARAMETER) != null) { + setTrustedProxiesMasks(getInitParameter(TRUSTED_PROXIES_MASKS_PARAMETER)); + setTrustedProxies(null); + } + if (getInitParameter(HTTP_SERVER_PORT_PARAMETER) != null) { try { setHttpServerPort(Integer.parseInt(getInitParameter(HTTP_SERVER_PORT_PARAMETER))); @@ -1157,6 +1212,35 @@ public void setInternalProxies(String internalProxies) { } } + /** + *

+ * List of network masks defining the internal proxies. + * This value will be used if internalProxies is not defined + *

+ *

+ * Default value: + *

    + *
  • 10.0.0.0/8
    • + *
  • 192.168.0.0/16
    • + *
  • 169.254.0.0/16
    • + *
  • 127.0.0.0/8
    • + *
  • 100.64.0.0/10
    • + *
  • 172.16.0.0/12
    • + *
  • ::1/128
    • + *
+ *

+ * @param internalProxiesMasks The list of network masks + */ + public void setInternalProxiesMasks(String internalProxiesMasks) { + if (internalProxiesMasks == null || internalProxiesMasks.length() == 0) { + this.internalProxiesMasks = null; + } else { + this.internalProxiesMasks = Arrays.stream(internalProxiesMasks.split(",")) + .map(NetMask::new) + .toList(); + } + } + /** *

* Header that holds the incoming host, usually named X-Forwarded-Host. @@ -1292,6 +1376,23 @@ public void setTrustedProxies(String trustedProxies) { } } + /** + *

+ * List of network masks defining the trusted proxies. + * This value will be used if trustedProxies is not defined + *

+ * @param trustedProxiesMasks The list of network masks + */ + public void setTrustedProxiesMasks(String trustedProxiesMasks) { + if (trustedProxiesMasks == null || trustedProxiesMasks.length() == 0) { + this.trustedProxiesMasks = null; + } else { + this.trustedProxiesMasks = Arrays.stream(trustedProxiesMasks.split(",")) + .map(NetMask::new) + .toList(); + } + } + public void setEnableLookups(boolean enableLookups) { this.enableLookups = enableLookups; } diff --git a/test/org/apache/catalina/filters/TestRemoteIpFilter.java b/test/org/apache/catalina/filters/TestRemoteIpFilter.java index 82c8bb8f7948..ced733684a86 100644 --- a/test/org/apache/catalina/filters/TestRemoteIpFilter.java +++ b/test/org/apache/catalina/filters/TestRemoteIpFilter.java @@ -18,8 +18,11 @@ import java.io.IOException; import java.io.PrintWriter; +import java.io.UncheckedIOException; import java.net.HttpURLConnection; +import java.net.InetAddress; import java.net.URI; +import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -37,6 +40,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.apache.catalina.util.NetMask; import org.junit.Assert; import org.junit.Test; @@ -492,6 +496,40 @@ public void testInvokeAllProxiesAreTrustedOrInternal() throws Exception { Assert.assertEquals("remoteHost", "140.211.11.130", actualRemoteHost); } + @Test + public void testInvokeAllProxiesAreTrustedOrInternal_withNetMasks() throws Exception { + + // PREPARE + FilterDef filterDef = new FilterDef(); + filterDef.addInitParameter("internalProxiesMasks", "192.168.0.0/24,192.168.5.0/24"); + filterDef.addInitParameter("trustedProxiesMasks", "192.0.2.0/24,198.51.100.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"); + request.setHeader("x-forwarded-for", "140.211.11.130, 192.0.2.50, 198.51.100.33, 192.168.0.100, 192.168.5.1"); + + // TEST + HttpServletRequest actualRequest = testRemoteIpFilter(filterDef, request).getRequest(); + + // VERIFY + String actualXForwardedFor = actualRequest.getHeader("x-forwarded-for"); + Assert.assertNull("all proxies are trusted, x-forwarded-for must be null", actualXForwardedFor); + + String actualXForwardedBy = actualRequest.getHeader("x-forwarded-by"); + Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "192.0.2.50,198.51.100.33", + actualXForwardedBy); + + String actualRemoteAddr = actualRequest.getRemoteAddr(); + Assert.assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr); + + String actualRemoteHost = actualRequest.getRemoteHost(); + Assert.assertEquals("remoteHost", "140.211.11.130", actualRemoteHost); + } + @Test public void testInvokeNotAllowedRemoteAddr() throws Exception { // PREPARE @@ -868,4 +906,38 @@ 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 testInternalProxiesMasks() { + RemoteIpFilter remoteIpFilter = new RemoteIpFilter(); + List masks = remoteIpFilter.getInternalProxiesMasks(); + + doTestMasks(masks, "8.8.8.8", false); + doTestMasks(masks, "100.62.0.0", false); + doTestMasks(masks, "100.63.255.255", false); + doTestMasks(masks, "100.64.0.0", true); + doTestMasks(masks, "100.65.0.0", true); + doTestMasks(masks, "100.68.0.0", true); + doTestMasks(masks, "100.72.0.0", true); + doTestMasks(masks, "100.88.0.0", true); + doTestMasks(masks, "100.95.0.0", true); + doTestMasks(masks, "100.102.0.0", true); + doTestMasks(masks, "100.110.0.0", true); + doTestMasks(masks, "100.126.0.0", true); + doTestMasks(masks, "100.127.255.255", true); + doTestMasks(masks, "100.128.0.0", false); + doTestMasks(masks, "100.130.0.0", false); + + } + + private void doTestMasks(List masks, String input, boolean expectedMatch) { + boolean match = masks.stream().anyMatch(mask -> { + try { + return mask.matches(InetAddress.getByName(input)); + } catch (UnknownHostException e) { + throw new UncheckedIOException(e); + } + }); + Assert.assertEquals(input, Boolean.valueOf(expectedMatch), Boolean.valueOf(match)); + } }