Merge pull request #8945 from NotMyFault/backporting-2.440.1

Backporting for 2.440.1

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<UUID, FullDuplexHttpService> 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;

core/src/main/java/hudson/cli/CLICommand.java

@@ -26,6 +26,7 @@
 
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.AbortException;
 import hudson.Extension;
 import hudson.ExtensionList;
@@ -51,6 +52,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.Jenkins;
+import jenkins.util.SystemProperties;
 import org.apache.commons.discovery.ResourceClassIterator;
 import org.apache.commons.discovery.ResourceNameIterator;
 import org.apache.commons.discovery.resource.ClassLoaders;
@@ -62,6 +64,7 @@
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.args4j.CmdLineException;
 import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.ParserProperties;
 import org.kohsuke.args4j.spi.OptionHandler;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authentication.BadCredentialsException;
@@ -107,6 +110,16 @@
  */
 @LegacyInstancesAreScopedToHudson
 public abstract class CLICommand implements ExtensionPoint, Cloneable {
+
+    /**
+     * Boolean values to either allow or disallow parsing of @-prefixes.
+     * If a command line value starts with @, it is interpreted as being a file, loaded,
+     * and interpreted as if the file content would have been passed to the command line
+     */
+    @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Accessible via System Groovy Scripts")
+    @Restricted(NoExternalUse.class)
+    public static boolean ALLOW_AT_SYNTAX = SystemProperties.getBoolean(CLICommand.class.getName() + ".allowAtSyntax");
+
     /**
      * Connected to stdout and stderr of the CLI agent that initiated the session.
      * IOW, if you write to these streams, the person who launched the CLI command
@@ -307,7 +320,8 @@ private void logAndPrintError(Throwable e, String errorMessage, String logMessag
      * @since 1.538
      */
     protected CmdLineParser getCmdLineParser() {
-        return new CmdLineParser(this);
+        ParserProperties properties = ParserProperties.defaults().withAtSyntax(ALLOW_AT_SYNTAX);
+        return new CmdLineParser(this, properties);
     }
 
     /**

core/src/main/java/hudson/cli/declarative/CLIRegisterer.java

@@ -59,6 +59,7 @@
 import org.jvnet.localizer.ResourceBundleHolder;
 import org.kohsuke.args4j.CmdLineException;
 import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.ParserProperties;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.core.Authentication;
@@ -131,7 +132,8 @@ protected CmdLineParser getCmdLineParser() {
                         private CmdLineParser bindMethod(List<MethodBinder> binders) {
 
                             registerOptionHandlers();
-                            CmdLineParser parser = new CmdLineParser(null);
+                            ParserProperties properties = ParserProperties.defaults().withAtSyntax(ALLOW_AT_SYNTAX);
+                            CmdLineParser parser = new CmdLineParser(null, properties);
 
                             //  build up the call sequence
                             Stack<Method> chains = new Stack<>();

core/src/main/resources/hudson/views/StatusColumn/column.jelly

@@ -23,18 +23,18 @@ THE SOFTWARE.
 -->
 
 <?jelly escape-by-default='true'?>
-<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout">
-  <td class="jenkins-table__cell--tight jenkins-table__icon" data="${job.iconColor.ordinal()}">
-    <div class="jenkins-table__cell__button-wrapper">
-      <j:choose>
-        <j:when test="${job.iconColor.iconName != null}">
+<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:t="/lib/hudson">
+  <j:choose>
+    <j:when test="${job.iconColor.iconName != null}">
+      <td class="jenkins-table__cell--tight jenkins-table__icon" data="${job.iconColor.ordinal()}">
+        <div class="jenkins-table__cell__button-wrapper">
           <l:icon src="symbol-status-${job.iconColor.iconName}" tooltip="${job.iconColor.description}" />
-        </j:when>
-        <j:otherwise>
-          <!-- This is for special conditions, such as folders -->
-          <l:icon src="${job.iconColor.iconClassName}" />
-        </j:otherwise>
-      </j:choose>
-    </div>
-  </td>
+        </div>
+      </td>
+    </j:when>
+    <j:otherwise>
+      <!-- This is for special conditions, such as folders -->
+      <t:ballColorTd it="${job.iconColor}" />
+    </j:otherwise>
+  </j:choose>
 </j:jelly>

test/pom.xml

@@ -230,7 +230,7 @@ THE SOFTWARE.
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>
       <artifactId>matrix-project</artifactId>
-      <version>822.v01b_8c85d16d2</version>
+      <version>822.824.v14451b_c0fd42</version>
       <scope>test</scope>
     </dependency>
     <dependency>

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<Boolean> escapeHatch;
+
+    private final Boolean allowWs;
+
+    @Parameterized.Parameters
+    public static List<String> 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
+        }
+    }
+}

test/src/test/java/jenkins/security/Security3314Test.java

@@ -0,0 +1,55 @@
+package jenkins.security;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+
+import hudson.cli.CLICommandInvoker;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.List;
+import jenkins.model.Jenkins;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.jvnet.hudson.test.JenkinsRule;
+
+@RunWith(Parameterized.class)
+public class Security3314Test {
+    private String commandName;
+
+    @Rule
+    public final JenkinsRule j = new JenkinsRule();
+
+    /**
+     * connect-node to test the CLICommand behavior
+     * disable-job to test the CLIRegisterer behavior (@CLIMethod)
+     */
+    @Parameterized.Parameters
+    public static List<String> commands() {
+        return Arrays.asList("connect-node", "disable-job");
+    }
+
+    public Security3314Test(String commandName) {
+        this.commandName = commandName;
+    }
+
+    @Test
+    public void commandShouldNotParseAt() throws Exception {
+        CLICommandInvoker command = new CLICommandInvoker(j, commandName);
+
+        Path tempPath = Files.createTempFile("tempFile", ".txt");
+        tempPath.toFile().deleteOnExit();
+        String content = "AtGotParsed";
+        Files.write(tempPath, content.getBytes());
+
+        final CLICommandInvoker.Result result = command
+                .authorizedTo(Jenkins.READ)
+                .invokeWithArgs("@" + tempPath);
+
+        assertThat(result.stderr(), containsString("@" + tempPath));
+        assertThat(result.stderr(), not(containsString("AtGotParsed")));
+    }
+}

