diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java index c265e477df..02e817729b 100644 --- a/core/src/main/java/hudson/model/Queue.java +++ b/core/src/main/java/hudson/model/Queue.java @@ -113,6 +113,7 @@ import org.acegisecurity.AccessDeniedException; import org.acegisecurity.Authentication; import org.jenkinsci.bytecode.AdaptField; import org.jenkinsci.remoting.RoleChecker; +import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.export.Exported; @@ -779,18 +780,68 @@ public class Queue extends ResourceController implements Saveable { @Exported(inline=true) public Item[] getItems() { Snapshot s = this.snapshot; - Item[] r = new Item[s.waitingList.size() + s.blockedProjects.size() + s.buildables.size() + - s.pendings.size()]; - s.waitingList.toArray(r); - int idx = s.waitingList.size(); - for (BlockedItem p : s.blockedProjects) { - r[idx++] = p; + List r = new ArrayList(); + + for(WaitingItem p : s.waitingList) { + r = filterItemListBasedOnPermissions(r, p); + } + for (BlockedItem p : s.blockedProjects){ + r = filterItemListBasedOnPermissions(r, p); } for (BuildableItem p : reverse(s.buildables)) { - r[idx++] = p; + r = filterItemListBasedOnPermissions(r, p); } for (BuildableItem p : reverse(s.pendings)) { - r[idx++] = p; + r= filterItemListBasedOnPermissions(r, p); + } + Item[] items = new Item[r.size()]; + r.toArray(items); + return items; + } + + private List filterItemListBasedOnPermissions(List r, Item t) { + if (t.task instanceof hudson.model.Item) { + if (((hudson.model.Item)t.task).hasPermission(hudson.model.Item.READ)) { + r.add(t); + } + } + return r; + } + + /** + * Returns an array of Item for which it is only visible the name of the task. + * + * Generally speaking the array is sorted such that the items that are most likely built sooner are + * at the end. + */ + @Restricted(NoExternalUse.class) + @Exported(inline=true) + public StubItem[] getDiscoverableItems() { + Snapshot s = this.snapshot; + List r = new ArrayList(); + + for(WaitingItem p : s.waitingList) { + r = filterDiscoverableItemListBasedOnPermissions(r, p); + } + for (BlockedItem p : s.blockedProjects){ + r = filterDiscoverableItemListBasedOnPermissions(r, p); + } + for (BuildableItem p : reverse(s.buildables)) { + r = filterDiscoverableItemListBasedOnPermissions(r, p); + } + for (BuildableItem p : reverse(s.pendings)) { + r= filterDiscoverableItemListBasedOnPermissions(r, p); + } + StubItem[] items = new StubItem[r.size()]; + r.toArray(items); + return items; + } + + private List filterDiscoverableItemListBasedOnPermissions(List r, Item t) { + if (t.task instanceof hudson.model.Item) { + if (!((hudson.model.Item)t.task).hasPermission(hudson.model.Item.READ) && ((hudson.model.Item)t.task).hasPermission(hudson.model.Item.DISCOVER)) { + r.add(new StubItem(new StubTask(t.task))); + } } return r; } @@ -2160,6 +2211,42 @@ public class Queue extends ResourceController implements Saveable { } + /** + * A Stub class for {@link Task} which exposes only the name of the Task to be displayed when the user + * has DISCOVERY permissions only. + */ + @Restricted(NoExternalUse.class) + @ExportedBean(defaultVisibility = 999) + public static class StubTask { + + private String name; + + public StubTask(@Nonnull Queue.Task base) { + this.name = base.getName(); + } + + @Exported + public String getName() { + return name; + } + } + + /** + * A Stub class for {@link Item} which exposes only the name of the Task to be displayed when the user + * has DISCOVERY permissions only. + */ + @Restricted(NoExternalUse.class) + @ExportedBean(defaultVisibility = 999) + public class StubItem { + + @Exported public StubTask task; + + public StubItem(StubTask task) { + this.task = task; + } + + } + /** * An optional interface for actions on Queue.Item. * Lets the action cooperate in queue management. diff --git a/core/src/main/java/hudson/node_monitors/ResponseTimeMonitor.java b/core/src/main/java/hudson/node_monitors/ResponseTimeMonitor.java index 69da2e6bcd..fea2555687 100644 --- a/core/src/main/java/hudson/node_monitors/ResponseTimeMonitor.java +++ b/core/src/main/java/hudson/node_monitors/ResponseTimeMonitor.java @@ -188,7 +188,7 @@ public class ResponseTimeMonitor extends NodeMonitor { } /** - * HTML rendering of the data + * String rendering of the data */ @Override public String toString() { @@ -200,7 +200,7 @@ public class ResponseTimeMonitor extends NodeMonitor { // return buf.toString(); int fc = failureCount(); if(fc>0) - return Util.wrapToErrorSpan(Messages.ResponseTimeMonitor_TimeOut(fc)); + return Messages.ResponseTimeMonitor_TimeOut(fc); return getAverage()+"ms"; } diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java index 553d74afb4..b51ea86df2 100644 --- a/core/src/main/java/hudson/slaves/SlaveComputer.java +++ b/core/src/main/java/hudson/slaves/SlaveComputer.java @@ -468,7 +468,7 @@ public class SlaveComputer extends Computer { /** * Sets up the connection through an existing channel. - * + * @param channel the channel to use; warning: callers are expected to have called {@link ChannelConfigurator} already * @since 1.444 */ public void setChannel(Channel channel, OutputStream launchLog, Channel.Listener listener) throws IOException, InterruptedException { diff --git a/core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java b/core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java index d4386567c0..db08fa3c92 100644 --- a/core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java +++ b/core/src/main/java/jenkins/slaves/DefaultJnlpSlaveReceiver.java @@ -92,7 +92,7 @@ public class DefaultJnlpSlaveReceiver extends JnlpAgentReceiver { String handshakeSecret = handshake.getRequestProperty("Secret-Key"); // Verify that the slave secret matches the handshake secret. if (!computer.getJnlpMac().equals(handshakeSecret)) { - LOGGER.log(Level.WARNING, "An attempt was made to connect as {0} from {1} with an incorrect secret", new Object[]{nodeName, handshake.getSocket().getRemoteSocketAddress()}); + LOGGER.log(Level.WARNING, "An attempt was made to connect as {0} from {1} with an incorrect secret", new Object[]{nodeName, handshake.getSocket()!=null?handshake.getSocket().getRemoteSocketAddress():null}); return false; } else { return true; diff --git a/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java b/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java index 06e54349f5..b82826fd57 100644 --- a/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java +++ b/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol.java @@ -9,6 +9,7 @@ import hudson.remoting.Engine; import hudson.slaves.SlaveComputer; import jenkins.AgentProtocol; import jenkins.model.Jenkins; +import jenkins.security.ChannelConfigurator; import jenkins.security.HMACConfidentialKey; import org.jenkinsci.remoting.nio.NioChannelHub; @@ -118,6 +119,10 @@ public class JnlpSlaveAgentProtocol extends AgentProtocol { try { ChannelBuilder cb = createChannelBuilder(nodeName); + for (ChannelConfigurator cc : ChannelConfigurator.all()) { + cc.onChannelBuilding(cb, computer); + } + computer.setChannel(cb.withHeaderStream(log).build(socket), log, new Listener() { @Override diff --git a/core/src/main/resources/hudson/slaves/OfflineCause/cause.jelly b/core/src/main/resources/hudson/slaves/OfflineCause/cause.jelly index a6676efe9f..51a05a144a 100644 --- a/core/src/main/resources/hudson/slaves/OfflineCause/cause.jelly +++ b/core/src/main/resources/hudson/slaves/OfflineCause/cause.jelly @@ -29,7 +29,7 @@ THE SOFTWARE.
- + ${it}
diff --git a/core/src/test/java/jenkins/slaves/DefaultJnlpSlaveReceiverTest.java b/core/src/test/java/jenkins/slaves/DefaultJnlpSlaveReceiverTest.java index 9cc6425d34..7fe3d9253b 100644 --- a/core/src/test/java/jenkins/slaves/DefaultJnlpSlaveReceiverTest.java +++ b/core/src/test/java/jenkins/slaves/DefaultJnlpSlaveReceiverTest.java @@ -56,6 +56,8 @@ public class DefaultJnlpSlaveReceiverTest { when(mockComputer.getChannel()).thenReturn(null); when(mockChannel.getProperty(any(String.class))).thenReturn("some cookie"); when(mockHandshake.jnlpConnect(mockComputer)).thenReturn(mockChannel); + when(mockHandshake.getRequestProperty("Secret-Key")).thenReturn("mock-secret"); + when(mockComputer.getJnlpMac()).thenReturn("mock-secret"); assertTrue(receiver.handle("node", mockHandshake)); verify(mockHandshake).success(any(Properties.class)); @@ -77,6 +79,8 @@ public class DefaultJnlpSlaveReceiverTest { when(mockChannel.getProperty(any(String.class))).thenReturn("some cookie"); when(mockComputer.disconnect(any(ConnectionFromCurrentPeer.class))).thenReturn(mockFuture); when(mockHandshake.jnlpConnect(mockComputer)).thenReturn(mockChannel); + when(mockHandshake.getRequestProperty("Secret-Key")).thenReturn("mock-secret"); + when(mockComputer.getJnlpMac()).thenReturn("mock-secret"); assertTrue(receiver.handle("node", mockHandshake)); verify(mockFuture).get(15, TimeUnit.SECONDS); diff --git a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java index 778294e034..09e2400cbb 100644 --- a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java +++ b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java @@ -26,13 +26,17 @@ package hudson.bugs; import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.html.HtmlPage; import com.gargoylesoftware.htmlunit.xml.XmlPage; +import hudson.cli.util.ScriptLoader; import hudson.model.Node.Mode; import hudson.model.Slave; +import hudson.remoting.Channel; import hudson.remoting.Launcher; import hudson.remoting.Which; import hudson.slaves.JNLPLauncher; import hudson.slaves.RetentionStrategy; import hudson.slaves.DumbSlave; +import jenkins.security.MasterToSlaveCallable; +import jenkins.security.s2m.AdminWhitelistRule; import org.dom4j.Document; import org.dom4j.Element; import org.dom4j.io.DOMReader; @@ -41,6 +45,7 @@ import org.jvnet.hudson.test.HudsonTestCase; import org.jvnet.hudson.test.recipes.PresetData; import org.jvnet.hudson.test.recipes.PresetData.DataSet; +import java.io.File; import java.net.HttpURLConnection; import java.net.URL; import java.util.Collections; @@ -112,6 +117,17 @@ public class JnlpAccessWithSecuredHudsonTest extends HudsonTestCase { for (int i = 0; i < /* one minute */600; i++) { if (slave.getComputer().isOnline()) { System.err.println("JNLP slave successfully connected"); + Channel channel = slave.getComputer().getChannel(); + assertFalse("SECURITY-206", channel.isRemoteClassLoadingAllowed()); + jenkins.getExtensionList(AdminWhitelistRule.class).get(AdminWhitelistRule.class).setMasterKillSwitch(false); + final File f = new File(jenkins.getRootDir(), "config.xml"); + assertTrue(f.exists()); + try { + fail("SECURITY-206: " + channel.call(new Attack(f.getAbsolutePath()))); + } catch (SecurityException x) { + System.out.println("expected: " + x); + assertTrue(x.getMessage().contains("http://jenkins-ci.org/security-144")); + } return; } Thread.sleep(100); @@ -122,4 +138,15 @@ public class JnlpAccessWithSecuredHudsonTest extends HudsonTestCase { } } + private static class Attack extends MasterToSlaveCallable { + private final String path; + Attack(String path) { + this.path = path; + } + @Override + public String call() throws Exception { + return Channel.current().call(new ScriptLoader(path)); + } + } + } diff --git a/test/src/test/java/hudson/model/QueueTest.java b/test/src/test/java/hudson/model/QueueTest.java index 3cc1ebf872..e8b6293794 100644 --- a/test/src/test/java/hudson/model/QueueTest.java +++ b/test/src/test/java/hudson/model/QueueTest.java @@ -23,6 +23,8 @@ */ package hudson.model; +import com.gargoylesoftware.htmlunit.html.DomElement; +import com.gargoylesoftware.htmlunit.html.DomNode; import com.gargoylesoftware.htmlunit.html.HtmlFileInput; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlFormUtil; @@ -45,11 +47,15 @@ import hudson.model.Queue.WaitingItem; import hudson.model.labels.LabelExpression; import hudson.model.queue.AbstractQueueTask; import hudson.model.queue.CauseOfBlockage; +import hudson.model.queue.QueueTaskDispatcher; import hudson.model.queue.QueueTaskFuture; import hudson.model.queue.ScheduleResult; import hudson.model.queue.SubTask; import hudson.security.ACL; +import hudson.security.AuthorizationMatrixProperty; import hudson.security.GlobalMatrixAuthorizationStrategy; +import hudson.security.Permission; +import hudson.security.ProjectMatrixAuthorizationStrategy; import hudson.security.SparseACL; import hudson.slaves.DumbSlave; import hudson.slaves.DummyCloudImpl; @@ -67,7 +73,10 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -562,11 +571,11 @@ public class QueueTest { FreeStyleBuild b = v.waitForStart(); assertEquals(1,b.getNumber()); assertTrue(b.isBuilding()); - assertSame(p,b.getProject()); + assertSame(p, b.getProject()); ev.signal(); // let the build complete FreeStyleBuild b2 = r.assertBuildStatusSuccess(v); - assertSame(b,b2); + assertSame(b, b2); } /** @@ -891,4 +900,69 @@ public class QueueTest { @TestExtension("shouldBeAbleToBlockFlyWeightTaskOnLastMinute") public static class DescriptorImpl extends NodePropertyDescriptor {} } + + @Test + public void queueApiOutputShouldBeFilteredByUserPermission() throws Exception { + + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + ProjectMatrixAuthorizationStrategy str = new ProjectMatrixAuthorizationStrategy(); + str.add(Jenkins.READ, "bob"); + str.add(Jenkins.READ, "alice"); + str.add(Jenkins.READ, "james"); + r.jenkins.setAuthorizationStrategy(str); + + FreeStyleProject project = r.createFreeStyleProject("project"); + + Map> permissions = new HashMap>(); + permissions.put(Item.READ, Collections.singleton("bob")); + permissions.put(Item.DISCOVER, Collections.singleton("james")); + AuthorizationMatrixProperty prop1 = new AuthorizationMatrixProperty(permissions); + project.addProperty(prop1); + project.getBuildersList().add(new SleepBuilder(10)); + project.scheduleBuild2(0); + + JenkinsRule.WebClient webClient = r.createWebClient(); + webClient.login("bob", "bob"); + XmlPage p = webClient.goToXml("queue/api/xml"); + + //bob has permission on the project and will be able to see it in the queue together with information such as the URL and the name. + for (DomNode element: p.getFirstChild().getFirstChild().getChildNodes()){ + if (element.getNodeName().equals("task")) { + for (DomNode child: ((DomElement) element).getChildNodes()) { + if (child.getNodeName().equals("name")) { + assertEquals(child.asText(), "project"); + } else if (child.getNodeName().equals("url")) { + assertNotNull(child.asText()); + } + } + } + } + webClient = r.createWebClient(); + webClient.login("alice"); + XmlPage p2 = webClient.goToXml("queue/api/xml"); + //alice does not have permission on the project and will not see it in the queue. + assertEquals("", p2.getContent()); + + webClient = r.createWebClient(); + webClient.login("james"); + XmlPage p3 = webClient.goToXml("queue/api/xml"); + //james has DISCOVER permission on the project and will only be able to see the task name. + assertEquals("project", + p3.getContent()); + + } + + //we force the project not to be executed so that it stays in the queue + @TestExtension("queueApiOutputShouldBeFilteredByUserPermission") + public static class MyQueueTaskDispatcher extends QueueTaskDispatcher { + @Override + public CauseOfBlockage canTake(Node node, Queue.BuildableItem item) { + return new CauseOfBlockage() { + @Override + public String getShortDescription() { + return "blocked by canTake"; + } + }; + } + } }