From 134c9428353247c22b69f497b904593faf93292d Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Fri, 3 Sep 2021 03:29:36 +0000 Subject: [PATCH] Reject mismatched layer sizes, with some exceptions The image manifest specification specifies a "size" for each layer that " ... exists so that a client will have an expected size for the content before validating. If the length of the retrieved content does not match the specified length, the content should not be trusted." This validates that a pushed manifest has the correct size for the layers, and if it does not, returns an error. A function is added to clear a blob+metadata which essentially rolls-back all the layers from the manifest (otherwise you get errors when trying again about existing objects). This is a change to the status quo, although I believe a correct one. A flag is added if the old behaviour is required for whatever obscure reason. The logic was getting a bit tangled, so I've refactored slightly to hopefully make it more understandable. Current docker seems to have an issue where is misreports the size; we put in a workaround for this since it has been identified. [1] https://docs.docker.com/registry/spec/manifest-v2-2/#image-manifest Change-Id: I70a7bb5f73d1dddc540e96529784bb8c9bb0b9e3 --- tools/conf.yaml | 5 +++ zuul_registry/main.py | 81 ++++++++++++++++++++++++++++++------------- 2 files changed, 62 insertions(+), 24 deletions(-) diff --git a/tools/conf.yaml b/tools/conf.yaml index b334253..5da6e70 100644 --- a/tools/conf.yaml +++ b/tools/conf.yaml @@ -21,3 +21,8 @@ registry: storage: driver: filesystem root: /tmp/storage + # Check the size of layers matches the size specified in the + # container manifest. Some versions of docker can push invalid + # manifests (and *also* don't care about a size mismatch when + # pulling); set this to false to ignore layer-size mismatches. + strict: true diff --git a/zuul_registry/main.py b/zuul_registry/main.py index c9b4e1a..7835592 100644 --- a/zuul_registry/main.py +++ b/zuul_registry/main.py @@ -175,10 +175,11 @@ class RegistryAPI: 'application/vnd.oci.image.manifest.v1+json', ] - def __init__(self, store, namespaced, authz): + def __init__(self, store, namespaced, authz, conf): self.storage = store self.authz = authz self.namespaced = namespaced + self.conf = conf def get_namespace(self, repository): if not self.namespaced: @@ -296,7 +297,18 @@ class RegistryAPI: res.headers['Content-Length'] = '0' res.status = '201 Created' - def _fix_manifest(self, namespace, content_type, body): + def _fix_manifest(self, namespace, request): + body = request.body.read() + content_type = request.headers.get('Content-Type') + + # Only v2 manifests need fixing + if (content_type != + 'application/vnd.docker.distribution.manifest.v2+json'): + return body + + data = json.loads(body) + changed = False + # The "docker build" command can produce a manifest with a # config that lacks a size attribute. It appears that Docker # Hub will silently add the size, so any image fetched from @@ -304,34 +316,54 @@ class RegistryAPI: # with the size attribute. The podman family of tools fails # to pull images without a config size. To avoid this error, # we emulate the Docker Hub behavior. - # The same is true for the layer sizes, but it's not a fatal - # error; podman just doesn't draw its progress bar. - if (content_type == - 'application/vnd.docker.distribution.manifest.v2+json'): - data = json.loads(body) - changed = False - if 'size' not in data['config']: - digest = data['config']['digest'] - size = self.storage.blob_size(namespace, digest) - data['config']['size'] = size + if 'size' not in data['config']: + digest = data['config']['digest'] + size = self.storage.blob_size(namespace, digest) + data['config']['size'] = size + changed = True + + for layer in data['layers']: + digest = layer['digest'] + actual_size = self.storage.blob_size(namespace, digest) + + # As above, we may or may not have a size for layers. If + # this layer doesn't have a size, add it. + if 'size' not in layer: + layer['size'] = actual_size changed = True - for layer in data['layers']: - if 'size' not in layer: - digest = layer['digest'] - size = self.storage.blob_size(namespace, digest) - layer['size'] = size - changed = True - if changed: - body = json.dumps(data).encode('utf8') + continue + + # However, if we got a size, we validate it + size = layer['size'] + if size == actual_size: + continue + + msg = ("Manifest has invalid size for layer %s " + "(size:%d actual:%d)" % (digest, size, actual_size)) + self.log.error(msg) + # Docker pushes a manifest with sizes one byte larger + # than it actaully sends. We choose to ignore this. + # https://github.com/docker/for-linux/issues/1296 + if ('docker/' in request.headers.get('User-Agent', '') + and (actual_size + 1 == size)): + self.log.info("Fix docker layer size for %s" % digest) + layer['size'] = actual_size + changed = True + elif self.conf.get('strict', True): + # We don't delete layers here as they may be used by + # different images with valid manifests. Return an error to + # the client so it can try again. + raise cherrypy.HTTPError(400, msg) + + if changed: + body = json.dumps(data).encode('utf8') return body @cherrypy.expose @cherrypy.config(**{'tools.check_auth.level': Authorization.WRITE}) def put_manifest(self, repository, ref): namespace, repository = self.get_namespace(repository) - body = cherrypy.request.body.read() - content_type = cherrypy.request.headers['Content-Type'] - body = self._fix_manifest(namespace, content_type, body) + body = self._fix_manifest(namespace, cherrypy.request) hasher = hashlib.sha256() hasher.update(body) digest = 'sha256:' + hasher.hexdigest() @@ -433,7 +465,8 @@ class RegistryServer: route_map = cherrypy.dispatch.RoutesDispatcher() api = RegistryAPI(self.store, False, - authz) + authz, + self.conf) cherrypy.tools.check_auth = authz route_map.connect('api', '/v2/',