Skip to content

Commit

Permalink
SECURITY-3362
Browse files Browse the repository at this point in the history
  • Loading branch information
Kevin-CB committed Nov 4, 2024
1 parent 55dd42a commit 478dd9e
Show file tree
Hide file tree
Showing 5 changed files with 227 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,17 @@
import net.sf.json.JSON;
import net.sf.json.JSONObject;
import org.acegisecurity.AccessDeniedException;
import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext;
import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException;
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
import org.jenkinsci.plugins.workflow.flow.FlowExecution;
import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
Expand Down Expand Up @@ -119,7 +124,8 @@ private ReplayAction(Run run) {
}

/** Fetches execution, blocking if needed while we wait for some of the loading process. */
private @CheckForNull CpsFlowExecution getExecutionBlocking() {
@Restricted(NoExternalUse.class)
public @CheckForNull CpsFlowExecution getExecutionBlocking() {
FlowExecutionOwner owner = ((FlowExecutionOwner.Executable) run).asFlowExecutionOwner();
if (owner == null) {
return null;
Expand Down Expand Up @@ -163,6 +169,14 @@ private ReplayAction(Run run) {
}
}

private boolean isSandboxed() {
CpsFlowExecution exec = getExecutionLazy();
if (exec != null) {
return exec.isSandbox();
}
return false;
}

/** Runs the extra tests for replayability beyond {@link #isEnabled()} that require a blocking load of the execution. */
/* accessible to Jelly */ public boolean isReplayableSandboxTest() {
CpsFlowExecution exec = getExecutionBlocking();
Expand Down Expand Up @@ -261,6 +275,16 @@ public void doRebuild(StaplerRequest req, StaplerResponse rsp) throws ServletExc
if (execution == null) {
return null;
}

if (!execution.isSandbox()) {
ScriptApproval.get().configuring(replacementMainScript,GroovyLanguage.get(), ApprovalContext.create(), true);
try {
ScriptApproval.get().using(replacementMainScript, GroovyLanguage.get());
} catch (UnapprovedUsageException e) {
throw new Failure("The script is not approved.");
}
}

actions.add(new ReplayFlowFactoryAction(replacementMainScript, replacementLoadedScripts, execution.isSandbox()));
actions.add(new CauseAction(new Cause.UserIdCause(), new ReplayCause(run)));

Expand Down Expand Up @@ -357,12 +381,26 @@ private static String diff(String script, String oldText, String nueText) throws
return hunks.isEmpty() ? "" : hunks.toUnifiedDiff("old/" + script, "new/" + script, new StringReader(oldText), new StringReader(nueText), 3);
}

// Stub, we do not need to do anything here.
/**
* Loaded scripts do not need to be approved.
*/
@RequirePOST
public FormValidation doCheckScript() {
public FormValidation doCheckLoadedScript() {
return FormValidation.ok();
}

/**
* Form validation for the main script
* Jelly only
* @param value the script being checked
* @return a message indicating that the script needs to be approved; nothing if the script is empty;
* a corresponding message if the script is approved
*/
@RequirePOST
public FormValidation doCheckScript(@QueryParameter String value) {
return Jenkins.get().getDescriptorByType(CpsFlowDefinition.DescriptorImpl.class).doCheckScript(value, "", isSandboxed());
}

@RequirePOST
public JSON doCheckScriptCompile(@AncestorInPath Item job, @QueryParameter String value) {
return Jenkins.get().getDescriptorByType(CpsFlowDefinition.DescriptorImpl.class).doCheckScriptCompile(job, value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@
import hudson.model.Run;
import java.util.HashMap;
import java.util.Map;
import jenkins.model.Jenkins;
import org.apache.commons.io.IOUtils;
import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
import org.jenkinsci.plugins.workflow.cps.replay.Messages;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.CmdLineParser;
Expand All @@ -51,6 +55,9 @@
@Option(name="-s", aliases="--script", metaVar="SCRIPT", usage="Name of script to edit, such as Script3, if not the main Jenkinsfile.")
public String script;

@Option(name = "-a", aliases="--approve", metaVar="APPROVE", usage ="Approve the main Jenkinsfile if the build is unsandboxed.")
public boolean approve = false;

@Override public String getShortDescription() {
return Messages.ReplayCommand_shortDescription();
}
Expand All @@ -77,13 +84,31 @@
throw new AbortException("Unrecognized script name among " + replacementLoadedScripts.keySet());
}
replacementLoadedScripts.put(script, text);
if (approve) {
approveScript(action.getOriginalScript(), action);
}
action.run(action.getOriginalScript(), replacementLoadedScripts);
} else {
approveScript(text, action);
action.run(text, action.getOriginalLoadedScripts());
}
return 0;
}

public void approveScript(String script, ReplayAction action) {
CpsFlowExecution exec = action.getExecutionBlocking();
if (exec == null) {
return;
}
if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER) || exec.isSandbox()) {
return;
}
if (!ScriptApproval.get().isScriptApproved(script, GroovyLanguage.get())) {
ScriptApproval.get().preapprove(script, GroovyLanguage.get());
ScriptApproval.get().save();
}
}

@SuppressWarnings("rawtypes")
public static class JobHandler extends GenericItemOptionHandler<Job> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
</f:entry>
<j:forEach var="loadedScript" items="${it.originalLoadedScripts.entrySet()}">
<f:entry field="${loadedScript.key.replace('.', '_')}" title="${loadedScript.key}">
<wfe:workflow-editor script="${loadedScript.value}" theme="${it.theme}" checkUrl="${rootURL}/${it.owner.url}${it.urlName}/checkScript" checkDependsOn=""/>
<wfe:workflow-editor script="${loadedScript.value}" theme="${it.theme}" checkUrl="${rootURL}/${it.owner.url}${it.urlName}/checkLoadedScript" checkDependsOn=""/>
</f:entry>
</j:forEach>
<f:block>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package org.jenkinsci.plugins.workflow.cps.replay;

