From 8c4e97d1716d45130fa65a82cb8350055e9f5c98 Mon Sep 17 00:00:00 2001 From: dty Date: Tue, 9 Jun 2009 07:09:02 +0000 Subject: [PATCH] Merge in changes that implement the cross-site request forgery crumb feature. git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@18738 71c3de6d-444a-0410-be80-ed276b4c234a --- .../java/hudson/cli/FullDuplexHttpStream.java | 57 +++++++ core/pom.xml | 11 ++ core/src/main/java/hudson/Functions.java | 51 ++++++ .../java/hudson/model/DownloadService.java | 17 +- core/src/main/java/hudson/model/Hudson.java | 47 +++++- .../main/java/hudson/scm/SubversionSCM.java | 5 + .../hudson/security/csrf/CrumbFilter.java | 133 +++++++++++++++ .../hudson/security/csrf/CrumbIssuer.java | 155 ++++++++++++++++++ .../security/csrf/CrumbIssuerDescriptor.java | 117 +++++++++++++ .../security/csrf/DefaultCrumbIssuer.java | 111 +++++++++++++ .../hudson/PluginManager/installed.jelly | 5 +- .../model/AbstractProject/sidepanel.jelly | 5 +- .../hudson/model/Hudson/configure.jelly | 15 +- .../hudson/model/Hudson/newView.jelly | 3 +- .../PageDecoratorImpl/footer.jelly | 2 + .../hudson/model/UpdateCenter/index.jelly | 3 +- .../security/csrf/CrumbIssuer/config.jelly | 36 ++++ .../hudson/widgets/HistoryWidget/index.jelly | 4 +- core/src/main/resources/lib/form/crumb.jelly | 11 ++ core/src/main/resources/lib/form/submit.jelly | 5 +- .../lib/hudson/editableDescription.jelly | 4 +- .../main/resources/lib/hudson/executors.jelly | 4 +- .../lib/hudson/newFromList/form.jelly | 3 +- .../lib/hudson/progressiveText.jelly | 4 +- .../resources/lib/hudson/project/matrix.jelly | 4 +- .../src/main/resources/lib/hudson/queue.jelly | 4 +- .../org/jvnet/hudson/test/HudsonTestCase.java | 33 ++++ .../jvnet/hudson/test/TestCrumbIssuer.java | 55 +++++++ .../test/java/hudson/model/HudsonTest.java | 6 +- .../java/lib/form/ExpandableTextboxTest.java | 6 +- war/resources/WEB-INF/web.xml | 13 +- war/resources/help/security/csrf/field.html | 4 + war/resources/help/security/csrf/salt.html | 8 + war/resources/help/system-config/csrf.html | 11 ++ war/resources/scripts/hudson-behavior.js | 37 ++++- 35 files changed, 951 insertions(+), 38 deletions(-) create mode 100644 core/src/main/java/hudson/security/csrf/CrumbFilter.java create mode 100644 core/src/main/java/hudson/security/csrf/CrumbIssuer.java create mode 100644 core/src/main/java/hudson/security/csrf/CrumbIssuerDescriptor.java create mode 100644 core/src/main/java/hudson/security/csrf/DefaultCrumbIssuer.java create mode 100644 core/src/main/resources/hudson/security/csrf/CrumbIssuer/config.jelly create mode 100644 core/src/main/resources/lib/form/crumb.jelly create mode 100644 test/src/main/java/org/jvnet/hudson/test/TestCrumbIssuer.java create mode 100644 war/resources/help/security/csrf/field.html create mode 100644 war/resources/help/security/csrf/salt.html create mode 100644 war/resources/help/system-config/csrf.html diff --git a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java index f7f73e69ae..27e8a618b2 100644 --- a/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java +++ b/cli/src/main/java/hudson/cli/FullDuplexHttpStream.java @@ -1,11 +1,17 @@ package hudson.cli; +import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.Reader; import java.net.HttpURLConnection; import java.net.URL; import java.util.UUID; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Creates a capacity-unlimited bi-directional {@link InputStream}/{@link OutputStream} pair over @@ -34,12 +40,17 @@ public class FullDuplexHttpStream { public FullDuplexHttpStream(URL target) throws IOException { this.target = target; + CrumbData crumbData = new CrumbData(); + // server->client HttpURLConnection con = (HttpURLConnection) target.openConnection(); con.setDoOutput(true); // request POST to avoid caching con.setRequestMethod("POST"); con.addRequestProperty("Session",uuid.toString()); con.addRequestProperty("Side","download"); + if(crumbData.isValid) { + con.addRequestProperty(crumbData.crumbName, crumbData.crumb); + } con.getOutputStream().close(); input = con.getInputStream(); // make sure we hit the right URL @@ -53,8 +64,54 @@ public class FullDuplexHttpStream { con.setChunkedStreamingMode(0); con.addRequestProperty("Session",uuid.toString()); con.addRequestProperty("Side","upload"); + if(crumbData.isValid) { + con.addRequestProperty(crumbData.crumbName, crumbData.crumb); + } output = con.getOutputStream(); } static final int BLOCK_SIZE = 1024; + static final Logger LOGGER = Logger.getLogger(FullDuplexHttpStream.class.getName()); + + private final class CrumbData { + String crumbName; + String crumb; + boolean isValid; + + private CrumbData() { + this.crumbName = ""; + this.crumb = ""; + this.isValid = false; + getData(); + } + + private void getData() { + try { + String base = createCrumbUrlBase(); + crumbName = readData(base+"?xpath=/*/crumbRequestField/text()"); + crumb = readData(base+"?xpath=/*/crumb/text()"); + isValid = true; + LOGGER.fine("Crumb data: "+crumbName+"="+crumb); + } + catch (IOException e) { + LOGGER.log(Level.WARNING,"Failed to get crumb data",e); + } + } + + private String createCrumbUrlBase() { + String url = target.toExternalForm(); + return new StringBuilder(url.substring(0, url.lastIndexOf("/cli"))).append("/crumbIssuer/api/xml/").toString(); + } + + private String readData(String dest) throws IOException { + HttpURLConnection con = (HttpURLConnection) new URL(dest).openConnection(); + try { + BufferedReader reader = new BufferedReader(new InputStreamReader(con.getInputStream())); + return reader.readLine(); + } + finally { + con.disconnect(); + } + } + } } diff --git a/core/pom.xml b/core/pom.xml index ceb73d4166..cc95dee338 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -702,6 +702,17 @@ THE SOFTWARE. jinterop-wmi 1.0 + + org.jvnet.hudson + htmlunit + 2.2-hudson-9 + + + xalan + xalan + + + new Ajax.Request(btn.getAttribute('url')+"/make"+(btn.checked?"Enable":"Disable")+"d", { + + parameters : { "${h.getCrumbRequestField()}": "${h.getCrumb(request)}" }, + onFailure : function(req,o) { $('needRestart').innerHTML = req.responseText; } diff --git a/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly b/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly index 9aa3fd4288..f69a46e461 100644 --- a/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly +++ b/core/src/main/resources/hudson/model/AbstractProject/sidepanel.jelly @@ -1,7 +1,7 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/core/src/main/resources/hudson/widgets/HistoryWidget/index.jelly b/core/src/main/resources/hudson/widgets/HistoryWidget/index.jelly index 2673fcd83a..12aaa5c616 100644 --- a/core/src/main/resources/hudson/widgets/HistoryWidget/index.jelly +++ b/core/src/main/resources/hudson/widgets/HistoryWidget/index.jelly @@ -1,7 +1,7 @@ + + + + + + \ No newline at end of file diff --git a/core/src/main/resources/lib/form/submit.jelly b/core/src/main/resources/lib/form/submit.jelly index 87b025115a..14b4369c3c 100644 --- a/core/src/main/resources/lib/form/submit.jelly +++ b/core/src/main/resources/lib/form/submit.jelly @@ -1,7 +1,7 @@ - + Submit button themed by YUI. This should be always used instead of the plain <input tag. @@ -36,5 +36,6 @@ THE SOFTWARE. The text of the submit button. Something like "submit", "OK", etc. + \ No newline at end of file diff --git a/core/src/main/resources/lib/hudson/editableDescription.jelly b/core/src/main/resources/lib/hudson/editableDescription.jelly index 65131513e9..b4e0f44897 100644 --- a/core/src/main/resources/lib/hudson/editableDescription.jelly +++ b/core/src/main/resources/lib/hudson/editableDescription.jelly @@ -1,7 +1,7 @@ diff --git a/core/src/main/resources/lib/hudson/newFromList/form.jelly b/core/src/main/resources/lib/hudson/newFromList/form.jelly index 929f192db8..a8ce7dfc10 100644 --- a/core/src/main/resources/lib/hudson/newFromList/form.jelly +++ b/core/src/main/resources/lib/hudson/newFromList/form.jelly @@ -1,7 +1,7 @@ + diff --git a/core/src/main/resources/lib/hudson/progressiveText.jelly b/core/src/main/resources/lib/hudson/progressiveText.jelly index ea3a5b8293..777c860121 100644 --- a/core/src/main/resources/lib/hudson/progressiveText.jelly +++ b/core/src/main/resources/lib/hudson/progressiveText.jelly @@ -1,7 +1,7 @@ var stickToBottom = scroller.isSticking(); diff --git a/core/src/main/resources/lib/hudson/project/matrix.jelly b/core/src/main/resources/lib/hudson/project/matrix.jelly index f41ee18570..231383c527 100644 --- a/core/src/main/resources/lib/hudson/project/matrix.jelly +++ b/core/src/main/resources/lib/hudson/project/matrix.jelly @@ -1,7 +1,7 @@ diff --git a/test/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java b/test/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java index 95b6181094..0222931633 100644 --- a/test/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java +++ b/test/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java @@ -54,6 +54,8 @@ import hudson.model.AbstractProject; import hudson.model.UpdateCenter.UpdateCenterConfiguration; import hudson.model.Node.Mode; import hudson.scm.SubversionSCM; +import hudson.security.csrf.CrumbIssuer; +import hudson.security.csrf.CrumbIssuerDescriptor; import hudson.slaves.CommandLauncher; import hudson.slaves.DumbSlave; import hudson.slaves.RetentionStrategy; @@ -98,6 +100,7 @@ import javax.servlet.ServletContextEvent; import junit.framework.TestCase; +import org.apache.commons.httpclient.NameValuePair; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.beanutils.PropertyUtils; @@ -128,8 +131,10 @@ import org.xml.sax.SAXException; import com.gargoylesoftware.htmlunit.AjaxController; import com.gargoylesoftware.htmlunit.BrowserVersion; import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.HttpMethod; import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.WebRequestSettings; +import com.gargoylesoftware.htmlunit.html.DomNode; import com.gargoylesoftware.htmlunit.html.HtmlButton; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; @@ -199,6 +204,10 @@ public abstract class HudsonTestCase extends TestCase { hudson = newHudson(); hudson.setNoUsageStatistics(true); // collecting usage stats from tests are pointless. + + hudson.setUseCrumbs(true); + hudson.setCrumbIssuer(((CrumbIssuerDescriptor)Hudson.getInstance().getDescriptor(TestCrumbIssuer.class)).newInstance(null,null)); + hudson.servletContext.setAttribute("app",hudson); hudson.servletContext.setAttribute("version","?"); WebAppMain.installExpressionFactory(new ServletContextEvent(hudson.servletContext)); @@ -918,6 +927,30 @@ public abstract class HudsonTestCase extends TestCase { public String getContextPath() { return "http://localhost:"+localPort+contextPath; } + + /** + * Adds a security crumb to the quest + */ + public WebRequestSettings addCrumb(WebRequestSettings req) { + NameValuePair crumb[] = { new NameValuePair() }; + + crumb[0].setName(hudson.getCrumbIssuer().getDescriptor().getCrumbRequestField()); + crumb[0].setValue(hudson.getCrumbIssuer().getCrumb( null )); + + req.setRequestParameters(Arrays.asList( crumb )); + return req; + } + + /** + * Creates a URL with crumb parameters relative to {{@link #getContextPath()} + */ + public URL createCrumbedUrl(String relativePath) throws MalformedURLException { + CrumbIssuer issuer = hudson.getCrumbIssuer(); + String crumbName = issuer.getDescriptor().getCrumbRequestField(); + String crumb = issuer.getCrumb(null); + + return new URL(getContextPath()+relativePath+"?"+crumbName+"="+crumb); + } } // needs to keep reference, or it gets GC-ed. diff --git a/test/src/main/java/org/jvnet/hudson/test/TestCrumbIssuer.java b/test/src/main/java/org/jvnet/hudson/test/TestCrumbIssuer.java new file mode 100644 index 0000000000..93e081330e --- /dev/null +++ b/test/src/main/java/org/jvnet/hudson/test/TestCrumbIssuer.java @@ -0,0 +1,55 @@ +/** + * Copyright (c) 2008-2009 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 org.jvnet.hudson.test; + +import javax.servlet.ServletRequest; + +import net.sf.json.JSONObject; + +import org.kohsuke.stapler.StaplerRequest; + +import hudson.Extension; +import hudson.model.ModelObject; +import hudson.security.csrf.CrumbIssuer; +import hudson.security.csrf.CrumbIssuerDescriptor; + +/** + * A crumb issuer that issues a constant crumb value. Used for unit testing. + * @author dty + */ +public class TestCrumbIssuer extends CrumbIssuer +{ + @Override + protected String issueCrumb( ServletRequest request, String salt ) + { + return "test"; + } + + @Override + public boolean validateCrumb( ServletRequest request, String salt, String crumb ) + { + return "test".equals(crumb); + } + + @Extension + public static final class DescriptorImpl extends CrumbIssuerDescriptor implements ModelObject { + public DescriptorImpl() + { + super(null, null); + load(); + } + + @Override + public String getDisplayName() { + return "Test Crumb"; + } + + public TestCrumbIssuer newInstance(StaplerRequest req, JSONObject formData) throws FormException { + return new TestCrumbIssuer(); + } + } + +} diff --git a/test/src/test/java/hudson/model/HudsonTest.java b/test/src/test/java/hudson/model/HudsonTest.java index e2ff5c299b..6f18641b6c 100644 --- a/test/src/test/java/hudson/model/HudsonTest.java +++ b/test/src/test/java/hudson/model/HudsonTest.java @@ -1,7 +1,7 @@ /* * The MIT License * - * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi + * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Yahoo! Inc. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -43,7 +43,7 @@ import org.jvnet.hudson.test.Email; import org.jvnet.hudson.test.HudsonTestCase; import org.jvnet.hudson.test.recipes.LocalData; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; +import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import java.net.URL; import java.util.List; @@ -140,7 +140,7 @@ public class HudsonTest extends HudsonTestCase { wc.getPage(req); fail("Error code expected"); } catch (FailingHttpStatusCodeException e) { - assertEquals(SC_BAD_REQUEST,e.getStatusCode()); + assertEquals(SC_FORBIDDEN,e.getStatusCode()); } // the master computer object should be still here diff --git a/test/src/test/java/lib/form/ExpandableTextboxTest.java b/test/src/test/java/lib/form/ExpandableTextboxTest.java index fd3590b493..e3acb0b27e 100644 --- a/test/src/test/java/lib/form/ExpandableTextboxTest.java +++ b/test/src/test/java/lib/form/ExpandableTextboxTest.java @@ -1,7 +1,7 @@ /* * The MIT License * - * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi + * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Yahoo! Inc. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -54,8 +54,8 @@ public class ExpandableTextboxTest extends HudsonTestCase { */ protected HtmlPage evaluateAsHtml(String jellyScript) throws Exception { HudsonTestCase.WebClient wc = new WebClient(); - - WebRequestSettings req = new WebRequestSettings(new URL(wc.getContextPath()+"eval"), POST); + + WebRequestSettings req = new WebRequestSettings(wc.createCrumbedUrl("eval"), POST); req.setRequestBody(""+jellyScript+""); Page page = wc.getPage(req); return (HtmlPage) page; diff --git a/war/resources/WEB-INF/web.xml b/war/resources/WEB-INF/web.xml index e6025709fd..f5cf546aaa 100644 --- a/war/resources/WEB-INF/web.xml +++ b/war/resources/WEB-INF/web.xml @@ -2,7 +2,7 @@