diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index 2af02aa4c5..d7eb14f7b6 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/core/src/main/java/hudson/PluginManager.java @@ -875,6 +875,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas /** * Bare-minimum configuration mechanism to change the update center. */ + @RequirePOST public HttpResponse doSiteConfigure(@QueryParameter String site) throws IOException { Jenkins hudson = Jenkins.getInstance(); hudson.checkPermission(CONFIGURE_UPDATECENTER); @@ -890,6 +891,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas } + @RequirePOST public HttpResponse doProxyConfigure(StaplerRequest req) throws IOException, ServletException { Jenkins jenkins = Jenkins.getInstance(); jenkins.checkPermission(CONFIGURE_UPDATECENTER); @@ -908,6 +910,7 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas /** * Uploads a plugin. */ + @RequirePOST public HttpResponse doUploadPlugin(StaplerRequest req) throws IOException, ServletException { try { Jenkins.getInstance().checkPermission(UPLOAD_PLUGINS); diff --git a/core/src/main/java/hudson/logging/LogRecorderManager.java b/core/src/main/java/hudson/logging/LogRecorderManager.java index d16ece6c2b..e35a098d4b 100644 --- a/core/src/main/java/hudson/logging/LogRecorderManager.java +++ b/core/src/main/java/hudson/logging/LogRecorderManager.java @@ -40,6 +40,7 @@ import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.HttpRedirect; +import org.kohsuke.stapler.interceptor.RequirePOST; import javax.servlet.ServletException; import java.io.File; @@ -106,6 +107,7 @@ public class LogRecorderManager extends AbstractModelObject implements ModelObje /** * Creates a new log recorder. */ + @RequirePOST public HttpResponse doNewLogRecorder(@QueryParameter String name) { Jenkins.checkGoodName(name); diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java index f2fc2f5660..458d1b8f64 100644 --- a/core/src/main/java/hudson/model/AbstractProject.java +++ b/core/src/main/java/hudson/model/AbstractProject.java @@ -1930,6 +1930,7 @@ public abstract class AbstractProject

,R extends A /** * Wipes out the workspace. */ + @RequirePOST public HttpResponse doDoWipeOutWorkspace() throws IOException, ServletException, InterruptedException { checkPermission(Functions.isWipeOutPermissionEnabled() ? WIPEOUT : BUILD); R b = getSomeBuildWithWorkspace(); diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index 16dacc90a2..8be60142d0 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -47,6 +47,8 @@ import jenkins.util.VirtualFile; import org.apache.commons.io.IOUtils; import org.apache.tools.zip.ZipEntry; import org.apache.tools.zip.ZipOutputStream; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; @@ -309,6 +311,17 @@ public final class DirectoryBrowserSupport implements HttpResponse { // pseudo file name to let the Stapler set text/plain rsp.serveFile(req, in, lastModified, -1, length, "plain.txt"); } else { + String csp = System.getProperty(DirectoryBrowserSupport.class.getName() + ".CSP"); + if (csp == null) { + // default value unless overridden with system property + csp = DEFAULT_CSP_VALUE; + } + if (!csp.trim().equals("")) { + // allow users to prevent sending this header by setting empty system property + for (String header : new String[]{"Content-Security-Policy", "X-WebKit-CSP", "X-Content-Security-Policy"}) { + rsp.setHeader(header, csp); + } + } rsp.serveFile(req, in, lastModified, -1, length, baseFile.getName() ); } } @@ -576,4 +589,7 @@ public final class DirectoryBrowserSupport implements HttpResponse { private static final Logger LOGGER = Logger.getLogger(DirectoryBrowserSupport.class.getName()); + + @Restricted(NoExternalUse.class) + public static final String DEFAULT_CSP_VALUE = "sandbox; default-src 'none'; img-src 'self'; style-src 'self';"; } diff --git a/core/src/main/java/hudson/security/csrf/CrumbFilter.java b/core/src/main/java/hudson/security/csrf/CrumbFilter.java index 9cecfa5daa..8e0fe64cb6 100644 --- a/core/src/main/java/hudson/security/csrf/CrumbFilter.java +++ b/core/src/main/java/hudson/security/csrf/CrumbFilter.java @@ -81,8 +81,8 @@ public class CrumbFilter implements Filter { LOGGER.log(Level.WARNING, "Found invalid crumb {0}. Will check remaining parameters for a valid one...", crumb); } } - // Multipart requests need to be handled by each handler. - if (valid || isMultipart(httpRequest)) { + + if (valid) { chain.doFilter(request, response); } else { LOGGER.log(Level.WARNING, "No valid crumb was included in request for {0}. Returning {1}.", new Object[] {httpRequest.getRequestURI(), HttpServletResponse.SC_FORBIDDEN}); diff --git a/test/src/test/groovy/hudson/model/AbstractProjectTest.groovy b/test/src/test/groovy/hudson/model/AbstractProjectTest.groovy index c25abfa5e6..43fefd3103 100644 --- a/test/src/test/groovy/hudson/model/AbstractProjectTest.groovy +++ b/test/src/test/groovy/hudson/model/AbstractProjectTest.groovy @@ -101,7 +101,13 @@ public class AbstractProjectTest extends HudsonTestCase { assert b.getWorkspace().exists(): "Workspace should exist by now"; // make sure that the action link is protected - createWebClient().assertFails(project.getUrl() + "doWipeOutWorkspace", HttpURLConnection.HTTP_FORBIDDEN); + com.gargoylesoftware.htmlunit.WebClient wc = createWebClient(); + try { + wc.getPage(new WebRequestSettings(new URL(wc.getContextPath() + project.getUrl() + "doWipeOutWorkspace"), HttpMethod.POST)); + fail("Expected HTTP status code 403") + } catch (FailingHttpStatusCodeException e) { + assertEquals(HttpURLConnection.HTTP_FORBIDDEN, e.getStatusCode()); + } } /** diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index 244189e246..dc9ebea798 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -170,6 +170,36 @@ public class DirectoryBrowserSupportTest { zipfile.delete(); } + @Issue("SECURITY-95") + @Test + public void contentSecurityPolicy() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + p.setScm(new SingleFileSCM("test.html", "

Hello world!

")); + p.getPublishersList().add(new ArtifactArchiver("*", "", true)); + assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get().getResult()); + + HtmlPage page = j.createWebClient().goTo("job/" + p.getName() + "/lastSuccessfulBuild/artifact/test.html"); + for (String header : new String[]{"Content-Security-Policy", "X-WebKit-CSP", "X-Content-Security-Policy"}) { + assertEquals("Header set: " + header, page.getWebResponse().getResponseHeaderValue(header), DirectoryBrowserSupport.DEFAULT_CSP_VALUE); + } + + String propName = DirectoryBrowserSupport.class.getName() + ".CSP"; + String initialValue = System.getProperty(propName); + try { + System.setProperty(propName, ""); + page = j.createWebClient().goTo("job/" + p.getName() + "/lastSuccessfulBuild/artifact/test.html"); + for (String header : new String[]{"Content-Security-Policy", "X-WebKit-CSP", "X-Content-Security-Policy"}) { + assertFalse("Header not set: " + header, page.getWebResponse().getResponseHeaders().contains(header)); + } + } finally { + if (initialValue == null) { + System.clearProperty(DirectoryBrowserSupport.class.getName() + ".CSP"); + } else { + System.setProperty(DirectoryBrowserSupport.class.getName() + ".CSP", initialValue); + } + } + } + private File download(UnexpectedPage page) throws IOException { File file = File.createTempFile("DirectoryBrowserSupport", "zipDownload"); diff --git a/test/src/test/java/hudson/model/ProjectTest.java b/test/src/test/java/hudson/model/ProjectTest.java index 88e7a1304f..0233d7824d 100644 --- a/test/src/test/java/hudson/model/ProjectTest.java +++ b/test/src/test/java/hudson/model/ProjectTest.java @@ -23,11 +23,15 @@ */ package hudson.model; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.WebRequestSettings; import hudson.model.queue.QueueTaskFuture; import hudson.security.AccessDeniedException2; import org.acegisecurity.context.SecurityContextHolder; import hudson.security.HudsonPrivateSecurityRealm; import hudson.security.GlobalMatrixAuthorizationStrategy; + +import java.net.URL; import java.util.Collections; import org.jvnet.hudson.reactor.ReactorException; @@ -604,7 +608,9 @@ public class ProjectTest { project.setAssignedLabel(slave.getSelfLabel()); project.getBuildersList().add(new Shell("echo hello > change.log")); j.buildAndAssertSuccess(project); - HtmlPage page = j.createWebClient().login(user.getId(), "password").goTo(project.getUrl() + "doWipeOutWorkspace"); + JenkinsRule.WebClient wc = j.createWebClient().login(user.getId(), "password"); + WebRequestSettings request = new WebRequestSettings(new URL(wc.getContextPath() + project.getUrl() + "doWipeOutWorkspace"), HttpMethod.POST); + HtmlPage p = wc.getPage(request); Thread.sleep(500); assertFalse("Workspace should not exist.", project.getSomeWorkspace().exists()); } diff --git a/war/src/main/webapp/scripts/hudson-behavior.js b/war/src/main/webapp/scripts/hudson-behavior.js index 04aa378231..b0cf9e368c 100644 --- a/war/src/main/webapp/scripts/hudson-behavior.js +++ b/war/src/main/webapp/scripts/hudson-behavior.js @@ -122,6 +122,13 @@ var crumb = { var div = document.createElement("div"); div.innerHTML = ""; form.appendChild(div); + if (form.enctype == "multipart/form-data") { + if (form.action.indexOf("?") != -1) { + form.action = form.action+"&"+this.fieldName+"="+this.value; + } else { + form.action = form.action+"?"+this.fieldName+"="+this.value; + } + } } } @@ -2553,6 +2560,7 @@ function buildFormTree(form) { // switch to multipart/form-data to support file submission // @enctype is the standard, but IE needs @encoding. form.enctype = form.encoding = "multipart/form-data"; + crumb.appendToForm(form); break; case "radio": if(!e.checked) break;