Author: markt Date: Thu Dec 6 14:30:51 2018 New Revision: 1848320 URL: http://svn.apache.org/viewvc?rev=1848320&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62978 Update the RemoteIpValve to handle multiple values in the x-forwarded-proto header. Patch provided by Tom Groot.
Modified: tomcat/trunk/java/org/apache/catalina/valves/RemoteIpValve.java tomcat/trunk/test/org/apache/catalina/valves/TestRemoteIpValve.java tomcat/trunk/webapps/docs/changelog.xml 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=1848320&r1=1848319&r2=1848320&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/valves/RemoteIpValve.java (original) +++ tomcat/trunk/java/org/apache/catalina/valves/RemoteIpValve.java Thu Dec 6 14:30:51 2018 @@ -60,7 +60,7 @@ import org.apache.tomcat.util.http.MimeH * <li>otherwise, the ip/host is declared to be the remote ip and looping is stopped.</li> * </ul> * </li> - * <li>If the request http header named <code>$protocolHeader</code> (e.g. <code>x-forwarded-for</code>) equals to the value of + * <li>If the request http header named <code>$protocolHeader</code> (e.g. <code>x-forwarded-proto</code>) consists only of forwards that match * <code>protocolHeaderHttpsValue</code> configuration parameter (default <code>https</code>) then <code>request.isSecure = true</code>, * <code>request.scheme = https</code> and <code>request.serverPort = 443</code>. Note that 443 can be overwritten with the * <code>$httpsServerPort</code> configuration parameter.</li> @@ -642,7 +642,7 @@ public class RemoteIpValve extends Valve if (protocolHeaderValue == null) { // don't modify the secure,scheme and serverPort attributes // of the request - } else if (protocolHeaderHttpsValue.equalsIgnoreCase(protocolHeaderValue)) { + } else if (isForwardedProtoHeaderValueSecure(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"); @@ -709,6 +709,26 @@ public class RemoteIpValve extends Valve } } + /** + * Considers the value to be secure if it exclusively holds forwards for + * {@link #protocolHeaderHttpsValue}. + */ + private boolean isForwardedProtoHeaderValueSecure(String protocolHeaderValue) { + if (!protocolHeaderValue.contains(",")) { + return protocolHeaderHttpsValue.equalsIgnoreCase(protocolHeaderValue); + } + String[] forwardedProtocols = commaDelimitedListToStringArray(protocolHeaderValue); + if (forwardedProtocols.length == 0) { + return false; + } + for (int i = 0; i < forwardedProtocols.length; i++) { + if (!protocolHeaderHttpsValue.equalsIgnoreCase(forwardedProtocols[i])) { + return false; + } + } + return true; + } + private void setPorts(Request request, int defaultPort) { int port = defaultPort; if (portHeader != null) { 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=1848320&r1=1848319&r2=1848320&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/valves/TestRemoteIpValve.java (original) +++ tomcat/trunk/test/org/apache/catalina/valves/TestRemoteIpValve.java Thu Dec 6 14:30:51 2018 @@ -684,6 +684,107 @@ public class TestRemoteIpValve { } @Test + public void testInvokeXforwardedProtoSaysMultipleHttpsForwardsForIncomingHttpsRequest() throws Exception { + performXForwardedProtoWithMultipleForwardsTest("https,https", true, true); + } + + @Test + public void testInvokeXforwardedProtoSaysMultipleForwardsWithFirstBeingHttpForIncomingHttpsRequest() throws Exception { + performXForwardedProtoWithMultipleForwardsTest("http,https", true, false); + } + + @Test + public void testInvokeXforwardedProtoSaysMultipleForwardsWithLastBeingHttpForIncomingHttpsRequest() throws Exception { + performXForwardedProtoWithMultipleForwardsTest("https,http", false, false); + } + + @Test + public void testInvokeXforwardedProtoSaysMultipleForwardsWithMiddleBeingHttpForIncomingHttpsRequest() throws Exception { + performXForwardedProtoWithMultipleForwardsTest("https,http,https", true, false); + } + + @Test + public void testInvokeXforwardedProtoSaysMultipleHttpForwardsForIncomingHttpsRequest() throws Exception { + performXForwardedProtoWithMultipleForwardsTest("http,http", false, false); + } + + @Test + public void testInvokeXforwardedProtoSaysInvalidValueForIncomingHttpsRequest() throws Exception { + performXForwardedProtoWithMultipleForwardsTest(",", false, false); + } + + private void performXForwardedProtoWithMultipleForwardsTest(String incomingHeaderValue, + boolean arrivesAsSecure, boolean shouldBeSecure) throws Exception { + + // PREPARE + String incomingScheme = arrivesAsSecure ? "https" : "http"; + String expectedScheme = shouldBeSecure ? "https" : "http"; + int incommingServerPort = arrivesAsSecure ? 8443 : 8080; + int expectedServerPort = shouldBeSecure ? 443 : 80; + RemoteIpValve remoteIpValve = new RemoteIpValve(); + remoteIpValve.setRemoteIpHeader("x-forwarded-for"); + remoteIpValve.setProtocolHeader("x-forwarded-proto"); + RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new RemoteAddrAndHostTrackerValve(); + remoteIpValve.setNext(remoteAddrAndHostTrackerValve); + + Request request = new MockRequest(); + 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(incomingHeaderValue); + request.setSecure(arrivesAsSecure); + request.setServerPort(incommingServerPort); + request.getCoyoteRequest().scheme().setString(incomingScheme); + + // TEST + remoteIpValve.invoke(request, null); + + // VERIFY + // client ip + String actualXForwardedFor = remoteAddrAndHostTrackerValve.getForwardedFor(); + Assert.assertNull("no intermediate non-trusted proxy, x-forwarded-for must be null", actualXForwardedFor); + + String actualXForwardedBy = remoteAddrAndHostTrackerValve.getForwardedBy(); + Assert.assertNull("no intermediate trusted proxy", 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", "192.168.0.10", actualPostInvokeRemoteAddr); + + String actualPostInvokeRemoteHost = request.getRemoteHost(); + Assert.assertEquals("postInvoke remoteAddr", "192.168.0.10", actualPostInvokeRemoteHost); + + // protocol + String actualScheme = remoteAddrAndHostTrackerValve.getScheme(); + Assert.assertEquals("x-forwarded-proto says " + expectedScheme, expectedScheme, actualScheme); + + int actualServerPort = remoteAddrAndHostTrackerValve.getServerPort(); + Assert.assertEquals("x-forwarded-proto says " + expectedScheme, expectedServerPort, actualServerPort); + + boolean actualSecure = remoteAddrAndHostTrackerValve.isSecure(); + Assert.assertEquals("x-forwarded-proto says " + expectedScheme, + Boolean.valueOf(shouldBeSecure), Boolean.valueOf(actualSecure)); + + boolean actualPostInvokeSecure = request.isSecure(); + Assert.assertEquals("postInvoke secure", + Boolean.valueOf(arrivesAsSecure), Boolean.valueOf(actualPostInvokeSecure)); + + int actualPostInvokeServerPort = request.getServerPort(); + Assert.assertEquals("postInvoke serverPort", incommingServerPort, actualPostInvokeServerPort); + + String actualPostInvokeScheme = request.getScheme(); + Assert.assertEquals("postInvoke scheme", incomingScheme, actualPostInvokeScheme); + } + + @Test public void testInvokeXforwardedProtoIsNullForIncomingHttpsRequest() throws Exception { // PREPARE Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1848320&r1=1848319&r2=1848320&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu Dec 6 14:30:51 2018 @@ -114,6 +114,11 @@ <update> Update the recommended minimum Tomcat Native version to 1.2.19. (markt) </update> + <fix> + <bug>62978</bug>: Update the RemoteIpValve to handle multiple values in + the <code>x-forwarded-proto</code> header. Patch provided by Tom Groot. + (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