svn commit: r1848320 - in /tomcat/trunk: java/org/apache/catalina/valves/RemoteIpValve.java test/org/apache/catalina/valves/TestRemoteIpValve.java webapps/docs/changelog.xml

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1848320 - in /tomcat/trunk: java/org/apache/catalina/valves/RemoteIpValve.java test/org/apache/catalina/valves/TestRemoteIpValve.java webapps/docs/changelog.xml

markt
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: [hidden email]
For additional commands, e-mail: [hidden email]