[SECURITY-2422][SECURITY-2441][SECURITY-2463][SECURITY-2476][SECURITY-2479][SECURITY-2586]

Author: dwnusbaum

src/main/java/org/jenkinsci/plugins/workflow/libs/FolderLibraries.java

@@ -77,7 +77,10 @@ private Collection<LibraryConfiguration> forGroup(@CheckForNull ItemGroup<?> gro
                 if (!checkPermission || f.hasPermission(Item.CONFIGURE)) {
                     FolderLibraries prop = f.getProperties().get(FolderLibraries.class);
                     if (prop != null) {
-                        libraries.addAll(prop.getLibraries());
+                        String source = FolderLibraries.ForJob.class.getName() + " " + f.getFullName();
+                        for (LibraryConfiguration library : prop.getLibraries()) {
+                            libraries.add(new ResolvedLibraryConfiguration(library, source));
+                        }
                     }
                 }
             }

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java

@@ -89,7 +89,7 @@
         if (action != null) {
             // Resuming a build, so just look up what we loaded before.
             for (LibraryRecord record : action.getLibraries()) {
-                FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.name);
+                FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
                 for (String root : new String[] {"src", "vars"}) {
                     FilePath dir = libDir.child(root);
                     if (dir.isDirectory()) {
@@ -120,7 +120,11 @@
                 }
                 String version = cfg.defaultedVersion(libraryVersions.remove(name));
                 Boolean changelog = cfg.defaultedChangelogs(libraryChangelogs.remove(name));
-                librariesAdded.put(name, new LibraryRecord(name, version, kindTrusted, changelog, cfg.getCachingConfiguration()));
+                String source = kind.getClass().getName();
+                if (cfg instanceof LibraryResolver.ResolvedLibraryConfiguration) {
+                    source = ((LibraryResolver.ResolvedLibraryConfiguration) cfg).getSource();
+                }
+                librariesAdded.put(name, new LibraryRecord(name, version, kindTrusted, changelog, cfg.getCachingConfiguration(), source));
                 retrievers.put(name, cfg.getRetriever());
             }
         }
@@ -135,7 +139,7 @@
         // Now actually try to retrieve the libraries.
         for (LibraryRecord record : librariesAdded.values()) {
             listener.getLogger().println("Loading library " + record.name + "@" + record.version);
-            for (URL u : retrieve(record.name, record.version, retrievers.get(record.name), record.trusted, record.changelog, record.cachingConfiguration, listener, build, execution, record.variables)) {
+            for (URL u : retrieve(record, retrievers.get(record.name), listener, build, execution)) {
                 additions.add(new Addition(u, record.trusted));
             }
         }
@@ -152,11 +156,14 @@
     }
 
     /** Retrieve library files. */
-    static List<URL> retrieve(@NonNull String name, @NonNull String version, @NonNull LibraryRetriever retriever, boolean trusted, Boolean changelog, LibraryCachingConfiguration cachingConfiguration, @NonNull TaskListener listener, @NonNull Run<?,?> run, @NonNull CpsFlowExecution execution, @NonNull Set<String> variables) throws Exception {
-        FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + name);
+    static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever retriever, @NonNull TaskListener listener, @NonNull Run<?,?> run, @NonNull CpsFlowExecution execution) throws Exception {
+        String name = record.name;
+        String version = record.version;
+        boolean changelog = record.changelog;
+        LibraryCachingConfiguration cachingConfiguration = record.cachingConfiguration;
+        FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
         Boolean shouldCache = cachingConfiguration != null;
-        final FilePath libraryCacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), name);
-        final FilePath versionCacheDir = new FilePath(libraryCacheDir, version);
+        final FilePath versionCacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName());
         final FilePath retrieveLockFile = new FilePath(versionCacheDir, LibraryCachingConfiguration.RETRIEVE_LOCK_FILE);
         final FilePath lastReadFile = new FilePath(versionCacheDir, LibraryCachingConfiguration.LAST_READ_FILE);
 
@@ -195,9 +202,11 @@ static List<URL> retrieve(@NonNull String name, @NonNull String version, @NonNul
         } else {
             retriever.retrieve(name, version, changelog, libDir, run, listener);
         }
