Skip to content
Permalink
Browse files
Jenkins 47593 avoid script block (#253)
* [JENKINS-47593][SECURITY-643] Allow command to be run without approval
- use the second constructor of CommandLauncher to avoid being blocked by the security fix

* - adding permission check on unsafe parameter modification
- adding warning message in case someone is modifying the unsafe parameters

* - also put the right check on the readResolve
- regroup the permission error message

* - change name of the required permission

* [SECURITY-643] Fix exception during start of cloud config

* [maven-release-plugin] prepare release ec2-1.38

* [maven-release-plugin] prepare for next development iteration
  • Loading branch information
Francis Upton IV committed Dec 16, 2017
1 parent fd17282 commit 180f7d0eae6031d67259a5d86d9d7d382f9eb05b
Showing 6 changed files with 88 additions and 11 deletions.
@@ -34,7 +34,7 @@ THE SOFTWARE.
</parent>

<artifactId>ec2</artifactId>
<version>1.38-SNAPSHOT</version>
<version>1.39-SNAPSHOT</version>
<packaging>hpi</packaging>
<name>Amazon EC2 plugin</name>
<url>http://wiki.jenkins-ci.org/display/JENKINS/Amazon+EC2+Plugin</url>
@@ -23,6 +23,7 @@
*/
package hudson.plugins.ec2;

import com.amazonaws.SdkClientException;
import hudson.Extension;
import hudson.Util;
import hudson.model.Failure;
@@ -152,13 +153,18 @@ public ListBoxModel doFillRegionItems(@QueryParameter boolean useInstanceProfile
return model;
}

AWSCredentialsProvider credentialsProvider = createCredentialsProvider(useInstanceProfileForCredentials, credentialsId);
AmazonEC2 client = connect(credentialsProvider, new URL("http://ec2.amazonaws.com"));
DescribeRegionsResult regions = client.describeRegions();
List<Region> regionList = regions.getRegions();
for (Region r : regionList) {
String name = r.getRegionName();
model.add(name, name);
try {
AWSCredentialsProvider credentialsProvider = createCredentialsProvider(useInstanceProfileForCredentials,
credentialsId);
AmazonEC2 client = connect(credentialsProvider, new URL("http://ec2.amazonaws.com"));
DescribeRegionsResult regions = client.describeRegions();
List<Region> regionList = regions.getRegions();
for (Region r : regionList) {
String name = r.getRegionName();
model.add(name, name);
}
} catch (SdkClientException ex) {
// Ignore, as this may happen before the credentials are specified
}
return model;
}
@@ -34,6 +34,8 @@

import org.apache.commons.codec.binary.Base64;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;

@@ -152,6 +154,15 @@ public SlaveTemplate(String ami, String zone, SpotConfiguration spotConfig, Stri
boolean usePrivateDnsName, String instanceCapStr, String iamInstanceProfile, boolean deleteRootOnTermination,
boolean useEphemeralDevices, boolean useDedicatedTenancy, String launchTimeoutStr, boolean associatePublicIp,
String customDeviceMapping, boolean connectBySSHProcess, boolean connectUsingPublicIp) {

if(StringUtils.isNotBlank(remoteAdmin) || StringUtils.isNotBlank(jvmopts) || StringUtils.isNotBlank(tmpDir)){
LOGGER.log(Level.FINE, "As remoteAdmin, jvmopts or tmpDir is not blank, we must ensure the user has RUN_SCRIPTS rights.");
Jenkins j = Jenkins.getInstance();
if(j != null){
j.checkPermission(Jenkins.RUN_SCRIPTS);
}
}

this.ami = ami;
this.zone = zone;
this.spotConfig = spotConfig;
@@ -1011,6 +1022,8 @@ public EC2AbstractSlave attach(String instanceId, TaskListener listener) throws
* Initializes data structure that we don't persist.
*/
protected Object readResolve() {
Jenkins.getInstance().checkPermission(Jenkins.RUN_SCRIPTS);

labelSet = Label.parse(labels);
securityGroupSet = parseSecurityGroups();

@@ -1089,6 +1102,33 @@ public String getHelpFile(String fieldName) {
return p;
}

@Restricted(NoExternalUse.class)
public FormValidation doCheckRemoteAdmin(@QueryParameter String value){
if(StringUtils.isBlank(value) || Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS)){
return FormValidation.ok();
}else{
return FormValidation.error(Messages.General_MissingPermission());
}
}

@Restricted(NoExternalUse.class)
public FormValidation doCheckTmpDir(@QueryParameter String value){
if(StringUtils.isBlank(value) || Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS)){
return FormValidation.ok();
}else{
return FormValidation.error(Messages.General_MissingPermission());
}
}

@Restricted(NoExternalUse.class)
public FormValidation doCheckJvmopts(@QueryParameter String value){
if(StringUtils.isBlank(value) || Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS)){
return FormValidation.ok();
}else{
return FormValidation.error(Messages.General_MissingPermission());
}
}

