Information disclosure through error stack traces related to agents

@@ -26,7 +26,6 @@
  
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
-import hudson.Functions;
 import hudson.model.Computer;
 import hudson.model.User;
 import java.io.ObjectStreamException;
@@ -112,7 +111,7 @@ public String getShortDescription() {
         }
  
         @Override public String toString() {
-            return Messages.OfflineCause_connection_was_broken_(Functions.printThrowable(cause));
+            return Messages.OfflineCause_connection_was_broken_simple();
         }
     }
  

@@ -31,6 +31,7 @@ DumbSlave.displayName=Permanent Agent
 NodeProvisioner.EmptyString=
 OfflineCause.LaunchFailed=This agent is offline because Jenkins failed to launch the agent process on it.
 OfflineCause.connection_was_broken_=Connection was broken: {0}
+OfflineCause.connection_was_broken_simple=Connection was broken
 SimpleScheduledRetentionStrategy.FinishedUpTime=Computer has finished its scheduled uptime
 SimpleScheduledRetentionStrategy.displayName=Bring this agent online according to a schedule
 EnvironmentVariablesNodeProperty.displayName=Environment variables

@@ -25,5 +25,4 @@ THE SOFTWARE.
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
   <p class="error">${%Connection was broken}</p>
-  <pre>${h.printThrowable(it.cause)}</pre>
 </j:jelly>

@@ -0,0 +1,18 @@
+package hudson.slaves;
+ 
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringContains.containsString;
+ 
+import org.junit.Test;
+ 
+public class OfflineCauseTest {
+ 
+    @Test
+    public void testChannelTermination_NoStacktrace() {
+        String exceptionMessage = "exception message";
+        OfflineCause.ChannelTermination cause = new OfflineCause.ChannelTermination(new RuntimeException(exceptionMessage));
+        assertThat(cause.toString(), not(containsString(exceptionMessage)));
+    }
+ 
+}

@@ -0,0 +1,85 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2015 Red Hat, Inc.; Christopher Simons
+ *
+ * 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 static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+ 
+import com.gargoylesoftware.htmlunit.Page;
+import hudson.slaves.DumbSlave;
+import hudson.slaves.OfflineCause;
+import java.util.concurrent.Future;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.JenkinsRule.WebClient;
+import org.jvnet.hudson.test.SmokeTest;
+ 
+@Category(SmokeTest.class)
+public class ComputerSEC2120Test {
+ 
+    @Rule public JenkinsRule j = new JenkinsRule();
+ 
+    @Test
+    public void testTerminatedNodeStatusPageDoesNotShowTrace() throws Exception {
+        DumbSlave agent = j.createOnlineSlave();
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.setAssignedNode(agent);
+ 
+        Future<FreeStyleBuild> r = ExecutorTest.startBlockingBuild(p);
+ 
+        String message = "It went away";
+        p.getLastBuild().getBuiltOn().toComputer().disconnect(
+                new OfflineCause.ChannelTermination(new RuntimeException(message))
+        );
+ 
+        WebClient wc = j.createWebClient();
+        Page page = wc.getPage(wc.createCrumbedUrl(agent.toComputer().getUrl()));
+        String content = page.getWebResponse().getContentAsString();
+        assertThat(content, not(containsString(message)));
+    }
+ 
+    @Test
+    public void testTerminatedNodeAjaxExecutorsDoesNotShowTrace() throws Exception {
+        DumbSlave agent = j.createOnlineSlave();
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.setAssignedNode(agent);
+ 
+        Future<FreeStyleBuild> r = ExecutorTest.startBlockingBuild(p);
+ 
+        String message = "It went away";
+        p.getLastBuild().getBuiltOn().toComputer().disconnect(
+                new OfflineCause.ChannelTermination(new RuntimeException(message))
+        );
+ 
+        WebClient wc = j.createWebClient();
+        Page page = wc.getPage(wc.createCrumbedUrl("ajaxExecutors"));
+        String content = page.getWebResponse().getContentAsString();
+        assertThat(content, not(containsString(message)));
+    }
+ 
+}

@@ -54,6 +54,7 @@
 import java.net.HttpURLConnection;
 import java.nio.charset.StandardCharsets;
 import java.util.Map;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import jenkins.model.Jenkins;