+        // Write the user-provided name to a file as a debugging aid.
+        libDir.withSuffix("-name.txt").write(name, "UTF-8");
 
         // Replace any classes requested for replay:
-        if (!trusted) {
+        if (!record.trusted) {
             for (String clazz : ReplayAction.replacementsIn(execution)) {
                 for (String root : new String[] {"src", "vars"}) {
                     String rel = root + "/" + clazz.replace('.', '/') + ".groovy";
@@ -221,7 +230,7 @@ static List<URL> retrieve(@NonNull String name, @NonNull String version, @NonNul
         if (varsDir.isDirectory()) {
             urls.add(varsDir.toURI().toURL());
             for (FilePath var : varsDir.list("*.groovy")) {
-                variables.add(var.getBaseName());
+                record.variables.add(var.getBaseName());
             }
         }
         if (urls.isEmpty()) {
@@ -245,8 +254,11 @@ static List<URL> retrieve(@NonNull String name, @NonNull String version, @NonNul
             if (action != null) {
                 FilePath libs = new FilePath(run.getRootDir()).child("libs");
                 for (LibraryRecord library : action.getLibraries()) {
-                    FilePath f = libs.child(library.name + "/resources/" + name);
-                    if (f.exists()) {
+                    FilePath libResources = libs.child(library.getDirectoryName() + "/resources/");
+                    FilePath f = libResources.child(name);
+                    if (!new File(f.getRemote()).getCanonicalFile().toPath().startsWith(libResources.absolutize().getRemote())) {
+                        throw new AbortException(name + " references a file that is not contained within the library: " + library.name);
+                    } else if (f.exists()) {
                         resources.put(library.name, readResource(f, encoding));
                     }
                 }
@@ -278,7 +290,7 @@ private static String readResource(FilePath file, @CheckForNull String encoding)
             List<GlobalVariable> vars = new ArrayList<>();
             for (LibraryRecord library : action.getLibraries()) {
                 for (String variable : library.variables) {
-                    vars.add(new UserDefinedGlobalVariable(variable, new File(run.getRootDir(), "libs/" + library.name + "/vars/" + variable + ".txt")));
+                    vars.add(new UserDefinedGlobalVariable(variable, new File(run.getRootDir(), "libs/" + library.getDirectoryName() + "/vars/" + variable + ".txt")));
                 }
             }
             return vars;
@@ -304,7 +316,7 @@ private static String readResource(FilePath file, @CheckForNull String encoding)
                                 continue; // TODO JENKINS-41157 allow replay of trusted libraries if you have ADMINISTER
                             }
                             for (String rootName : new String[] {"src", "vars"}) {
-                                FilePath root = libs.child(library.name + "/" + rootName);
+                                FilePath root = libs.child(library.getDirectoryName() + "/" + rootName);
                                 if (!root.isDirectory()) {
                                     continue;
                                 }

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryRecord.java

@@ -27,6 +27,7 @@
 import java.util.Collections;
 import java.util.Set;
 import java.util.TreeSet;
+import jenkins.security.HMACConfidentialKey;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 
@@ -36,26 +37,49 @@
 @ExportedBean
 public final class LibraryRecord {
 
+    private static final HMACConfidentialKey DIRECTORY_NAME_KEY = new HMACConfidentialKey(LibraryRecord.class, "directoryName", 32);
+    private static final String ASCII_UNIT_SEPARATOR = String.valueOf((char)31);
+
     final String name;
     final String version;
     final Set<String> variables = new TreeSet<>();
     final boolean trusted;
     final boolean changelog;
     final LibraryCachingConfiguration cachingConfiguration;
+    private String directoryName;
 
-    LibraryRecord(String name, String version, boolean trusted, boolean changelog, LibraryCachingConfiguration cachingConfiguration) {
+    /**
+     * @param name The name of the library, as entered by the user. Not validated or restricted in any way.
+     * @param version The version of the library, as entered by the user. Not validated or restricted in any way.
+     * @param trusted Whether the library is trusted. Typically determined by {@link LibraryResolver#isTrusted}, but see also {@link LibraryStep}.
+     * @param changelog Whether we should include any SCM changes in this library in the build's changelog.
+     * @param cachingConfiguration If non-null, contains cache-related configuration.
+     * @param source A string describing the source of the configuration of this library. Typically the class name of a {@link LibraryResolver}, sometimes with additional data, but see also {@link LibraryStep}.
+     */
+    LibraryRecord(String name, String version, boolean trusted, boolean changelog, LibraryCachingConfiguration cachingConfiguration, String source) {
         this.name = name;
         this.version = version;
         this.trusted = trusted;
         this.changelog = changelog;
         this.cachingConfiguration = cachingConfiguration;
+        this.directoryName = directoryNameFor(name, version, String.valueOf(trusted), source);
     }
 
     @Exported
     public String getName() {
         return name;
     }
 
+    /**
+     * Returns a partially unique name that can be safely used as a directory name.
+     *
+     * Uniqueness is based on the library name, version, whether it is trusted, and the source of the library.
+     * {@link LibraryRetriever}-specific information such as the SCM is not used to produce this name.
+     */
+    public String getDirectoryName() {
+        return directoryName;
+    }
+
     @Exported
     public String getVersion() {
         return version;
@@ -78,7 +102,24 @@ public boolean isChangelog() {
 
     @Override public String toString() {
         String cachingConfigurationStr = cachingConfiguration != null ? cachingConfiguration.toString() : "null";
-        return "LibraryRecord{name=" + name + ", version=" + version + ", variables=" + variables + ", trusted=" + trusted + ", changelog=" + changelog + ", cachingConfiguration=" + cachingConfigurationStr + '}';
+        return "LibraryRecord{name=" + name + ", version=" + version + ", variables=" + variables + ", trusted=" + trusted + ", changelog=" + changelog + ", cachingConfiguration=" + cachingConfigurationStr + ", directoryName=" + directoryName + '}';
+    }
+
+    private Object readResolve() {
+        if (directoryName == null) {
+            // Builds started before directoryName was added must continue to use the library name as the directory name.
+            directoryName = name;
+        }
+        return this;
+    }
+
+    public static String directoryNameFor(String... data) {
+        for (String datum : data) {
+            if (datum.contains(ASCII_UNIT_SEPARATOR)) { // Very unlikely to appear in legitimate user-controlled text.
+                throw new IllegalStateException("Unable to create directory name due to control character in " + datum);
+            }
+        }
+        return DIRECTORY_NAME_KEY.mac(String.join(ASCII_UNIT_SEPARATOR, data));
     }
 
 }

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryResolver.java

@@ -85,4 +85,31 @@ public abstract class LibraryResolver implements ExtensionPoint {
         return Collections.emptySet();
     }
 
+    /**
+     * Used by some implementations of {@link LibraryResolver} to prevent libraries with the same name that are
+     * configured in distinct trust domains from using the same cache directory.
+     *
+     * For example, {@link FolderLibraries.ForJob} may return libraries from different folders, and a user who is able
+     * to configure a library in one folder may not be able to configure a library in another folder. To prevent this
+     * from causing issues, {@link FolderLibraries.ForJob#forGroup} returns instances of this class where {@code source}
+     * includes the full name of the folder where the library is configured.
+     */
+    static class ResolvedLibraryConfiguration extends LibraryConfiguration {
+        private final String source;
+
+        ResolvedLibraryConfiguration(LibraryConfiguration config, @NonNull String source) {
+            super(config.getName(), config.getRetriever());
+            setDefaultVersion(config.getDefaultVersion());
+            setImplicit(config.isImplicit());
+            setAllowVersionOverride(config.isAllowVersionOverride());
+            setIncludeInChangesets(config.getIncludeInChangesets());
+            setCachingConfiguration(config.getCachingConfiguration());
+            this.source = source;
+        }
+
+        @NonNull String getSource() {
+            return source;
+        }
+    }
+
 }

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java

@@ -166,6 +166,8 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
             boolean trusted = false;
             Boolean changelog = step.getChangelog();
             LibraryCachingConfiguration cachingConfiguration = null;
+            // Note that cachingConfiguration is only ever non-null if source is overwritten below, so the default value of source will never be used when caching is enabled.
+            String source = LibraryStep.class.getName() + " " + run.getExternalizableId();
             LibraryRetriever retriever = step.getRetriever();
             if (retriever == null) {
                 for (LibraryResolver resolver : ExtensionList.lookup(LibraryResolver.class)) {
@@ -176,6 +178,10 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
                             version = cfg.defaultedVersion(version);
                             changelog = cfg.defaultedChangelogs(changelog);
                             cachingConfiguration = cfg.getCachingConfiguration();
+                            source = resolver.getClass().getName();
+                            if (cfg instanceof LibraryResolver.ResolvedLibraryConfiguration) {
+                                source = ((LibraryResolver.ResolvedLibraryConfiguration) cfg).getSource();
+                            }
                             break;
                         }
                     }
@@ -186,8 +192,7 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
             } else if (version == null) {
                 throw new AbortException("Must specify a version for library " + name);
             }
-
-            LibraryRecord record = new LibraryRecord(name, version, trusted, changelog, cachingConfiguration);
+            LibraryRecord record = new LibraryRecord(name, version, trusted, changelog, cachingConfiguration, source);
             LibrariesAction action = run.getAction(LibrariesAction.class);
             if (action == null) {
                 action = new LibrariesAction(Lists.newArrayList(record));
@@ -197,7 +202,7 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
                 for (LibraryRecord existing : libraries) {
                     if (existing.name.equals(name)) {
                         listener.getLogger().println("Only using first definition of library " + name);
-                        return new LoadedClasses(name, trusted, changelog, run);
+                        return new LoadedClasses(name, existing.getDirectoryName(), trusted, changelog, run);
                     }
                 }
                 List<LibraryRecord> newLibraries = new ArrayList<>(libraries);
@@ -207,11 +212,11 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
             listener.getLogger().println("Loading library " + record.name + "@" + record.version);
             CpsFlowExecution exec = (CpsFlowExecution) getContext().get(FlowExecution.class);
             GroovyClassLoader loader = (trusted ? exec.getTrustedShell() : exec.getShell()).getClassLoader();
-            for (URL u : LibraryAdder.retrieve(record.name, record.version, retriever, record.trusted, record.changelog, record.cachingConfiguration, listener, run, (CpsFlowExecution) getContext().get(FlowExecution.class), record.variables)) {
+            for (URL u : LibraryAdder.retrieve(record, retriever, listener, run, (CpsFlowExecution) getContext().get(FlowExecution.class))) {
                 loader.addURL(u);
             }
             run.save(); // persist changes to LibrariesAction.libraries*.variables
-            return new LoadedClasses(name, trusted, changelog, run);
+            return new LoadedClasses(name, record.getDirectoryName(), trusted, changelog, run);
         }
 
     }
@@ -228,8 +233,8 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S
         /** {@code file:/…/libs/NAME/src/} */
         private final @NonNull String srcUrl;
 
-        LoadedClasses(String library, boolean trusted, Boolean changelog, Run<?,?> run) {
-            this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ new File(run.getRootDir(), "libs/" + library + "/src").toURI().toString());
+        LoadedClasses(String library, String libraryDirectoryName, boolean trusted, Boolean changelog, Run<?,?> run) {
+            this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ new File(run.getRootDir(), "libs/" + libraryDirectoryName + "/src").toURI().toString());
         }
 
         LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String srcUrl) {

src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java

@@ -185,7 +185,8 @@ static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String
             if (baseWorkspace == null) {
                 throw new IOException(node.getDisplayName() + " may be offline");
             }
-            dir = baseWorkspace.withSuffix(getFilePathSuffix() + "libs").child(name);
+            String checkoutDirName = LibraryRecord.directoryNameFor(scm.getKey());
+            dir = baseWorkspace.withSuffix(getFilePathSuffix() + "libs").child(checkoutDirName);
         } else { // should not happen, but just in case:
             throw new AbortException("Cannot check out in non-top-level build");
         }
@@ -194,6 +195,8 @@ static void doRetrieve(String name, boolean changelog, @NonNull SCM scm, String
             throw new IOException(node.getDisplayName() + " may be offline");
         }
         try (WorkspaceList.Lease lease = computer.getWorkspaceList().allocate(dir)) {
+            // Write the SCM key to a file as a debugging aid.
+            lease.path.withSuffix("-scm-key.txt").write(scm.getKey(), "UTF-8");
             retrySCMOperation(listener, () -> {
                 delegate.checkout(run, lease.path, listener, node.createLauncher(listener));
                 return null;