Skip to content

Commit 3277a8e

Browse files
committed
1 parent 7cedd2f commit 3277a8e

File tree

5 files changed

+212
-7
lines changed

5 files changed

+212
-7
lines changed

src/main/java/hudson/remoting/Channel.java

+44-5
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,14 @@
5252
import java.net.URL;
5353
import java.nio.channels.ClosedChannelException;
5454
import java.nio.charset.StandardCharsets;
55+
import java.util.Arrays;
5556
import java.util.Collections;
5657
import java.util.Date;
58+
import java.util.HashSet;
5759
import java.util.List;
5860
import java.util.Locale;
5961
import java.util.Map;
62+
import java.util.Set;
6063
import java.util.WeakHashMap;
6164
import java.util.concurrent.ConcurrentHashMap;
6265
import java.util.concurrent.CopyOnWriteArrayList;
@@ -925,15 +928,51 @@ public boolean preloadJar(Callable<?,?> classLoaderRef, Class<?>... classesInJar
925928
return preloadJar(UserRequest.getClassLoader(classLoaderRef), classesInJar);
926929
}
927930

931+
@SuppressFBWarnings(
932+
value = "DMI_COLLECTION_OF_URLS",
933+
justification = "All URLs point to local files, so no DNS lookup.")
928934
public boolean preloadJar(ClassLoader local, Class<?>... classesInJar) throws IOException, InterruptedException {
929-
URL[] jars = new URL[classesInJar.length];
930-
for (int i = 0; i < classesInJar.length; i++)
931-
jars[i] = Which.jarFile(classesInJar[i]).toURI().toURL();
932-
return call(new PreloadJarTask(jars, local));
935+
Set<URL> jarSet = new HashSet<>();
936+
for (Class<?> clazz : classesInJar) {
937+
jarSet.add(Which.jarFile(clazz).toURI().toURL());
938+
}
939+
URL[] jars = jarSet.toArray(new URL[0]);
940+
return preloadJar(local, jars);
933941
}
934942

943+
@SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "Callers are privileged controller-side code.")
935944
public boolean preloadJar(ClassLoader local, URL... jars) throws IOException, InterruptedException {
936-
return call(new PreloadJarTask(jars,local));
945+
byte[][] contents = new byte[jars.length][0];
946+
947+
List<URL> jarList = Arrays.asList(jars);
948+
for (int i = 0; i < jarList.size(); i++) {
949+
final URL url = jarList.get(i);
950+
jars[i] = url;
951+
contents[i] = Util.readFully(url.openStream());
952+
}
953+
try {
954+
return call(new PreloadJarTask2(jars, contents, local));
955+
} catch (IOException ex) {
956+
if (ex.getCause() instanceof IllegalAccessError) {
957+
logger.log(
958+
Level.FINE,
959+
ex,
960+
() -> "Failed to call PreloadJarTask2 on " + this + ", retrying with PreloadJarTask");
961+
// When the agent is running an outdated version of remoting, we cannot access nonpublic classes in the
962+
// same package, as PreloadJarTask2 would be loaded from the controller, and hence a different module/
963+
// classloader, than the rest of remoting. As a result PreloadJarTask2 will throw IllegalAccessError:
964+
//
965+
// java.lang.IllegalAccessError: failed to access class hudson.remoting.RemoteClassLoader from class
966+
// hudson.remoting.PreloadJarTask2 (hudson.remoting.RemoteClassLoader is in unnamed module of loader
967+
// 'app'; hudson.remoting.PreloadJarTask2 is in unnamed module of loader 'Jenkins v${project.version}'
968+
// @795f104a)
969+
//
970+
// Identify this error here and fall back to PreloadJarTask, relying on the restrictive controller-side
971+
// implementation of IClassLoader#fetchJar.
972+
return call(new PreloadJarTask(jars, local));
973+
}
974+
throw ex;
975+
}
937976
}
938977

939978
/**
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package hudson.remoting;
2+
3+
import java.io.IOException;
4+
import java.net.URL;
5+
6+
/**
7+
* Validate a URL attempted to be read by the remote end (agent side).
8+
*
9+
* @deprecated Do not use, intended as a temporary workaround only.
10+
*/
11+
// TODO Remove once we no longer require compatibility with remoting before 2024-08.
12+
@Deprecated
13+
public interface JarURLValidator {
14+
void validate(URL url) throws IOException;
15+
}

