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

WIP: Build Secrets #28079

Closed
wants to merge 33 commits into from
Closed

WIP: Build Secrets #28079

wants to merge 33 commits into from

Conversation

ehazlett
Copy link
Contributor

@ehazlett ehazlett commented Nov 4, 2016

RE-OPENED in #30637

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 (rw,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 ..

Depends on: #27794 I re-used some structs from that PR to reduce duplication.

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

secrets: vendor swarmkit

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

wip: use tmpfs for swarm secrets

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

wip: inject secrets from swarm secret store

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

secrets: use secret names in cli for service create

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

switch to use mounts instead of volumes

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

vendor: use ehazlett swarmkit

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

secrets: finish secret update

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
- fix lint issues
- use errors pkg for wrapping errors
- cleanup on error when setting up secrets mount
- fix erroneous import
- remove unneeded switch for secret reference mode
- return single mount for secrets instead of slice

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
- use /secrets for swarm secret create route
- do not specify omitempty for secret and secret reference
- simplify lookup for secret ids
- do not use pointer for secret grpc conversion

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
- add nosuid and noexec to tmpfs

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

docs: minor update to service create usage

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
- use Filters instead of Filter for secret list
- UID, GID -> string
- getSecrets -> getSecretsByName
- updated test case for secrets with better source
- use golang.org/x/context instead of context
- for grpc conversion allocate with make
- check for nil with task.Spec.GetContainer()

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@vdemeester
Copy link
Member

Something is not quite in the current code :

$ docker build --build-secret source=./Makefile,target=key .
# […]
Step 2/3 : RUN ls /run/secrets
 ---> Running in 3e94087a41d1
Makefile
 ---> e16d40d33e00
Removing intermediate container 3e94087a41d1
Step 3/3 : RUN cat /run/secrets/key
 ---> Running in dd1a7f25908a
cat: can't open '/run/secrets/key': No such file or directory
The command '/bin/sh -c cat /run/secrets/key' returned a non-zero code: 1

Other than that, design LGTM 🐸 for me. I don't think we should invalidate the cache automatically with --build-secret flags — documenting that --no-cache might be required if the secret is modified should be enough. @thaJeztah WDYT ?

@thaJeztah
Copy link
Member

documenting that --no-cache might be required if the secret is modified should be enough

Let me think; even if we would detect changes to the secrets, it would always invalidate the entire cache, because we don't know which layers (possibly) access the secrets, so that would be similar to --no-cache.

Still, it would be good if we could detect changes (as I think users would expect a change to result in a cache bust), but only of course if that doesn't lead to security issues (was thinking of a hash of the encrypted secrets?)

ping @NathanMcCauley @cyli @diogomonica any thoughts?

@ehazlett
Copy link
Contributor Author

ehazlett commented Nov 8, 2016

@vdemeester @justincormack I should have added WIP. This was to get feedback on the idea. I'll add tests.

@vdemeester

Should we create this in /var/lib/docker instead than default temp dir ?

This creates it in /var/lib/docker/tmp. Since it's short lived I didn't think it should have a dedicated spot in /var/lib/docker.

@diogomonica
Copy link
Contributor

Should we even allow secret modifications? Why not have it be read-only and not worry about it?

@ehazlett
Copy link
Contributor Author

ehazlett commented Nov 8, 2016

Should we even allow secret modifications? Why not have it be read-only and not worry about it?

Yes I need to remount to ro.

@ehazlett ehazlett changed the title Build Secrets WIP: Build Secrets Nov 8, 2016
@cyli
Copy link
Contributor

cyli commented Nov 9, 2016

@thaJeztah Possibly if we wanted to do detect changes, salting + stretching the secret might be better than a hash? (e.g. PBKDF2 or scrypt, as opposed to SHA#)

I'm not sure about automatically invalidating the cache - I can see arguments for both cases. --no-cache or using build args to cause cache invalidations would be more explicit and might let the user specify what behavior they want.

@cpuguy83
Copy link
Member

I don't think invalidation is necessarily going to be hard here. Since secrets are effectively immutable and live within Docker, not just some random file, we can use the name+create time of the secret hashed in the build history even. Or if not timestamp, some randomly generated token that we can compare with.

@ehazlett
Copy link
Contributor Author

@cpuguy83 these are build time secrets that come from client side. for example this could be an ssh key that is used for git that could change, or a password for a service, etc.

@NikolausDemmel
Copy link

This would be very useful for us, thanks!

Two small things:

  • I would have expected a syntax like for volumes: docker build -t test --no-cache --build-secret ~/git-key:git-key
  • Would it be possible to specifiy a whole folder, or just single files?

@diogomonica
Copy link
Contributor

I think secrets are orthogonal to builds, and a secret changing doesn't mean changes to the content of the layer itself. Therefore, I think we shouldn't invalidate, and if people so desire, they can use the --no-cache argument.

Additionally, we can always change this to invalidate the cache later (worse case scenario, builds will be less efficient), and we can really never take it back if we ship it with invalidation and people start depending on the cache-busting side-effects.

@cpuguy83
Copy link
Member

I agree. If they really need to rebuild there is --no-cache.

@ehazlett
Copy link
Contributor Author

@diogomonica good point. let's keep it that way and allow the user to decide on invalidation.

thx @diogomonica @cpuguy83

@TomasTomecek
Copy link
Contributor

Care to elaborate why this got closed?

@vdemeester
Copy link
Member

@ehazlett would better elaborate, but I think it's mainly to be concentrated on secrets in services for 1.13 (and make it right) and come back later on that one. Let say its a temporary close to help us focus on 1.13 for now 👼

@ehazlett
Copy link
Contributor Author

ehazlett commented Dec 5, 2016

This is very outdated and not targeted for 1.13. We can re-open if we decide to add this.

@thaJeztah thaJeztah mentioned this pull request Dec 14, 2016
4 tasks
@AndreKR
Copy link

AndreKR commented Dec 28, 2016

Sooo? Those service secrets don't solve the problem of build secrets, do they?

@ehazlett
Copy link
Contributor Author

ehazlett commented Feb 1, 2017

Since there was interest, I've rebased and opened a PR (apparently GH won't let me re-open this since I had to rebase and force push) #30637

@ehazlett ehazlett mentioned this pull request Feb 1, 2017
@joelpresence
Copy link

Hi, any progress on this? Not being able to specify build-time secrets (like GitHub keys or tokens) is holding us back from using Docker. I feel like we've been waiting over a full year for this ... Any ETA on when this will be available?

I don't mean to pile on, and we certainly appreciate all of your hard work in making Docker free for us to use, but unless we can securely pull code from a private github repo into our Docker build, I don't see how we can use Docker?

Thanks! :-)

@BenoitNorrin
Copy link

@joelpresence [WIP] Build Secrets #30637

In the meantime, you have 2 possibilities:

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.

None yet