Skip to content

Commit

Permalink
[SECURITY-478] Require RUN_SCRIPTS before configuring CommandLauncher…
Browse files Browse the repository at this point in the history
… or CommandConnector.
  • Loading branch information
jglick committed Sep 13, 2017
1 parent 1b90346 commit d7ea3f4
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 12 deletions.
22 changes: 22 additions & 0 deletions core/src/main/java/hudson/slaves/CommandConnector.java
Expand Up @@ -25,11 +25,15 @@

import hudson.EnvVars;
import hudson.Extension;
import hudson.Util;
import hudson.model.TaskListener;
import hudson.util.FormValidation;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;

import java.io.IOException;
import jenkins.model.Jenkins;
import org.kohsuke.stapler.QueryParameter;

/**
* Executes a program on the master and expect that script to connect.
Expand All @@ -42,6 +46,12 @@ public class CommandConnector extends ComputerConnector {
@DataBoundConstructor
public CommandConnector(String command) {
this.command = command;
Jenkins.getInstance().checkPermission(Jenkins.RUN_SCRIPTS);
}

private Object readResolve() {
Jenkins.getInstance().checkPermission(Jenkins.RUN_SCRIPTS);
return this;
}

@Override
Expand All @@ -55,5 +65,17 @@ public static class DescriptorImpl extends ComputerConnectorDescriptor {
public String getDisplayName() {
return Messages.CommandLauncher_displayName();
}

public FormValidation doCheckCommand(@QueryParameter String value) {
if (!Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS)) {
return FormValidation.warning(Messages.CommandLauncher_cannot_be_configured_by_non_administrato());
}
if (Util.fixEmptyAndTrim(value) == null) {
return FormValidation.error(Messages.CommandLauncher_NoLaunchCommand());
} else {
return FormValidation.ok();
}
}

}
}
10 changes: 10 additions & 0 deletions core/src/main/java/hudson/slaves/CommandLauncher.java
Expand Up @@ -33,6 +33,7 @@
import jenkins.model.Jenkins;
import hudson.model.TaskListener;
import hudson.remoting.Channel;
import hudson.security.ACL;
import hudson.util.StreamCopyThread;
import hudson.util.FormValidation;
import hudson.util.ProcessTree;
Expand Down Expand Up @@ -74,6 +75,12 @@ public CommandLauncher(String command) {
public CommandLauncher(String command, EnvVars env) {
this.agentCommand = command;
this.env = env;
Jenkins.getInstance().checkPermission(Jenkins.RUN_SCRIPTS);
}

private Object readResolve() {
Jenkins.getInstance().checkPermission(Jenkins.RUN_SCRIPTS);
return this;
}

public String getCommand() {
Expand Down Expand Up @@ -191,6 +198,9 @@ public String getDisplayName() {
}

public FormValidation doCheckCommand(@QueryParameter String value) {
if (!Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS)) {
return FormValidation.warning(Messages.CommandLauncher_cannot_be_configured_by_non_administrato());
}
if(Util.fixEmptyAndTrim(value)==null)
return FormValidation.error(Messages.CommandLauncher_NoLaunchCommand());
else
Expand Down
1 change: 1 addition & 0 deletions core/src/main/resources/hudson/slaves/Messages.properties
Expand Up @@ -27,6 +27,7 @@ CommandLauncher.displayName=Launch agent via execution of command on the master
JNLPLauncher.displayName=Launch agent via Java Web Start
ComputerLauncher.unexpectedError=Unexpected error in launching an agent. This is probably a bug in Jenkins
ComputerLauncher.abortedLaunch=Launching agent process aborted.
CommandLauncher.cannot_be_configured_by_non_administrato=cannot be configured by non-administrators
CommandLauncher.NoLaunchCommand=No launch command specified
ConnectionActivityMonitor.OfflineCause=Repeated ping attempts failed
DumbSlave.displayName=Permanent Agent
Expand Down
22 changes: 10 additions & 12 deletions test/src/test/java/hudson/model/ProjectTest.java
Expand Up @@ -27,7 +27,6 @@
import com.gargoylesoftware.htmlunit.WebRequest;
import hudson.model.queue.QueueTaskFuture;
import hudson.security.AccessDeniedException2;
import org.acegisecurity.context.SecurityContextHolder;
import hudson.security.HudsonPrivateSecurityRealm;
import hudson.security.GlobalMatrixAuthorizationStrategy;

Expand Down Expand Up @@ -67,6 +66,8 @@
import hudson.EnvVars;
import hudson.model.labels.LabelAtom;
import hudson.scm.SCMDescriptor;
import hudson.security.ACL;
import hudson.security.ACLContext;
import hudson.slaves.Cloud;
import hudson.slaves.DumbSlave;
import hudson.slaves.NodeProvisioner;
Expand Down Expand Up @@ -537,8 +538,7 @@ public void testDoCancelQueue() throws Exception{
HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false);
j.jenkins.setSecurityRealm(realm);
User user = realm.createAccount("John Smith", "password");
SecurityContextHolder.getContext().setAuthentication(user.impersonate());
try{
try (ACLContext as = ACL.as(user)) {
project.doCancelQueue(null, null);
fail("User should not have permission to build project");
}
Expand All @@ -558,8 +558,7 @@ public void testDoDoDelete() throws Exception{
HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false);
j.jenkins.setSecurityRealm(realm);
User user = realm.createAccount("John Smith", "password");
SecurityContextHolder.getContext().setAuthentication(user.impersonate());
try{
try (ACLContext as = ACL.as(user)) {
project.doDoDelete(null, null);
fail("User should not have permission to build project");
}
Expand Down Expand Up @@ -590,8 +589,7 @@ public void testDoDoWipeOutWorkspace() throws Exception{
HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false);
j.jenkins.setSecurityRealm(realm);
User user = realm.createAccount("John Smith", "password");
SecurityContextHolder.getContext().setAuthentication(user.impersonate());
try{
try (ACLContext as = ACL.as(user)) {
project.doDoWipeOutWorkspace();
fail("User should not have permission to build project");
}
Expand Down Expand Up @@ -624,8 +622,7 @@ public void testDoDisable() throws Exception{
HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false);
j.jenkins.setSecurityRealm(realm);
User user = realm.createAccount("John Smith", "password");
SecurityContextHolder.getContext().setAuthentication(user.impersonate());
try{
try (ACLContext as = ACL.as(user)) {
project.doDisable();
fail("User should not have permission to build project");
}
Expand Down Expand Up @@ -655,9 +652,10 @@ public void testDoEnable() throws Exception{
HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false);
j.jenkins.setSecurityRealm(realm);
User user = realm.createAccount("John Smith", "password");
SecurityContextHolder.getContext().setAuthentication(user.impersonate());
project.disable();
try{
try (ACLContext as = ACL.as(user)) {
project.disable();
}
try (ACLContext as = ACL.as(user)) {
project.doEnable();
fail("User should not have permission to build project");
}
Expand Down
140 changes: 140 additions & 0 deletions test/src/test/java/hudson/slaves/CommandLauncher2Test.java
@@ -0,0 +1,140 @@
/*
* The MIT License
*
* Copyright 2017 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.slaves;

import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.HttpMethod;
import com.gargoylesoftware.htmlunit.WebRequest;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
import hudson.cli.CLICommand;
import hudson.cli.CLICommandInvoker;
import hudson.cli.UpdateNodeCommand;
import hudson.model.Computer;
import hudson.model.User;
import java.net.HttpURLConnection;
import jenkins.model.Jenkins;
import org.apache.tools.ant.filters.StringInputStream;
import static org.hamcrest.Matchers.containsString;
import org.junit.Test;
import static org.junit.Assert.*;
import org.junit.Rule;
import org.junit.runners.model.Statement;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.MockAuthorizationStrategy;
import org.jvnet.hudson.test.RestartableJenkinsRule;

public class CommandLauncher2Test {

@Rule
public RestartableJenkinsRule rr = new RestartableJenkinsRule();

@Issue("SECURITY-478")
@Test
public void requireRunScripts() throws Exception {
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
rr.j.jenkins.setSecurityRealm(rr.j.createDummySecurityRealm());
rr.j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
grant(Jenkins.ADMINISTER).everywhere().to("admin").
grant(Jenkins.READ, Computer.CONFIGURE).everywhere().to("dev"));
DumbSlave s = new DumbSlave("s", "/", new CommandLauncher("echo unconfigured"));
rr.j.jenkins.addNode(s);
// First, reconfigure using GUI.
JenkinsRule.WebClient wc = rr.j.createWebClient().login("admin");
HtmlForm form = wc.getPage(s, "configure").getFormByName("config");
HtmlTextInput input = form.getInputByName("_.command");
assertEquals("echo unconfigured", input.getText());
input.setText("echo configured by GUI");
rr.j.submit(form);
s = (DumbSlave) rr.j.jenkins.getNode("s");
assertEquals("echo configured by GUI", ((CommandLauncher) s.getLauncher()).getCommand());
// Then by REST.
String configDotXml = s.toComputer().getUrl() + "config.xml";
String xml = wc.goTo(configDotXml, "application/xml").getWebResponse().getContentAsString();
assertThat(xml, containsString("echo configured by GUI"));
WebRequest req = new WebRequest(wc.createCrumbedUrl(configDotXml), HttpMethod.POST);
req.setEncodingType(null);
req.setRequestBody(xml.replace("echo configured by GUI", "echo configured by REST"));
wc.getPage(req);
s = (DumbSlave) rr.j.jenkins.getNode("s");
assertEquals("echo configured by REST", ((CommandLauncher) s.getLauncher()).getCommand());
// Then by CLI.
CLICommand cmd = new UpdateNodeCommand();
cmd.setTransportAuth(User.get("admin").impersonate());
assertThat(new CLICommandInvoker(rr.j, cmd).withStdin(new StringInputStream(xml.replace("echo configured by GUI", "echo configured by CLI"))).invokeWithArgs("s"), CLICommandInvoker.Matcher.succeededSilently());
s = (DumbSlave) rr.j.jenkins.getNode("s");
assertEquals("echo configured by CLI", ((CommandLauncher) s.getLauncher()).getCommand());
// Now verify that all modes failed as dev. First as GUI.
s.setLauncher(new CommandLauncher("echo configured by admin"));
wc = rr.j.createWebClient().login("dev");
form = wc.getPage(s, "configure").getFormByName("config");
input = form.getInputByName("_.command");
assertEquals("echo configured by admin", input.getText());
input.setText("echo ATTACK");
try {
rr.j.submit(form);
fail();
} catch (FailingHttpStatusCodeException x) {
assertEquals("403 would be more natural but Descriptor.newInstance wraps AccessDeniedException2 in Error", 500, x.getStatusCode());
}
s = (DumbSlave) rr.j.jenkins.getNode("s");
assertEquals("echo configured by admin", ((CommandLauncher) s.getLauncher()).getCommand());
// Then by REST.
req = new WebRequest(wc.createCrumbedUrl(configDotXml), HttpMethod.POST);
req.setEncodingType(null);
req.setRequestBody(xml.replace("echo configured by GUI", "echo ATTACK"));
try {
wc.getPage(req);
} catch (FailingHttpStatusCodeException x) {
assertEquals(HttpURLConnection.HTTP_FORBIDDEN, x.getStatusCode());
}
s = (DumbSlave) rr.j.jenkins.getNode("s");
assertNotEquals(CommandLauncher.class, s.getLauncher().getClass()); // currently seems to reset it to JNLPLauncher, whatever
s.setLauncher(new CommandLauncher("echo configured by admin"));
// Then by CLI.
cmd = new UpdateNodeCommand();
cmd.setTransportAuth(User.get("dev").impersonate());
assertThat(new CLICommandInvoker(rr.j, cmd).withStdin(new StringInputStream(xml.replace("echo configured by GUI", "echo ATTACK"))).invokeWithArgs("s"),
CLICommandInvoker.Matcher./* gets swallowed by RobustReflectionConverter, hmm*/succeededSilently());
s = (DumbSlave) rr.j.jenkins.getNode("s");
assertNotEquals(CommandLauncher.class, s.getLauncher().getClass());
// Now also check that SYSTEM deserialization works after a restart.
s.setLauncher(new CommandLauncher("echo configured by admin"));
s.save();
}
});
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
DumbSlave s = (DumbSlave) rr.j.jenkins.getNode("s");
assertEquals("echo configured by admin", ((CommandLauncher) s.getLauncher()).getCommand());
}
});
}

}

0 comments on commit d7ea3f4

Please sign in to comment.