Skip to content

Commit 1af7753

Browse files
Kevin-CBKevin-CB
committed
SECURITY-3499
1 parent a107ab3 commit 1af7753

File tree

4 files changed

+186
-4
lines changed

4 files changed

+186
-4
lines changed

‎pom.xml‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<parent>
66
<groupId>org.jenkins-ci.plugins</groupId>
77
<artifactId>plugin</artifactId>
8-
<version>5.1</version>
8+
<version>5.9</version>
99
<relativePath/>
1010
</parent>
1111

@@ -17,8 +17,11 @@
1717
<url>https://github.com/jenkinsci/${project.artifactId}-plugin</url>
1818
<properties>
1919
<changelist>999999-SNAPSHOT</changelist>
20-
<jenkins.version>2.479</jenkins.version>
20+
<!-- https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/ -->
21+
<jenkins.baseline>2.479</jenkins.baseline>
22+
<jenkins.version>${jenkins.baseline}.3</jenkins.version>
2123
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
24+
<useBeta>true</useBeta> <!-- FailureHandler -->
2225
</properties>
2326
<licenses>
2427
<license>
@@ -51,11 +54,23 @@
5154
<dependencies>
5255
<dependency>
5356
<groupId>io.jenkins.tools.bom</groupId>
54-
<artifactId>bom-2.462.x</artifactId>
55-
<version>3435.v238d66a_043fb_</version>
57+
<artifactId>bom-${jenkins.baseline}.x</artifactId>
58+
<version>4488.v7fe26526366e</version>
5659
<scope>import</scope>
5760
<type>pom</type>
5861
</dependency>
62+
<!-- TODO: Remove once the BOM has caught up -->
63+
<dependency>
64+
<groupId>org.jenkins-ci.plugins.workflow</groupId>
65+
<artifactId>workflow-step-api</artifactId>
66+
<version>704.ve4f0967e98fa_</version>
67+
</dependency>
68+
<!-- TODO: Remove once the BOM has caught up -->
69+
<dependency>
70+
<groupId>org.jenkins-ci.plugins.workflow</groupId>
71+
<artifactId>workflow-cps</artifactId>
72+
<version>4106.4108.v841a_e1819d4d</version>
73+
</dependency>
5974
</dependencies>
6075
</dependencyManagement>
6176
<dependencies>

‎src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
5858
import org.jenkinsci.plugins.workflow.steps.BodyInvoker;
5959
import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
60+
import org.jenkinsci.plugins.workflow.steps.FailureHandler;
6061
import org.jenkinsci.plugins.workflow.steps.GeneralNonBlockingStepExecution;
6162
import org.jenkinsci.plugins.workflow.steps.MissingContextVariableException;
6263
import org.jenkinsci.plugins.workflow.steps.Step;
@@ -140,13 +141,32 @@ private void doStart() throws Exception {
140141
v -> unix ? "$" + v : "%" + v + "%"
141142
).collect(Collectors.joining(" or ")));
142143
}
144+
143145
getContext().newBodyInvoker().
144146
withContext(EnvironmentExpander.merge(getContext().get(EnvironmentExpander.class), new Overrider(secretOverrides, publicOverrides))).
145147
withContext(BodyInvoker.mergeConsoleLogFilters(getContext().get(ConsoleLogFilter.class), new Filter(secretOverrides.values(), run.getCharset().name()))).
148+
withContext(FailureHandler.merge(getContext().get(FailureHandler.class), new Handler(secretOverrides.values()))).
146149
withCallback(new Callback2(unbinders)).
147150
start();
148151
}
149152

