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

diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java
index 226e3f8..ac30d37 100644 (file)
--- 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;
  * <td><code>https</code></td>
  * </tr>
  * <tr>
+ * <td>httpServerPort</td>
+ * <td>Value returned by {@link ServletRequest#getServerPort()} when the <code>protocolHeader</code> indicates <code>http</code> protocol</td>
+ * <td>N/A</td>
+ * <td>integer</td>
+ * <td>80</td>
+ * </tr>
+ * <tr>
+ * <td>httpsServerPort</td>
+ * <td>Value returned by {@link ServletRequest#getServerPort()} when the <code>protocolHeader</code> indicates <code>https</code> protocol</td>
+ * <td>N/A</td>
+ * <td>integer</td>
+ * <td>443</td>
+ * </tr>
  * </table>
  * </p>
  * <p>
@@ -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 {
     
     /**
      * <p>
+     * Server Port value if the {@link #protocolHeader} is not <code>null</code> and does not indicate HTTP
+     * </p>
+     * <p>
+     * Default value : 80
+     * </p>
+     */
+    public void setHttpServerPort(int httpServerPort) {
+        this.httpServerPort = httpServerPort;
+    }
+    
+    /**
+     * <p>
      * Server Port value if the {@link #protocolHeader} indicates HTTPS
      * </p>
      * <p>

diff --git a/java/org/apache/catalina/valves/mbeans-descriptors.xml b/java/org/apache/catalina/valves/mbeans-descriptors.xml
index 76ecf30..d6adb0f 100644 (file)
--- 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" />
-               
+
+    <attribute name="httpServerPort"
+               description="Value returned by ServletRequest.getServerPort() when the protocolHeader indicates http protocol"
+               type="java.lang.String"
+               writeable="false" />
+    
+    <attribute name="httpsServerPort"
+               description="Value returned by ServletRequest.getServerPort() when the protocolHeader indicates https protocol"
+               type="java.lang.String"
+               writeable="false" />
+    
     <attribute name="protocolHeader"
                description="The protocol header (e.g. &quot;X-Forwarded-Proto&quot;)"
                type="java.lang.String"
@@ -381,7 +391,7 @@
                type="java.lang.String"
                writeable="false" />
                
-    <attribute name="remoteIpHedaer"
+    <attribute name="remoteIpHeader"
                description="The remote IP header name (e.g. &quot;X-Forwarded-For&quot;)"
                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 d3a4d9a..3242941 100644 (file)
--- 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 875bcdd..4f580fc 100644 (file)
--- a/webapps/docs/config/valve.xml
+++ b/webapps/docs/config/valve.xml
@@ -622,9 +622,10 @@
     via a request headers (e.g. &quot;X-Forwarded-For&quot;).</p>
 
     <p>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. &quot;X-Forwarded-Proto&quot;).</p>
- 
+    (http/https), server port and <code>request.secure</code> with the scheme presented 
+    by a proxy or a load balancer via a request header 
+    (e.g. &quot;X-Forwarded-Proto&quot;).</p>
+    
     <p>This Valve may be used at the <code>Engine</code>, <code>Host</code> or
     <code>Context</code> level as required. Normally, this Valve would be used
     at the <code>Engine</code> level.</p>
@@ -690,6 +691,20 @@
         used.</p>
       </attribute>
 
+      <attribute name="httpServerPort" required="false">
+         <p>Value returned by <code>ServletRequest.getServerPort()</code> 
+         when the <strong>protocolHeader</strong> indicates <code>http</code> 
+         protocol. If not specified, the default of <code>80</code> is
+        used.</p>
+      </attribute>
+
+      <attribute name="httpsServerPort" required="false">
+         <p>Value returned by <code>ServletRequest.getServerPort()</code> 
+         when the <strong>protocolHeader</strong> indicates <code>https</code> 
+         protocol. If not specified, the default of <code>443</code> is
+        used.</p>
+      </attribute>
+      
     </attributes>
 
   </subsection>