From 409508a675ffc4ed9681e30bb46c8d9cb375b78c Mon Sep 17 00:00:00 2001 From: Daniel Beck Date: Mon, 22 Jul 2024 15:55:41 +0200 Subject: [PATCH] [SECURITY-3430] --- src/main/java/hudson/remoting/Channel.java | 49 ++++++++- .../java/hudson/remoting/JarURLValidator.java | 15 +++ .../java/hudson/remoting/PreloadJarTask.java | 4 +- .../java/hudson/remoting/PreloadJarTask2.java | 100 ++++++++++++++++++ .../hudson/remoting/RemoteClassLoader.java | 51 ++++++++- 5 files changed, 212 insertions(+), 7 deletions(-) create mode 100644 src/main/java/hudson/remoting/JarURLValidator.java create mode 100644 src/main/java/hudson/remoting/PreloadJarTask2.java diff --git a/src/main/java/hudson/remoting/Channel.java b/src/main/java/hudson/remoting/Channel.java index 702006f4f..d79756f78 100644 --- a/src/main/java/hudson/remoting/Channel.java +++ b/src/main/java/hudson/remoting/Channel.java @@ -52,11 +52,14 @@ import java.net.URL; import java.nio.channels.ClosedChannelException; import java.nio.charset.StandardCharsets; +import java.util.Arrays; import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -925,15 +928,51 @@ public boolean preloadJar(Callable classLoaderRef, Class... classesInJar return preloadJar(UserRequest.getClassLoader(classLoaderRef), classesInJar); } + @SuppressFBWarnings( + value = "DMI_COLLECTION_OF_URLS", + justification = "All URLs point to local files, so no DNS lookup.") public boolean preloadJar(ClassLoader local, Class... classesInJar) throws IOException, InterruptedException { - URL[] jars = new URL[classesInJar.length]; - for (int i = 0; i < classesInJar.length; i++) - jars[i] = Which.jarFile(classesInJar[i]).toURI().toURL(); - return call(new PreloadJarTask(jars, local)); + Set jarSet = new HashSet<>(); + for (Class clazz : classesInJar) { + jarSet.add(Which.jarFile(clazz).toURI().toURL()); + } + URL[] jars = jarSet.toArray(new URL[0]); + return preloadJar(local, jars); } + @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Callers are privileged controller-side code.") public boolean preloadJar(ClassLoader local, URL... jars) throws IOException, InterruptedException { - return call(new PreloadJarTask(jars,local)); + byte[][] contents = new byte[jars.length][0]; + + List jarList = Arrays.asList(jars); + for (int i = 0; i < jarList.size(); i++) { + final URL url = jarList.get(i); + jars[i] = url; + contents[i] = Util.readFully(url.openStream()); + } + try { + return call(new PreloadJarTask2(jars, contents, local)); + } catch (IOException ex) { + if (ex.getCause() instanceof IllegalAccessError) { + logger.log( + Level.FINE, + ex, + () -> "Failed to call PreloadJarTask2 on " + this + ", retrying with PreloadJarTask"); + // When the agent is running an outdated version of remoting, we cannot access nonpublic classes in the + // same package, as PreloadJarTask2 would be loaded from the controller, and hence a different module/ + // classloader, than the rest of remoting. As a result PreloadJarTask2 will throw IllegalAccessError: + // + // java.lang.IllegalAccessError: failed to access class hudson.remoting.RemoteClassLoader from class + // hudson.remoting.PreloadJarTask2 (hudson.remoting.RemoteClassLoader is in unnamed module of loader + // 'app'; hudson.remoting.PreloadJarTask2 is in unnamed module of loader 'Jenkins v${project.version}' + // @795f104a) + // + // Identify this error here and fall back to PreloadJarTask, relying on the restrictive controller-side + // implementation of IClassLoader#fetchJar. + return call(new PreloadJarTask(jars, local)); + } + throw ex; + } } /** diff --git a/src/main/java/hudson/remoting/JarURLValidator.java b/src/main/java/hudson/remoting/JarURLValidator.java new file mode 100644 index 000000000..4e95f07d7 --- /dev/null +++ b/src/main/java/hudson/remoting/JarURLValidator.java @@ -0,0 +1,15 @@ +package hudson.remoting; + +import java.io.IOException; +import java.net.URL; + +/** + * Validate a URL attempted to be read by the remote end (agent side). + * + * @deprecated Do not use, intended as a temporary workaround only. + */ +// TODO Remove once we no longer require compatibility with remoting before 2024-08. +@Deprecated +public interface JarURLValidator { + void validate(URL url) throws IOException; +} diff --git a/src/main/java/hudson/remoting/PreloadJarTask.java b/src/main/java/hudson/remoting/PreloadJarTask.java index 44a816976..a266c8790 100644 --- a/src/main/java/hudson/remoting/PreloadJarTask.java +++ b/src/main/java/hudson/remoting/PreloadJarTask.java @@ -34,8 +34,10 @@ * {@link Callable} used to deliver a jar file to {@link RemoteClassLoader}. * * @author Kohsuke Kawaguchi + * @deprecated Retained for compatibility with pre-2024-08 remoting only (see {@link Channel#preloadJar(ClassLoader, java.net.URL...)}), use {@link hudson.remoting.PreloadJarTask2}. */ -final class PreloadJarTask implements DelegatingCallable { +@Deprecated +final class PreloadJarTask implements DelegatingCallable { /** * Jar file to be preloaded. */ diff --git a/src/main/java/hudson/remoting/PreloadJarTask2.java b/src/main/java/hudson/remoting/PreloadJarTask2.java new file mode 100644 index 000000000..1f76b5b90 --- /dev/null +++ b/src/main/java/hudson/remoting/PreloadJarTask2.java @@ -0,0 +1,100 @@ +/* + * The MIT License + * + * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi + * + * 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 hudson.remoting; + +import edu.umd.cs.findbugs.annotations.CheckForNull; +import java.io.IOException; +import java.net.URL; +import org.jenkinsci.remoting.Role; +import org.jenkinsci.remoting.RoleChecker; + +/** + * {@link Callable} used to deliver a jar file to {@link RemoteClassLoader}. + *