/***
* Check that the AMI requested is available in the cloud and can be used.
*/
@@ -3,11 +3,15 @@
import hudson.Extension;
import hudson.model.Descriptor;

import hudson.util.FormValidation;
import jenkins.model.Jenkins;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;

public class UnixData extends AMITypeData {

private final String rootCommandPrefix;
private final String slaveCommandPrefix;
private final String sshPort;
@@ -17,6 +21,13 @@ public UnixData(String rootCommandPrefix, String slaveCommandPrefix, String sshP
this.rootCommandPrefix = rootCommandPrefix;
this.slaveCommandPrefix = slaveCommandPrefix;
this.sshPort = sshPort;

this.readResolve();
}

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

@Override
@@ -35,6 +46,24 @@ public static class DescriptorImpl extends Descriptor<AMITypeData> {
public String getDisplayName() {
return "unix";
}

@Restricted(NoExternalUse.class)
public FormValidation doCheckRootCommandPrefix(@QueryParameter String value){
if(StringUtils.isBlank(value) || Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS)){
return FormValidation.ok();
}else{
return FormValidation.error(Messages.General_MissingPermission());
}
}

@Restricted(NoExternalUse.class)
public FormValidation doCheckSlaveCommandPrefix(@QueryParameter String value){
if(StringUtils.isBlank(value) || Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS)){
return FormValidation.ok();
}else{
return FormValidation.error(Messages.General_MissingPermission());
}
}
}

public String getRootCommandPrefix() {
@@ -204,7 +204,7 @@ protected void launch(EC2Computer computer, TaskListener listener, Instance inst
String sshClientLaunchString = String.format("ssh -o StrictHostKeyChecking=no -i %s %s@%s -p %d %s", identityKeyFile.getAbsolutePath(), node.remoteAdmin, getEC2HostAddress(computer, inst), node.getSshPort(), launchString);

logInfo(computer, listener, "Launching slave agent (via SSH client process): " + sshClientLaunchString);
CommandLauncher commandLauncher = new CommandLauncher(sshClientLaunchString);
CommandLauncher commandLauncher = new CommandLauncher(sshClientLaunchString, null);
commandLauncher.launch(computer, listener);
} finally {
identityKeyFile.delete();
@@ -8,4 +8,6 @@ EC2SpotSlave.AmazonEC2SpotInstance=Amazon EC2 Spot Instance
EC2SpotSlave.Spot1=Spot $
EC2SpotSlave.Spot2= max bid price

AmazonEC2Cloud.NonUniqName=Cloud name must be unique across EC2 clouds
AmazonEC2Cloud.NonUniqName=Cloud name must be unique across EC2 clouds

General.MissingPermission=You do not have the Overall/RunScripts right to modify this field

0 comments on commit 180f7d0

Please sign in to comment.