1093833 - docker-worker: Investigate and update volume mount permissions

Status: RESOLVED WONTFIX

(Taskcluster :: Workers, defect, P1)

Description

[Greg Arndt [:garndt]]

Currently tasks are being run under the root user which does not cause a problem with volume mounts since those mounts are created by the root user on the host machine.

if a task is run under a different user, the root user owns the volume and thus the user in the container cannot write to it.  

It might be possible to startup the docker daemon and the docker-worker under a different account that plays nicely with the docker containers.

Comment 1

[Jonas Finnemann Jensen (:jonasfj)]

Are you talking about cacheFolders?

Note, docker daemon AFAIK needs root, but one can run docker-worker under any user, as long as it has read/write access to docker socket...

Comment 2

[Greg Arndt [:garndt]]

These are the cached folders we specify in a task and get mounted within a container.  docker-worker creates these directories on the host first and then docker volume mounts them.  I'm going to see if it's possible to run the docker-worker using another user (such as the already configured ubuntu user with uid/gid 1000) and see if things getter better.

Comment 3

[Jonas Finnemann Jensen (:jonasfj)]

Running docker-worker under another user makes sense in that if docker-worker is compromised, exploitability will be somewhat limited... It's the same old layers of security story.

@garndt,
The cacheFolder problems seems more like a permission issue.
You can chown a folder to another user if you like, but unless the user in the docker container is the same I suspect it pointless.
You're probably better of just chmod 777 /.../cachefolder/

If users of the cache folder aren't always the same user... Or they mess around with permissions, that seems like not our issue. Giving 777 should give all users read/write/execute rights. Which is basically what we want.
Security-wise it's never ideal that cacheFolders exists on the host system, but as long as nothing is able to call into the cacheFolder it'll be fine.

Just do:
fs.chmodSync('/path/to/cache/folder/', '777');

And see if that doesn't solve the issue.

Comment 4

[Jonas Finnemann Jensen (:jonasfj)]

Note, this assume host permissions are carried into docker container, when folders from host is mounted.
I think I remember reading that they do... But I could be wrong.

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

It should be possible to get the USER from the image metadata.  However, I don't know if there's a way to predict the host userid to which that username will be mapped when the container runs.  I've struggled with this at home, too -- at some point a docker update occurred and userid to which 'worker' mapped on my system changed from 1000 to 500.

At worst, we could inject 'chmod' commands into the container, with -u root, for all caches before running the command specified in the payload (as the image's desired user).  Or even inject a shell script which does that.

Comment 6

[Jonas Finnemann Jensen (:jonasfj)]

A possible option is that we might have to use data volumes.
I suspect from random mailing list that this _might_ be a solution, still needs to be verified.

But I also slightly I fear that this might prevent us from preloading the cache folders programmatically.
(Note, this possibly affects abstractions pmoore and I are designing for generic-worker)

Curious can't we just 777 the cache folders, or maybe that'll create problems with some programs that
expect permissions to be somewhat restricted. Yeah, well it was a tempting solution.

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

777 might work but doesn't seem "right", particularly since the ownership of the directory will still be incorrect. I don't think Firefox would mind, but SSH would complain if a home directory were mounted with root:root:0777.

http://stackoverflow.com/questions/23544282/what-is-the-best-way-to-manage-permissions-for-docker-shared-volumes/27021154#27021154 is a good description of doing this with data containers.

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

I don't see it stated in the documentation anywhere, but it looks like docker uses a single user namespace for all of its containers on a host, so that uid U on one container will map to uid U on another container, even if it maps to some other uid V on the host.  So if you have a data container with directories owned by uid 515, and your task container user's uid is 515, all is well.  But aligning uids for matching usernames is not automatic - it's based on the uid that 'useradd' selects.  This seems to be hidden in the SO post above by requiring that data and app containers both use the same base image and call `useradd` early in the Dockerfile.

One easy fix for this would be to pick an arbitrary userid for caches - 1500, say, to keep out of the way of auto-numbering beginning at 500 or 1000 - and require task containers to match that (with `-u 1500` or whatever the argument to `useradd` is).

An advantage to this approach is that it would be easier for developers to use.  I've had no end of troubles with uid mismatches in trying to run task images locally (I've been running tasks as non-root locally), but if I had a few commands to run to set up data containers and run with --volumes-from tc-vcs-cache, etc., that'd be a lot easier.