@@ -218,4 +219,43 @@ public void configDotXmlWithValidXmlAndBadField() throws Exception {
                     "  <fullName>Foo User</fullName>\n" +
                     "  <badField/>\n" +
                     "</hudson.model.User>\n";
+ 
+    @Test
+    public void testTerminatedNodeStatusPageDoesNotShowTrace() throws Exception {
+        DumbSlave agent = j.createOnlineSlave();
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.setAssignedNode(agent);
+ 
+        Future<FreeStyleBuild> r = ExecutorTest.startBlockingBuild(p);
+ 
+        String message = "It went away";
+        p.getLastBuild().getBuiltOn().toComputer().disconnect(
+                new OfflineCause.ChannelTermination(new RuntimeException(message))
+        );
+ 
+        WebClient wc = j.createWebClient();
+        Page page = wc.getPage(wc.createCrumbedUrl(agent.toComputer().getUrl()));
+        String content = page.getWebResponse().getContentAsString();
+        assertThat(content, not(containsString(message)));
+    }
+ 
+    @Test
+    public void testTerminatedNodeAjaxExecutorsDoesNotShowTrace() throws Exception {
+        DumbSlave agent = j.createOnlineSlave();
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.setAssignedNode(agent);
+ 
+        Future<FreeStyleBuild> r = ExecutorTest.startBlockingBuild(p);
+ 
+        String message = "It went away";
+        p.getLastBuild().getBuiltOn().toComputer().disconnect(
+                new OfflineCause.ChannelTermination(new RuntimeException(message))
+        );
+ 
+        WebClient wc = j.createWebClient();
+        Page page = wc.getPage(wc.createCrumbedUrl("ajaxExecutors"));
+        String content = page.getWebResponse().getContentAsString();
+        assertThat(content, not(containsString(message)));
+    }
+ 
 }

@@ -0,0 +1,86 @@
+package hudson.model;
+ 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+ 
+import hudson.Launcher;
+import hudson.remoting.VirtualChannel;
+import hudson.slaves.DumbSlave;
+import hudson.slaves.OfflineCause;
+import hudson.tasks.Builder;
+import hudson.util.OneShotEvent;
+import java.io.IOException;
+import java.util.concurrent.Future;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.TestExtension;
+ 
+public class ExecutorSEC2120Test {
+ 
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+ 
+    @Test
+    public void disconnectCause_WithoutTrace() throws Exception {
+        DumbSlave slave = j.createOnlineSlave();
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.setAssignedNode(slave);
+ 
+        Future<FreeStyleBuild> r = startBlockingBuild(p);
+ 
+        String message = "It went away";
+        p.getLastBuild().getBuiltOn().toComputer().disconnect(
+                new OfflineCause.ChannelTermination(new RuntimeException(message))
+        );
+ 
+        OfflineCause offlineCause = p.getLastBuild().getBuiltOn().toComputer().getOfflineCause();
+        assertThat(offlineCause.toString(), not(containsString(message)));
+    }
+ 
+    /**
+     * Start a project with an infinite build step
+     *
+     * @param project {@link FreeStyleProject} to start
+     * @return A {@link Future} object represents the started build
+     * @throws Exception if somethink wrong happened
+     */
+    public static Future<FreeStyleBuild> startBlockingBuild(FreeStyleProject project) throws Exception {
+        final OneShotEvent e = new OneShotEvent();
+ 
+        project.getBuildersList().add(new BlockingBuilder(e));
+ 
+        Future<FreeStyleBuild> r = project.scheduleBuild2(0);
+        e.block();  // wait until we are safe to interrupt
+        assertTrue(project.getLastBuild().isBuilding());
+ 
+        return r;
+    }
+ 
+    private static final class BlockingBuilder extends Builder {
+        private final OneShotEvent e;
+ 
+        private BlockingBuilder(OneShotEvent e) {
+            this.e = e;
+        }
+ 
+        @Override
+        public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
+            VirtualChannel channel = launcher.getChannel();
+            Node node = build.getBuiltOn();
+ 
+            e.signal(); // we are safe to be interrupted
+            for (;;) {
+                // Keep using the channel
+                channel.call(node.getClockDifferenceCallable());
+                Thread.sleep(100);
+            }
+        }
+ 
+        @TestExtension
+        public static class DescriptorImpl extends Descriptor<Builder> {}
+    }
+ 
+}