Cross-Site Request Forgery in Jenkins

@@ -17,14 +17,19 @@
 import jenkins.model.Jenkins;
 import hudson.model.ModelObject;
  
+import javax.annotation.Nonnull;
 import javax.servlet.ServletRequest;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpSession;
+ 
 import jenkins.security.HexStringConfidentialKey;
  
 import net.sf.json.JSONObject;
  
 import org.acegisecurity.Authentication;
 import org.jenkinsci.Symbol;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.StaplerRequest;
  
@@ -38,6 +43,9 @@ public class DefaultCrumbIssuer extends CrumbIssuer {
     private transient MessageDigest md;
     private boolean excludeClientIPFromCrumb;
  
+    @Restricted(NoExternalUse.class)
+    public static /* non-final: Groovy Console */ boolean EXCLUDE_SESSION_ID = SystemProperties.getBoolean(DefaultCrumbIssuer.class.getName() + ".EXCLUDE_SESSION_ID");
+ 
     @DataBoundConstructor
     public DefaultCrumbIssuer(boolean excludeClientIPFromCrumb) {
         try {
@@ -75,13 +83,15 @@ protected synchronized String issueCrumb(ServletRequest request, String salt) {
                 HttpServletRequest req = (HttpServletRequest) request;
                 StringBuilder buffer = new StringBuilder();
                 Authentication a = Jenkins.getAuthentication();
-                if (a != null) {
-                    buffer.append(a.getName());
-                }
+                buffer.append(a.getName());
                 buffer.append(';');
                 if (!isExcludeClientIPFromCrumb()) {
                     buffer.append(getClientIP(req));
                 }
+                if (!EXCLUDE_SESSION_ID) {
+                    buffer.append(';');
+                    buffer.append(getSessionId(req));
+                }
  
                 md.update(buffer.toString().getBytes());
                 return Util.toHexString(md.digest(salt.getBytes()));
@@ -90,6 +100,14 @@ protected synchronized String issueCrumb(ServletRequest request, String salt) {
         return null;
     }
  
+    private String getSessionId(@Nonnull HttpServletRequest request) {
+        HttpSession session = request.getSession(false);
+        if (session == null) {
+            return "NO_SESSION";
+        }
+        return session.getId();
+    }
+ 
     /**
      * {@inheritDoc}
      */

@@ -0,0 +1,89 @@
+/**
+ * Copyright (c) 2008-2010 Yahoo! Inc.
+ * All rights reserved.
+ * The copyrights to the contents of this file are licensed under the MIT License (http://www.opensource.org/licenses/mit-license.php)
+ */
+ 
+package hudson.security.csrf;
+ 
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import com.gargoylesoftware.htmlunit.html.DomElement;
+import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import hudson.model.User;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.JenkinsRule.WebClient;
+ 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+ 
+/**
+ *
+ * @author dty
+ */
+//TODO merge back to DefaultCrumbIssuerTest
+public class DefaultCrumbIssuerSEC626Test {
+    
+    @Rule public JenkinsRule r = new JenkinsRule();
+ 
+    @Before public void setIssuer() {
+        r.jenkins.setCrumbIssuer(new DefaultCrumbIssuer(false));
+    }
+ 
+    @Test
+    @Issue("SECURITY-626")
+    public void crumbOnlyValidForOneSession() throws Exception {
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        DefaultCrumbIssuer issuer = new DefaultCrumbIssuer(false);
+        r.jenkins.setCrumbIssuer(issuer);
+ 
+        User.getById("foo", true);
+ 
+        DefaultCrumbIssuer.EXCLUDE_SESSION_ID = true;
+        compareDifferentSessions_tokenAreEqual(true);
+ 
+        DefaultCrumbIssuer.EXCLUDE_SESSION_ID = false;
+        compareDifferentSessions_tokenAreEqual(false);
+    }
+    
+    private void compareDifferentSessions_tokenAreEqual(boolean areEqual) throws Exception {
+        WebClient wc = r.createWebClient();
+        wc.login("foo");
+ 
+        HtmlPage p = wc.goTo("configure");
+        String crumb1 = p.getElementByName("Jenkins-Crumb").getAttribute("value");
+        r.submit(p.getFormByName("config"));
+ 
+        wc.goTo("logout");
+        wc.login("foo");
+ 
+        p = wc.goTo("configure");
+        String crumb2 = p.getElementByName("Jenkins-Crumb").getAttribute("value");
+        r.submit(p.getFormByName("config"));
+ 
+        assertEquals(crumb1.equals(crumb2), areEqual);
+ 
+        if (areEqual) {
+            r.submit(p.getFormByName("config"));
+        } else {
+            replaceAllCrumbInPageBy(p, crumb1);
+            try {
+                // submit the form with previous session crumb
+                r.submit(p.getFormByName("config"));
+                fail();
+            } catch (FailingHttpStatusCodeException e) {
+                assertTrue(e.getMessage().contains("No valid crumb"));
+            }
+        }
+    }
+ 
+    private void replaceAllCrumbInPageBy(HtmlPage page, String newCrumb) {
+        for (DomElement el : page.getElementsByName("Jenkins-Crumb")) {
+            el.setAttribute("value", newCrumb);
+        }
+    }
+}