[JENKINS-50211] Reject connections from agents with unsupported Remoting versions (#4579)

Co-authored-by: NBasil Crow <me@basilcrow.com>

core/src/main/java/hudson/slaves/SlaveComputer.java

@@ -661,9 +661,22 @@ public class SlaveComputer extends Computer {
         log.println("Remoting version: " + slaveVersion);
         VersionNumber agentVersion = new VersionNumber(slaveVersion);
         if (agentVersion.isOlderThan(RemotingVersionInfo.getMinimumSupportedVersion())) {
-            log.printf("WARNING: Remoting version is older than a minimum required one (%s). " +
-                            "Connection will not be rejected, but the compatibility is NOT guaranteed%n",
-                    RemotingVersionInfo.getMinimumSupportedVersion());
+            if (!ALLOW_UNSUPPORTED_REMOTING_VERSIONS) {
+                taskListener.fatalError(
+                        "Rejecting the connection because the Remoting version is older than the"
+                            + " minimum required version (%s). To allow the connection anyway, set"
+                            + " the hudson.slaves.SlaveComputer.allowUnsupportedRemotingVersions"
+                            + " system property to true.%n",
+                        RemotingVersionInfo.getMinimumSupportedVersion());
+                disconnect(new OfflineCause.LaunchFailed());
+                return;
+            } else {
+                taskListener.error(
+                        "The Remoting version is older than the minimum required version (%s)."
+                            + " The connection will be allowed, but compatibility is NOT"
+                            + " guaranteed.%n",
+                        RemotingVersionInfo.getMinimumSupportedVersion());
+            }
         }
 
         log.println("Launcher: " + getLauncher().getClass().getSimpleName());
@@ -1142,6 +1155,13 @@ public class SlaveComputer extends Computer {
         }
     }
 
+    /**
+     * Escape hatch for allowing connections from agents with unsupported Remoting versions.
+     */
+    @Restricted(NoExternalUse.class)
+    @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "for script console")
+    public static /* not final */ boolean ALLOW_UNSUPPORTED_REMOTING_VERSIONS = SystemProperties.getBoolean(SlaveComputer.class.getName() + ".allowUnsupportedRemotingVersions");
+
     // use RingBufferLogHandler class name to configure for backward compatibility
     private static final int DEFAULT_RING_BUFFER_SIZE = SystemProperties.getInteger(RingBufferLogHandler.class.getName() + ".defaultSize", 256);
 

test/pom.xml

@@ -237,6 +237,14 @@ THE SOFTWARE.
                   <outputDirectory>${project.build.outputDirectory}/old-remoting</outputDirectory>
                   <destFileName>remoting-minimal-supported.jar</destFileName>
                 </artifactItem>
+                <artifactItem>
+                  <groupId>org.jenkins-ci.main</groupId>
+                  <artifactId>remoting</artifactId>
+                  <version>3.13</version>
+                  <type>jar</type>
+                  <outputDirectory>${project.build.outputDirectory}/old-remoting</outputDirectory>
+                  <destFileName>remoting-unsupported.jar</destFileName>
+                </artifactItem>
               </artifactItems>
             </configuration>
           </execution>

test/src/test/java/hudson/slaves/SlaveComputerTest.java

@@ -38,11 +38,8 @@ import hudson.model.User;
 import hudson.remoting.Launcher;
 import hudson.security.ACL;
 import hudson.security.ACLContext;
-import java.io.File;
 import java.io.IOError;
 import java.io.IOException;
-import java.nio.charset.Charset;
-import java.nio.file.Files;
 import jenkins.model.Jenkins;
 import net.sf.json.JSONNull;
 import net.sf.json.JSONObject;