A few other potential issues with this approach:
 - not clear that you can map the same volume to different directories in different containers
 - data containers still require an OS image, which means loading a few GB of layers that may not correspond to any running tasks

We could fix the first by indexing caches with <cache name, mount location>.

We could fix the second by creating a base image containing only a statically-linked 'chown' or something like it (a simple go executable, perhaps, that calls mkdir(2) and chown(2)).

The 0777 solution sounds a lot easier!  However, I think the heat is off for the moment since the Firefox build-linux.sh script now drops root after chown'ing the cache directories.  So there's time to implement this well.

Comment 9

[Jonas Finnemann Jensen (:jonasfj)]

> aligning uids for matching usernames is not automatic - it's based on the uid that 'useradd' selects
Well, I'm not sure we really want caches to be shared between different images.

> uid mismatches in trying to run task images locally
Don't use caches locally, unless you're specifically debugging that.

> - not clear that you can map the same volume to different directories in different containers
Yeah, I have yet to figure out how volumes map between containers.

> - data containers still require an OS image, which means loading a few GB of layers 
You can make a dummy image, with: "FROM scratch" (also mostly docker images aren't GBs).

> We could fix the first by indexing caches with <cache name, mount location>.
Maybe it's: cacheId = hash(<cacheName> + <mount path> + <task.payload.image>)
This would dramatically reduce cache collisions too, obviously this is not free though.

Or perhaps caches should just be given as a list of paths:
  task.payload.cache = ["/home/worker/.tc-vcs", "/home/worker/obj-dir"]
As same image, same path and same workerType is the same cache in most cases anyways.
In cases where people want to parameterize caches on per task level they can just make a folder for it,
and inject the name of that folder by env variable.

This also makes it hard to make cache explosion. And in case of compiler upgrades, we're sure to not
get cache poisoning. The cost is hard to estimate, but this is simpler and removes a major footgun :)

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

> Well, I'm not sure we really want caches to be shared between different images.
Whut?  We're doing lots of that now (tc-vcs, for example).  Sure, cache invalidation is hard (thanks Phil Karlton), but limiting the sharing of caches will, I suspect, do a great deal of damage to hit rates, while never eliminating invalidation issues.

> Don't use caches locally, unless you're specifically debugging that.
And spend many extra hours waiting for builds? No thanks..

Comment 11

[Greg Arndt [:garndt]]

Just making a mental note here that the agreed upon solution while in Berlin is that a particular UID for a user within the container will be required if using caches.  This UID should be one that our linux platforms do not automatically assign.

Comment 12

[Nick Alexander :nalexander [he/him]]

