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

Build Secrets #30637

Closed
wants to merge 1 commit into from
Closed

Build Secrets #30637

wants to merge 1 commit into from

Conversation

ehazlett
Copy link
Contributor

@ehazlett ehazlett commented Feb 1, 2017

This replaces #28079 as I had to re-open.

This enables build time secrets using the --build-secret flag.

Similar to #27794 this creates a tmpfs during each run of the build and exposes the specified "secrets" into the container. These are not committed to the image.

Here is an example Dockerfile:

FROM busybox:latest
RUN mount | grep tmpfs
RUN find /run
RUN cat /run/secrets/git-key

Given the following command:

docker build -t test --no-cache --build-secret source=~/git-key,target=git-key .

This will expose the key from ~/git-key as /run/secrets/git-key. This can be used however needed during build by the corresponding RUN directives.

Here is the example output:

Sending build context to Docker daemon 3.072 kB
Step 1/4 : FROM busybox:latest
 ---> e02e811dd08f
Step 2/4 : RUN mount | grep tmpfs
 ---> Running in 9c7863ab331b
tmpfs on /dev type tmpfs (rw,nosuid,mode=755)
tmpfs on /sys/fs/cgroup type tmpfs (ro,nosuid,nodev,noexec,relatime,mode=755)
tmpfs on /run type tmpfs (ro,nosuid,nodev,noexec,relatime)
shm on /dev/shm type tmpfs (rw,nosuid,nodev,noexec,relatime,size=65536k)
tmpfs on /proc/kcore type tmpfs (rw,nosuid,mode=755)
tmpfs on /proc/timer_list type tmpfs (rw,nosuid,mode=755)
tmpfs on /proc/timer_stats type tmpfs (rw,nosuid,mode=755)
tmpfs on /proc/sched_debug type tmpfs (rw,nosuid,mode=755)
tmpfs on /sys/firmware type tmpfs (ro,relatime)
 ---> 60836fd9a783
Removing intermediate container 9c7863ab331b
Step 3/4 : RUN find /run
 ---> Running in 56a46a797034
/run
/run/secrets
/run/secrets/git-key
 ---> 908a73ff32a1
Removing intermediate container 56a46a797034
Step 4/4 : RUN cat /run/secrets/git-key
 ---> Running in e8f6b05e7a81
TESTKEY
 ---> 3c13c956d170
Removing intermediate container e8f6b05e7a81
Successfully built 3c13c956d170
root@dev:~/build# docker run --rm -ti test ls -lah /run/
total 8
drwxr-xr-x    2 root     root        4.0K Nov  4 19:55 .
drwxr-xr-x    1 root     root        4.0K Nov  4 19:56 ..

Example using a private git key

Dockerfile

FROM alpine:latest
RUN apk add -U git openssh
COPY ssh_config /root/.ssh/config
RUN ls -lah /run/secrets
RUN git clone ssh://git@hello.frie.nd:/x/repo
$> docker build -t testimage --build-secret source=~/hello.key,target=id_rsa,mode=0400 .
Sending build context to Docker daemon 5.632 kB
Step 1/5 : FROM alpine:latest
 ---> 88e169ea8f46
Step 2/5 : RUN apk add -U git openssh
 ---> Using cache
 ---> fce3270066bb
Step 3/5 : COPY ssh_config /root/.ssh/config
 ---> Using cache
 ---> 7030c50fbc41
Step 4/5 : RUN ls -lah /run/secrets
 ---> Running in 90a1ec20dc54
total 4
drwxr-xr-x    2 root     root          60 Feb  7 19:57 .
drwxrwxrwt    3 root     root          60 Feb  7 19:57 ..
-r--------    1 root     root        1.6K Feb  7 19:57 id_rsa
 ---> 9f45932eb25b
Removing intermediate container 90a1ec20dc54
Step 5/5 : RUN git clone ssh://git@hello.frie.nd:/x/repo
 ---> Running in 910bcff08a04
Cloning into 'repo'...
Warning: Permanently added '[hello.frie.nd]:22,[192.251.68.254]:22' (ECDSA) to the list of known hosts.
 ---> 3939f5d3e275
Removing intermediate container 910bcff08a04
Successfully built 3939f5d3e275

@ehazlett ehazlett changed the title enable build secrets [WIP] Build Secrets Feb 1, 2017
@ehazlett ehazlett mentioned this pull request Feb 1, 2017
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

