Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-42950] Support more credential masking scenarios #59

Merged
merged 36 commits into from
May 31, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
927eb26
[JENKINS-42950] Support more credential masking scenarios
jvz Apr 9, 2019
3fb359e
Improve assertions
jvz May 3, 2019
ee51298
Move package and add link to relevant PR
jvz May 3, 2019
2273356
Rename test
jvz May 3, 2019
19fec43
Rename test
jvz May 3, 2019
6dcc230
Rename tests and improve some
jvz May 3, 2019
cd94206
Improve help docs
daniel-beck May 3, 2019
c82bcc3
Simplify set class usage
jvz May 3, 2019
fa4ff53
Improve javadoc
jvz May 3, 2019
5f55aca
Improve help docs
jvz May 3, 2019
a432974
Add batch test for fancier password
jvz May 3, 2019
0dde12f
Fix assertion
jvz May 3, 2019
30d0900
Improve tests to work with bash properly
jvz May 3, 2019
ad1b6aa
Move bash assumption to BeforeClass
jvz May 3, 2019
9463aaf
Fix java 11 build issue
jvz May 7, 2019
c18e1ec
Add tests for /bin/sh
jvz May 7, 2019
394a08e
Simplify batch pattern masker
jvz May 9, 2019
bd82279
Fix bash assumptions in tests and other test fixes
Wadeck May 10, 2019
ee00db0
Clarify character range
jvz May 20, 2019
f07204b
Refactor credentials registration in tests
jvz May 21, 2019
5425b67
Add one more data point to bash test
jvz May 21, 2019
ebb4a53
Improve password generation readability
jvz May 21, 2019
1512830
Fix test regressions
jvz May 22, 2019
c707d92
Remove unneeded TODO
jvz May 30, 2019
7b05cd2
Clean up javadoc
jvz May 30, 2019
f7690e0
Apply suggestions from code review
jvz May 30, 2019
c9938de
Re-add docs on set +x
jvz May 30, 2019
14f8c62
Fix imports
jvz May 30, 2019
4ebbd28
Use @ClassRule JenkinsRule and improve comments
jvz May 30, 2019
9bd38ca
Refactor masking API to use Pattern
jvz May 30, 2019
cf2178c
Remove redundant pattern
jvz May 30, 2019
89d9c74
Remove redundant pattern
jvz May 30, 2019
c7d3cf6
Update log message to reflect masking improvements
jvz May 30, 2019
e2efe9a
Restrict API
jvz May 30, 2019
717d5db
Update test for log message update
jvz May 30, 2019
163fdba
Simplify API
jvz May 30, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ private void doStart() throws Exception {
}
if (!overrides.isEmpty()) {
boolean unix = launcher != null ? launcher.isUnix() : true;
listener.getLogger().println("Masking only exact matches of " + overrides.keySet().stream().map(
listener.getLogger().println("Masking supported pattern matches of " + overrides.keySet().stream().map(
v -> unix ? "$" + v : "%" + v + "%"
).collect(Collectors.joining(" or ")));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ public class BashSecretPatternFactory implements SecretPatternFactory {
}

@Override
public @Nonnull Collection<Pattern> getSecretPatterns(@Nonnull String input) {
Collection<Pattern> patterns = new HashSet<>();
public @Nonnull Collection<String> getEncodedForms(@Nonnull String input) {
Collection<String> patterns = new HashSet<>();
String quotedForm = getQuotedForm(input);
patterns.add(SecretPatternFactory.quotedCompile(quotedForm));
patterns.add(SecretPatternFactory.quotedCompile(surroundWithQuotes(quotedForm)));
patterns.add(SecretPatternFactory.quotedCompile(getUnquotedForm(input)));
patterns.add(quotedForm);
patterns.add(surroundWithQuotes(quotedForm));
patterns.add(getUnquotedForm(input));
return patterns;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or a bit more simply

return Arrays.asList(quotedForm, surroundWithQuotes(quotedForm), getUnquotedForm(input));

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ public class BatchSecretPatternFactory implements SecretPatternFactory {
private static final Pattern QUOTED_CHARS = Pattern.compile("(\\^)(\\^?)");

@Override
public @Nonnull Collection<Pattern> getSecretPatterns(@Nonnull String input) {
public @Nonnull Collection<String> getEncodedForms(@Nonnull String input) {
return input.contains("^")
? Collections.singleton(SecretPatternFactory.quotedCompile(QUOTED_CHARS.matcher(input).replaceAll("$2")))
? Collections.singleton(QUOTED_CHARS.matcher(input).replaceAll("$2"))
: Collections.emptySet();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import javax.annotation.Nonnull;
import java.util.Collection;
import java.util.Collections;
import java.util.regex.Pattern;

/**
* Trivial secret pattern factory that matches the literal value of the secret.
Expand All @@ -41,7 +40,7 @@
public class LiteralSecretPatternFactory implements SecretPatternFactory {
@Nonnull
@Override
public Collection<Pattern> getSecretPatterns(@Nonnull String input) {
return Collections.singleton(SecretPatternFactory.quotedCompile(input));
public Collection<String> getEncodedForms(@Nonnull String input) {
return Collections.singleton(input);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,24 @@

import hudson.ExtensionList;
import hudson.ExtensionPoint;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

import javax.annotation.Nonnull;
import java.util.Collection;
import java.util.regex.Pattern;

/**
* Creates regular expressions to match encoded forms of secrets in logs.
* Provides encoded forms to an input for use in masking those forms in logs.
* These are typically implemented to handle various shell quoting algorithms (sometimes confused with escaping) to
* pass literal string values to an interpreter.
*/
@Restricted(NoExternalUse.class)
public interface SecretPatternFactory extends ExtensionPoint {

/**
* Returns a collection of alternative forms the given input may show up as in logs.
* Note that these patterns must embed their flags in the pattern rather than as parameters to Pattern.
* Returns a collection of alternative forms the given input may be encoded as in logs.
*/
@Nonnull Collection<Pattern> getSecretPatterns(@Nonnull String input);
@Nonnull Collection<String> getEncodedForms(@Nonnull String input);

/**
* Returns all SecretPatternFactory extensions known at runtime.
Expand All @@ -51,11 +52,4 @@ public interface SecretPatternFactory extends ExtensionPoint {
return ExtensionList.lookup(SecretPatternFactory.class);
}

/**
* Composes {@link Pattern#compile(String)} and {@link Pattern#quote(String)} for convenience.
*/
static @Nonnull Pattern quotedCompile(@Nonnull String input) {
return Pattern.compile(Pattern.quote(input));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@
import javax.annotation.Nonnull;
import java.util.Collection;
import java.util.Comparator;
import java.util.TreeSet;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

@Restricted(NoExternalUse.class)
public class SecretPatterns {

private static final Comparator<Pattern> EXTRACT_PATTERN = Comparator.comparing(Pattern::pattern);
private static final Comparator<Pattern> BY_LENGTH_DESCENDING = Comparator.comparing(Pattern::pattern,
Comparator.comparingInt(String::length).reversed().thenComparing(String::compareTo));
private static final Comparator<String> BY_LENGTH_DESCENDING =
Comparator.comparingInt(String::length).reversed().thenComparing(String::compareTo);

/**
* Constructs a regular expression to match against all known forms that the given collection of input strings may
Expand All @@ -51,18 +49,14 @@ public class SecretPatterns {
* absence of quoting, the longer form is masked.
*/
public static @Nonnull Pattern getAggregateSecretPattern(@Nonnull Collection<String> inputs) {
Collection<Pattern> patterns = new TreeSet<>(EXTRACT_PATTERN);
for (String input : inputs) {
if (input.isEmpty()) {
continue;
}
for (SecretPatternFactory provider : SecretPatternFactory.all()) {
patterns.addAll(provider.getSecretPatterns(input));
}
}
String pattern = patterns.stream()
String pattern = inputs.stream()
.filter(input -> !input.isEmpty())
.flatMap(input ->
SecretPatternFactory.all().stream().flatMap(factory ->
factory.getEncodedForms(input).stream()))
.sorted(BY_LENGTH_DESCENDING)
.map(Pattern::pattern)
.distinct()
.map(Pattern::quote)
.collect(Collectors.joining("|"));
return Pattern.compile(pattern);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public static class Execution extends AbstractSynchronousStepExecution<Void> {
+ "}", true));
WorkflowRun b = p.scheduleBuild2(0).waitForStart();
SemaphoreStep.waitForStart("basics/1", b);
story.j.assertLogContains(Functions.isWindows() ? "Masking only exact matches of %USERNAME% or %PASSWORD%" : "Masking only exact matches of $USERNAME or $PASSWORD", b);
story.j.assertLogContains(Functions.isWindows() ? "Masking supported pattern matches of %USERNAME% or %PASSWORD%" : "Masking supported pattern matches of $USERNAME or $PASSWORD", b);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to introduce a package-private utility method for this message to be shared between src/main/ and src/test/.

}
});
story.addStep(new Statement() {
Expand Down