From 55c31c892c224b7bd12714ec1c20dbeae68defde Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 Nov 2021 02:26:35 -0500 Subject: [PATCH 1/9] Record an informative stack trace & telemetry whenever `FilePath` agent-to-controller access is permitted (#5890) * Record an informative stack trace whenever `SlaveToMasterFileCallable` is permitted * Also record telemetry as `SlaveToMasterFileCallableUsage` * Deduplicate stack traces, and verify in test * Push out end date to 2022-03-01 * `noopener noreferrer` needed on external links Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com> * s/should/does/ in help Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com> * Link to redirect w/ more info Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com> * `mvn spotless:apply` Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com> --- .../security/s2m/DefaultFilePathFilter.java | 16 ++++ .../impl/SlaveToMasterFileCallableUsage.java | 75 +++++++++++++++++++ .../description.jelly | 13 ++++ .../security/s2m/AdminFilePathFilterTest.java | 14 ++++ 4 files changed, 118 insertions(+) create mode 100644 core/src/main/java/jenkins/telemetry/impl/SlaveToMasterFileCallableUsage.java create mode 100644 core/src/main/resources/jenkins/telemetry/impl/SlaveToMasterFileCallableUsage/description.jelly diff --git a/core/src/main/java/jenkins/security/s2m/DefaultFilePathFilter.java b/core/src/main/java/jenkins/security/s2m/DefaultFilePathFilter.java index ef4955213db7..c591827ec8ee 100644 --- a/core/src/main/java/jenkins/security/s2m/DefaultFilePathFilter.java +++ b/core/src/main/java/jenkins/security/s2m/DefaultFilePathFilter.java @@ -26,12 +26,17 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Extension; +import hudson.ExtensionList; import hudson.remoting.ChannelBuilder; +import hudson.remoting.Command; +import hudson.remoting.Request; import java.io.File; +import java.lang.reflect.Field; import java.util.logging.Level; import java.util.logging.Logger; import jenkins.ReflectiveFilePathFilter; import jenkins.security.ChannelConfigurator; +import jenkins.telemetry.impl.SlaveToMasterFileCallableUsage; import jenkins.util.SystemProperties; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; @@ -59,6 +64,17 @@ protected boolean op(String op, File f) throws SecurityException { LOGGER.log(Level.FINE, "agent allowed to {0} {1}", new Object[] {op, f}); return true; } else { + try { + Field current = Request.class.getDeclaredField("CURRENT"); + current.setAccessible(true); + Field createdAt = Command.class.getDeclaredField("createdAt"); + createdAt.setAccessible(true); + Throwable trace = (Throwable) createdAt.get(((ThreadLocal) current.get(null)).get()); + ExtensionList.lookupSingleton(SlaveToMasterFileCallableUsage.class).recordTrace(trace); + LOGGER.log(Level.WARNING, "Permitting agent-to-controller '" + op + "' on '" + f + "'. This is deprecated and will soon be rejected. Learn more: https://www.jenkins.io/redirect/permitted-agent-to-controller-file-access", trace); + } catch (Exception x) { + LOGGER.log(Level.WARNING, null, x); + } return false; } } diff --git a/core/src/main/java/jenkins/telemetry/impl/SlaveToMasterFileCallableUsage.java b/core/src/main/java/jenkins/telemetry/impl/SlaveToMasterFileCallableUsage.java new file mode 100644 index 000000000000..dd628b767773 --- /dev/null +++ b/core/src/main/java/jenkins/telemetry/impl/SlaveToMasterFileCallableUsage.java @@ -0,0 +1,75 @@ +/* + * The MIT License + * + * Copyright 2021 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.telemetry.impl; + +import hudson.Extension; +import hudson.Functions; +import java.time.LocalDate; +import java.util.Collections; +import java.util.Set; +import java.util.TreeSet; +import jenkins.SlaveToMasterFileCallable; +import jenkins.security.s2m.DefaultFilePathFilter; +import jenkins.telemetry.Telemetry; +import net.sf.json.JSONObject; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +/** + * Records when {@link DefaultFilePathFilter} found {@link SlaveToMasterFileCallable} or similar being used. + */ +@Extension +@Restricted(NoExternalUse.class) +public class SlaveToMasterFileCallableUsage extends Telemetry { + + private Set traces = new TreeSet<>(); + + @Override + public String getDisplayName() { + return "Access to files on controllers from code running on an agent"; + } + + @Override + public LocalDate getStart() { + return LocalDate.of(2021, 11, 4); // https://www.jenkins.io/security/advisory/2021-11-04/ + } + + @Override + public LocalDate getEnd() { + return LocalDate.of(2022, 3, 1); + } + + @Override + public synchronized JSONObject createContent() { + JSONObject json = JSONObject.fromObject(Collections.singletonMap("traces", traces)); + traces.clear(); + return json; + } + + public synchronized void recordTrace(Throwable trace) { + traces.add(Functions.printThrowable(trace).replaceAll("@[a-f0-9]+", "@…")); + } + +} diff --git a/core/src/main/resources/jenkins/telemetry/impl/SlaveToMasterFileCallableUsage/description.jelly b/core/src/main/resources/jenkins/telemetry/impl/SlaveToMasterFileCallableUsage/description.jelly new file mode 100644 index 000000000000..83f02a200bf1 --- /dev/null +++ b/core/src/main/resources/jenkins/telemetry/impl/SlaveToMasterFileCallableUsage/description.jelly @@ -0,0 +1,13 @@ + + + Jenkins controllers construct a remote-procedure-call (RPC) channel to agents to instruct them to perform work. + This channel is bidirectional and in a handful of cases agents made requests of the controller. + This was always tricky to secure, + and recently + the category of usages which involved access to files was more tightly restricted than before; + Jenkins developers are considering disabling this kind of usage entirely. + Since it is difficult to determine via static analysis or even manual code inspection which plugins are using this system, + we are collecting information on how widely it is used. + The data includes names of Java classes mainly in Jenkins core and plugins as well as method names and line numbers. + It does not include the names of files being accessed or anything else not determined by versions of software components in use. + diff --git a/test/src/test/java/jenkins/security/s2m/AdminFilePathFilterTest.java b/test/src/test/java/jenkins/security/s2m/AdminFilePathFilterTest.java index 4c4d8f41a29a..383417a2db9e 100644 --- a/test/src/test/java/jenkins/security/s2m/AdminFilePathFilterTest.java +++ b/test/src/test/java/jenkins/security/s2m/AdminFilePathFilterTest.java @@ -25,13 +25,17 @@ package jenkins.security.s2m; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.jvnet.hudson.test.LoggerRule.recorded; +import hudson.ExtensionList; import hudson.FilePath; import hudson.model.Slave; import hudson.remoting.Callable; @@ -40,9 +44,11 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.lang.reflect.Field; +import java.util.List; import java.util.logging.Level; import javax.inject.Inject; import jenkins.SoloFilePathFilter; +import jenkins.telemetry.impl.SlaveToMasterFileCallableUsage; import org.jenkinsci.remoting.RoleChecker; import org.junit.Before; import org.junit.Rule; @@ -107,6 +113,14 @@ public void slaveCannotReadFileFromSecrets_butCanFromUserContent() throws Except checkSlave_can_readFile(s, rootTargetPrivate); } + + @SuppressWarnings("unchecked") + List traces = (List) ExtensionList.lookupSingleton(SlaveToMasterFileCallableUsage.class).createContent().getJSONArray("traces"); + assertThat(traces, hasSize(1)); + assertThat(traces.get(0), allOf(containsString("Command UserRequest:hudson.FilePath$ReadToString@… created at"), containsString(ReadFileS2MCallable.class.getName() + ".call"))); + @SuppressWarnings("unchecked") + List cleared = (List) ExtensionList.lookupSingleton(SlaveToMasterFileCallableUsage.class).createContent().getJSONArray("traces"); + assertThat(cleared, empty()); } private static class ReadFileS2MCallable implements Callable { From e3ebd488d0cb04d2c91beb14d775d5414b38cd4f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 Nov 2021 05:00:24 -0500 Subject: [PATCH 2/9] Warn in `Queue.init` if items scheduled during startup will be clobbered (#5934) --- core/src/main/java/hudson/model/Queue.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java index a15dedce5a6e..64af981be6fc 100644 --- a/core/src/main/java/hudson/model/Queue.java +++ b/core/src/main/java/hudson/model/Queue.java @@ -3103,7 +3103,12 @@ public static Queue getInstance() { */ @Initializer(after=JOB_CONFIG_ADAPTED) public static void init(Jenkins h) { - h.getQueue().load(); + Queue queue = h.getQueue(); + Item[] items = queue.getItems(); + if (items.length > 0) { + LOGGER.warning(() -> "Loading queue will discard previously scheduled items: " + Arrays.toString(items)); + } + queue.load(); } /** From 86040dc68389f828ad3f13e102e0648e68dbe0b8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 Nov 2021 05:00:35 -0500 Subject: [PATCH 3/9] Supply a `SlaveComputer` context for both channels associated with a `maven-plugin` build (#5891) --- core/src/main/java/hudson/slaves/Channels.java | 9 +++++++-- .../main/java/jenkins/security/ChannelConfigurator.java | 6 ++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/hudson/slaves/Channels.java b/core/src/main/java/hudson/slaves/Channels.java index 9f0a6c80921f..9fb5cab11bb6 100644 --- a/core/src/main/java/hudson/slaves/Channels.java +++ b/core/src/main/java/hudson/slaves/Channels.java @@ -27,6 +27,7 @@ import hudson.Launcher.LocalLauncher; import hudson.Proc; import hudson.model.Computer; +import hudson.model.Executor; import hudson.model.TaskListener; import hudson.remoting.Channel; import hudson.remoting.ChannelBuilder; @@ -108,8 +109,10 @@ public synchronized void join() throws InterruptedException { }; cb.withHeaderStream(header); + Executor executor = Executor.currentExecutor(); + Object context = executor != null ? executor.getOwner() : proc; for (ChannelConfigurator cc : ChannelConfigurator.all()) { - cc.onChannelBuilding(cb,null); // TODO: what to pass as a context? + cc.onChannelBuilding(cb, context); } return cb.build(in,out); @@ -145,8 +148,10 @@ public synchronized void join() throws InterruptedException { }; cb.withHeaderStream(header); + Executor executor = Executor.currentExecutor(); + Object context = executor != null ? executor.getOwner() : proc; for (ChannelConfigurator cc : ChannelConfigurator.all()) { - cc.onChannelBuilding(cb,null); // TODO: what to pass as a context? + cc.onChannelBuilding(cb, context); } return cb.build(proc.getInputStream(),proc.getOutputStream()); diff --git a/core/src/main/java/jenkins/security/ChannelConfigurator.java b/core/src/main/java/jenkins/security/ChannelConfigurator.java index 4bd9f0638f21..77923f50fe48 100644 --- a/core/src/main/java/jenkins/security/ChannelConfigurator.java +++ b/core/src/main/java/jenkins/security/ChannelConfigurator.java @@ -3,9 +3,13 @@ import edu.umd.cs.findbugs.annotations.Nullable; import hudson.ExtensionList; import hudson.ExtensionPoint; +import hudson.Proc; import hudson.remoting.Channel; import hudson.remoting.ChannelBuilder; +import hudson.slaves.Channels; import hudson.slaves.SlaveComputer; +import java.io.OutputStream; +import java.util.concurrent.ExecutorService; /** * Intercepts the new creation of {@link Channel} and tweak its configuration. @@ -29,6 +33,8 @@ public abstract class ChannelConfigurator implements ExtensionPoint { *
*
{@link SlaveComputer} *
When a channel is being established to talk to a agent. + *
{@link Proc} + *
When {@link Channels#forProcess(String, ExecutorService, Process, OutputStream)} or overloads are used without a contextual {@link SlaveComputer}. *
*/ public void onChannelBuilding(ChannelBuilder builder, @Nullable Object context) {} From 91e8c4a5954586f8c687335cd97d443a9145d6a5 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 Nov 2021 05:00:43 -0500 Subject: [PATCH 4/9] [JENKINS-67188] Deadlock loading `TaskListener` vs. `StreamTaskListener` (#5932) --- .../java/hudson/model/NullTaskListener.java | 42 +++++++++++++++++++ .../main/java/hudson/model/TaskListener.java | 3 +- 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/hudson/model/NullTaskListener.java diff --git a/core/src/main/java/hudson/model/NullTaskListener.java b/core/src/main/java/hudson/model/NullTaskListener.java new file mode 100644 index 000000000000..5d0454c512b7 --- /dev/null +++ b/core/src/main/java/hudson/model/NullTaskListener.java @@ -0,0 +1,42 @@ +/* + * The MIT License + * + * Copyright 2021 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 hudson.model; + +import java.io.PrintStream; +import org.apache.commons.io.output.NullPrintStream; + +/** + * @see TaskListener#NULL + */ +class NullTaskListener implements TaskListener { + + private static final long serialVersionUID = 1L; + + @Override + public PrintStream getLogger() { + return new NullPrintStream(); + } + +} diff --git a/core/src/main/java/hudson/model/TaskListener.java b/core/src/main/java/hudson/model/TaskListener.java index 4b05ede48c0f..a466613d027f 100644 --- a/core/src/main/java/hudson/model/TaskListener.java +++ b/core/src/main/java/hudson/model/TaskListener.java @@ -27,7 +27,6 @@ import hudson.console.ConsoleNote; import hudson.console.HyperlinkNote; import hudson.remoting.Channel; -import hudson.util.NullStream; import hudson.util.StreamTaskListener; import java.io.IOException; import java.io.OutputStreamWriter; @@ -155,5 +154,5 @@ default PrintWriter fatalError(String format, Object... args) { /** * {@link TaskListener} that discards the output. */ - TaskListener NULL = new StreamTaskListener(new NullStream()); + TaskListener NULL = new NullTaskListener(); } From ab64e651aef0643c08a6e8a31994a08832a4966b Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Tue, 23 Nov 2021 02:00:58 -0800 Subject: [PATCH 5/9] Include `symbol-annotation` in core BOM (#5940) --- bom/pom.xml | 5 +++++ core/pom.xml | 3 +-- test/pom.xml | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/bom/pom.xml b/bom/pom.xml index 991070392674..7cb6b17401d9 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -288,6 +288,11 @@ THE SOFTWARE. robust-http-client 1.2 + + org.jenkins-ci + symbol-annotation + 1.1 + com.sun.mail jakarta.mail diff --git a/core/pom.xml b/core/pom.xml index a1e62367dde5..7af7df22c56d 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -479,10 +479,9 @@ THE SOFTWARE. org.jvnet.robust-http-client robust-http-client - + org.jenkins-ci symbol-annotation - 1.1 diff --git a/test/pom.xml b/test/pom.xml index 9b02134aa2cc..9a28e82d1dd1 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -135,7 +135,7 @@ THE SOFTWARE. org.jenkins-ci.plugins structs - 1.23 + 1.24 test From a0e933a1d46caa57e7e86dfddc1be6f75a9b9808 Mon Sep 17 00:00:00 2001 From: Jenkins Release Bot <66998184+jenkins-release-bot@users.noreply.github.com> Date: Tue, 23 Nov 2021 12:37:02 +0000 Subject: [PATCH 6/9] [maven-release-plugin] prepare release jenkins-2.322 --- bom/pom.xml | 2 +- cli/pom.xml | 2 +- core/pom.xml | 2 +- pom.xml | 4 ++-- test/pom.xml | 2 +- war/pom.xml | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bom/pom.xml b/bom/pom.xml index 7cb6b17401d9..db9b9c5895a9 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -28,7 +28,7 @@ THE SOFTWARE. org.jenkins-ci.main jenkins-parent - ${revision}${changelist} + 2.322 jenkins-bom diff --git a/cli/pom.xml b/cli/pom.xml index 3838594a564f..d2931db4b3ff 100644 --- a/cli/pom.xml +++ b/cli/pom.xml @@ -5,7 +5,7 @@ org.jenkins-ci.main jenkins-parent - ${revision}${changelist} + 2.322 cli diff --git a/core/pom.xml b/core/pom.xml index 7af7df22c56d..644d7acd35b8 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -29,7 +29,7 @@ THE SOFTWARE. org.jenkins-ci.main jenkins-parent - ${revision}${changelist} + 2.322 jenkins-core diff --git a/pom.xml b/pom.xml index d5bd491675e0..a17e13fceb1e 100644 --- a/pom.xml +++ b/pom.xml @@ -34,7 +34,7 @@ THE SOFTWARE. org.jenkins-ci.main jenkins-parent - ${revision}${changelist} + 2.322 pom Jenkins main module @@ -61,7 +61,7 @@ THE SOFTWARE. scm:git:git://github.com/jenkinsci/jenkins.git scm:git:ssh://git@github.com/jenkinsci/jenkins.git https://github.com/jenkinsci/jenkins - ${scmTag} + jenkins-2.322 diff --git a/test/pom.xml b/test/pom.xml index 9a28e82d1dd1..47c547c116c5 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -28,7 +28,7 @@ THE SOFTWARE. org.jenkins-ci.main jenkins-parent - ${revision}${changelist} + 2.322 jenkins-test diff --git a/war/pom.xml b/war/pom.xml index 413436828bda..ca5622472553 100644 --- a/war/pom.xml +++ b/war/pom.xml @@ -28,7 +28,7 @@ THE SOFTWARE. org.jenkins-ci.main jenkins-parent - ${revision}${changelist} + 2.322 jenkins-war From 4faf13cd38d105e90a1352222264e1a8aa3c0491 Mon Sep 17 00:00:00 2001 From: Jenkins Release Bot <66998184+jenkins-release-bot@users.noreply.github.com> Date: Tue, 23 Nov 2021 12:38:02 +0000 Subject: [PATCH 7/9] [maven-release-plugin] prepare for next development iteration --- bom/pom.xml | 2 +- cli/pom.xml | 2 +- core/pom.xml | 2 +- pom.xml | 6 +++--- test/pom.xml | 2 +- war/pom.xml | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/bom/pom.xml b/bom/pom.xml index db9b9c5895a9..7cb6b17401d9 100644 --- a/bom/pom.xml +++ b/bom/pom.xml @@ -28,7 +28,7 @@ THE SOFTWARE. org.jenkins-ci.main jenkins-parent - 2.322 + ${revision}${changelist} jenkins-bom diff --git a/cli/pom.xml b/cli/pom.xml index d2931db4b3ff..3838594a564f 100644 --- a/cli/pom.xml +++ b/cli/pom.xml @@ -5,7 +5,7 @@ org.jenkins-ci.main jenkins-parent - 2.322 + ${revision}${changelist} cli diff --git a/core/pom.xml b/core/pom.xml index 644d7acd35b8..7af7df22c56d 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -29,7 +29,7 @@ THE SOFTWARE. org.jenkins-ci.main jenkins-parent - 2.322 + ${revision}${changelist} jenkins-core diff --git a/pom.xml b/pom.xml index a17e13fceb1e..231f7a20bad3 100644 --- a/pom.xml +++ b/pom.xml @@ -34,7 +34,7 @@ THE SOFTWARE. org.jenkins-ci.main jenkins-parent - 2.322 + ${revision}${changelist} pom Jenkins main module @@ -61,7 +61,7 @@ THE SOFTWARE. scm:git:git://github.com/jenkinsci/jenkins.git scm:git:ssh://git@github.com/jenkinsci/jenkins.git https://github.com/jenkinsci/jenkins - jenkins-2.322 + ${scmTag} @@ -70,7 +70,7 @@ THE SOFTWARE. - 2.322 + 2.323 -SNAPSHOT