From ed78a8beeb7e2b293b80418df2416d8d8c85cdfd Mon Sep 17 00:00:00 2001 From: Ivan Bazulic Date: Thu, 12 Jun 2025 11:31:05 -0400 Subject: [PATCH] api: Disallow push of manifests with negative layer size (PROJQUAY-8560) (#3683) * v2: Disallow push of manifests with negative layer size (PROJQUAY-8560) Under certain conditions, clients may create a manifest (OCI or Docker v2) that contains negative layer sizes. Our current validation schema does not take that corner case into account, it only checks if the manifest is properly formatted or not. With this change, Quay will reject manifests that have negative layer sizes and raise a `400` with a proper exception. An example can be seen here: ~~~ gunicorn-registry stdout | 2025-02-10 22:34:54,930 [377] [ERROR] [endpoints.v2.manifest] failed to parse manifest when writing by tagname gunicorn-registry stdout | Traceback (most recent call last): gunicorn-registry stdout | File "/quay-registry/endpoints/v2/manifest.py", line 362, in _parse_manifest gunicorn-registry stdout | return parse_manifest_from_bytes( gunicorn-registry stdout | File "/quay-registry/image/shared/schemas.py", line 40, in parse_manifest_from_bytes gunicorn-registry stdout | return DockerSchema2Manifest(manifest_bytes) gunicorn-registry stdout | File "/quay-registry/image/docker/schema2/manifest.py", line 172, in __init__ gunicorn-registry stdout | raise MalformedSchema2Manifest("layer size is negative") gunicorn-registry stdout | image.docker.schema2.manifest.MalformedSchema2Manifest: layer size is negative gunicorn-registry stdout | 2025-02-10 22:34:54,931 [377] [DEBUG] [endpoints.v2] sending response: b'{"errors":[{"code":"MANIFEST_INVALID","detail":{"message":"failed to parse manifest: layer size is negative"},"message":"manifest invalid"}]}\n ~~~ * Add tests, changed error message on malformed manifest exception * Fix v2 API test * Add match expression to pytest to make sure a proper exception is raised * Add exception for layers with size 0 bytes, fix tests * Fix indentation on previous changes * Fix indetation --- endpoints/v2/test/test_manifest.py | 152 ++++++++++++++++++++- image/docker/schema2/manifest.py | 4 + image/docker/schema2/test/test_config.py | 5 + image/docker/schema2/test/test_manifest.py | 60 ++++++++ image/oci/manifest.py | 4 + image/oci/test/test_oci_manifest.py | 111 +++++++++++++++ 6 files changed, 335 insertions(+), 1 deletion(-) diff --git a/endpoints/v2/test/test_manifest.py b/endpoints/v2/test/test_manifest.py index 60859c106..922b1cce9 100644 --- a/endpoints/v2/test/test_manifest.py +++ b/endpoints/v2/test/test_manifest.py @@ -1,5 +1,6 @@ +import hashlib +import json import time -from test.fixtures import * # noqa: F401, F403 from unittest.mock import patch from flask import url_for @@ -11,6 +12,12 @@ from auth.auth_context_type import ValidatedAuthContext from data import model from data.registry_model import registry_model from endpoints.test.shared import conduct_call +from image.docker.schema2.test.test_config import ( + CONFIG_BYTES, + CONFIG_DIGEST, + CONFIG_SIZE, +) +from test.fixtures import * # noqa: F401, F403 from util.security.registry_jwt import build_context_and_subject, generate_bearer_token @@ -75,3 +82,146 @@ def test_e2e_query_count_manifest_norewrite(client, app): ) assert counter.count <= 27 + + +INVALID_DOCKER_V2_MANIFEST = json.dumps( + { + "schemaVersion": 2, + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "config": { + "mediaType": "application/vnd.docker.container.image.v1+json", + "size": CONFIG_SIZE, + "digest": CONFIG_DIGEST, + }, + "layers": [ + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 1234, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + }, + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 32654, + "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", + }, + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": -1, + "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b", + }, + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 73109, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + }, + ], + } +).encode("utf-8") + + +def test_push_malformed_manifest_docker_v2s2(client, app): + repo_ref = registry_model.lookup_repository("devtable", "simple") + + params = { + "repository": "devtable/simple", + "manifest_ref": "sha256:" + hashlib.sha256(INVALID_DOCKER_V2_MANIFEST).hexdigest(), + } + + user = model.user.get_user("devtable") + access = [ + { + "type": "repository", + "name": "devtable/simple", + "actions": ["pull", "push"], + } + ] + + context, subject = build_context_and_subject(ValidatedAuthContext(user=user)) + token = generate_bearer_token( + realapp.config["SERVER_HOSTNAME"], subject, context, access, 600, instance_keys + ) + + headers = { + "Authorization": "Bearer %s" % token, + } + + # Conduct a call to prime the instance key and other caches. + conduct_call( + client, + "v2.write_manifest_by_digest", + url_for, + "PUT", + params, + expected_code=400, + headers=headers, + raw_body=INVALID_DOCKER_V2_MANIFEST, + ) + + +INVALID_OCI_MANIFEST = json.dumps( + { + "schemaVersion": 2, + "config": { + "mediaType": "application/vnd.oci.image.config.v1+json", + "size": 7023, + "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", + }, + "layers": [ + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 32654, + "digest": "sha256:9834876dcfb05cb167a5c24953eba58c4ac89b1adf57f28f2f9d09af107ee8f0", + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": -1, + "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b", + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 73109, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + }, + ], + "annotations": {"com.example.key1": "value1", "com.example.key2": "value2"}, + } +).encode("utf-8") + + +def test_push_malformed_manifest_oci_manifest(client, app): + repo_ref = registry_model.lookup_repository("devtable", "simple") + + params = { + "repository": "devtable/simple", + "manifest_ref": "sha256:" + hashlib.sha256(INVALID_OCI_MANIFEST).hexdigest(), + } + + user = model.user.get_user("devtable") + access = [ + { + "type": "repository", + "name": "devtable/simple", + "actions": ["pull", "push"], + } + ] + + context, subject = build_context_and_subject(ValidatedAuthContext(user=user)) + token = generate_bearer_token( + realapp.config["SERVER_HOSTNAME"], subject, context, access, 600, instance_keys + ) + + headers = { + "Authorization": "Bearer %s" % token, + } + + # Conduct a call to prime the instance key and other caches. + conduct_call( + client, + "v2.write_manifest_by_digest", + url_for, + "PUT", + params, + expected_code=400, + headers=headers, + raw_body=INVALID_OCI_MANIFEST, + ) diff --git a/image/docker/schema2/manifest.py b/image/docker/schema2/manifest.py index 6246f706b..64e4755b9 100644 --- a/image/docker/schema2/manifest.py +++ b/image/docker/schema2/manifest.py @@ -163,6 +163,10 @@ class DockerSchema2Manifest(ManifestInterface): except ValidationError as ve: raise MalformedSchema2Manifest("manifest data does not match schema: %s" % ve) + for layer in self._parsed["layers"]: + if layer["size"] < 0: + raise MalformedSchema2Manifest("invalid layer size") + for layer in self.filesystem_layers: if layer.is_remote and not layer.urls: raise MalformedSchema2Manifest("missing `urls` for remote layer") diff --git a/image/docker/schema2/test/test_config.py b/image/docker/schema2/test/test_config.py index 578e11501..c5440e03c 100644 --- a/image/docker/schema2/test/test_config.py +++ b/image/docker/schema2/test/test_config.py @@ -130,3 +130,8 @@ def test_valid_config(): assert v1_compat["container_config"]["Cmd"] == [history_entry.command] assert config.labels == {} + + +EMPTY_CONFIG_BYTES = json.dumps({}).encode("utf-8") +EMPTY_CONFIG_SIZE = len(EMPTY_CONFIG_BYTES) +EMPTY_CONFIG_DIGEST = "sha256:" + hashlib.sha256(EMPTY_CONFIG_BYTES).hexdigest() diff --git a/image/docker/schema2/test/test_manifest.py b/image/docker/schema2/test/test_manifest.py index a38d9b278..555be5194 100644 --- a/image/docker/schema2/test/test_manifest.py +++ b/image/docker/schema2/test/test_manifest.py @@ -22,6 +22,8 @@ from image.docker.schema2.test.test_config import ( CONFIG_BYTES, CONFIG_DIGEST, CONFIG_SIZE, + EMPTY_CONFIG_DIGEST, + EMPTY_CONFIG_SIZE, ) from image.shared.schemautil import ContentRetrieverForTesting from util.bytes import Bytes @@ -44,6 +46,64 @@ def test_malformed_manifests(json_data): DockerSchema2Manifest(Bytes.for_string_or_unicode(json_data)) +EMPTY_CONFIG_MANIFEST_BYTES = json.dumps( + { + "schemaVersion": 2, + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "config": { + "mediaType": "application/vnd.docker.container.image.v1+json", + "size": EMPTY_CONFIG_SIZE, + "digest": EMPTY_CONFIG_DIGEST, + }, + "layers": [ + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 1234, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + }, + ], + } +).encode("utf-8") + + +def test_empty_config_manifest(): + manifest = DockerSchema2Manifest(Bytes.for_string_or_unicode(EMPTY_CONFIG_MANIFEST_BYTES)) + assert manifest.config.size == EMPTY_CONFIG_SIZE + assert manifest.config.digest == EMPTY_CONFIG_DIGEST + assert manifest.media_type == "application/vnd.docker.distribution.manifest.v2+json" + assert manifest.config_media_type == "application/vnd.docker.container.image.v1+json" + + +MANIFEST_WITH_INVALID_LAYER_SIZE = json.dumps( + { + "schemaVersion": 2, + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "config": { + "mediaType": "application/vnd.docker.container.image.v1+json", + "size": CONFIG_SIZE, + "digest": CONFIG_DIGEST, + }, + "layers": [ + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 1234, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + }, + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": -1, + "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f", + }, + ], + } +).encode("utf-8") + + +def test_invalid_layer_size_manifest(): + with pytest.raises(MalformedSchema2Manifest, match="invalid layer size"): + DockerSchema2Manifest(Bytes.for_string_or_unicode(MANIFEST_WITH_INVALID_LAYER_SIZE)) + + MANIFEST_BYTES = json.dumps( { "schemaVersion": 2, diff --git a/image/oci/manifest.py b/image/oci/manifest.py index ddb357007..8be13ce6d 100644 --- a/image/oci/manifest.py +++ b/image/oci/manifest.py @@ -167,6 +167,10 @@ class OCIManifest(ManifestInterface): except ValidationError as ve: raise MalformedOCIManifest("manifest data does not match schema: %s" % ve) + for layer in self._parsed["layers"]: + if layer["size"] < 0: + raise MalformedOCIManifest("invalid layer size") + for layer in self.filesystem_layers: if layer.is_remote and not layer.urls: raise MalformedOCIManifest("missing `urls` for remote layer") diff --git a/image/oci/test/test_oci_manifest.py b/image/oci/test/test_oci_manifest.py index f57217a3f..696c75cda 100644 --- a/image/oci/test/test_oci_manifest.py +++ b/image/oci/test/test_oci_manifest.py @@ -1,3 +1,4 @@ +import hashlib import json import pytest @@ -38,6 +39,39 @@ SAMPLE_MANIFEST = """{ } }""" +SAMPLE_MANIFEST2 = """{ + "schemaVersion": 2, + "config": { + "mediaType": "application/vnd.oci.image.config.v1+json", + "size": 7023, + "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7" + }, + "layers": [ + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 32654, + "digest": "sha256:9834876dcfb05cb167a5c24953eba58c4ac89b1adf57f28f2f9d09af107ee8f0" + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 16724, + "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b" + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 73109, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + "annotations": { + "com.example.layerkey1": "value1", + "com.example.layerkey2": "value2" + } + } + ], + "annotations": { + "com.example.key1": "value1", + "com.example.key2": "value2" + } +}""" SAMPLE_MANIFEST2 = """{ "schemaVersion": 2, "config": { @@ -302,6 +336,83 @@ def test_validate_helm_oci_manifest(): manifest = OCIManifest(Bytes.for_string_or_unicode(manifest_bytes)) +INVALID_LAYER_SIZE_MANIFEST = json.dumps( + { + "schemaVersion": 2, + "config": { + "mediaType": "application/vnd.oci.image.config.v1+json", + "size": 7023, + "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", + }, + "layers": [ + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 32654, + "digest": "sha256:9834876dcfb05cb167a5c24953eba58c4ac89b1adf57f28f2f9d09af107ee8f0", + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": -1, + "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b", + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 73109, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + }, + ], + "annotations": {"com.example.key1": "value1", "com.example.key2": "value2"}, + } +).encode("utf-8") + + +def test_invalid_layer_size_manifest(): + with pytest.raises(MalformedOCIManifest, match="invalid layer size"): + OCIManifest(Bytes.for_string_or_unicode(INVALID_LAYER_SIZE_MANIFEST)) + + +MANIFEST_WITH_LAYER_SIZE_0 = json.dumps( + { + "schemaVersion": 2, + "config": { + "mediaType": "application/vnd.oci.image.config.v1+json", + "size": 7023, + "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", + }, + "layers": [ + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 32654, + "digest": "sha256:9834876dcfb05cb167a5c24953eba58c4ac89b1adf57f28f2f9d09af107ee8f0", + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 0, + "digest": "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + }, + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "size": 73109, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + }, + ], + "annotations": {"com.example.key1": "value1", "com.example.key2": "value2"}, + } +).encode() + + +def test_manifest_with_layer_size_0(): + digest = "sha256:" + hashlib.sha256(MANIFEST_WITH_LAYER_SIZE_0).hexdigest() + manifest = OCIManifest(Bytes.for_string_or_unicode(MANIFEST_WITH_LAYER_SIZE_0)) + assert manifest.digest == digest + assert manifest.blob_digests == [ + "sha256:9834876dcfb05cb167a5c24953eba58c4ac89b1adf57f28f2f9d09af107ee8f0", + "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736", + "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", + ] + + def test_manifest_layer_annotations(): manifest = OCIManifest(Bytes.for_string_or_unicode(SAMPLE_MANIFEST2)) assert manifest.annotations == {"com.example.key1": "value1", "com.example.key2": "value2"}