diff --git a/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java b/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java index 7f833c17f326..64c6e04ec212 100644 --- a/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java +++ b/core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java @@ -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} */ diff --git a/test/src/test/java/hudson/security/csrf/DefaultCrumbIssuerSEC626Test.java b/test/src/test/java/hudson/security/csrf/DefaultCrumbIssuerSEC626Test.java new file mode 100644 index 000000000000..3abd3ccb1816 --- /dev/null +++ b/test/src/test/java/hudson/security/csrf/DefaultCrumbIssuerSEC626Test.java @@ -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); + } + } +}