Author: markt
Date: Mon Jun 4 18:57:59 2018
New Revision: 1832882
URL: http://svn.apache.org/viewvc?rev=1832882&view=rev
Log:
Correctly handle the case when the request passes through one or more
trustedProxies but no internalProxies.
Based on a patch by zhanhb
This closes #45
Modified:
tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java
tomcat/trunk/java/org/apache/catalina/valves/RemoteIpValve.java
tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java
tomcat/trunk/test/org/apache/catalina/valves/TestRemoteIpValve.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java?rev=1832882&r1=1832881&r2=1832882&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java Mon Jun
4 18:57:59 2018
@@ -67,7 +67,8 @@ import org.apache.juli.logging.LogFactor
* This servlet filter proceeds as follows:
* </p>
* <p>
- * If the incoming <code>request.getRemoteAddr()</code> matches the servlet
filter's list of internal proxies :
+ * If the incoming <code>request.getRemoteAddr()</code> matches the servlet
+ * filter's list of internal or trusted proxies:
* </p>
* <ul>
* <li>Loop on the comma delimited list of IPs and hostnames passed by the
preceding load balancer or proxy in the given request's Http
@@ -761,8 +762,11 @@ public class RemoteIpFilter extends Gene
public void doFilter(HttpServletRequest request, HttpServletResponse
response, FilterChain chain) throws IOException, ServletException {
- if (internalProxies != null &&
- internalProxies.matcher(request.getRemoteAddr()).matches()) {
+ boolean isInternal = internalProxies != null &&
+ internalProxies.matcher(request.getRemoteAddr()).matches();
+
+ if (isInternal || (trustedProxies != null &&
+ trustedProxies.matcher(request.getRemoteAddr()).matches())) {
String remoteIp = null;
// In java 6, proxiesHeaderValue should be declared as a
java.util.Deque
LinkedList<String> proxiesHeaderValue = new LinkedList<>();
@@ -778,11 +782,14 @@ public class RemoteIpFilter extends Gene
String[] remoteIpHeaderValue =
commaDelimitedListToStringArray(concatRemoteIpHeaderValue.toString());
int idx;
+ if (!isInternal) {
+ proxiesHeaderValue.addFirst(request.getRemoteAddr());
+ }
// loop on remoteIpHeaderValue to find the first trusted remote ip
and to build the proxies chain
for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
String currentRemoteIp = remoteIpHeaderValue[idx];
remoteIp = currentRemoteIp;
- if (internalProxies.matcher(currentRemoteIp).matches()) {
+ if (internalProxies !=null &&
internalProxies.matcher(currentRemoteIp).matches()) {
// do nothing, internalProxies IPs are not appended to the
} else if (trustedProxies != null &&
trustedProxies.matcher(currentRemoteIp).matches()) {
Modified: tomcat/trunk/java/org/apache/catalina/valves/RemoteIpValve.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/RemoteIpValve.java?rev=1832882&r1=1832881&r2=1832882&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/RemoteIpValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/RemoteIpValve.java Mon Jun 4
18:57:59 2018
@@ -47,7 +47,8 @@ import org.apache.tomcat.util.http.MimeH
* This valve proceeds as follows:
* </p>
* <p>
- * If the incoming <code>request.getRemoteAddr()</code> matches the valve's
list of internal proxies :
+ * If the incoming <code>request.getRemoteAddr()</code> matches the valve's
list
+ * of internal or trusted proxies:
* </p>
* <ul>
* <li>Loop on the comma delimited list of IPs and hostnames passed by the
preceding load balancer or proxy in the given request's Http
@@ -572,9 +573,11 @@ public class RemoteIpValve extends Valve
final int originalServerPort = request.getServerPort();
final String originalProxiesHeader = request.getHeader(proxiesHeader);
final String originalRemoteIpHeader =
request.getHeader(remoteIpHeader);
+ boolean isInternal = internalProxies != null &&
+ internalProxies.matcher(originalRemoteAddr).matches();
- if (internalProxies !=null &&
- internalProxies.matcher(originalRemoteAddr).matches()) {
+ if (isInternal || (trustedProxies != null &&
+ trustedProxies.matcher(originalRemoteAddr).matches())) {
String remoteIp = null;
// In java 6, proxiesHeaderValue should be declared as a
java.util.Deque
LinkedList<String> proxiesHeaderValue = new LinkedList<>();
@@ -590,11 +593,14 @@ public class RemoteIpValve extends Valve
String[] remoteIpHeaderValue =
commaDelimitedListToStringArray(concatRemoteIpHeaderValue.toString());
int idx;
+ if (!isInternal) {
+ proxiesHeaderValue.addFirst(originalRemoteAddr);
+ }
// loop on remoteIpHeaderValue to find the first trusted remote ip
and to build the proxies chain
for (idx = remoteIpHeaderValue.length - 1; idx >= 0; idx--) {
String currentRemoteIp = remoteIpHeaderValue[idx];
remoteIp = currentRemoteIp;
- if (internalProxies.matcher(currentRemoteIp).matches()) {
+ if (internalProxies !=null &&
internalProxies.matcher(currentRemoteIp).matches()) {
// do nothing, internalProxies IPs are not appended to the
} else if (trustedProxies != null &&
trustedProxies.matcher(currentRemoteIp).matches()) {
Modified: tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java?rev=1832882&r1=1832881&r2=1832882&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/filters/TestRemoteIpFilter.java Mon
Jun 4 18:57:59 2018
@@ -348,6 +348,75 @@ public class TestRemoteIpFilter extends
}
@Test
+ public void testInvokeAllProxiesAreTrustedEmptyInternal() throws Exception
{
+
+ // PREPARE
+ RemoteIpFilter remoteIpFilter = new RemoteIpFilter();
+ FilterDef filterDef = new FilterDef();
+ filterDef.addInitParameter("internalProxies", "");
+ filterDef.addInitParameter("trustedProxies", "proxy1|proxy2|proxy3");
+ filterDef.addInitParameter("remoteIpHeader", "x-forwarded-for");
+ filterDef.addInitParameter("proxiesHeader", "x-forwarded-by");
+
+ filterDef.setFilter(remoteIpFilter);
+ MockHttpServletRequest request = new MockHttpServletRequest();
+
+ request.setRemoteAddr("proxy3");
+ request.setRemoteHost("remote-host-original-value");
+ request.setHeader("x-forwarded-for", "140.211.11.130, proxy1, proxy2");
+
+ // 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", "proxy1, proxy2, proxy3", 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 testInvokeAllProxiesAreTrustedUnusedInternal() throws
Exception {
+
+ // PREPARE
+ RemoteIpFilter remoteIpFilter = new RemoteIpFilter();
+ FilterDef filterDef = new FilterDef();
+ filterDef.addInitParameter("trustedProxies", "proxy1|proxy2|proxy3");
+ filterDef.addInitParameter("remoteIpHeader", "x-forwarded-for");
+ filterDef.addInitParameter("proxiesHeader", "x-forwarded-by");
+
+ filterDef.setFilter(remoteIpFilter);
+ MockHttpServletRequest request = new MockHttpServletRequest();
+
+ request.setRemoteAddr("proxy3");
+ request.setRemoteHost("remote-host-original-value");
+ request.setHeader("x-forwarded-for", "140.211.11.130, proxy1, proxy2");
+
+ // 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", "proxy1, proxy2, proxy3", 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 testInvokeAllProxiesAreTrustedAndRemoteAddrMatchRegexp()
throws Exception {
// PREPARE
Modified: tomcat/trunk/test/org/apache/catalina/valves/TestRemoteIpValve.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/valves/TestRemoteIpValve.java?rev=1832882&r1=1832881&r2=1832882&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/valves/TestRemoteIpValve.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/valves/TestRemoteIpValve.java Mon Jun
4 18:57:59 2018
@@ -203,6 +203,87 @@ public class TestRemoteIpValve {
}
@Test
+ public void testInvokeAllProxiesAreTrustedEmptyInternal() throws Exception
{
+
+ // PREPARE
+ RemoteIpValve remoteIpValve = new RemoteIpValve();
+ remoteIpValve.setInternalProxies("");
+ remoteIpValve.setTrustedProxies("proxy1|proxy2|proxy3");
+ remoteIpValve.setRemoteIpHeader("x-forwarded-for");
+ remoteIpValve.setProxiesHeader("x-forwarded-by");
+ RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new
RemoteAddrAndHostTrackerValve();
+ remoteIpValve.setNext(remoteAddrAndHostTrackerValve);
+
+ Request request = new MockRequest();
+ request.setCoyoteRequest(new org.apache.coyote.Request());
+ request.setRemoteAddr("proxy3");
+ request.setRemoteHost("remote-host-original-value");
+
request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130,
proxy1, proxy2");
+
+ // TEST
+ remoteIpValve.invoke(request, null);
+
+ // VERIFY
+ String actualXForwardedFor =
remoteAddrAndHostTrackerValve.getForwardedFor();
+ Assert.assertNull("all proxies are trusted, x-forwarded-for must be
null", actualXForwardedFor);
+
+ String actualXForwardedBy =
remoteAddrAndHostTrackerValve.getForwardedBy();
+ Assert.assertEquals("all proxies are trusted, they must appear in
x-forwarded-by", "proxy1, proxy2, proxy3", actualXForwardedBy);
+
+ String actualRemoteAddr =
remoteAddrAndHostTrackerValve.getRemoteAddr();
+ Assert.assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr);
+
+ String actualRemoteHost =
remoteAddrAndHostTrackerValve.getRemoteHost();
+ Assert.assertEquals("remoteHost", "140.211.11.130", actualRemoteHost);
+
+ String actualPostInvokeRemoteAddr = request.getRemoteAddr();
+ Assert.assertEquals("postInvoke remoteAddr", "proxy3",
actualPostInvokeRemoteAddr);
+
+ String actualPostInvokeRemoteHost = request.getRemoteHost();
+ Assert.assertEquals("postInvoke remoteAddr",
"remote-host-original-value", actualPostInvokeRemoteHost);
+ }
+
+ @Test
+ public void testInvokeAllProxiesAreTrustedUnusedInternal() throws
Exception {
+
+ // PREPARE
+ RemoteIpValve remoteIpValve = new RemoteIpValve();
+ remoteIpValve.setTrustedProxies("proxy1|proxy2|proxy3");
+ remoteIpValve.setRemoteIpHeader("x-forwarded-for");
+ remoteIpValve.setProxiesHeader("x-forwarded-by");
+ RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new
RemoteAddrAndHostTrackerValve();
+ remoteIpValve.setNext(remoteAddrAndHostTrackerValve);
+
+ Request request = new MockRequest();
+ request.setCoyoteRequest(new org.apache.coyote.Request());
+ request.setRemoteAddr("proxy3");
+ request.setRemoteHost("remote-host-original-value");
+
request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130,
proxy1, proxy2");
+
+ // TEST
+ remoteIpValve.invoke(request, null);
+
+ // VERIFY
+ String actualXForwardedFor =
remoteAddrAndHostTrackerValve.getForwardedFor();
+ Assert.assertNull("all proxies are trusted, x-forwarded-for must be
null", actualXForwardedFor);
+
+ String actualXForwardedBy =
remoteAddrAndHostTrackerValve.getForwardedBy();
+ Assert.assertEquals("all proxies are trusted, they must appear in
x-forwarded-by", "proxy1, proxy2, proxy3", actualXForwardedBy);
+
+ String actualRemoteAddr =
remoteAddrAndHostTrackerValve.getRemoteAddr();
+ Assert.assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr);
+
+ String actualRemoteHost =
remoteAddrAndHostTrackerValve.getRemoteHost();
+ Assert.assertEquals("remoteHost", "140.211.11.130", actualRemoteHost);
+
+ String actualPostInvokeRemoteAddr = request.getRemoteAddr();
+ Assert.assertEquals("postInvoke remoteAddr", "proxy3",
actualPostInvokeRemoteAddr);
+
+ String actualPostInvokeRemoteHost = request.getRemoteHost();
+ Assert.assertEquals("postInvoke remoteAddr",
"remote-host-original-value", actualPostInvokeRemoteHost);
+ }
+
+ @Test
public void testInvokeAllProxiesAreTrustedOrInternal() throws Exception {
// PREPARE
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1832882&r1=1832881&r2=1832882&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Jun 4 18:57:59 2018
@@ -137,6 +137,12 @@
<code>internalProxies</code> regular expression. Patch by Craig
Andrews.
(markt)
</add>
+ <fix>
+ In the <code>RemoteIpValve</code> and <code>RemoteIpFilter</code>,
+ correctly handle the case when the request passes through one or more
+ <code>trustedProxies</code> but no <code>internalProxies</code>. Based
+ on a patch by zhanhb. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