Design LGTM

if err != nil {
return nil, errors.Wrap(err, "unable to create temp directory for secrets")
}
if err := mount.Mount("tmpfs", tempDir, "tmpfs", "nodev,nosuid,noexec"); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

should be readonly perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added the remount to ro.


This will expose the contents of `github-key` in the container during build
as `/run/secrets/git.key`.

Copy link
Member

Choose a reason for hiding this comment

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

It would be better to clarify that a file in secret cannot be executed, so that no one try to put install-some-proprietary-software.sh to secret

Choose a reason for hiding this comment

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

Why would you want to prevent people from installing proprietary software?

In any case, can't you always

RUN cp /run/secrets/foo.sh /tmp/ && chmod +x /tmp/foo.sh && /tmp/foo.sh

regardless.

Copy link
Member

Choose a reason for hiding this comment

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

@ehazlett is this different from secrets on docker service? On services, I was able to actually provide an executable as secret?

Copy link
Contributor Author

@ehazlett ehazlett Feb 7, 2017

Choose a reason for hiding this comment

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

@thaJeztah it was probably using the cache as by default the mode is read-only, no-execute. Here is an example:

docker build --no-cache -t testimage --build-secret src=/usr/local/bin/docker,target=docker,mode=0755 .
Sending build context to Docker daemon 12.49 MB
Step 1/4 : FROM alpine:latest
 ---> 88e169ea8f46
Step 2/4 : RUN mount | grep /run
 ---> Running in d158b3ff377c
tmpfs on /run type tmpfs (ro,relatime)
 ---> 9a3f370992b0
Removing intermediate container d158b3ff377c
Step 3/4 : RUN ls -lah /run/secrets/
 ---> Running in f99f664b5034
total 12200
drwxr-xr-x    2 root     root          60 Feb  7 19:52 .
drwxrwxrwt    3 root     root          60 Feb  7 19:52 ..
-rwxr-xr-x    1 root     root       11.9M Feb  7 19:52 docker
 ---> d12b13566935
Removing intermediate container f99f664b5034
Step 4/4 : RUN /run/secrets/docker -v
 ---> Running in 2c64f083244f
Docker version 1.14.0-dev, build 46c4cda71
 ---> 8e94d203c623
Removing intermediate container 2c64f083244f
Successfully built 8e94d203c623