@@ -66,8 +63,7 @@ public class SlaveComputerTest {
     @Test
     public void testAgentLogs() throws Exception {
         DumbSlave node = j.createOnlineSlave();
-        File logFile = node.getComputer().getLogFile();
-        String log = new String(Files.readAllBytes(logFile.toPath()), Charset.defaultCharset());
+        String log = node.getComputer().getLog();
         Assert.assertTrue(log.contains("Remoting version: " + Launcher.VERSION));
         Assert.assertTrue(log.contains("Launcher: " + SimpleCommandLauncher.class.getSimpleName()));
         Assert.assertTrue(log.contains("Communication Protocol: Standard in/out"));

test/src/test/java/jenkins/slaves/UnsupportedRemotingAgentEscapeHatchTest.java

+package jenkins.slaves;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+
+import hudson.EnvVars;
+import hudson.model.FreeStyleProject;
+import hudson.model.Slave;
+import hudson.slaves.ComputerLauncher;
+import hudson.slaves.SlaveComputer;
+import hudson.tasks.BatchFile;
+import hudson.tasks.Shell;
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import org.apache.commons.io.FileUtils;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestRule;
+import org.jvnet.hudson.test.FlagRule;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.SimpleCommandLauncher;
+
+/**
+ * Test for the escape hatch for unsupported Remoting agent versions
+ */
+public class UnsupportedRemotingAgentEscapeHatchTest {
+
+    @Rule public JenkinsRule j = new JenkinsRuleWithUnsupportedAgent();
+
+    @Rule public TemporaryFolder tmpDir = new TemporaryFolder();
+
+    @Rule
+    public TestRule allowUnsupportedRemotingVersions = FlagRule.systemProperty(
+            SlaveComputer.class.getName() + ".allowUnsupportedRemotingVersions",
+            Boolean.toString(true));
+
+    private File agentJar;
+
+    @Before
+    public void extractAgent() throws Exception {
+        agentJar = new File(tmpDir.getRoot(), "unsupported-agent.jar");
+        FileUtils.copyURLToFile(UnsupportedRemotingAgentEscapeHatchTest.class.getResource("/old-remoting/remoting-unsupported.jar"), agentJar);
+    }
+
+    @Issue("JENKINS-50211")
+    @Test
+    public void shouldBeAbleToConnectAgentWithUnsupportedVersionWithEscapeHatch() throws Exception {
+        Slave agent = j.createOnlineSlave();
+        assertThat(agent.toComputer().getLog(), containsString("The Remoting version is older than the minimum required version"));
+        assertThat(agent.toComputer().getLog(), containsString("The connection will be allowed, but compatibility is NOT guaranteed"));
+
+        // Ensure we are able to run something on the agent
+        FreeStyleProject project = j.createFreeStyleProject("foo");
+        project.setAssignedLabel(agent.getSelfLabel());
+        project.getBuildersList().add(agent.getComputer().isUnix()
+                ? new Shell("echo Hello")
+                : new BatchFile("echo 'hello'"));
+        j.buildAndAssertSuccess(project);
+    }
+
+    private class JenkinsRuleWithUnsupportedAgent extends JenkinsRule {
+        @Override
+        public ComputerLauncher createComputerLauncher(EnvVars env) throws URISyntaxException, IOException {
+            int sz = this.jenkins.getNodes().size();
+            return new SimpleCommandLauncher(
+                    String.format(
+                            "\"%s/bin/java\" %s -jar \"%s\"",
+                            System.getProperty("java.home"),
+                            SLAVE_DEBUG_PORT > 0
+                                    ? " -Xdebug -Xrunjdwp:transport=dt_socket,server=y,address="
+                                            + (SLAVE_DEBUG_PORT + sz)
+                                    : "",
+                            agentJar.getAbsolutePath()));
+        }
+    }
+}

test/src/test/java/jenkins/slaves/UnsupportedRemotingAgentTest.java

+package jenkins.slaves;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.assertThrows;
+
+import hudson.EnvVars;
+import hudson.model.Slave;
+import hudson.slaves.ComputerLauncher;
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.concurrent.ExecutionException;
+import org.apache.commons.io.FileUtils;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.SimpleCommandLauncher;
+
+/**
+ * Test for unsupported Remoting agent versions
+ */
+public class UnsupportedRemotingAgentTest {
+
+    @Rule public JenkinsRule j = new JenkinsRuleWithUnsupportedAgent();
+
+    @Rule public TemporaryFolder tmpDir = new TemporaryFolder();
+
+    private File agentJar;
+
+    @Before
+    public void extractAgent() throws Exception {
+        agentJar = new File(tmpDir.getRoot(), "unsupported-agent.jar");
+        FileUtils.copyURLToFile(UnsupportedRemotingAgentTest.class.getResource("/old-remoting/remoting-unsupported.jar"), agentJar);
+    }
+
+    @Issue("JENKINS-50211")
+    @Test
+    public void shouldNotBeAbleToConnectAgentWithUnsupportedVersion() throws Exception {
+        Slave agent = j.createSlave();
+        ExecutionException e = assertThrows(ExecutionException.class, () -> agent.toComputer().connect(false).get());
+        assertThat(e.getCause(), instanceOf(IOException.class));
+        assertThat(e.getMessage(), containsString("Agent failed to connect"));
+        assertThat(agent.toComputer().getLog(), containsString("Rejecting the connection because the Remoting version is older than the minimum required version"));
+    }
+
+    private class JenkinsRuleWithUnsupportedAgent extends JenkinsRule {
+        @Override
+        public ComputerLauncher createComputerLauncher(EnvVars env) throws URISyntaxException, IOException {
+            int sz = this.jenkins.getNodes().size();
+            return new SimpleCommandLauncher(
+                    String.format(
+                            "\"%s/bin/java\" %s -jar \"%s\"",
+                            System.getProperty("java.home"),
+                            SLAVE_DEBUG_PORT > 0
+                                    ? " -Xdebug -Xrunjdwp:transport=dt_socket,server=y,address="
+                                            + (SLAVE_DEBUG_PORT + sz)
+                                    : "",
+                            agentJar.getAbsolutePath()));
+        }
+    }
+}