(In reply to Greg Arndt [:garndt] from comment #11)
> Just making a mental note here that the agreed upon solution while in Berlin
> is that a particular UID for a user within the container will be required if
> using caches.  This UID should be one that our linux platforms do not
> automatically assign.

How do I actually do this?  I'm trying to run a build that uses tooltool locally.  Dustin suggested that I use -v to mount volumes for tooltool caches.  That immediately runs into https://github.com/boot2docker/boot2docker/issues/587, as suggested by this log:

++ tc-vcs checkout /home/worker/workspace/build/tools https://hg.mozilla.org/build/tools https://hg.mozilla.org/build/tools default default
[taskcluster-vcs] detectHg: start fetching head of https://hg.mozilla.org/build/tools
[taskcluster-vcs] detectHg: end fetching head of https://hg.mozilla.org/build/tools
Error: EACCES, mkdir '/home/worker/.tc-vcs/clones'

/usr/lib/node_modules/taskcluster-vcs/build/bin/tc-vcs.js:58
        throw err;
              ^
Error: EACCES, mkdir '/home/worker/.tc-vcs/clones'

garndt, dustin: can you help here?  I'm running docker-machine on Mac OS X to try to run a modified "android-api-11" locally.

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

Docker doesn't make this easy, sadly, since it doesn't actually namespace uids.  The solution we've worked out above is basically to only use one uid for everything, but if that uid doesn't happen to correspond do your homedir, then it still wouldn't help you run it locally.

When I have done this sort of hacking, I've had to chmod the directory to the uid used in the docker container (500, if I recall).  It's super-clunky, but it does work.  I ended up creating dedicated directories for linking to containers, and just copied the TC cache in there.

Comment 14

[Wander Lairson Costa]

Well, I have a proposal, and would like to hear some comments. Here it is. Given an uid U:
1. At startup, docker-worker tries to drop its privileges to U.
2. If it is successful, go to step 4. If U doesn't exist, create an user with uid U.
3. docker-worker drops its privileges to U.
4. End.

What do you think? I don't remember if Linux allows to create users with specific uid, I need to check my old books.

Comment 15

[Dustin J. Mitchell [:dustin] (he/him)]

Linux does let you create users with a specific uid.  Also, I think the setuid call doesn't check whether the given UID exists in /etc/passwd, so I doubt the call would ever be unsuccessful.

But I don't understand where this U comes from, and how it solves the problem of cache permissions?  Also, why is docker-worker itself changing its privilege, instead of just running the docker containers under a specific userid?

Comment 16

[Wander Lairson Costa]

(In reply to Dustin J. Mitchell [:dustin] from comment #15)
> Linux does let you create users with a specific uid.  Also, I think the
> setuid call doesn't check whether the given UID exists in /etc/passwd, so I
> doubt the call would ever be unsuccessful.
> 
> But I don't understand where this U comes from, and how it solves the
> problem of cache permissions?  Also, why is docker-worker itself changing
> its privilege, instead of just running the docker containers under a
> specific userid?

U is just a generic name for the uid number we agree on. Putting the logic inside docker-worker, we make sure it always run with the same uid, and create cache folders with that same uid permissions. The only downside would be to purge all caches before running it for the first time.

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

I don't think we actually want docker-worker to drop privileges -- doing so might make other things difficult, and anyway wouldn't change the UID under which the containers run.  Running as root, docker-worker can chown the cache directories it creates.

Comment 18

[Jonas Finnemann Jensen (:jonasfj)]

> I don't think we actually want docker-worker to drop privileges -- doing so might make other things difficult

Ideally, we ought to explore docker as non-root. But that's completely unrelated to this.
And not something we should anytime soon -- it's just security :)

I agree doing chown from docker-worker seems like a solid option.

Comment 19

[Greg Arndt [:garndt]]

This has been sitting out there for a long time but no major progress.  I'm going to remove ni? and assign it to me so I'll try to get to it soon.  One of the things introduced in docker 1.10 was the availability of user namespaces that maybe will help with the over all issues with this.  In doing so I'll also be working on a better story around the caches that may or may not exist within the container when running locally or in taskcluster.

Comment 20

[Dustin J. Mitchell [:dustin] (he/him)]

In bug 1289249 gps is setting things up so that the 'worker' userid is 1000 in all Gecko docker images.

So we could potentially close this by just documenting:
 - containers run as root
 - if you drop privileges and want to share caches, we recommend userid 1000

Comment 21

[Gregory Szorc [:gps]]

I dropped the patch to normalize on uid/gid 1000 because it caused too much pain. We can cross that bridge later.

Comment 22

[Wander Lairson Costa]

gps refactored volume mounting and this was fixed as a side effect.