In the original issue we decided that an explicit cache bust was preferable instead of busting or trying to maintain a hash of a secret (#28079 (comment))

@NikolausDemmel
Copy link

Wouldn't the syntax

docker build -t test --no-cache --build-secret source=~/git-key:git-key .

be more in line with other command line options like volumes?

@ehazlett
Copy link
Contributor Author

ehazlett commented Feb 3, 2017 via email

@ehazlett
Copy link
Contributor Author

ehazlett commented Feb 7, 2017

Added an example using a private git key in the description.

@@ -373,6 +374,21 @@ the command line.
> repeatable builds on remote Docker hosts. This is also the reason why
> `ADD ../file` will not work.

### Build with Secrets
Copy link
Member

Choose a reason for hiding this comment

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

It might be worthwhile for the daemon to be able to specify default build secrets.
The use case for this would be something like importing rhel subscriptions.

Copy link
Member

Choose a reason for hiding this comment

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

@cpuguy83 @ehazlett should this be discussed in this PR or in a follow-up one though ? I would vote for a follow-up, let's do simple "small" steps 👼

Copy link
Member

Choose a reason for hiding this comment

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

would be nice to have rhel subs while building daemon wide (but we also need them at run time :( and we currently inject those)

@joelpresence
Copy link

We desperately need this ... Any chance we could get a patch version 1.13.1 out quick with this merged? :-)

@AkihiroSuda
Copy link
Member

@joelpresence Since this is a feature addition, I think this is unlikely to be included in 1.13.1.
But you could:

$ hub checkout https://github.com/docker/docker/pull/30637
$ make deb

@nathanleclaire
Copy link
Contributor

@ehazlett Dude, this is awesome! Folks (myself included) have been wanting this for literally years.

I'm a bit curious about how we intend to keep users from accidentally doing something like:

RUN cp /run/secrets/mykey /home/containeruser/.ssh/id_rsa
RUN git clone git@github.com:me/privaterepo

(which if I understand correctly would end up with /home/containeruser/.ssh/id_rsa committed to the image)

Since that would "just work" in most cases (but would leak the secret), it's going to be pretty tempting for average user. I'm assuming maybe you're specifying SSH key path in the config file you pass in above to be /run/secrets/mykey to prevent this?

It might be nice to have some helpers to symlink files from the tmpfs to somewhere else (that way folks can cargo cult the solution comfortably vs. having to customize for SSH or something every time). I'm not 100% sure what would be a nice solution though. Maybe there are limited enough use cases for build time secrets that it might be OK.


```bash
docker build -t app \
--secret source=/path/to/github-key,target=git.key .
Copy link
Contributor

Choose a reason for hiding this comment

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

Inconsistent with --build-secret here. (Although isn't --build-secret a bit redundant anyway? ;) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya good catch. I used --build-secretto match the --build-env flag

@@ -373,6 +374,21 @@ the command line.
> repeatable builds on remote Docker hosts. This is also the reason why
> `ADD ../file` will not work.

### Build with Secrets

Copy link
Member

Choose a reason for hiding this comment

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

I think there should be some comparison between the build secret and the swarm secret, but it can be another PR

@ehazlett
Copy link
Contributor Author

ehazlett commented Feb 10, 2017

@nathanleclaire Ya the ssh keys are tough. I solved it by using a custom ssh config. Like so:

Host *
    IdentityFile /run/secrets/id_rsa

As far as copying, I'm not sure what we can do. If we had a fuse fs we could block that but since it's just tmpfs probably education at this point similar to swarm secrets.

@nathanleclaire
Copy link
Contributor

nathanleclaire commented Feb 10, 2017

As far as copying, I'm not sure what we can do. If we had a fuse fs we could block that but since it's just tmpfs probably education at this point similar to swarm secrets.

Yeah, it might just be one of those cases where we have to have big all-caps warnings and clear education about best practices in the docs, and that SSH config is pretty simple which is nice.

Maybe it might be feasible to have an additional option which would set a symlink from the tmpfs mount to a target FS location in container, e.g.,

$ docker build \
    --build-secret source=~/.aws/config,target=awsconfig,link=/home/user/.aws/config \
    .

Then it would be a universal way to share secret files safely instead of having to re-design each time and having different solutions for certs, AWS keys, etc., etc.

@nathanleclaire
Copy link
Contributor

nathanleclaire commented Feb 10, 2017

Also, --build-secret source=~/.aws/config is a Docker client side path?

edit: yep.

@thaJeztah
Copy link
Member

Bit late to the party, and haven't had a lot of time diving into this so far. I love the ability to have secrets for build, as it's something that's really missing now. I do have some concerns / thoughts though;

  • build secrets allow you to use local files as a secret. This encourages (enforces even) to store those secrets on disk.
  • in addition, there's no relation with the docker secrets commands, which is confusing (i.e., I cannot "manage" those secrets
  • not even sure if that's on the roadmap, but I can see an opportunity of being able to use a swarm for building; i.e., docker build uploading the build context to a node in the swarm, and executing the build there

This will add a "pre-condition", but would it make sense to require swarm mode to be active, and actually using the swarm-mode secrets for this? Basically;

docker swarm init
echo "passw0rd" | docker secret create --name github-key -
docker build --secret github-key -t my-image .

Given that docker build uses regular containers to run the build steps in, this would probably mean we need to implement "secrets" for docker run as well (having a dependency on having swarm mode enabled).

@ehazlett
Copy link
Contributor Author

Given that docker build uses regular containers to run the build steps in, this would probably mean we need to implement "secrets" for docker run as well (having a dependency on having swarm mode enabled).

Yes this is why it works the way it does today. In my original PR for secrets I pushed to support containers so we could do just this but it was decided otherwise for various security reasons. I agree that if we could get secrets for containers we could use Swarm as the store but I'm not sure what the timeline would be for that. I also think that there is a use case for not having them in a Swarm store (i.e. they could collide with a production secret, etc) and rather locally.

@vdemeester
Copy link
Member

I also love the ability to have secrets for build, as it's something that's really missing now.
I do agree with @thaJeztah on the potential "confusion" with swarm secrets.

As long as we don't support "swarm mode" (and thus secret) for containers and build, maybe this shouldn't be called secret but something more abstract — as it's basically limited mounts, maybe, a first steps would be to allow some sort of "mount" for build that could be used as secret as implemented in this PR. (/me thinking outloud 🤔)

@neomantra
Copy link
Contributor

@vdemeester We were looking for this In a discussion about injecting HashiCorp Vault secrets in the Docker builds.

It started with me talking about the sorta complicated way I take a "wrapped token" from Vault and pass it as build-arg and then unwrap it in the build context. Later in it, it became clear that it would be more useful to have access to a tmpfs mount, but of course we can't build our own solution (without modifying docker) because tmpfs require privileges and builds run unprivileged.

I'm mostly noting this as a use case: some users of this secret feature might not be pulling files from an existing file system. but rather from some secrets system. [This points to the need for a secrets backend, but I'm super happy to have this sooner rather than later.]

For this Hashi Vault use case, I think what I will do is extract secrets into a host tmpfs then map all those files to the build with multiple --build-secret args, and then tear down my local tmpfs.

@eatdrinksleepcode
Copy link

What is needed to move this forward? Build-time secrets have been in discussion for years, and all discussions seem to (eventually) point here. A month ago reviewers seemed to generally approve. Does it just need a rebase?

@briceburg
Copy link

briceburg commented Apr 28, 2017

Is there a possibility of a SECRET directive in the builder? I think the comments in #30637 (comment) make a lot of sense considering transparency and build repeatability.

The directive could be correlated as ARG is to the --build-arg flag. E.g.

$ docker build --secret=source=acme-ro.key,target=github_key,mode=0400 .
# Dockerfile example, dest option symlinks secret to path
#SECRET <name> [options]
SECRET github_key dest=/root/.ssh/id_rsa

RUN git clone git@github.com:acme/private-repo.git
...

Also, should the secret source (if a file) be limited to the build context? This potentially guards against non-repeatable builds.

Obviously having the source file on disk and unencrypted is not good -- and we'd probably want (pre)create and (post)remove it as part of a build script / orchestration. What would be very cool and solve this is to allow plugins to handle the source, such as reading from hashicorp vault. This does go against repeatability however. Something like;

$ docker build --secret=plugin=hashicorp_vault,source=keys/acme-ro.key,target=private_key .

In any case, build time secrets will be incredibly useful -- and, at least to me, they're more important than runtime which we can get by using vault or tmpfs bindmounts or swarm. Thanks!

@ehazlett
Copy link
Contributor Author

@eatdrinksleepcode i'll bring this up again in the maintainer meeting. there were a few design questions (i.e. wanting to use swarm instead of these locally).

@briceburg Yes in a former branch I had implemented SECRET. We decided to split it apart as you said runtime secrets have some more requirements and limitations such as secrets only work in Swarm mode. I also agree that build secrets are needed and we can address runtime in a separate PR.

In the meantime, I'll work on rebasing to get this ready in case maintainers can agree :)

@dnephin
Copy link
Member

dnephin commented Apr 28, 2017

Looking at the implementation I think #32507 is a better solution for this.

@diogomonica
Copy link
Contributor

@dnephin agreed, looks good.

@ehazlett
Copy link
Contributor Author

wfm

@ehazlett ehazlett closed this Apr 29, 2017
@tonistiigi
Copy link
Member

@ehazlett Can you provide some clarification behind closing this. Are you suggesting #32507 instead of the SECRET dockerfile syntax? #32507 itself does not provide things like using tmpfs for storage, not persisting secret values in build cache, or sending them with a client(#32677 could help with that). Or is it blocked by swarmkit dependency? Also, note that nobody is currently working on #32507, if that becomes a requirement for build secrets we might need to assign a higher priority.

@thaJeztah
Copy link
Member

I'd still be for having the same approach as for swarm services; possibly requiring swarm mode to be enabled if people want to make use of this feature. Feels confusing to use completely different mechanisms for one or the other (build vs run / service create)

@ehazlett
Copy link
Contributor Author

ehazlett commented May 2, 2017

@tonistiigi This PR has stagnated and maintainers can't agree on a design. This PR didn't add the SECRET directive (that was a different one, #28079 I think). I agree with @thaJeztah that having different methods would not be ideal and only cause more issues in implementation on the backend. Security and another maintainer like #32507 better so that, along with the deadlock in design, is why I closed this.

@tonistiigi tonistiigi mentioned this pull request May 22, 2017
@bilby91
Copy link

bilby91 commented Jul 11, 2017

Is there any version of docker that supports this flag already ? I'm running Docker version 17.06.0-ce, build 02c1d87 and don't see the flag with --help.

@cpuguy83
Copy link
Member

@bilby91 No, this is not merged.

@BenoitNorrin
Copy link

The new multi-stage build feature (17.06) may solves all ours problems, right ?

@bilby91
Copy link

bilby91 commented Jul 12, 2017

@BenoitNorrin what is the multi-stage feature you are mentioning ?

@jamesongithub
Copy link

@pvanderlinden
Copy link

This will still bake the secrets into the image

@BenoitNorrin
Copy link

@pvanderlinden Can you explain why please ?

@pvanderlinden
Copy link

@BenoitNorrin For example if you use internal software repositories:
In our case, roughly:

  • private install dependencies which can be used for the pip/conda package manager
  • we build the package outside the container
  • we then copy that to the build container
  • we then create a pip.conf file with the credentials for the pypi repository from an environment variable
  • we also add a conda channel in certain cases from an environment variable
  • we then install the package, pulling in all dependencies

This will add the secrets in multiple way to the end image:

  • the files used by pip & anaconda will be in a layer
  • the environment variable with the content of these files, including credentials, will be part of the image metadata.

I think a good collection of these problems can be found in this ticket:
#13490

@AndreKR
Copy link

AndreKR commented Aug 21, 2017

  • the files used by pip & anaconda will be in a layer
  • the environment variable with the content of these files, including credentials, will be part of the image metadata.

Nope, all this will be discarded together with the first image. The result image only contains layers and ENVs starting from the second FROM in your Dockerfile.

@BenoitNorrin
Copy link

BenoitNorrin commented Aug 21, 2017

I probably miss something.

Here an example.

FROM alpine as builder
ARG LOGIN
ARG PASSWORD
RUN echo Login : ${LOGIN}
RUN echo Password : ${PASSWORD}
RUN echo "Hello World" > /my-file.txt

FROM nginx
COPY --from=builder /my-file.txt /usr/share/nginx/html

docker build -t multi-stages --build-arg LOGIN=foo --build-arg PASSWORD=bar .

IMAGE               CREATED             CREATED BY                                      SIZE                COMMENT
e303c4944241        14 minutes ago      /bin/sh -c #(nop) COPY file:6d2ecc9da483e8...   12B
3448f27c273f        3 months ago        /bin/sh -c #(nop)  CMD ["nginx" "-g" "daem...   0B
<missing>           3 months ago        /bin/sh -c #(nop)  STOPSIGNAL [SIGQUIT]         0B
<missing>           3 months ago        /bin/sh -c #(nop)  EXPOSE 80/tcp                0B
<missing>           3 months ago        /bin/sh -c ln -sf /dev/stdout /var/log/ngi...   22B
<missing>           3 months ago        /bin/sh -c apt-get update  && apt-get inst...   52.2MB
<missing>           3 months ago        /bin/sh -c #(nop)  ENV NJS_VERSION=1.13.0....   0B
<missing>           3 months ago        /bin/sh -c #(nop)  ENV NGINX_VERSION=1.13....   0B
<missing>           3 months ago        /bin/sh -c #(nop)  MAINTAINER NGINX Docker...   0B
<missing>           3 months ago        /bin/sh -c #(nop)  CMD ["/bin/bash"]            0B
<missing>           3 months ago        /bin/sh -c #(nop) ADD file:a90ec883129f86b...   57.1MB

No matter what is done in the first stage (builder), this not embed in the final stage.
There are probably some precautions to be taken with some package managers and with some actions not to bring back cache files (need some RUN of cleaning in the builder before copying files) but it is now possible to access private things.

@pvanderlinden
Copy link

@AndreKR You confuse a build container and installations.
With some installations it is as simple as copying a single binary (with go for example), with other ones you will install all dependencies in the installation step, so that won't happen in the first container, but in the last container.

@AndreKR
Copy link

AndreKR commented Aug 21, 2017

I don't know about pip, but I know about npm and composer and in both cases you install all dependencies in the first stage and you just copy node_modules and vendor to the second stage together with the rest of the applications. No credentials copied.

@pvanderlinden
Copy link

That would work indeed, but I hardly call handpicking the files you need from a different container a solution to the build secret problem (and this method of handpicking files was already possible). In many occasions it is much simpler to only ignore the credentials you explicitly pass into a build container.

@BenoitNorrin
Copy link

See : #13490 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.