Closed Bug 1187544 Opened 9 years ago Closed 9 years ago

Verify base Docker images

Categories

(Developer Services :: Mercurial: hg.mozilla.org, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(1 file)

We're using Docker heavily in the version-control-tools testing environment. In bug 1139218 we'll be involving Docker in the operation of our production infrastructure for the very first time (not actually using Docker in production but using Docker to produce a chroot environment that we'll use in production).

`docker pull` has no sane concept of trust (see https://titanous.com/posts/docker-insecurity). So, we need to build our own system for obtaining the base images in a secure and deterministic manner.

This probably involves us uploading base images to S3 and doing SHA-256 verification after pulling them.

P1 since it blocks bug 1139218.
docker: use SHA-256 digests to refer to base images (bug 1187544); r?kang

Modern versions of Docker support pulling images identified by their
content-derived digest. Let's pin the base Docker images to a known good
version so behavior doesn't change over time. This should also give us
some security against tampering.

I don't believe Docker currently implements all the advice from
https://titanous.com/posts/docker-insecurity, specifically the bit about
verifying digests before consuming the data (a vulnerability in
processing images could lead to modification of the Docker daemon to not
report digest failures). Do we care enough to add additional digest
verification on top of what Docker provides?
Attachment #8638862 - Flags: review?(gdestuynder)
Comment on attachment 8638862 [details]
MozReview Request: docker: secure pulling of Docker base images (bug 1187544); r?smacleod, kang

I'm cancelling review because the more Docker source code I read the more I get worried. We already have an abstraction layer for Docker in version-control-tools. I'm going to implement proper security around pulling base images.
Attachment #8638862 - Flags: review?(gdestuynder)
Comment on attachment 8638862 [details]
MozReview Request: docker: secure pulling of Docker base images (bug 1187544); r?smacleod, kang

docker: secure pulling of Docker base images (bug 1187544); r?smacleod, kang

`docker pull` isn't secure because it doesn't verify digests *before*
processing incoming data. This opens the door for malicious images to
exploit a vulnerability in image application in the Docker daemon.
See https://titanous.com/posts/docker-insecurity for more.

In the near future, we are going to be using Docker to generate
images used to execute code on hg.mozilla.org. Importing malicious
Docker images (possibly as result of a MITM attack) could lead to subtle
compromise of the generation of these images, which could lead to
compromise of hg.mozilla.org. We introduce content based verification
of Docker base images to help prevent against this scenario.

We introduce a new special syntax to Dockerfiles. "FROM secure:"
defines a tuple of (repository, tag prefix, SHA-256, URL) defining
a remotely hosted image with known content. The "repository" and "tag
prefix" fields are used to locally tag the image so `docker images`
prints something sane.

When this syntax is encountered, we obtain the base image from the URL
specified, verify it, and import it. Or, we quickly obtain the image ID
by seeing if the hash of the (URL, SHA-256) already exists by scanning
local images. Compromise of the local Docker store could thus lead to
false cache hits. However, if your local Docker store is compromised,
your local system is already compromised and all bets are already off,
so we don't care.

We modify the code for "importing" the Docker image to use modified
Dockerfile content that references the ID of the imported image. Keep in
mind that Docker image IDs are not derived from just image content and
therefore not deterministic. That is why we can't hard-code the image
IDs in the Dockerfile. If we did, we could probably add special syntax
behind comments (e.g. "# sha256 XYZ" - like peep) and pass through the
Dockerfile unmodified. The unofficial pre-processed Dockerfile syntax is
unfortunate. But, it gets the job done.

We've modified relevant "FROM" statements in our in-tree Dockerfiles to
reference trusted Docker image archives. Because 3rd party Docker
registries and Git repositories containing these base images are
unreliable and may change from underneath us, I've uploaded these files
to S3 and inserted those URLs here.

The CentOS image comes from
https://github.com/CentOS/sig-cloud-instance-images at commit
311d80f2e558eba3a6ea88c387714ae2e4175702 and is simply a copy of
the docker/centos-6-20150615_2019-docker.tar.xz file in that revision.

The Ubuntu image is derived from
https://github.com/tianon/docker-brew-ubuntu-core at commit
4f7764b8eb70271b6a736341e090699fe058a198. Unlike the CentOS image,
modifications to the image need to occur. The image was built by running
`docker build .` in the trust/ directory of that repository to produce a
Docker image. When then created a container via `docker run -it <image>
/bin/false` so we could `docker export` that container to a tarball. The
xz compressed version of the tarball is what is uploaded. This process
may or may not be deterministic over time.
Attachment #8638862 - Attachment description: MozReview Request: docker: use SHA-256 digests to refer to base images (bug 1187544); r?kang → MozReview Request: docker: secure pulling of Docker base images (bug 1187544); r?smacleod, kang
Attachment #8638862 - Flags: review?(smacleod)
Attachment #8638862 - Flags: review?(gdestuynder)
Attachment #8638862 - Flags: review?(smacleod) → review+
Comment on attachment 8638862 [details]
MozReview Request: docker: secure pulling of Docker base images (bug 1187544); r?smacleod, kang

https://reviewboard.mozilla.org/r/14153/#review12805

The code looks good to me. I think what's lacking though is some documentation of the new custom syntax which isn't burried in the code itself. I won't let this block landing, but following up on this would be appreciated.

::: testing/vcttesting/docker.py:224
(Diff revision 2)
> +        (images don't need to be buffered before being applied), is is insecure

"is is"
I'm going to go ahead and land this (with docs). I figure kang can leave feedback whenever and we can make follow-ups, if necessary.
url:        https://hg.mozilla.org/hgcustom/version-control-tools/rev/d008cc3e17ed397b3a3e0e3a15e9f643db1e29c6
changeset:  d008cc3e17ed397b3a3e0e3a15e9f643db1e29c6
user:       Gregory Szorc <gps@mozilla.com>
date:       Mon Jul 27 11:20:15 2015 -0700
description:
docker: secure pulling of Docker base images (bug 1187544); r=smacleod

`docker pull` isn't secure because it doesn't verify digests *before*
processing incoming data. This opens the door for malicious images to
exploit a vulnerability in image application in the Docker daemon.
See https://titanous.com/posts/docker-insecurity for more.

In the near future, we are going to be using Docker to generate
images used to execute code on hg.mozilla.org. Importing malicious
Docker images (possibly as result of a MITM attack) could lead to subtle
compromise of the generation of these images, which could lead to
compromise of hg.mozilla.org. We introduce content based verification
of Docker base images to help prevent against this scenario.

We introduce a new special syntax to Dockerfiles. "FROM secure:"
defines a tuple of (repository, tag prefix, SHA-256, URL) defining
a remotely hosted image with known content. The "repository" and "tag
prefix" fields are used to locally tag the image so `docker images`
prints something sane.

When this syntax is encountered, we obtain the base image from the URL
specified, verify it, and import it. Or, we quickly obtain the image ID
by seeing if the hash of the (URL, SHA-256) already exists by scanning
local images. Compromise of the local Docker store could thus lead to
false cache hits. However, if your local Docker store is compromised,
your local system is already compromised and all bets are already off,
so we don't care.

We modify the code for "importing" the Docker image to use modified
Dockerfile content that references the ID of the imported image. Keep in
mind that Docker image IDs are not derived from just image content and
therefore not deterministic. That is why we can't hard-code the image
IDs in the Dockerfile. If we did, we could probably add special syntax
behind comments (e.g. "# sha256 XYZ" - like peep) and pass through the
Dockerfile unmodified. The unofficial pre-processed Dockerfile syntax is
unfortunate. But, it gets the job done.

We've modified relevant "FROM" statements in our in-tree Dockerfiles to
reference trusted Docker image archives. Because 3rd party Docker
registries and Git repositories containing these base images are
unreliable and may change from underneath us, I've uploaded these files
to S3 and inserted those URLs here.

The CentOS image comes from
https://github.com/CentOS/sig-cloud-instance-images at commit
311d80f2e558eba3a6ea88c387714ae2e4175702 and is simply a copy of
the docker/centos-6-20150615_2019-docker.tar.xz file in that revision.

The Ubuntu image is derived from
https://github.com/tianon/docker-brew-ubuntu-core at commit
4f7764b8eb70271b6a736341e090699fe058a198. Unlike the CentOS image,
modifications to the image need to occur. The image was built by running
`docker build .` in the trust/ directory of that repository to produce a
Docker image. When then created a container via `docker run -it <image>
/bin/false` so we could `docker export` that container to a tarball. The
xz compressed version of the tarball is what is uploaded. This process
may or may not be deterministic over time.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 8638862 [details]
MozReview Request: docker: secure pulling of Docker base images (bug 1187544); r?smacleod, kang

https://reviewboard.mozilla.org/r/14153/#review12833

::: testing/vcttesting/docker.py:259
(Diff revision 2)
> +                raise Exception('downloaded Docker image does not match digest: '

this is generally where we'd want to forward this through logs for alerting

::: testing/docker/builder-ansible-centos6/Dockerfile:5
(Diff revisions 1 - 2)
> -# CentOS 6
> +FROM secure:mozsecure:centos6:sha256 88cc1475bd58f6ff6f41d50b813c9998c4d3a7167a8a45f8bd0b4a8342dec0db:https://s3-us-west-2.amazonaws.com/moz-packages/docker-images/centos-6-20150615_2019-docker.tar.xz

Note that in all cases this makes the repository with the docker files the "source of truth" for hashes, since they're not signed, they cannot be externally verified.

This is fine though as the control works - ideally I'd hope for Docker to implement this with a signed hash, so that any receiver of the image can check it's been untouched and validated by a trusted source, regardless who's hosting the dockerfile.
Attachment #8638862 - Flags: review?(gdestuynder)
Comment on attachment 8638862 [details]
MozReview Request: docker: secure pulling of Docker base images (bug 1187544); r?smacleod, kang

https://reviewboard.mozilla.org/r/14153/#review12835

Ship It!
Attachment #8638862 - Flags: review+
https://reviewboard.mozilla.org/r/14153/#review12833

> Note that in all cases this makes the repository with the docker files the "source of truth" for hashes, since they're not signed, they cannot be externally verified.
> 
> This is fine though as the control works - ideally I'd hope for Docker to implement this with a signed hash, so that any receiver of the image can check it's been untouched and validated by a trusted source, regardless who's hosting the dockerfile.

Definitely. And I think I saw something about this in some Docker planning notes or something. I just don't know when it will be available. Hopefully soon. I definitely want the ability to add individual "signing keys" to trust specific repositories and/or images.

> this is generally where we'd want to forward this through logs for alerting

This code doesn't run on servers, so there is nowhere to forward it to :/

Hopefully if a developer sees it they will sound the alarm.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: