[FIX BUG 3854] Add support for the X-Forwarded-For HTTP header, which proxies

commonly filled in to pass along the IP address of the client. If this header
exists, use the first IP address as the client IP instead of calling
getRemoteAddr.

Added test cases to test various situations.



git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@19132 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java

@@ -29,7 +29,7 @@ import org.kohsuke.stapler.StaplerRequest;
  *
  */
 public class DefaultCrumbIssuer extends CrumbIssuer {
-
+    
     private MessageDigest md;
 
     DefaultCrumbIssuer() {
@@ -55,7 +55,7 @@ public class DefaultCrumbIssuer extends CrumbIssuer {
                     buffer.append(a.getName());
                 }
                 buffer.append(';');
-                buffer.append(req.getRemoteAddr());
+                buffer.append(getClientIP(req));
 
                 md.update(buffer.toString().getBytes());
                 byte[] crumbBytes = md.digest(salt.getBytes());
@@ -88,6 +88,20 @@ public class DefaultCrumbIssuer extends CrumbIssuer {
         return false;
     }
 
+    private final String PROXY_HEADER = "X-Forwarded-For";
+
+    private String getClientIP(HttpServletRequest req) {
+        String defaultAddress = req.getRemoteAddr();
+        String forwarded = req.getHeader(PROXY_HEADER);
+        if (forwarded != null) {
+	        String[] hopList = forwarded.split(",");
+            if (hopList.length >= 1) {
+                return hopList[0];
+            }
+        }
+        return defaultAddress;
+    }
+    
     @Extension
     public static final class DescriptorImpl extends CrumbIssuerDescriptor<DefaultCrumbIssuer> implements ModelObject {
 

test/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java

@@ -204,7 +204,7 @@ public abstract class HudsonTestCase extends TestCase {
         hudson.setNoUsageStatistics(true); // collecting usage stats from tests are pointless.
         
         hudson.setUseCrumbs(true);
-        hudson.setCrumbIssuer(((CrumbIssuerDescriptor<CrumbIssuer>)Hudson.getInstance().getDescriptor(TestCrumbIssuer.class)).newInstance(null,null));
+        hudson.setCrumbIssuer(((CrumbIssuerDescriptor<CrumbIssuer>)hudson.getDescriptor(TestCrumbIssuer.class)).newInstance(null,null));
 
         hudson.servletContext.setAttribute("app",hudson);
         hudson.servletContext.setAttribute("version","?");

test/src/test/java/hudson/security/csrf/DefaultCrumbIssuerTest.java

+/*
+ * To change this template, choose Tools | Templates
+ * and open the template in the editor.
+ */
+
+package hudson.security.csrf;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.HudsonTestCase;
+
+/**
+ *
+ * @author dty
+ */
+public class DefaultCrumbIssuerTest extends HudsonTestCase {
+    protected void setUp() throws Exception {
+        super.setUp();
+        assertNotNull(hudson);
+        CrumbIssuerDescriptor<CrumbIssuer> descriptor = (CrumbIssuerDescriptor<CrumbIssuer>)hudson.getDescriptor(DefaultCrumbIssuer.class);
+        assertNotNull(descriptor);
+        CrumbIssuer issuer = descriptor.newInstance(null,null);
+        assertNotNull(issuer);
+        hudson.setCrumbIssuer(issuer);
+    }
+
+    private static final String[] testData = {
+        "10.2.3.1",
+        "10.2.3.1,10.20.30.40",
+        "10.2.3.1,10.20.30.41",
+        "10.2.3.3,10.20.30.40,10.20.30.41"
+    };
+    private static final String HEADER_NAME = "X-Forwarded-For";
+
+    @Bug(3854)
+    public void testClientIPFromHeader() throws Exception {
+        WebClient wc = new WebClient();
+
+        wc.addRequestHeader(HEADER_NAME, testData[0]);
+        HtmlPage p = wc.goTo("configure");
+        submit(p.getFormByName("config"));
+    }
+
+    @Bug(3854)
+    public void testHeaderChange() throws Exception {
+        WebClient wc = new WebClient();
+
+        wc.addRequestHeader(HEADER_NAME, testData[0]);
+        HtmlPage p = wc.goTo("configure");
+
+        wc.removeRequestHeader(HEADER_NAME);
+        try {
+            // The crumb should no longer match if we remove the proxy info
+            submit(p.getFormByName("config"));
+        }
+        catch (FailingHttpStatusCodeException e) {
+            assertEquals(403,e.getStatusCode());
+        }
+    }
+
+    @Bug(3854)
+    public void testProxyIPChanged() throws Exception {
+        WebClient wc = new WebClient();
+
+        wc.addRequestHeader(HEADER_NAME, testData[1]);
+        HtmlPage p = wc.goTo("configure");
+
+        wc.removeRequestHeader(HEADER_NAME);
+        wc.addRequestHeader(HEADER_NAME, testData[2]);
+
+        // The crumb should be the same even if the proxy IP changes
+        submit(p.getFormByName("config"));
+    }
+
+    @Bug(3854)
+    public void testProxyIPChain() throws Exception {
+        WebClient wc = new WebClient();
+
+        wc.addRequestHeader(HEADER_NAME, testData[3]);
+        HtmlPage p = wc.goTo("configure");
+        submit(p.getFormByName("config"));
+    }
+}