+ * This replaces {@link hudson.remoting.PreloadJarTask} and delivers the jar contents as part of the Callable rather + * than needing to call {@link hudson.remoting.RemoteClassLoader#prefetch(java.net.URL)}. + *

+ * @since TODO 2024-08 + */ +final class PreloadJarTask2 implements DelegatingCallable { + /** + * Jar file to be preloaded. + */ + private final URL[] jars; + + private final byte[][] contents; + + // TODO: This implementation exists starting from + // https://github.com/jenkinsci/remoting/commit/f3d0a81fdf46a10c3c6193faf252efaeaee98823 + // Since this time nothing has blown up, but it still seems to be suspicious. + // The solution for null classloaders is available in RemoteDiagnostics.Script#call() in the Jenkins core codebase + @CheckForNull + private transient ClassLoader target = null; + + PreloadJarTask2(URL[] jars, byte[][] contents, @CheckForNull ClassLoader target) { + if (jars.length != contents.length) { + throw new IllegalArgumentException("Got " + jars.length + " jars and " + contents.length + " contents"); + } + this.jars = jars; + this.contents = contents; + this.target = target; + } + + @Override + public ClassLoader getClassLoader() { + return target; + } + + @Override + public Boolean call() throws IOException { + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + + try { + if (!(cl instanceof RemoteClassLoader)) { + return false; + } + final RemoteClassLoader rcl = (RemoteClassLoader) cl; + + boolean r = false; + for (int i = 0; i < jars.length; i++) { + r |= rcl.prefetch(jars[i], contents[i]); + } + return r; + } catch (IllegalAccessError iae) { + // Catch the IAE instead of letting it be wrapped by remoting to suppress warnings logged on the agent-side + throw new IOException(iae); + } + } + + /** + * This task is only useful in the context that allows remote classloading, and by that point + * any access control check is pointless. So just declare the worst possible role. + */ + @Override + public void checkRoles(RoleChecker checker) throws SecurityException { + checker.check(this, Role.UNKNOWN); + } + + private static final long serialVersionUID = -773448303394727271L; +} diff --git a/src/main/java/hudson/remoting/RemoteClassLoader.java b/src/main/java/hudson/remoting/RemoteClassLoader.java index 84518a82c..ba91c0b73 100644 --- a/src/main/java/hudson/remoting/RemoteClassLoader.java +++ b/src/main/java/hudson/remoting/RemoteClassLoader.java @@ -669,8 +669,12 @@ public static void deleteDirectoryOnExit(File dir) { * @param jar Jar to be prefetched. Note that this file is an file on the other end, * and doesn't point to anything meaningful locally. * @return true if the prefetch happened. false if the jar is already prefetched. + * @deprecated Only left in for compatibility with pre-2024-08 remoting. Use {@link #prefetch(java.net.URL, byte[])} instead. * @see Channel#preloadJar(Callable, Class[]) + * @see hudson.remoting.PreloadJarTask + * @see hudson.remoting.PreloadJarTask2 */ + @Deprecated /*package*/ boolean prefetch(URL jar) throws IOException { synchronized (prefetchedJars) { if (prefetchedJars.contains(jar)) { @@ -686,6 +690,31 @@ public static void deleteDirectoryOnExit(File dir) { } } + /** + * Prefetches the specified jar with the specified content into this classloader. + * @param jar Jar to be prefetched. Note that this file is an file on the other end, + * and doesn't point to anything meaningful locally. + * @param content the jar content + * @return true if the prefetch happened. false if the jar is already prefetched. + * @see Channel#preloadJar(Callable, Class[]) + * @see hudson.remoting.PreloadJarTask2 + * @since TODO 2024-08 + */ + /*package*/ boolean prefetch(URL jar, byte[] content) throws IOException { + synchronized (prefetchedJars) { + if (prefetchedJars.contains(jar)) { + return false; + } + + String p = jar.getPath().replace('\\', '/'); + p = Util.getBaseName(p); + File localJar = Util.makeResource(p, content); + addURL(localJar.toURI().toURL()); + prefetchedJars.add(jar); + return true; + } + } + /** * Receiver-side of {@link ClassFile2} uses this to remember the prefetch information. */ @@ -828,6 +857,7 @@ public static class ClassFile2 extends ResourceFile { * Remoting interface. */ public interface IClassLoader { + @Deprecated byte[] fetchJar(URL url) throws IOException; /** @@ -945,8 +975,27 @@ public ClassLoaderProxy(@NonNull ClassLoader cl, Channel channel) { } @Override - @SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "This is only used for managing the jar cache as files.") + @SuppressFBWarnings( + value = "URLCONNECTION_SSRF_FD", + justification = "URL validation is being done through JarURLValidator") public byte[] fetchJar(URL url) throws IOException { + final Object o = channel.getProperty(JarURLValidator.class); + if (o == null) { + final boolean disabled = Boolean.getBoolean(Channel.class.getName() + ".DISABLE_JAR_URL_VALIDATOR"); + LOGGER.log(Level.FINE, "Default behavior for URL: " + url + " with disabled flag: " + disabled); + if (!disabled) { + throw new IOException( + "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 { + if (o instanceof JarURLValidator) { + ((JarURLValidator) o).validate(url); + } else { + throw new IOException("Unexpected channel property hudson.remoting.JarURLValidator value: " + o); + } + } return readFully(url.openStream()); }