src/main/java/hudson/remoting/PreloadJarTask.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@
3434
* {@link Callable} used to deliver a jar file to {@link RemoteClassLoader}.
3535
*
3636
* @author Kohsuke Kawaguchi
37+
* @deprecated Retained for compatibility with pre-2024-08 remoting only (see {@link Channel#preloadJar(ClassLoader, java.net.URL...)}), use {@link hudson.remoting.PreloadJarTask2}.
3738
*/
38-
final class PreloadJarTask implements DelegatingCallable<Boolean,IOException> {
39+
@Deprecated
40+
final class PreloadJarTask implements DelegatingCallable<Boolean, IOException> {
3941
/**
4042
* Jar file to be preloaded.
4143
*/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* The MIT License
3+
*
4+
* Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy
7+
* of this software and associated documentation files (the "Software"), to deal
8+
* in the Software without restriction, including without limitation the rights
9+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
* copies of the Software, and to permit persons to whom the Software is
11+
* furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in
14+
* all copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
* THE SOFTWARE.
23+
*/
24+
package hudson.remoting;
25+
26+
import edu.umd.cs.findbugs.annotations.CheckForNull;
27+
import java.io.IOException;
28+
import java.net.URL;
29+
import org.jenkinsci.remoting.Role;
30+
import org.jenkinsci.remoting.RoleChecker;
31+
32+
/**
33+
* {@link Callable} used to deliver a jar file to {@link RemoteClassLoader}.
34+
* <p>
35+
* This replaces {@link hudson.remoting.PreloadJarTask} and delivers the jar contents as part of the Callable rather
36+
* than needing to call {@link hudson.remoting.RemoteClassLoader#prefetch(java.net.URL)}.
37+
* </p>
38+
* @since TODO 2024-08
39+
*/
40+
final class PreloadJarTask2 implements DelegatingCallable<Boolean, IOException> {
41+
/**
42+
* Jar file to be preloaded.
43+
*/
44+
private final URL[] jars;
45+
46+
private final byte[][] contents;
47+
48+
// TODO: This implementation exists starting from
49+
// https://github.com/jenkinsci/remoting/commit/f3d0a81fdf46a10c3c6193faf252efaeaee98823
50+
// Since this time nothing has blown up, but it still seems to be suspicious.
51+
// The solution for null classloaders is available in RemoteDiagnostics.Script#call() in the Jenkins core codebase
52+
@CheckForNull
53+
private transient ClassLoader target = null;
54+
55+
PreloadJarTask2(URL[] jars, byte[][] contents, @CheckForNull ClassLoader target) {
56+
if (jars.length != contents.length) {
57+
throw new IllegalArgumentException("Got " + jars.length + " jars and " + contents.length + " contents");
58+
}
59+
this.jars = jars;
60+
this.contents = contents;
61+
this.target = target;
62+
}
63+
64+
@Override
65+
public ClassLoader getClassLoader() {
66+
return target;
67+
}
68+
69+
@Override
70+
public Boolean call() throws IOException {
71+
ClassLoader cl = Thread.currentThread().getContextClassLoader();
72+
73+
try {
74+
if (!(cl instanceof RemoteClassLoader)) {
75+
return false;
76+
}
77+
final RemoteClassLoader rcl = (RemoteClassLoader) cl;
78+
79+
boolean r = false;
80+
for (int i = 0; i < jars.length; i++) {
81+
r |= rcl.prefetch(jars[i], contents[i]);
82+
}
83+
return r;
84+
} catch (IllegalAccessError iae) {
85+
// Catch the IAE instead of letting it be wrapped by remoting to suppress warnings logged on the agent-side
86+
throw new IOException(iae);
87+
}
88+
}
89+
90+
/**
91+
* This task is only useful in the context that allows remote classloading, and by that point
92+
* any access control check is pointless. So just declare the worst possible role.
93+
*/
94+
@Override
95+
public void checkRoles(RoleChecker checker) throws SecurityException {
96+
checker.check(this, Role.UNKNOWN);
97+
}
98+
99+
private static final long serialVersionUID = -773448303394727271L;
100+
}

src/main/java/hudson/remoting/RemoteClassLoader.java

+50-1
Original file line numberDiff line numberDiff line change
@@ -682,8 +682,12 @@ public static void deleteDirectoryOnExit(File dir) {
682682
* @param jar Jar to be prefetched. Note that this file is an file on the other end,
683683
* and doesn't point to anything meaningful locally.
684684
* @return true if the prefetch happened. false if the jar is already prefetched.
685+
* @deprecated Only left in for compatibility with pre-2024-08 remoting. Use {@link #prefetch(java.net.URL, byte[])} instead.
685686
* @see Channel#preloadJar(Callable, Class[])
687+
* @see hudson.remoting.PreloadJarTask
688+
* @see hudson.remoting.PreloadJarTask2
686689
*/
690+
@Deprecated
687691
/*package*/ boolean prefetch(URL jar) throws IOException {
688692
synchronized (prefetchedJars) {
689693
if (prefetchedJars.contains(jar)) {
@@ -699,6 +703,31 @@ public static void deleteDirectoryOnExit(File dir) {
699703
}
700704
}
701705

706+
/**
707+
* Prefetches the specified jar with the specified content into this classloader.
708+
* @param jar Jar to be prefetched. Note that this file is an file on the other end,
709+
* and doesn't point to anything meaningful locally.
710+
* @param content the jar content
711+
* @return true if the prefetch happened. false if the jar is already prefetched.
712+
* @see Channel#preloadJar(Callable, Class[])
713+
* @see hudson.remoting.PreloadJarTask2
714+
* @since TODO 2024-08
715+
*/
716+
/*package*/ boolean prefetch(URL jar, byte[] content) throws IOException {
717+
synchronized (prefetchedJars) {
718+
if (prefetchedJars.contains(jar)) {
719+
return false;
720+
}
721+
722+
String p = jar.getPath().replace('\\', '/');
723+
p = Util.getBaseName(p);
724+
File localJar = Util.makeResource(p, content);
725+
addURL(localJar.toURI().toURL());
726+
prefetchedJars.add(jar);
727+
return true;
728+
}
729+
}
730+
702731
/**
703732
* Receiver-side of {@link ClassFile2} uses this to remember the prefetch information.
704733
*/
@@ -841,6 +870,7 @@ public static class ClassFile2 extends ResourceFile {
841870
* Remoting interface.
842871
*/
843872
public interface IClassLoader {
873+
@Deprecated
844874
byte[] fetchJar(URL url) throws IOException;
845875

846876
/**
@@ -964,8 +994,27 @@ public ClassLoaderProxy(@NonNull ClassLoader cl, Channel channel) {
964994
}
965995

966996
@Override
967-
@SuppressFBWarnings(value = "URLCONNECTION_SSRF_FD", justification = "This is only used for managing the jar cache as files.")
997+
@SuppressFBWarnings(
998+
value = "URLCONNECTION_SSRF_FD",
999+
justification = "URL validation is being done through JarURLValidator")
9681000
public byte[] fetchJar(URL url) throws IOException {
1001+
final Object o = channel.getProperty(JarURLValidator.class);
1002+
if (o == null) {
1003+
final boolean disabled = Boolean.getBoolean(Channel.class.getName() + ".DISABLE_JAR_URL_VALIDATOR");
1004+
LOGGER.log(Level.FINE, "Default behavior for URL: " + url + " with disabled flag: " + disabled);
1005+
if (!disabled) {
1006+
throw new IOException(
1007+
"No hudson.remoting.JarURLValidator has been set for this channel, so all #fetchJar calls are rejected."
1008+
+ " This is likely a bug in Jenkins."
1009+
+ " As a workaround, try updating the agent.jar file.");
1010+
}
1011+
} else {
1012+
if (o instanceof JarURLValidator) {
1013+
((JarURLValidator) o).validate(url);
1014+
} else {
1015+
throw new IOException("Unexpected channel property hudson.remoting.JarURLValidator value: " + o);
1016+
}
1017+
}
9691018
return readFully(url.openStream());
9701019
}
9711020

0 commit comments

Comments
 (0)