import org.htmlunit.FailingHttpStatusCodeException;
import org.htmlunit.WebAssert;
import org.htmlunit.html.HtmlForm;
import org.htmlunit.html.HtmlPage;
Expand Down Expand Up @@ -56,6 +57,8 @@
import jenkins.model.Jenkins;
import org.apache.commons.io.IOUtils;
import org.hamcrest.Matchers;
import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
Expand All @@ -77,6 +80,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

Expand Down Expand Up @@ -397,4 +401,60 @@ public void rebuild() throws Exception {
});
}

@Issue("SECURITY-3362")
@Test
public void rebuildNeedScriptApproval() throws Exception {
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
story.j.jenkins.setSecurityRealm(story.j.createDummySecurityRealm());
story.j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
grant(Jenkins.READ, Item.BUILD, Item.READ).everywhere().to("dev1"));

WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "SECURITY-3362");
String script = "pipeline {\n" +
" agent any\n" +
" stages {\n" +
" stage('List Jobs') {\n" +
" steps {\n" +
" script {\n" +
" println \"Jobs: ${jenkins.model.Jenkins.instance.getItemByFullName(env.JOB_NAME)?.parent?.items*.fullName.join(', ')}!\"" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}\n";
p.setDefinition(new CpsFlowDefinition(script, false));

ScriptApproval.get().preapprove(script, GroovyLanguage.get());

WorkflowRun b1 = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0));
story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b1));

ScriptApproval.get().clearApprovedScripts();

{ // First time around, verify that UI elements are present and functional.
ReplayAction a = b1.getAction(ReplayAction.class);
assertNotNull(a);
assertFalse(canReplay(b1, "dev1"));
assertTrue(canRebuild(b1, "dev1"));
JenkinsRule.WebClient wc = story.j.createWebClient();
wc.login("dev1");

HtmlPage page = wc.getPage(b1, a.getUrlName());
WebAssert.assertFormNotPresent(page, "config");
HtmlForm form = page.getFormByName("rebuild");

try {
story.j.submit(form);
} catch (FailingHttpStatusCodeException e) {
String responseBody = e.getResponse().getContentAsString();
assertTrue(responseBody.contains("The script is not approved."));
}
story.j.waitUntilNoActivity();
WorkflowRun b2 = p.getBuildByNumber(2);
assertNull(b2);
}
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* The MIT License
*
* Copyright 2024 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 org.jenkinsci.plugins.workflow.cps.replay;

import hudson.cli.CLICommandInvoker;
import jenkins.model.Jenkins;
import org.apache.tools.ant.filters.StringInputStream;
import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.MockAuthorizationStrategy;

import static org.hamcrest.MatcherAssert.assertThat;

public class ReplayPipelineCommandTest {

@Rule public JenkinsRule j = new JenkinsRule();

@Issue("SECURITY-3362")
@Test
public void rebuildNeedScriptApprovalCLIEdition() throws Exception {
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
grant(Jenkins.ADMINISTER).everywhere().toEveryone());

WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "SECURITY-3362");
j.jenkins.getWorkspaceFor(p).child("a.groovy").write("echo 'Hello LoadedWorld'", null);
String script =
"node() {\n" +
" a = load('a.groovy')\n" +
"}\n";
p.setDefinition(new CpsFlowDefinition(script, false));

ScriptApproval.get().preapprove(script, GroovyLanguage.get());

WorkflowRun b1 = j.assertBuildStatusSuccess(p.scheduleBuild2(0));
j.assertBuildStatusSuccess(j.waitForCompletion(b1));
String viaCliScript = "echo 'HelloWorld'";

assertThat(new CLICommandInvoker(j, new ReplayPipelineCommand()).withStdin(new StringInputStream(viaCliScript))
.invokeWithArgs(p.getName(), "-n", "1"),
CLICommandInvoker.Matcher.succeededSilently());
j.waitUntilNoActivity();
j.assertBuildStatusSuccess(p.getBuildByNumber(2));

assertThat(new CLICommandInvoker(j, new ReplayPipelineCommand()).withStdin(new StringInputStream(viaCliScript))
.invokeWithArgs(p.getName(), "-n", "1", "-s", "Script1"),
CLICommandInvoker.Matcher.succeededSilently());
j.waitUntilNoActivity();
j.assertBuildStatusSuccess(p.getBuildByNumber(3));

ScriptApproval.get().clearApprovedScripts();

assertThat(new CLICommandInvoker(j, new ReplayPipelineCommand()).withStdin(new StringInputStream(viaCliScript))
.invokeWithArgs(p.getName(), "-n", "1"),
CLICommandInvoker.Matcher.succeededSilently());
j.waitUntilNoActivity();
j.assertBuildStatusSuccess(p.getBuildByNumber(4));

ScriptApproval.get().clearApprovedScripts();

assertThat(new CLICommandInvoker(j, new ReplayPipelineCommand()).withStdin(new StringInputStream(viaCliScript))
.invokeWithArgs(p.getName(), "-n", "1", "-s", "Script1"),
CLICommandInvoker.Matcher.failedWith(1));

assertThat(new CLICommandInvoker(j, new ReplayPipelineCommand()).withStdin(new StringInputStream(viaCliScript))
.invokeWithArgs(p.getName(), "-n", "1", "-s", "Script1", "-a"),
CLICommandInvoker.Matcher.succeededSilently());
j.waitUntilNoActivity();
j.assertBuildStatusSuccess(p.getBuildByNumber(5));
}
}

0 comments on commit 478dd9e

Please sign in to comment.