From de450967f38398169650b55c002f1229a3fcdb1b Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Tue, 16 Jan 2024 17:07:37 +0000 Subject: [PATCH] [SECURITY-3315] --- core/src/main/java/hudson/cli/CLIAction.java | 21 ++++++- .../java/hudson/cli/Security3315Test.java | 62 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 test/src/test/java/hudson/cli/Security3315Test.java diff --git a/core/src/main/java/hudson/cli/CLIAction.java b/core/src/main/java/hudson/cli/CLIAction.java index 633ecca3437c..6123cdcf45d0 100644 --- a/core/src/main/java/hudson/cli/CLIAction.java +++ b/core/src/main/java/hudson/cli/CLIAction.java @@ -49,8 +49,10 @@ import javax.servlet.http.HttpServletResponse; import jenkins.model.Jenkins; import jenkins.util.FullDuplexHttpService; +import jenkins.util.SystemProperties; import jenkins.websocket.WebSocketSession; import jenkins.websocket.WebSockets; +import org.apache.commons.lang.StringUtils; import org.jenkinsci.Symbol; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; @@ -73,6 +75,12 @@ public class CLIAction implements UnprotectedRootAction, StaplerProxy { private static final Logger LOGGER = Logger.getLogger(CLIAction.class.getName()); + /** + * Boolean values map to allowing/disallowing WS CLI endpoint always, {@code null} is the default of doing an {@code Origin} check. + * {@code true} is only advisable if anonymous users have no permissions, and Jenkins sends SameSite=Lax cookies (or browsers use that as the implicit default). + */ + /* package-private for testing */ static /* non-final for Script Console */ Boolean ALLOW_WEBSOCKET = SystemProperties.optBoolean(CLIAction.class.getName() + ".ALLOW_WEBSOCKET"); + private final transient Map duplexServices = new HashMap<>(); @Override @@ -114,10 +122,21 @@ public boolean isWebSocketSupported() { /** * WebSocket endpoint. */ - public HttpResponse doWs() { + public HttpResponse doWs(StaplerRequest req) { if (!WebSockets.isSupported()) { return HttpResponses.notFound(); } + if (ALLOW_WEBSOCKET == null) { + final String actualOrigin = req.getHeader("Origin"); + final String expectedOrigin = StringUtils.removeEnd(StringUtils.removeEnd(Jenkins.get().getRootUrlFromRequest(), "/"), req.getContextPath()); + + if (actualOrigin == null || !actualOrigin.equals(expectedOrigin)) { + LOGGER.log(Level.FINE, () -> "Rejecting origin: " + actualOrigin + "; expected was from request: " + expectedOrigin); + return HttpResponses.forbidden(); + } + } else if (!ALLOW_WEBSOCKET) { + return HttpResponses.forbidden(); + } Authentication authentication = Jenkins.getAuthentication2(); return WebSockets.upgrade(new WebSocketSession() { ServerSideImpl connection; diff --git a/test/src/test/java/hudson/cli/Security3315Test.java b/test/src/test/java/hudson/cli/Security3315Test.java new file mode 100644 index 000000000000..83d207a49c66 --- /dev/null +++ b/test/src/test/java/hudson/cli/Security3315Test.java @@ -0,0 +1,62 @@ +package hudson.cli; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +import java.io.IOException; +import java.net.URL; +import java.util.Arrays; +import java.util.List; +import org.htmlunit.HttpMethod; +import org.htmlunit.Page; +import org.htmlunit.WebRequest; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.jvnet.hudson.test.FlagRule; +import org.jvnet.hudson.test.JenkinsRule; + +@RunWith(Parameterized.class) +public class Security3315Test { + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Rule + public FlagRule escapeHatch; + + private final Boolean allowWs; + + @Parameterized.Parameters + public static List escapeHatchValues() { + return Arrays.asList(null, "true", "false"); + } + + public Security3315Test(String allowWs) { + this.allowWs = allowWs == null ? null : Boolean.valueOf(allowWs); + this.escapeHatch = new FlagRule<>(() -> CLIAction.ALLOW_WEBSOCKET, v -> { CLIAction.ALLOW_WEBSOCKET = v; }, this.allowWs); + } + + @Test + public void test() throws IOException { + try (JenkinsRule.WebClient wc = j.createWebClient().withThrowExceptionOnFailingStatusCode(false)) { + // HTTP 400 is WebSocket "success" (HTMLUnit doesn't support it) + final URL jenkinsUrl = j.getURL(); + WebRequest request = new WebRequest(new URL(jenkinsUrl.toString() + "cli/ws"), HttpMethod.GET); + Page page = wc.getPage(request); + assertThat(page.getWebResponse().getStatusCode(), is(allowWs == Boolean.TRUE ? 400 : 403)); // no Origin header + + request.setAdditionalHeader("Origin", jenkinsUrl.getProtocol() + "://example.org:" + jenkinsUrl.getPort()); + page = wc.getPage(request); + assertThat(page.getWebResponse().getStatusCode(), is(allowWs == Boolean.TRUE ? 400 : 403)); // Wrong Origin host + + request.setAdditionalHeader("Origin", jenkinsUrl.getProtocol() + "://" + jenkinsUrl.getHost()); + page = wc.getPage(request); + assertThat(page.getWebResponse().getStatusCode(), is(allowWs == Boolean.TRUE ? 400 : 403)); // Wrong Origin port + + request.setAdditionalHeader("Origin", jenkinsUrl.getProtocol() + "://" + jenkinsUrl.getHost() + ":" + jenkinsUrl.getPort()); + page = wc.getPage(request); + assertThat(page.getWebResponse().getStatusCode(), is(allowWs == Boolean.FALSE ? 403 : 400)); // Reject correct Origin if ALLOW_WS is explicitly false + } + } +}