[SECURITY-883]

CHANGELOG.md

@@ -7,6 +7,10 @@ Known issues
 
 See the full list of issues at [JIRA](https://issues.jenkins-ci.org/issues/?filter=15575)
 
+1.7.1
+-----
+* Do not print credentials in build output or logs. Only affects certain pipeline steps like `withDockerRegistry`. `sh` step is not affected [SECURITY-883](https://issues.jenkins-ci.org/browse/SECURITY-883)
+
 1.7.0
 -----
 * Add option to apply caps only on alive pods [#252](https://github.com/jenkinsci/kubernetes-plugin/pull/252)

pom.xml

@@ -214,6 +214,12 @@
       <version>2.16.2</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci.plugins</groupId>
+      <artifactId>docker-workflow</artifactId>
+      <version>1.15</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <dependencyManagement>
@@ -225,6 +231,11 @@
       </dependency>
 
       <!-- just to fix enforcer RequireUpperBoundDeps -->
+      <dependency>
+        <groupId>org.jenkins-ci.plugins</groupId>
+        <artifactId>credentials-binding</artifactId>
+        <version>1.12</version>
+      </dependency>
       <dependency>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>structs</artifactId>

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java

@@ -38,9 +38,11 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.apache.commons.io.output.NullOutputStream;
 import org.apache.commons.io.output.TeeOutputStream;
 import org.csanchez.jenkins.plugins.kubernetes.pipeline.proc.CachedProc;
 import org.csanchez.jenkins.plugins.kubernetes.pipeline.proc.DeadProc;
+import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
 
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -60,8 +62,6 @@
 import io.fabric8.kubernetes.client.dsl.ExecWatch;
 import io.fabric8.kubernetes.client.dsl.Execable;
 import okhttp3.Response;
-import org.apache.commons.io.output.NullOutputStream;
-import org.jenkinsci.plugins.workflow.steps.EnvironmentExpander;
 
 /**
  * This decorator interacts directly with the Kubernetes exec API to run commands inside a container. It does not use
@@ -234,10 +234,12 @@ public Proc launch(ProcStarter starter) throws IOException {
                         }
                     }
                 }
-                return doLaunch(starter.quiet(), envVars, starter.stdout(), starter.pwd(), getCommands(starter));
+                return doLaunch(starter.quiet(), envVars, starter.stdout(), starter.pwd(), starter.masks(),
+                        getCommands(starter));
             }
 
-            private Proc doLaunch(boolean quiet, String [] cmdEnvs,  OutputStream outputForCaller, FilePath pwd, String... commands) throws IOException {
+            private Proc doLaunch(boolean quiet, String[] cmdEnvs, OutputStream outputForCaller, FilePath pwd,
+                    boolean[] masks, String... commands) throws IOException {
                 if (processes == null) {
                     processes = new HashMap<>();
                 }
@@ -365,7 +367,7 @@ public void onClose(int i, String s) {
                     }
 
                     this.setupEnvironmentVariable(envVars, watch);
-                    doExec(watch, printStream, commands);
+                    doExec(watch, printStream, masks, commands);
                     if (closables == null) {
                         closables = new ArrayList<>();
                     }
@@ -391,7 +393,7 @@ public void kill(Map<String, String> modelEnvVars) throws IOException, Interrupt
                 String cookie = modelEnvVars.get(COOKIE_VAR);
 
                 int exitCode = doLaunch(
-                        true, null, null, null,
+                        true, null, null, null, null,
                         "sh", "-c", "kill \\`grep -l '" + COOKIE_VAR + "=" + cookie  +"' /proc/*/environ | cut -d / -f 3 \\`"
                 ).join();
 
@@ -458,14 +460,19 @@ public void close() throws IOException {
         }
     }
 
-    private static void doExec(ExecWatch watch, PrintStream out, String... statements) {
+    private static void doExec(ExecWatch watch, PrintStream out, boolean[] masks, String... statements) {
         try {
             out.print("Executing command: ");
             StringBuilder sb = new StringBuilder();
-            for (String stmt : statements) {
-                String s = String.format("\"%s\" ", stmt);
-                sb.append(s);
-                out.print(s);
+            for (int i = 0; i < statements.length; i++) {
+                String s = String.format("\"%s\" ", statements[i]);
+                if (masks != null && masks[i]) {
+                    sb.append("******** ");
+                    out.print("******** ");
+                } else {
+                    sb.append(s);
+                    out.print(s);
+                }
                 watch.getInput().write(s.getBytes(StandardCharsets.UTF_8));
             }
             sb.append(NEWLINE);
@@ -477,7 +484,7 @@ private static void doExec(ExecWatch watch, PrintStream out, String... statement
             sb.append(ExitCodeOutputStream.EXIT_COMMAND);
             out.print(ExitCodeOutputStream.EXIT_COMMAND);
             LOGGER.log(Level.FINEST, "Executing command: {0}", sb);
-             watch.getInput().write(ExitCodeOutputStream.EXIT_COMMAND.getBytes(StandardCharsets.UTF_8));
+            watch.getInput().write(ExitCodeOutputStream.EXIT_COMMAND.getBytes(StandardCharsets.UTF_8));
 
             out.flush();
             watch.getInput().flush();

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecoratorPipelineTest.java

@@ -18,20 +18,33 @@
 
 import static org.junit.Assert.*;
 
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
 import org.apache.commons.compress.utils.IOUtils;
 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.LoggerRule;
 
 import com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKey;
 import com.cloudbees.jenkins.plugins.sshcredentials.impl.BasicSSHUserPrivateKey.PrivateKeySource;
 import com.cloudbees.plugins.credentials.CredentialsScope;
 import com.cloudbees.plugins.credentials.SystemCredentialsProvider;
+import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
+import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl;
 
 public class ContainerExecDecoratorPipelineTest extends AbstractKubernetesPipelineTest {
 
+    private static final String DOCKER_LOGIN_CMD = "Executing command: \"docker\" \"login\"";
+
+    @Rule
+    public LoggerRule containerExecLogs = new LoggerRule()
+            .record(Logger.getLogger(ContainerExecDecorator.class.getName()), Level.ALL);
+
     @Issue({ "JENKINS-47225", "JENKINS-42582" })
     @Test
     public void sshagent() throws Exception {
@@ -54,4 +67,26 @@ public void sshagent() throws Exception {
          //check that we don't accidentally start exporting sensitive info to the log
         r.assertLogNotContains("secret_passphrase", b);
     }
+
+    @Test
+    public void docker() throws Exception {
+        StandardUsernamePasswordCredentials credentials = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL,
+                "ContainerExecDecoratorPipelineTest-docker", "bob", "username", "secret_password");
+        SystemCredentialsProvider.getInstance().getCredentials().add(credentials);
+
+        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "docker");
+        p.setDefinition(new CpsFlowDefinition(loadPipelineScript("docker.groovy"), true));
+        containerExecLogs.capture(1000);
+        WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+        assertNotNull(b);
+        r.waitForCompletion(b);
+        r.assertLogContains(DOCKER_LOGIN_CMD, b);
+        // check that we don't accidentally start exporting sensitive info to the build log
+        r.assertLogNotContains("secret_password", b);
+        // check that we don't accidentally start exporting sensitive info to the Jenkins log
+        assertTrue("docker login was not executed",
+                containerExecLogs.getMessages().stream().anyMatch(msg -> msg.contains(DOCKER_LOGIN_CMD)));
+        assertFalse("credential leaked to log",
+                containerExecLogs.getMessages().stream().anyMatch(msg -> msg.contains("secret_password")));
+    }
 }

src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/docker.groovy

@@ -0,0 +1,24 @@
+pipeline {
+  agent {
+    kubernetes {
+      label 'docker'
+      containerTemplate {
+        name 'docker'
+        image 'docker:1.11'
+        ttyEnabled true
+        command 'cat'
+      }
+    }
+  }
+  stages {
+    stage('Run maven') {
+        steps {
+            container('docker') {
+                withDockerRegistry(registry: [credentialsId: 'ContainerExecDecoratorPipelineTest-docker']) {
+                    sh 'hostname'
+                }
+            }
+        }
+    }
+  }
+}