From 003efafd6e818ef9f964dc343c323751d010ca1f Mon Sep 17 00:00:00 2001 From: Benoit GUERIN Date: Tue, 22 Aug 2023 12:11:40 +0200 Subject: [PATCH] fix SECURITY-3257: do not expose usernames on console log --- jenkins-plugin/pom.xml | 4 + .../maven/WithMavenStepExecution2.java | 12 +- .../MaskPasswordsConsoleLogFilter.java | 10 +- .../MaskPasswordsConsoleLogFilterTest.java | 116 ++++++++++++++++++ 4 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 jenkins-plugin/src/test/java/org/jenkinsci/plugins/pipeline/maven/console/MaskPasswordsConsoleLogFilterTest.java diff --git a/jenkins-plugin/pom.xml b/jenkins-plugin/pom.xml index bec4a3ce..55794d87 100644 --- a/jenkins-plugin/pom.xml +++ b/jenkins-plugin/pom.xml @@ -321,6 +321,10 @@ xml-apis xml-apis + + xerces + xercesImpl + diff --git a/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2.java b/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2.java index 2a2299a4..988cee2b 100644 --- a/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2.java +++ b/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/WithMavenStepExecution2.java @@ -1214,10 +1214,8 @@ public String apply(@Nullable Entry entry) StandardUsernameCredentials credentials = entry.getValue(); return "[" + "mavenServerId: '" + mavenServerId + "', " + - "jenkinsCredentials: '" + credentials.getId() + "', " + - "username: '" + credentials.getUsername() + "', " + - "type: '" + ClassUtils.getShortName(credentials.getClass()) + - "']"; + "jenkinsCredentials: '" + credentials.getId() + "'" + + "]"; } } @@ -1230,13 +1228,9 @@ public String apply(@javax.annotation.Nullable Credentials credentials) { String result = ClassUtils.getShortName(credentials.getClass()) + "["; if (credentials instanceof IdCredentials) { IdCredentials idCredentials = (IdCredentials) credentials; - result += "id: " + idCredentials.getId() + ","; + result += "id: " + idCredentials.getId(); } - if (credentials instanceof UsernameCredentials) { - UsernameCredentials usernameCredentials = (UsernameCredentials) credentials; - result += "username: " + usernameCredentials.getUsername() + ""; - } result += "]"; return result; } diff --git a/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/console/MaskPasswordsConsoleLogFilter.java b/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/console/MaskPasswordsConsoleLogFilter.java index c0391802..d69a7228 100644 --- a/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/console/MaskPasswordsConsoleLogFilter.java +++ b/jenkins-plugin/src/main/java/org/jenkinsci/plugins/pipeline/maven/console/MaskPasswordsConsoleLogFilter.java @@ -3,6 +3,8 @@ import com.cloudbees.jenkins.plugins.sshcredentials.SSHUserPrivateKey; import com.cloudbees.plugins.credentials.Credentials; import com.cloudbees.plugins.credentials.common.PasswordCredentials; +import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials; + import hudson.console.ConsoleLogFilter; import hudson.model.Run; import hudson.util.Secret; @@ -50,7 +52,13 @@ public static MaskPasswordsConsoleLogFilter newMaskPasswordsConsoleLogFilter(@No protected static Collection toString(@NonNull Iterable credentials) { List result = new ArrayList<>(); for (Credentials creds : credentials) { - if (creds instanceof PasswordCredentials) { + if (creds instanceof UsernamePasswordCredentials) { + UsernamePasswordCredentials usernamePasswordCredentials = (UsernamePasswordCredentials) creds; + if (usernamePasswordCredentials.isUsernameSecret()) { + result.add(usernamePasswordCredentials.getUsername()); + } + result.add(usernamePasswordCredentials.getPassword().getPlainText()); + } else if (creds instanceof PasswordCredentials) { PasswordCredentials passwordCredentials = (PasswordCredentials) creds; result.add(passwordCredentials.getPassword().getPlainText()); } else if (creds instanceof SSHUserPrivateKey) { diff --git a/jenkins-plugin/src/test/java/org/jenkinsci/plugins/pipeline/maven/console/MaskPasswordsConsoleLogFilterTest.java b/jenkins-plugin/src/test/java/org/jenkinsci/plugins/pipeline/maven/console/MaskPasswordsConsoleLogFilterTest.java new file mode 100644 index 00000000..15917f1f --- /dev/null +++ b/jenkins-plugin/src/test/java/org/jenkinsci/plugins/pipeline/maven/console/MaskPasswordsConsoleLogFilterTest.java @@ -0,0 +1,116 @@ +package org.jenkinsci.plugins.pipeline.maven.console; + +import static com.cloudbees.plugins.credentials.CredentialsScope.GLOBAL; +import static java.util.Arrays.asList; + +import java.util.ArrayList; +import java.util.List; + +import org.acegisecurity.Authentication; +import org.jenkinsci.plugins.configfiles.GlobalConfigFiles; +import org.jenkinsci.plugins.configfiles.maven.GlobalMavenSettingsConfig; +import org.jenkinsci.plugins.configfiles.maven.job.MvnGlobalSettingsProvider; +import org.jenkinsci.plugins.configfiles.maven.security.ServerCredentialMapping; +import org.jenkinsci.plugins.pipeline.maven.AbstractIntegrationTest; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.jenkinsci.plugins.workflow.job.WorkflowRun; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.jvnet.hudson.test.Issue; + +import com.cloudbees.plugins.credentials.Credentials; +import com.cloudbees.plugins.credentials.CredentialsProvider; +import com.cloudbees.plugins.credentials.domains.DomainRequirement; +import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; + +import hudson.ExtensionList; +import hudson.model.ItemGroup; +import hudson.model.Result; +import jenkins.model.Jenkins; +import jenkins.mvn.GlobalMavenConfig; + +public class MaskPasswordsConsoleLogFilterTest extends AbstractIntegrationTest { + + @Issue("SECURITY-3257") + @ParameterizedTest + @ValueSource(booleans = { true, false }) + public void should_hide_server_username_and_password(boolean usernameIsSecret) throws Exception { + //@formatter:off + String pipelineScript = "node() {\n" + + " withMaven(traceability: true, globalMavenSettingsConfig: 'maven-global-config-test') {\n" + + " sh 'cat \"$GLOBAL_MVN_SETTINGS\"'\n" + + " }\n" + + "}"; + + //@formatter:off + String mavenGlobalSettings = "\n" + + "\n" + + " \n" + + " \n" + + " server-id\n" + + " \n" + + " \n" + + "\n"; + //@formatter:on + + ExtensionList extensionList = Jenkins.getInstance().getExtensionList(CredentialsProvider.class); + extensionList.add(extensionList.size(), new FakeCredentialsProvider(usernameIsSecret)); + + List mappings = new ArrayList<>(); + mappings.add(new ServerCredentialMapping("server-id", "creds-id")); + GlobalMavenSettingsConfig mavenGlobalSettingsConfig = new GlobalMavenSettingsConfig("maven-global-config-test", "maven-global-config-test", "", + mavenGlobalSettings, true, mappings); + + GlobalConfigFiles.get().save(mavenGlobalSettingsConfig); + GlobalMavenConfig.get().setGlobalSettingsProvider(new MvnGlobalSettingsProvider(mavenGlobalSettingsConfig.id)); + + try { + WorkflowJob pipeline = jenkinsRule.createProject(WorkflowJob.class, "hide-credentials"); + pipeline.setDefinition(new CpsFlowDefinition(pipelineScript, true)); + WorkflowRun build = jenkinsRule.assertBuildStatus(Result.SUCCESS, pipeline.scheduleBuild2(0)); + jenkinsRule.assertLogContains( + "[withMaven] using Maven global settings.xml 'maven-global-config-test' with Maven servers credentials provided by Jenkins (replaceAll: true): [mavenServerId: 'server-id', jenkinsCredentials: 'creds-id']", + build); + jenkinsRule.assertLogContains("server-id", build); + if (usernameIsSecret) { + jenkinsRule.assertLogNotContains("aUser", build); + } else { + jenkinsRule.assertLogContains("aUser", build); + } + jenkinsRule.assertLogNotContains("aPass", build); + } finally { + GlobalMavenConfig.get().setSettingsProvider(null); + } + } + + private static class FakeCredentialsProvider extends CredentialsProvider { + private boolean usernameIsSecret; + + public FakeCredentialsProvider(boolean usernameIsSecret) { + this.usernameIsSecret = usernameIsSecret; + } + + @Override + public boolean isEnabled(Object context) { + return true; + } + + @Override + public List getCredentials(Class type, ItemGroup itemGroup, Authentication authentication, + List domainRequirements) { + UsernamePasswordCredentialsImpl creds = new UsernamePasswordCredentialsImpl(GLOBAL, "creds-id", "", "aUser", "aPass"); + creds.setUsernameSecret(usernameIsSecret); + return (List) asList(creds); + } + + @Override + public List getCredentials(Class type, ItemGroup itemGroup, Authentication authentication) { + return getCredentials(type, itemGroup, authentication, null); + } + } + +}