test/src/test/java/lib/form/ValidateButtonTest.java

@@ -109,13 +109,16 @@ public static final class DescriptorImpl extends Descriptor<TestValidateIsCalled
 
             public void doValidateTest1(@QueryParameter("a") String a, @QueryParameter("b") boolean b,
                                         @QueryParameter("c") boolean c, @QueryParameter("d") String d,
-                                        @QueryParameter("e") String e) {
+                                        @QueryParameter("e") String e,
+                                        @QueryParameter("f") String f
+                                        ) {
                 try {
                     assertEquals("avalue", a);
                     assertTrue(b);
                     assertFalse(c);
                     assertEquals("dvalue", d);
                     assertEquals("e2", e);
+                    assertEquals("f", f);
                     test1Outcome = null;
                 } catch (RuntimeException t) {
                     test1Outcome = t;

test/src/test/resources/lib/form/ValidateButtonTest/TestValidateIsCalled/index.jelly

@@ -50,7 +50,27 @@ THE SOFTWARE.
           <f:option value="e2" selected="true">e2</f:option>
         </select>
       </f:entry>
-      <f:validateButton method="validateTest1" title="test" with="a,b,c,d,e" />
+
+      <f:entry title="f">
+        <f:radioBlock name="f" value="notf" title="fa" checked="false">
+          <f:entry title="fa">
+            <f:textbox name="fa"/>
+          </f:entry>
+        </f:radioBlock>
+
+        <f:radioBlock name="f" value="f" title="fb" checked="true">
+          <f:entry title="fb">
+            <f:textbox name="fb"/>
+          </f:entry>
+        </f:radioBlock>
+
+        <f:radioBlock name="f" value="reallynotf" title="fb" checked="false">
+          <f:entry title="fc">
+            <f:textbox name="fc"/>
+          </f:entry>
+        </f:radioBlock>
+      </f:entry>
+      <f:validateButton method="validateTest1" title="test" with="a,b,c,d,e,f" />
     </f:form>
   </l:main-panel>
 </l:layout>

war/pom.xml

@@ -310,7 +310,7 @@ THE SOFTWARE.
                   <!-- detached after 1.561 -->
                   <groupId>org.jenkins-ci.plugins</groupId>
                   <artifactId>matrix-project</artifactId>
-                  <version>818.v7eb_e657db_924</version>
+                  <version>822.824.v14451b_c0fd42</version>
                   <type>hpi</type>
                 </artifactItem>
                 <artifactItem>

war/src/main/webapp/scripts/hudson-behavior.js

@@ -2708,8 +2708,13 @@ function validateButton(checkUrl, paramList, button) {
   paramList.split(",").forEach(function (name) {
     var p = findPreviousFormItem(button, name);
     if (p != null) {
-      if (p.type == "checkbox") {
+      if (p.type === "checkbox") {
         parameters[name] = p.checked;
+      } else if (p.type === "radio") {
+        while (p && !p.checked) {
+          p = findPreviousFormItem(p, name);
+        }
+        parameters[name] = p.value;
       } else {
         parameters[name] = p.value;
       }