153+
private static final class Handler implements FailureHandler {
154+
155+
private static final long serialVersionUID = 1;
156+
157+
private final Secret secretPattern;
158+
159+
Handler(Collection<String> secrets) {
160+
this.secretPattern = Secret.fromString(SecretPatterns.getAggregateSecretPattern(secrets).pattern());
161+
}
162+
163+
@NonNull
164+
@Override
165+
public Throwable handle(@NonNull StepContext ctx, @NonNull Throwable t) {
166+
return MaskedException.of(t, Pattern.compile(secretPattern.getPlainText()));
167+
}
168+
}
169+
150170
private final class Callback2 extends TailCall {
151171

152172
private static final long serialVersionUID = 1;

‎src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/MaskedException.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package org.jenkinsci.plugins.credentialsbinding.impl;
2+
3+
import edu.umd.cs.findbugs.annotations.NonNull;
4+
import hudson.Functions;
5+
6+
import java.util.HashSet;
7+
import java.util.Objects;
8+
import java.util.Set;
9+
import java.util.regex.Pattern;
10+
11+
final class MaskedException extends Exception {
12+
private static final long serialVersionUID = 1L;
13+
14+
static Throwable of(@NonNull Throwable unmasked, Pattern pattern) {
15+
return of(unmasked, new HashSet<>(), pattern);
16+
}
17+
18+
private static Throwable of(@NonNull Throwable unmasked, @NonNull Set<Throwable> visited, Pattern pattern) {
19+
if (!visited.add(unmasked)) {
20+
return new Exception("cycle");
21+
}
22+
var text = Functions.printThrowable(unmasked);
23+
if (pattern.matcher(text).find()) {
24+
var masked = new MaskedException(pattern.matcher(Objects.requireNonNullElse(unmasked.getMessage(), "")).replaceAll("****"));
25+
masked.setStackTrace(unmasked.getStackTrace());
26+
var cause = unmasked.getCause();
27+
if (cause != null) {
28+
masked.initCause(of(cause, visited, pattern));
29+
}
30+
for (var suppressed : unmasked.getSuppressed()) {
31+
masked.addSuppressed(of(suppressed, visited, pattern));
32+
}
33+
return masked;
34+
} else {
35+
return unmasked;
36+
}
37+
}
38+
39+
private MaskedException(String message) {
40+
super(message);
41+
}
42+
43+
}

‎src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,24 +46,31 @@
4646
import java.io.File;
4747
import java.io.FileNotFoundException;
4848
import java.io.IOException;
49+
import java.io.PrintWriter;
50+
import java.io.StringWriter;
4951
import java.nio.charset.StandardCharsets;
5052
import java.nio.file.NoSuchFileException;
5153
import java.util.Collections;
5254
import java.util.Set;
5355
import java.util.TreeSet;
56+
import java.util.stream.Collectors;
57+
5458
import jenkins.security.QueueItemAuthenticator;
5559
import jenkins.security.QueueItemAuthenticatorConfiguration;
5660
import org.apache.commons.io.FileUtils;
5761
import static org.hamcrest.MatcherAssert.assertThat;
62+
import static org.hamcrest.Matchers.containsString;
5863
import static org.hamcrest.Matchers.equalTo;
5964
import static org.hamcrest.Matchers.hasItem;
6065
import static org.hamcrest.Matchers.is;
66+
import static org.hamcrest.Matchers.not;
6167
import static org.hamcrest.Matchers.notNullValue;
6268
import static org.hamcrest.Matchers.nullValue;
6369
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
6470
import org.jenkinsci.plugins.plaincredentials.impl.FileCredentialsImpl;
6571
import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl;
6672
import org.jenkinsci.plugins.workflow.actions.ArgumentsAction;
73+
import org.jenkinsci.plugins.workflow.actions.ErrorAction;
6774
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
6875
import org.jenkinsci.plugins.workflow.cps.SnippetizerTester;
6976
import org.jenkinsci.plugins.workflow.flow.FlowExecution;
@@ -460,6 +467,103 @@ public void usernameUnmaskedInStepArguments() throws Throwable {
460467
r.assertLogContains("got: P#1", r.assertBuildStatusSuccess(p.scheduleBuild2(0).get()));
461468
});
462469
}
470+
471+
@Issue("SECURITY-3499")
472+
@Test public void maskingExceptionInError() throws Throwable {
473+
rr.then(r -> {
474+
CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), new StringCredentialsImpl(CredentialsScope.GLOBAL, "creds", "sample", Secret.fromString("s3cr3t")));
475+
var p = r.jenkins.createProject(WorkflowJob.class, "p");
476+
p.setDefinition(new CpsFlowDefinition(
477+
"""
478+
withCredentials([string(credentialsId: 'creds', variable: 'SECRET')]) {
479+
error(/should not mention $SECRET/)
480+
}
481+
""", true));
482+
var b = r.buildAndAssertStatus(Result.FAILURE, p);
483+
r.assertLogNotContains("s3cr3t", b);
484+
r.assertLogContains("should not mention ****", b);
485+
assertErrorActionsDoNotContainString(b, "s3cr3t");
486+
});
487+
}
488+
489+
@Issue("SECURITY-3499")
490+
@Test public void maskingExceptionInNestedError() throws Throwable {
491+
rr.then(r -> {
492+
CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), new StringCredentialsImpl(CredentialsScope.GLOBAL, "creds", "sample", Secret.fromString("s3cr3t")));
493+
var p = r.jenkins.createProject(WorkflowJob.class, "p");
494+
p.setDefinition(new CpsFlowDefinition(
495+
"""
496+
withCredentials([string(credentialsId: 'creds', variable: 'SECRET')]) {
497+
try {
498+
error(/nested exception should not mention $SECRET/)
499+
}
500+
catch (err) {
501+
throw new Exception("normal exception should not mention $SECRET", err)
502+
}
503+
}
504+
""", false));
505+
var b = r.buildAndAssertStatus(Result.FAILURE, p);
506+
r.assertLogNotContains("s3cr3t", b);
507+
r.assertLogContains("nested exception should not mention ****", b);
508+
r.assertLogContains("normal exception should not mention ****", b);
509+
assertErrorActionsDoNotContainString(b, "s3cr3t");
510+
});
511+
}
512+
513+
@Issue("SECURITY-3499")
514+
@Test public void maskingExceptionInSuppressedError() throws Throwable {
515+
rr.then(r -> {
516+
CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), new StringCredentialsImpl(CredentialsScope.GLOBAL, "creds", "sample", Secret.fromString("s3cr3t")));
517+
var p = r.jenkins.createProject(WorkflowJob.class, "p");
518+
p.setDefinition(new CpsFlowDefinition(
519+
"""
520+
withCredentials([string(credentialsId: 'creds', variable: 'SECRET')]) {
521+
def main = new Exception("normal exception should not mention $SECRET")
522+
main.addSuppressed(new Exception("suppressed exception should not mention $SECRET"))
523+
throw main
524+
}
525+
""", false));
526+
var b = r.buildAndAssertStatus(Result.FAILURE, p);
527+
r.assertLogNotContains("s3cr3t", b);
528+
r.assertLogContains("suppressed exception should not mention ****", b);
529+
r.assertLogContains("normal exception should not mention ****", b);
530+
assertErrorActionsDoNotContainString(b, "s3cr3t");
531+
});
532+
}
533+
534+
@Issue("SECURITY-3499")
535+
@Test public void maskingExceptionForIncorrectArgs() throws Throwable {
536+
rr.then(r -> {
537+
CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), new StringCredentialsImpl(CredentialsScope.GLOBAL, "creds", "sample", Secret.fromString("s3cr3t")));
538+
var p = r.jenkins.createProject(WorkflowJob.class, "p");
539+
p.setDefinition(new CpsFlowDefinition(
540+
"""
541+
withCredentials([string(credentialsId: 'creds', variable: 'SECRET')]) {
542+
writeFile([file: "creds.txt", text: "${SECRET}", encoding: 'Base64']) {
543+
}
544+
}
545+
""", false));
546+
var b = r.buildAndAssertStatus(Result.FAILURE, p);
547+
r.assertLogNotContains("s3cr3t", b);
548+
r.assertLogContains("text=****", b);
549+
assertErrorActionsDoNotContainString(b, "s3cr3t");
550+
});
551+
}
552+
553+
private void assertErrorActionsDoNotContainString(WorkflowRun b, String needle) {
554+
var errorActionStackTraces = new DepthFirstScanner().allNodes(b.getExecution()).stream()
555+
.map(n -> n.getPersistentAction(ErrorAction.class))
556+
.filter(ea -> ea != null)
557+
.map(ea -> ea.getError())
558+
.map(t -> {
559+
var writer = new StringWriter();
560+
t.printStackTrace(new PrintWriter(writer));
561+
return writer.toString();
562+
})
563+
.collect(Collectors.toList());
564+
assertThat(errorActionStackTraces, not(hasItem(containsString(needle))));
565+
}
566+
463567
private static final class SpecialCredentials extends BaseStandardCredentials implements StringCredentials {
464568
private Run<?, ?> build;
465569
SpecialCredentials(CredentialsScope scope, String id, String description) {

0 commit comments

Comments
 (0)