From cf645086a6d7ea2b816b5aa6911e469dcabba0f6 Mon Sep 17 00:00:00 2001 From: markt Date: Tue, 2 Feb 2010 13:28:55 +0000 Subject: [PATCH] Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48653 RemoteIpValve : request.secure and request.scheme are not forced to "false" and "http" if X-Forwarded-Proto=http Patch provided by Cyrille Le Clerc git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@905627 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/valves/RemoteIpValve.java | 48 +++- .../apache/catalina/valves/mbeans-descriptors.xml | 14 +- .../apache/catalina/valves/TestRemoteIpValve.java | 274 +++++++++++++++++++++ webapps/docs/config/valve.xml | 21 +- 4 files changed, 350 insertions(+), 7 deletions(-) diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java index 226e3f802..ac30d373f 100644 --- a/java/org/apache/catalina/valves/RemoteIpValve.java +++ b/java/org/apache/catalina/valves/RemoteIpValve.java @@ -26,6 +26,7 @@ import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; import org.apache.tomcat.util.res.StringManager; import org.apache.catalina.connector.Request; @@ -126,6 +127,19 @@ import org.apache.juli.logging.LogFactory; * https * * + * httpServerPort + * Value returned by {@link ServletRequest#getServerPort()} when the protocolHeader indicates http protocol + * N/A + * integer + * 80 + * + * + * httpsServerPort + * Value returned by {@link ServletRequest#getServerPort()} when the protocolHeader indicates https protocol + * N/A + * integer + * 443 + * * *

*

@@ -415,6 +429,11 @@ public class RemoteIpValve extends ValveBase { } /** + * @see #setHttpServerPort(int) + */ + private int httpServerPort = 80; + + /** * @see #setHttpsServerPort(int) */ private int httpsServerPort = 443; @@ -456,6 +475,10 @@ public class RemoteIpValve extends ValveBase { return httpsServerPort; } + public int getHttpServerPort() { + return httpServerPort; + } + /** * Return descriptive information about this Valve implementation. */ @@ -507,7 +530,7 @@ public class RemoteIpValve extends ValveBase { public String getRemoteIpHeader() { return remoteIpHeader; } - + /** * @see #setTrustedProxies(String) * @return comma delimited list of trusted proxies @@ -580,12 +603,21 @@ public class RemoteIpValve extends ValveBase { if (protocolHeader != null) { String protocolHeaderValue = request.getHeader(protocolHeader); - if (protocolHeaderValue != null && protocolHeaderHttpsValue.equalsIgnoreCase(protocolHeaderValue)) { + if (protocolHeaderValue == null) { + // don't modify the secure,scheme and serverPort attributes + // of the request + } else if (protocolHeaderHttpsValue.equalsIgnoreCase(protocolHeaderValue)) { request.setSecure(true); // use request.coyoteRequest.scheme instead of request.setScheme() because request.setScheme() is no-op in Tomcat 6.0 request.getCoyoteRequest().scheme().setString("https"); request.setServerPort(httpsServerPort); + } else { + request.setSecure(false); + // use request.coyoteRequest.scheme instead of request.setScheme() because request.setScheme() is no-op in Tomcat 6.0 + request.getCoyoteRequest().scheme().setString("http"); + + request.setServerPort(httpServerPort); } } @@ -613,6 +645,18 @@ public class RemoteIpValve extends ValveBase { /** *

+ * Server Port value if the {@link #protocolHeader} is not null and does not indicate HTTP + *

+ *

+ * Default value : 80 + *

+ */ + public void setHttpServerPort(int httpServerPort) { + this.httpServerPort = httpServerPort; + } + + /** + *

* Server Port value if the {@link #protocolHeader} indicates HTTPS *

*

diff --git a/java/org/apache/catalina/valves/mbeans-descriptors.xml b/java/org/apache/catalina/valves/mbeans-descriptors.xml index 76ecf30f8..d6adb0f41 100644 --- a/java/org/apache/catalina/valves/mbeans-descriptors.xml +++ b/java/org/apache/catalina/valves/mbeans-descriptors.xml @@ -365,7 +365,17 @@ description="Comma delimited list of internal proxies" type="java.lang.String" writeable="false" /> - + + + + + - diff --git a/test/org/apache/catalina/valves/TestRemoteIpValve.java b/test/org/apache/catalina/valves/TestRemoteIpValve.java index d3a4d9ab6..324294187 100644 --- a/test/org/apache/catalina/valves/TestRemoteIpValve.java +++ b/test/org/apache/catalina/valves/TestRemoteIpValve.java @@ -38,6 +38,9 @@ public class TestRemoteIpValve extends TestCase { static class RemoteAddrAndHostTrackerValve extends ValveBase { private String remoteAddr; private String remoteHost; + private String scheme; + private boolean secure; + private int serverPort; public String getRemoteAddr() { return remoteAddr; @@ -47,10 +50,25 @@ public class TestRemoteIpValve extends TestCase { return remoteHost; } + public String getScheme() { + return scheme; + } + + public int getServerPort() { + return serverPort; + } + + public boolean isSecure() { + return secure; + } + @Override public void invoke(Request request, Response response) throws IOException, ServletException { this.remoteHost = request.getRemoteHost(); this.remoteAddr = request.getRemoteAddr(); + this.scheme = request.getScheme(); + this.secure = request.isSecure(); + this.serverPort = request.getServerPort(); } } @@ -271,6 +289,262 @@ public class TestRemoteIpValve extends TestCase { assertEquals("postInvoke remoteAddr", "remote-host-original-value", actualPostInvokeRemoteHost); } + public void testInvokeXforwardedProtoSaysHttpsForIncomingHttpRequest() throws Exception { + + // PREPARE + RemoteIpValve remoteIpValve = new RemoteIpValve(); + remoteIpValve.setRemoteIpHeader("x-forwarded-for"); + remoteIpValve.setProtocolHeader("x-forwarded-proto"); + RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new RemoteAddrAndHostTrackerValve(); + remoteIpValve.setNext(remoteAddrAndHostTrackerValve); + + Request request = new Request(); + request.setCoyoteRequest(new org.apache.coyote.Request()); + // client ip + request.setRemoteAddr("192.168.0.10"); + request.setRemoteHost("192.168.0.10"); + request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130"); + // protocol + request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-proto").setString("https"); + request.setSecure(false); + request.setServerPort(8080); + request.getCoyoteRequest().scheme().setString("http"); + + // TEST + remoteIpValve.invoke(request, null); + + // VERIFY + // client ip + String actualXForwardedFor = request.getHeader("x-forwarded-for"); + assertNull("no intermediate non-trusted proxy, x-forwarded-for must be null", actualXForwardedFor); + + String actualXForwardedBy = request.getHeader("x-forwarded-by"); + assertNull("no intermediate trusted proxy", actualXForwardedBy); + + String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr(); + assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr); + + String actualRemoteHost = remoteAddrAndHostTrackerValve.getRemoteHost(); + assertEquals("remoteHost", "140.211.11.130", actualRemoteHost); + + String actualPostInvokeRemoteAddr = request.getRemoteAddr(); + assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteAddr); + + String actualPostInvokeRemoteHost = request.getRemoteHost(); + assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteHost); + + // protocol + String actualScheme = remoteAddrAndHostTrackerValve.getScheme(); + assertEquals("x-forwarded-proto says https", "https", actualScheme); + + int actualServerPort = remoteAddrAndHostTrackerValve.getServerPort(); + assertEquals("x-forwarded-proto says https", 443, actualServerPort); + + boolean actualSecure = remoteAddrAndHostTrackerValve.isSecure(); + assertEquals("x-forwarded-proto says https", true, actualSecure); + + boolean actualPostInvokeSecure = request.isSecure(); + assertEquals("postInvoke secure", false, actualPostInvokeSecure); + + int actualPostInvokeServerPort = request.getServerPort(); + assertEquals("postInvoke serverPort", 8080, actualPostInvokeServerPort); + + String actualPostInvokeScheme = request.getScheme(); + assertEquals("postInvoke scheme", "http", actualPostInvokeScheme); + } + + public void testInvokeXforwardedProtoIsNullForIncomingHttpRequest() throws Exception { + + // PREPARE + RemoteIpValve remoteIpValve = new RemoteIpValve(); + remoteIpValve.setRemoteIpHeader("x-forwarded-for"); + remoteIpValve.setProtocolHeader("x-forwarded-proto"); + RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new RemoteAddrAndHostTrackerValve(); + remoteIpValve.setNext(remoteAddrAndHostTrackerValve); + + Request request = new Request(); + request.setCoyoteRequest(new org.apache.coyote.Request()); + // client ip + request.setRemoteAddr("192.168.0.10"); + request.setRemoteHost("192.168.0.10"); + request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130"); + // protocol + // null "x-forwarded-proto" + request.setSecure(false); + request.setServerPort(8080); + request.getCoyoteRequest().scheme().setString("http"); + + // TEST + remoteIpValve.invoke(request, null); + + // VERIFY + // client ip + String actualXForwardedFor = request.getHeader("x-forwarded-for"); + assertNull("no intermediate non-trusted proxy, x-forwarded-for must be null", actualXForwardedFor); + + String actualXForwardedBy = request.getHeader("x-forwarded-by"); + assertNull("no intermediate trusted proxy", actualXForwardedBy); + + String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr(); + assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr); + + String actualRemoteHost = remoteAddrAndHostTrackerValve.getRemoteHost(); + assertEquals("remoteHost", "140.211.11.130", actualRemoteHost); + + String actualPostInvokeRemoteAddr = request.getRemoteAddr(); + assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteAddr); + + String actualPostInvokeRemoteHost = request.getRemoteHost(); + assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteHost); + + // protocol + String actualScheme = remoteAddrAndHostTrackerValve.getScheme(); + assertEquals("x-forwarded-proto is null", "http", actualScheme); + + int actualServerPort = remoteAddrAndHostTrackerValve.getServerPort(); + assertEquals("x-forwarded-proto is null", 8080, actualServerPort); + + boolean actualSecure = remoteAddrAndHostTrackerValve.isSecure(); + assertEquals("x-forwarded-proto is null", false, actualSecure); + + boolean actualPostInvokeSecure = request.isSecure(); + assertEquals("postInvoke secure", false, actualPostInvokeSecure); + + int actualPostInvokeServerPort = request.getServerPort(); + assertEquals("postInvoke serverPort", 8080, actualPostInvokeServerPort); + + String actualPostInvokeScheme = request.getScheme(); + assertEquals("postInvoke scheme", "http", actualPostInvokeScheme); + } + + public void testInvokeXforwardedProtoSaysHttpForIncomingHttpsRequest() throws Exception { + + // PREPARE + RemoteIpValve remoteIpValve = new RemoteIpValve(); + remoteIpValve.setRemoteIpHeader("x-forwarded-for"); + remoteIpValve.setProtocolHeader("x-forwarded-proto"); + RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new RemoteAddrAndHostTrackerValve(); + remoteIpValve.setNext(remoteAddrAndHostTrackerValve); + + Request request = new Request(); + request.setCoyoteRequest(new org.apache.coyote.Request()); + // client ip + request.setRemoteAddr("192.168.0.10"); + request.setRemoteHost("192.168.0.10"); + request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130"); + // protocol + request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-proto").setString("http"); + request.setSecure(true); + request.setServerPort(8443); + request.getCoyoteRequest().scheme().setString("https"); + + // TEST + remoteIpValve.invoke(request, null); + + // VERIFY + // client ip + String actualXForwardedFor = request.getHeader("x-forwarded-for"); + assertNull("no intermediate non-trusted proxy, x-forwarded-for must be null", actualXForwardedFor); + + String actualXForwardedBy = request.getHeader("x-forwarded-by"); + assertNull("no intermediate trusted proxy", actualXForwardedBy); + + String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr(); + assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr); + + String actualRemoteHost = remoteAddrAndHostTrackerValve.getRemoteHost(); + assertEquals("remoteHost", "140.211.11.130", actualRemoteHost); + + String actualPostInvokeRemoteAddr = request.getRemoteAddr(); + assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteAddr); + + String actualPostInvokeRemoteHost = request.getRemoteHost(); + assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteHost); + + // protocol + String actualScheme = remoteAddrAndHostTrackerValve.getScheme(); + assertEquals("x-forwarded-proto says http", "http", actualScheme); + + int actualServerPort = remoteAddrAndHostTrackerValve.getServerPort(); + assertEquals("x-forwarded-proto says http", 80, actualServerPort); + + boolean actualSecure = remoteAddrAndHostTrackerValve.isSecure(); + assertEquals("x-forwarded-proto says http", false, actualSecure); + + boolean actualPostInvokeSecure = request.isSecure(); + assertEquals("postInvoke secure", true, actualPostInvokeSecure); + + int actualPostInvokeServerPort = request.getServerPort(); + assertEquals("postInvoke serverPort", 8443, actualPostInvokeServerPort); + + String actualPostInvokeScheme = request.getScheme(); + assertEquals("postInvoke scheme", "https", actualPostInvokeScheme); + } + + public void testInvokeXforwardedProtoIsNullForIncomingHttpsRequest() throws Exception { + + // PREPARE + RemoteIpValve remoteIpValve = new RemoteIpValve(); + remoteIpValve.setRemoteIpHeader("x-forwarded-for"); + remoteIpValve.setProtocolHeader("x-forwarded-proto"); + RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new RemoteAddrAndHostTrackerValve(); + remoteIpValve.setNext(remoteAddrAndHostTrackerValve); + + Request request = new Request(); + request.setCoyoteRequest(new org.apache.coyote.Request()); + // client ip + request.setRemoteAddr("192.168.0.10"); + request.setRemoteHost("192.168.0.10"); + request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130"); + // protocol + // Don't declare "x-forwarded-proto" + request.setSecure(true); + request.setServerPort(8443); + request.getCoyoteRequest().scheme().setString("https"); + + // TEST + remoteIpValve.invoke(request, null); + + // VERIFY + // client ip + String actualXForwardedFor = request.getHeader("x-forwarded-for"); + assertNull("no intermediate non-trusted proxy, x-forwarded-for must be null", actualXForwardedFor); + + String actualXForwardedBy = request.getHeader("x-forwarded-by"); + assertNull("no intermediate trusted proxy", actualXForwardedBy); + + String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr(); + assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr); + + String actualRemoteHost = remoteAddrAndHostTrackerValve.getRemoteHost(); + assertEquals("remoteHost", "140.211.11.130", actualRemoteHost); + + String actualPostInvokeRemoteAddr = request.getRemoteAddr(); + assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteAddr); + + String actualPostInvokeRemoteHost = request.getRemoteHost(); + assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteHost); + + // protocol + String actualScheme = remoteAddrAndHostTrackerValve.getScheme(); + assertEquals("x-forwarded-proto is null", "https", actualScheme); + + int actualServerPort = remoteAddrAndHostTrackerValve.getServerPort(); + assertEquals("x-forwarded-proto is null", 8443, actualServerPort); + + boolean actualSecure = remoteAddrAndHostTrackerValve.isSecure(); + assertEquals("x-forwarded-proto is null", true, actualSecure); + + boolean actualPostInvokeSecure = request.isSecure(); + assertEquals("postInvoke secure", true, actualPostInvokeSecure); + + int actualPostInvokeServerPort = request.getServerPort(); + assertEquals("postInvoke serverPort", 8443, actualPostInvokeServerPort); + + String actualPostInvokeScheme = request.getScheme(); + assertEquals("postInvoke scheme", "https", actualPostInvokeScheme); + } + public void testInvokeNotAllowedRemoteAddr() throws Exception { // PREPARE RemoteIpValve remoteIpValve = new RemoteIpValve(); diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml index 875bcdd8d..4f580fc44 100644 --- a/webapps/docs/config/valve.xml +++ b/webapps/docs/config/valve.xml @@ -622,9 +622,10 @@ via a request headers (e.g. "X-Forwarded-For").

Another feature of this valve is to replace the apparent scheme - (http/https) and server port with the scheme presented by a proxy or a load - balancer via a request header (e.g. "X-Forwarded-Proto").

- + (http/https), server port and request.secure with the scheme presented + by a proxy or a load balancer via a request header + (e.g. "X-Forwarded-Proto").

+

This Valve may be used at the Engine, Host or Context level as required. Normally, this Valve would be used at the Engine level.

@@ -690,6 +691,20 @@ used.

+ +

Value returned by ServletRequest.getServerPort() + when the protocolHeader indicates http + protocol. If not specified, the default of 80 is + used.

+ + + +

Value returned by ServletRequest.getServerPort() + when the protocolHeader indicates https + protocol. If not specified, the default of 443 is + used.

+ + -- 2.11.0