[JENKINS-56674] Mask environment variable if they passed in case of inside docker image run by vkravets · Pull Request #166 · jenkinsci/docker-workflow-plugin

vkravets · 2019-03-22T06:56:29Z

Resolves https://issues.jenkins-ci.org/browse/JENKINS-56674

I would like to propose and show which issue exists with masked env variables during such case

node {
  withDockerContainer('ubuntu') {
    env.TEST_PWD = 'pwd12345'
    withDockerContainer('ubuntu') {
      withDockerContainer('ubuntu') {
		sh('echo test')
      }
    }
  }
}

So the output of job will have include env variable which can contains sensitive data (first part of docker exec):

 6.514 [prj #1] [Pipeline] node
   6.617 [prj #1] Running on master in /Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj
   6.617 [prj #1] [Pipeline] {
   7.814 [prj #1] [Pipeline] withDockerContainer
   7.814 [prj #1] Jenkins does not seem to be running inside a container
   7.815 [prj #1] $ docker run -t -d -u 501:20 -w /Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj -v /Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj:/Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj:rw,z -v /Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj@tmp:/Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj@tmp:rw,z -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** ubuntu cat
   7.815 [prj #1] $ docker top c44d7264133f649cc80cc97aae11272e00c5023efe9c34e86d69ea71dc7beb91 -eo pid,comm
   7.815 [prj #1] [Pipeline] {
  10.504 [prj #1] [Pipeline] withDockerContainer
  10.504 [prj #1] ERROR: Failed to parse docker version. Please note there is a minimum docker version requirement of v1.7.
  10.505 [prj #1] Jenkins does not seem to be running inside a container
  10.505 [prj #1] $ docker exec --env BUILD_DISPLAY_NAME=#1 --env BUILD_ID=1 --env BUILD_NUMBER=1 --env BUILD_TAG=jenkins-prj-1 --env BUILD_URL=http://localhost:56168/jenkins/job/prj/1/ --env CLASSPATH= --env EXECUTOR_NUMBER=1 --env HUDSON_HOME=/Users/vkravets/work/my/docker-workflow-plugin/./tmp --env HUDSON_SERVER_COOKIE=586ce441e4ad2814 --env HUDSON_URL=http://localhost:56168/jenkins/ --env JENKINS_HOME=/Users/vkravets/work/my/docker-workflow-plugin/./tmp --env JENKINS_SERVER_COOKIE=586ce441e4ad2814 --env JENKINS_URL=http://localhost:56168/jenkins/ --env JOB_BASE_NAME=prj --env JOB_NAME=prj --env JOB_URL=http://localhost:56168/jenkins/job/prj/ --env NODE_LABELS=master --env NODE_NAME=master --env TEST_PWD=pwd12345 --env workspace=/Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj c44d7264133f649cc80cc97aae11272e00c5023efe9c34e86d69ea71dc7beb91 docker run -t -d -u 501:20 -w /Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj -v /Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj:/Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj:rw,z -v /Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj@tmp:/Users/vkravets/work/my/docker-workflow-plugin/tmp/workspace/prj@tmp:rw,z -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** ubuntu cat

As you can see second docker run (which is wrapped by docker exec logged all env variables) which is not security

…nside docker image run

vkravets · 2019-03-22T07:20:01Z

Guys, sorry that disturb you, but I think this should be fixed asap, since it's a security issue... Could you please review?
@jglick @ndeloof @abayer

conn · 2019-03-22T17:22:43Z

You shouldn't hardcode passwords in your Jenkinsfile or anywhere in your repositories.

Values in environment variables that are also provided by the credentials API will be masked in console output when docker.inside is ran. Values that are not provided by the credentials API are at risk of being leaked via other means.

Unless my understanding of how credentials and console output work together is flawed, this wouldn't be a risk for anyone that is retrieving credentials via appropriate channels.

vkravets · 2019-03-23T02:16:19Z

Yes we should not, case which I've shown in test it's just a test, environment variable can be field from any source(vault, file, DB, credentials api or something else) and passed to process by env... In the same way all environment variable in the end of command masked, but at the beginning not.

The case which I've try to described appeared only in case of recursive call of which is covered by logic of this step, but not masked env variables... Try to look at the code and I hope you will understand what I mean.... @conn

vkravets · 2019-03-23T02:21:06Z

Unless my understanding of how credentials and console output work together is flawed, this wouldn't be a risk for anyone that is retrieving credentials via appropriate channels.

@conn not a true, since you can get credentials from any source and it will not logged in plain text but in case if it filled as env and passed to inContainer it will logged in the docker exec command which is trying to pass all env variables as command parameters

vkravets · 2019-03-26T12:06:37Z

@jglick @ndeloof @abayer is it possible to review this PR?

jglick · 2019-03-27T19:51:31Z

src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java

+                        "  withDockerContainer(image:'docker',\n" +
+                        "                      args:'-v /var/run/docker.sock:/var/run/docker.sock --user root') {\n" +
+                        "    env.TEST_PWD = 'pwd12345'\n" +
+                        "    withDockerContainer(image:'docker',\n" +


This is not a plausible test case. I am working on a clearer one.

dwnusbaum · 2019-03-29T18:42:43Z

Thanks for the PR @vkravets! I've just merged #167 which includes this PR.

vkravets mentioned this pull request Mar 22, 2019

Mask environment variable if they passed in case of inside docker image run #165

Closed

[JENKINS-56674] Mask environment variable if they passed in case of i…

67b802f

…nside docker image run

vkravets force-pushed the JENKINS-56674-fix-env-masking branch from 875e3f0 to 67b802f Compare March 22, 2019 07:04

jglick requested changes Mar 27, 2019

View reviewed changes

jglick mentioned this pull request Mar 27, 2019

[JENKINS-56674] Masking of environment variables passed to docker exec #167

Merged

dwnusbaum merged commit 67b802f into jenkinsci:master Mar 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-56674] Mask environment variable if they passed in case of inside docker image run#166

[JENKINS-56674] Mask environment variable if they passed in case of inside docker image run#166
dwnusbaum merged 1 commit intojenkinsci:masterfrom
vkravets:JENKINS-56674-fix-env-masking

vkravets commented Mar 22, 2019 •

edited

Loading

Uh oh!

vkravets commented Mar 22, 2019

Uh oh!

conn commented Mar 22, 2019

Uh oh!

vkravets commented Mar 23, 2019

Uh oh!

vkravets commented Mar 23, 2019

Uh oh!

vkravets commented Mar 26, 2019

Uh oh!

jglick Mar 27, 2019

Uh oh!

jglick Mar 27, 2019

Uh oh!

dwnusbaum commented Mar 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

vkravets commented Mar 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vkravets commented Mar 22, 2019

Uh oh!

conn commented Mar 22, 2019

Uh oh!

vkravets commented Mar 23, 2019

Uh oh!

vkravets commented Mar 23, 2019

Uh oh!

vkravets commented Mar 26, 2019

Uh oh!

jglick Mar 27, 2019

Choose a reason for hiding this comment

Uh oh!

jglick Mar 27, 2019

Choose a reason for hiding this comment

Uh oh!

dwnusbaum commented Mar 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vkravets commented Mar 22, 2019 •

edited

Loading