Increase minimum required Remoting version from 3176.v207ec082a_8c0 to 3256.3258.v858f3c9a_f69d (#27439)

* Drop support for remoting releases before 2024-08

* Adapt Security3430Test to latest changes

Uses the original changes from @daniel-beck, merged with master branch.

One relevant change for discussion is the switch from IllegalStateException
to IOException in the assertions in the _run method.  I believe that is the
correct change, but would like validation from @daniel-back to be certain.

-                assertThat(itex.getCause(), instanceOf(IllegalStateException.class));
+                assertThat(itex.getCause(), instanceOf(IOException.class));

Testing done:

* Confirmed Security3430Test passes in my development environment

---------

Co-authored-by: Daniel Beck <daniel-beck@users.noreply.github.com>
Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com>
This commit is contained in:
authored and GitHub committed 2026-09-27 06:12:26 -06:00
1 parent 70d3bbb786
commit 2725fc3a00
6 files changed
+26 -263

No files matched your search

+1 -1
View File
@@ -40,7 +40,7 @@ THE SOFTWARE.
<properties>
<!-- Minimum Remoting version, which is tested for API compatibility, duplicated so that renovate only updates the latest remoting version property -->
<remoting.minimum.supported.version>3176.v207ec082a_8c0</remoting.minimum.supported.version>
<remoting.minimum.supported.version>3256.3258.v858f3c9a_f69d</remoting.minimum.supported.version>
<!-- Filled in by jacoco-maven-plugin -->
<jacocoSurefireArgs />
<!-- Not managed by jenkins-bom, so referenced from annotationProcessorPaths via this property -->
@@ -2455,22 +2455,6 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
// only for debugging purpose
return "classLoader " + getClass().getName();
}
// TODO Remove this once we require post 2024-07 remoting minimum version and deleted ClassLoaderProxy#fetchJar(URL)
@SuppressFBWarnings(
value = "DMI_COLLECTION_OF_URLS",
justification = "All URLs point to local files, so no DNS lookup.")
@Restricted(NoExternalUse.class)
public boolean isPluginJar(URL jarUrl) {
for (PluginWrapper plugin : activePlugins) {
if (plugin.classLoader instanceof URLClassLoader) {
if (Set.of(((URLClassLoader) plugin.classLoader).getURLs()).contains(jarUrl)) {
return true;
}
}
}
return false;
}
}
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "for script console")
@@ -1,105 +0,0 @@
/*
* The MIT License
*
* Copyright (c) 2024 CloudBees, 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
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package jenkins.security.s2m;
import edu.umd.cs.findbugs.annotations.Nullable;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.Extension;
import hudson.PluginManager;
import hudson.remoting.Channel;
import hudson.remoting.ChannelBuilder;
import hudson.remoting.JarURLValidator;
import java.io.IOException;
import java.net.URL;
import java.net.URLClassLoader;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.model.Jenkins;
import jenkins.security.ChannelConfigurator;
import jenkins.util.SystemProperties;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
@Restricted(NoExternalUse.class)
@Deprecated
@Extension
public class JarURLValidatorImpl extends ChannelConfigurator implements JarURLValidator {
public static final Logger LOGGER = Logger.getLogger(JarURLValidatorImpl.class.getName());
@Override
public void onChannelBuilding(ChannelBuilder builder, @Nullable Object context) {
LOGGER.log(Level.CONFIG, () -> "Setting up JarURLValidatorImpl for context: " + context);
builder.withProperty(JarURLValidator.class, this);
}
@Override
public void validate(URL url) throws IOException {
final String rejectAllProp = JarURLValidatorImpl.class.getName() + ".REJECT_ALL";
if (SystemProperties.getBoolean(rejectAllProp)) {
LOGGER.log(Level.FINE, () -> "Rejecting URL due to configuration: " + url);
throw new IOException("The system property '" + rejectAllProp + "' has been set, so all attempts by agents to load jars from the controller are rejected."
+ " Update the agent.jar of the affected agent to a version released in August 2024 or later to prevent this error."); // TODO better version spec
}
final String allowAllProp = Channel.class.getName() + ".DISABLE_JAR_URL_VALIDATOR";
if (SystemProperties.getBoolean(allowAllProp)) {
LOGGER.log(Level.FINE, () -> "Allowing URL due to configuration: " + url);
return;
}
if (!isAllowedJar(url)) {
LOGGER.log(Level.FINE, () -> "Rejecting URL: " + url);
throw new IOException("This URL does not point to a jar file allowed to be requested by agents: " + url + "."
+ " Update the agent.jar of the affected agent to a version released in August 2024 or later to prevent this error."
+ " Alternatively, set the system property '" + allowAllProp + "' to 'true' if all the code built by Jenkins is as trusted as an administrator.");
} else {
LOGGER.log(Level.FINE, () -> "Allowing URL: " + url);
}
}
@SuppressFBWarnings(
value = "DMI_COLLECTION_OF_URLS",
justification = "All URLs point to local files, so no DNS lookup.")
private static boolean isAllowedJar(URL url) {
final ClassLoader classLoader = Jenkins.get().getPluginManager().uberClassLoader;
if (classLoader instanceof PluginManager.UberClassLoader uberClassLoader) {
if (uberClassLoader.isPluginJar(url)) {
LOGGER.log(Level.FINER, () -> "Determined to be plugin jar: " + url);
return true;
}
}
final ClassLoader coreClassLoader = Jenkins.class.getClassLoader();
if (coreClassLoader instanceof URLClassLoader urlClassLoader) {
if (Set.of(urlClassLoader.getURLs()).contains(url)) {
LOGGER.log(Level.FINER, () -> "Determined to be core jar: " + url);
return true;
}
}
LOGGER.log(Level.FINER, () -> "Neither core nor plugin jar: " + url);
return false;
}
}
+1 -9
View File
@@ -39,7 +39,7 @@ THE SOFTWARE.
<properties>
<mavenDebug>false</mavenDebug>
<!-- Minimum Remoting version, which is tested for API compatibility, duplicated so that renovate only updates the latest remoting version property -->
<remoting.minimum.supported.version>3176.v207ec082a_8c0</remoting.minimum.supported.version>
<remoting.minimum.supported.version>3256.3258.v858f3c9a_f69d</remoting.minimum.supported.version>
<!-- Filled in by jacoco-maven-plugin -->
<jacocoSurefireArgs />
<!--
@@ -371,14 +371,6 @@ THE SOFTWARE.
<version>3256.v88a_f6e922152</version>
<type>jar</type>
<outputDirectory>${project.build.outputDirectory}/old-remoting</outputDirectory>
<destFileName>remoting-before-SECURITY-3430-fix.jar</destFileName>
</artifactItem>
<artifactItem>
<groupId>org.jenkins-ci.main</groupId>
<artifactId>remoting</artifactId>
<version>3085.vc4c6977c075a</version>
<type>jar</type>
<outputDirectory>${project.build.outputDirectory}/old-remoting</outputDirectory>
<destFileName>remoting-unsupported.jar</destFileName>
</artifactItem>
<artifactItem>
@@ -2,38 +2,31 @@ package jenkins.security;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
import hudson.ExtensionList;
import hudson.Functions;
import hudson.model.Computer;
import hudson.remoting.Channel;
import hudson.remoting.Launcher;
import hudson.slaves.SlaveComputer;
import hudson.util.RingBufferLogHandler;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.charset.StandardCharsets;
import java.security.Security;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import jenkins.bouncycastle.api.InstallBouncyCastleJCAProvider;
import jenkins.security.s2m.JarURLValidatorImpl;
import jenkins.slaves.RemotingVersionInfo;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
@@ -46,13 +39,12 @@ import org.junit.jupiter.api.io.TempDir;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.junit.jupiter.InboundAgentExtension;
import org.jvnet.hudson.test.junit.jupiter.RealJenkinsExtension;
import org.kohsuke.args4j.Argument;
import org.kohsuke.stapler.Stapler;
class Security3430Test {
@RegisterExtension
private final RealJenkinsExtension jj = new RealJenkinsExtension().withLogger(JarURLValidatorImpl.class, Level.FINEST);
private final RealJenkinsExtension jj = new RealJenkinsExtension();
@RegisterExtension
private final InboundAgentExtension agents = new InboundAgentExtension();
@@ -66,12 +58,6 @@ class Security3430Test {
runWithRemoting(RemotingVersionInfo.getMinimumSupportedVersion().toString(), "/old-remoting/remoting-minimum-supported.jar", true);
}
@Test
void runWithPreviousAgentJar() throws Throwable {
assumeFalse(Functions.isWindows() && System.getenv("CI") != null, "Low value, high cost on Windows");
runWithRemoting("3256.v88a_f6e922152", "/old-remoting/remoting-before-SECURITY-3430-fix.jar", true);
}
@Test
void runWithCurrentAgentJar() throws Throwable {
assumeFalse(Functions.isWindows() && System.getenv("CI") != null, "Low value, high cost on Windows");
@@ -83,18 +69,10 @@ class Security3430Test {
final String agentName = "agent1";
try {
createAgent(agentName, remotingResourcePath);
jj.runRemotely(Security3430Test::_run, agentName, expectedRemotingVersion, requestingJarFromAgent, true);
jj.runRemotely(Security3430Test::_run, agentName, expectedRemotingVersion, requestingJarFromAgent);
} finally {
agents.stop(jj, agentName);
}
jj.runRemotely(Security3430Test::disableJarURLValidatorImpl);
final String agentName2 = "agent2";
try {
createAgent(agentName2, remotingResourcePath);
jj.runRemotely(Security3430Test::_run, agentName2, expectedRemotingVersion, requestingJarFromAgent, false);
} finally {
agents.stop(jj, agentName2);
}
}
private void createAgent(String name, String remotingResourcePath) throws Throwable {
@@ -115,23 +93,13 @@ class Security3430Test {
return ((SlaveComputer) r.jenkins.getComputer(name)).getJnlpMac();
}
// This is quite artificial, but demonstrating that without JarURLValidatorImpl we do not allow any calls from the agent:
private static void disableJarURLValidatorImpl(JenkinsRule j) {
assertTrue(ExtensionList.lookup(ChannelConfigurator.class).remove(ExtensionList.lookupSingleton(JarURLValidatorImpl.class)));
}
/**
*
* @param agentName the name of the agent we're working with
* @param expectedRemotingVersion The version expected for remoting, or {@code null} if we're using whatever is bundled with this Jenkins.
* @param requestingJarFromAgent {@code true} if and only if we expect to go through {@code ClassLoaderProxy#fetchJar}
* @param hasJenkinsJarURLValidator {@code true} if and only we do not expect {@link jenkins.security.s2m.JarURLValidatorImpl} to be present. Only relevant when {@code requestingJarFromAgent} is {@code true}.
*/
private static void _run(JenkinsRule j, String agentName, String expectedRemotingVersion, Boolean requestingJarFromAgent, Boolean hasJenkinsJarURLValidator) throws Throwable {
final RingBufferLogHandler logHandler = new RingBufferLogHandler(50);
Logger.getLogger(JarURLValidatorImpl.class.getName()).addHandler(logHandler);
final List<LogRecord> logRecords = logHandler.getView();
private static void _run(JenkinsRule j, String agentName, String expectedRemotingVersion, Boolean requestingJarFromAgent) throws Throwable {
final Computer computer = j.jenkins.getComputer(agentName);
assertThat(computer, instanceOf(SlaveComputer.class));
SlaveComputer agent = (SlaveComputer) computer;
@@ -142,113 +110,37 @@ class Security3430Test {
assertThat(result, is(expectedRemotingVersion));
}
logHandler.clear();
{ // regular behavior
if (hasJenkinsJarURLValidator) {
// it works
assertTrue(channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, Stapler.class));
if (requestingJarFromAgent) {
assertThat(logRecords, hasItem(logMessageContainsString("Allowing URL: file:/")));
} else {
assertThat(logRecords.stream().map(LogRecord::getMessage).toList(), is(empty()));
}
logHandler.clear();
assertFalse(channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, Stapler.class));
assertThat(logRecords, not(hasItem(logMessageContainsString("Allowing URL"))));
assertThat(logRecords, not(hasItem(logMessageContainsString("Rejecting URL"))));
} else {
// outdated remoting.jar will fail, but up to date one passes
if (requestingJarFromAgent) {
final IOException ex = assertThrows(IOException.class, () -> channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, Stapler.class));
assertThat(ex.getMessage(), containsString(
"No hudson.remoting.JarURLValidator has been set for this channel, so all #fetchJar calls are rejected. This is likely a bug in Jenkins. As a workaround, try updating the agent.jar file."));
} else {
assertTrue(channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, Stapler.class));
assertThat(logRecords.stream().map(LogRecord::getMessage).toList(), is(empty()));
}
}
}
logHandler.clear();
if (hasJenkinsJarURLValidator) { // Start rejecting everything; only applies to JarURLValidatorImpl
System.setProperty(JarURLValidatorImpl.class.getName() + ".REJECT_ALL", "true");
// it works
assertTrue(channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, Stapler.class));
// Identify that a jar was already loaded:
assertFalse(channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, Stapler.class));
assertThat(logRecords, not(hasItem(logMessageContainsString("Allowing URL"))));
assertThat(logRecords, not(hasItem(logMessageContainsString("Rejecting URL"))));
logHandler.clear();
// different jar file than before, old remoting will fail due to call through ClassLoaderProxy#fetchJar, new remoting passes
if (requestingJarFromAgent) {
final IOException ioException = assertThrows(IOException.class, () -> channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, Argument.class));
assertThat(ioException.getMessage(), containsString("all attempts by agents to load jars from the controller are rejected"));
assertThat(logRecords, not(hasItem(logMessageContainsString("Allowing URL"))));
assertThat(logRecords, hasItem(logMessageContainsString("Rejecting URL due to configuration: ")));
} else {
assertTrue(channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, org.kohsuke.args4j.Argument.class));
assertThat(logRecords, not(hasItem(logMessageContainsString("Allowing URL"))));
assertThat(logRecords, not(hasItem(logMessageContainsString("Rejecting URL"))));
}
}
logHandler.clear();
if (hasJenkinsJarURLValidator) { // Disable block, only applies to JarURLValidatorImpl
System.clearProperty(JarURLValidatorImpl.class.getName() + ".REJECT_ALL");
if (requestingJarFromAgent) {
// now it works again for old remoting:
assertTrue(channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, org.kohsuke.args4j.Argument.class));
assertThat(logRecords, hasItem(logMessageContainsString("Allowing URL: file:/")));
} else {
// new remoting already has it.
assertFalse(channel.preloadJar(j.jenkins.getPluginManager().uberClassLoader, org.kohsuke.args4j.Argument.class));
assertThat(logRecords, not(hasItem(logMessageContainsString("Allowing URL"))));
assertThat(logRecords, not(hasItem(logMessageContainsString("Rejecting URL"))));
}
assertThat(logRecords, not(hasItem(logMessageContainsString("Rejecting URL due to configuration: "))));
}
logHandler.clear();
if (hasJenkinsJarURLValidator || !requestingJarFromAgent) { // prepare bouncycastle-api
assertTrue(j.jenkins.getPluginManager().getPlugin("bouncycastle-api").isActive());
InstallBouncyCastleJCAProvider.on(channel);
channel.call(new ConfirmBouncyCastleLibrary());
}
logHandler.clear();
assertTrue(j.jenkins.getPluginManager().getPlugin("bouncycastle-api").isActive());
InstallBouncyCastleJCAProvider.on(channel);
channel.call(new ConfirmBouncyCastleLibrary());
{ // Exploitation tests
final URL secretKeyFile = new File(j.jenkins.getRootDir(), "secret.key").toURI().toURL();
final String expectedContent = IOUtils.toString(secretKeyFile, StandardCharsets.UTF_8);
{ // Protection is effective when agents request non-jar files:
// Protection is effective when agents request non-jar files:
if (expectedRemotingVersion == null) {
assertThrows(NoSuchMethodException.class, () -> channel.call(new Exploit(secretKeyFile, expectedContent)));
} else {
final InvocationTargetException itex = assertThrows(InvocationTargetException.class, () -> channel.call(new Exploit(secretKeyFile, expectedContent)));
assertThat(itex.getCause(), instanceOf(IOException.class));
if (hasJenkinsJarURLValidator) {
assertThat(itex.getCause().getMessage(), containsString("This URL does not point to a jar file allowed to be requested by agents"));
assertThat(logRecords, not(hasItem(logMessageContainsString("Allowing URL"))));
assertThat(logRecords, hasItem(logMessageContainsString("Rejecting URL: ")));
} else {
assertThat(itex.getCause().getMessage(), containsString(
"No hudson.remoting.JarURLValidator has been set for this channel, so all #fetchJar calls are rejected. This is likely a bug in Jenkins. As a workaround, try updating the agent.jar file."));
}
}
logHandler.clear();
{ // Disable protection and non-jar files can be accessed:
System.setProperty(Channel.class.getName() + ".DISABLE_JAR_URL_VALIDATOR", "true");
channel.call(new Exploit(secretKeyFile, expectedContent));
if (hasJenkinsJarURLValidator) {
assertThat(logRecords, hasItem(logMessageContainsString("Allowing URL due to configuration")));
assertThat(logRecords, not(hasItem(logMessageContainsString("Rejecting URL"))));
}
System.clearProperty(Channel.class.getName() + ".DISABLE_JAR_URL_VALIDATOR");
}
{ // Support for pre-2024-08 remoting has been dropped, so even formerly legitimate jar files cannot be requested anymore:
final URLClassLoader classLoader = (URLClassLoader) j.jenkins.getPluginManager().getPlugin("bouncycastle-api").classLoader;
URL safeUrl = classLoader.getURLs()[0];
final String expectedContent = IOUtils.toString(safeUrl, StandardCharsets.UTF_8);
if (expectedRemotingVersion == null) {
assertThrows(NoSuchMethodException.class, () -> channel.call(new Exploit(safeUrl, expectedContent)));
} else {
final InvocationTargetException itex = assertThrows(InvocationTargetException.class, () -> channel.call(new Exploit(safeUrl, expectedContent)));
assertThat(itex.getCause(), instanceOf(IOException.class));
}
}
}
+1 -1
View File
@@ -49,7 +49,7 @@ THE SOFTWARE.
<!-- executable/Main is a plain bootstrap class; nothing here needs annotation processing. -->
<maven.compiler.proc>none</maven.compiler.proc>
<!-- Minimum Remoting version, which is tested for API compatibility, duplicated so that renovate only updates the latest remoting version property -->
<remoting.minimum.supported.version>3176.v207ec082a_8c0</remoting.minimum.supported.version>
<remoting.minimum.supported.version>3256.3258.v858f3c9a_f69d</remoting.minimum.supported.version>
</properties>