buildkite / sockguard

A proxy for docker.sock that enforces access control and isolated privileges
MIT License
143 stars 22 forks source link

"docker build" `/build` API operations are missing the owner label #25

Open CpuID opened 6 years ago

CpuID commented 6 years ago
#5 02:30:33.433058 POST - /v1.22/build?t=api_test_api_tester&q=False&nocache=False&rm=True&forcerm=False&pull=False - 51200b
#5 02:30:33.433263 Adding label sockguard.owner=/ecs/40b91296-460c-4b42-afd7-7d003f2e2529 to querystring: /v1.22/build t=api_test_api_tester&q=False&nocache=False&rm=True&forcerm=False&pull=False
root@ip-xxx-xxx-xxx-xx:~# docker inspect bb05f145b417 | jq .[0].Config.Labels | grep sockguard
root@ip-xxx-xxx-xxx-xx:~# 

I'll dig deeper, but I wouldn't be surprised if it's not applying the label to the querystring correctly for some reason.

CpuID commented 6 years ago

Confirmed, owner label not being applied:

bash-4.4# cat Dockerfile 
FROM alpine:3.8

RUN sleep 300

CMD [ "ls" ]
bash-4.4# docker build .
Sending build context to Docker daemon  2.048kB
Step 1/4 : FROM alpine:3.8
 ---> 11cd0b38bc3c
Step 2/4 : RUN sleep 300
 ---> Running in 045d9790972b

And from the host (unguarded):

[nathan@ns-desktop-ub cgroup_parent (master)]$ docker inspect 045d9790972b | jq -r .[0].Config.Labels
{}

Logs:

sockguard_1  | #6 02:52:23.982756 POST - /v1.37/build?buildargs=%7B%7D&cachefrom=%5B%5D&cgroupparent=&cpuperiod=0&cpuquota=0&cpusetcpus=&cpusetmems=&cpushares=0&dockerfile=Dockerfile&labels=%7B%7D&memory=0&memswap=0&networkmode=default&rm=1&shmsize=0&target=&ulimits=null - -1b
sockguard_1  | #6 02:52:23.983632 Adding label com.buildkite.sockguard.owner=sockguard-pid-1 to querystring: /v1.37/build buildargs=%7B%7D&cachefrom=%5B%5D&cgroupparent=&cpuperiod=0&cpuquota=0&cpusetcpus=&cpusetmems=&cpushares=0&dockerfile=Dockerfile&labels=%7B%7D&memory=0&memswap=0&networkmode=default&rm=1&shmsize=0&target=&ulimits=null
CpuID commented 6 years ago

Interesting, so there is no way to set the label as a query param on the containers used to build each layer, nor does it add the label to the intermediate layers during the Dockerfile processing (the query param puts them on the final image produced only).

But.... if you do a LABEL key=value right after the FROM os:ver in the Dockerfile, that label is applied:

... :) Confirmed via some local docker inspect hackery. Will involve modifying the input Dockerfile, so a little extra logic.

Some considerations:

CpuID commented 6 years ago

dockerfile
string "Dockerfile" Path within the build context to the Dockerfile. This is ignored if remote is specified and points to an external Dockerfile.

remote
string A Git repository URI or HTTP/HTTPS context URI. If the URI points to a single text file, the file’s contents are placed into a file called Dockerfile and the image is built from that file. If the URI points to a tarball, the file is downloaded by the daemon and the contents therein used as the context for the build. If the URI points to a tarball and the dockerfile parameter is also specified, there must be a file with the corresponding path inside the tarball.

Now that makes it interesting... we probably can't manipulate that as it would be pulled in by the Docker daemon in theory.

Also, reading over this, the input Dockerfile by default is actually part of a tarball (the /build POST has Content-Type: application/x-tar), which means to on-the-fly modify the Dockerfile would involve reading it in sockguard, extracting, modify the Dockerfile content, and rebuilding the tar to send on.

This also likely means we would have to buffer the entire build context in sockguard, which I am not too happy doing (for memory footprint blowout reasons).

Going to put this on the backburner and give it further thought. At least for shared tenancy environments https://github.com/buildkite/sockguard/issues/23 will cover off the container runtime aspects via CgroupParent.