From f0f2d9cdf4d8e7e7a32cbaff958136346b84843c Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 19 Dec 2018 13:42:07 -0500 Subject: [PATCH] Add architecture validation to manifest lists that contain schema 1 manifests Fixes https://jira.coreos.com/browse/QUAY-1266 --- data/model/oci/manifest.py | 9 +++++++- image/docker/interfaces.py | 7 ++++++ image/docker/schema1.py | 10 +++++++++ image/docker/schema2/list.py | 22 +++++++++++++++++++ image/docker/schema2/manifest.py | 6 ++++++ image/docker/schema2/test/test_list.py | 25 +++++++++++++++++++-- test/registry/protocol_v2.py | 4 ++-- test/registry/registry_tests.py | 30 +++++++++++++++++++++++++- workers/tagbackfillworker.py | 3 +++ 9 files changed, 110 insertions(+), 6 deletions(-) diff --git a/data/model/oci/manifest.py b/data/model/oci/manifest.py index b1b1d8148..d39a97f5f 100644 --- a/data/model/oci/manifest.py +++ b/data/model/oci/manifest.py @@ -74,8 +74,15 @@ def get_or_create_manifest(repository_id, manifest_interface_instance, storage): def _create_manifest(repository_id, manifest_interface_instance, storage): - # Load, parse and get/create the child manifests, if any. + # Validate the manifest. retriever = RepositoryContentRetriever.for_repository(repository_id, storage) + try: + manifest_interface_instance.validate(retriever) + except (ManifestException, MalformedSchema2ManifestList, BlobDoesNotExist, IOError): + logger.exception('Could not validate manifest `%s`', manifest_interface_instance.digest) + return None + + # Load, parse and get/create the child manifests, if any. child_manifest_refs = manifest_interface_instance.child_manifests(retriever) child_manifest_rows = {} child_manifest_label_dicts = [] diff --git a/image/docker/interfaces.py b/image/docker/interfaces.py index 85072216f..c5495027b 100644 --- a/image/docker/interfaces.py +++ b/image/docker/interfaces.py @@ -38,6 +38,13 @@ class ManifestInterface(object): cannot be computed locally. """ + @abstractmethod + def validate(self, content_retriever): + """ Performs validation of required assertions about the manifest. Raises a ManifestException + on failure. + """ + pass + @abstractmethod def get_layers(self, content_retriever): """ Returns the layers of this manifest, from base to leaf or None if this kind of manifest diff --git a/image/docker/schema1.py b/image/docker/schema1.py index 7ace40128..c5f2b8dea 100644 --- a/image/docker/schema1.py +++ b/image/docker/schema1.py @@ -213,6 +213,16 @@ class DockerSchema1Manifest(ManifestInterface): if not verified: raise InvalidSchema1Signature() + def validate(self, content_retriever): + """ Performs validation of required assertions about the manifest. Raises a ManifestException + on failure. + """ + # Already validated. + + @property + def architecture(self): + return self._architecture + @property def is_manifest_list(self): return False diff --git a/image/docker/schema2/list.py b/image/docker/schema2/list.py index 195a3faba..49e8b6aa8 100644 --- a/image/docker/schema2/list.py +++ b/image/docker/schema2/list.py @@ -40,6 +40,13 @@ class MalformedSchema2ManifestList(ManifestException): pass +class MismatchManifestException(MalformedSchema2ManifestList): + """ Raised when a manifest list contains a schema 1 manifest with a differing architecture + from that specified in the manifest list for the manifest. + """ + pass + + class LazyManifestLoader(object): def __init__(self, manifest_data, content_retriever): self._manifest_data = manifest_data @@ -239,6 +246,21 @@ class DockerSchema2ManifestList(ManifestInterface): manifests = self._parsed[DOCKER_SCHEMA2_MANIFESTLIST_MANIFESTS_KEY] return [LazyManifestLoader(m, content_retriever) for m in manifests] + def validate(self, content_retriever): + """ Performs validation of required assertions about the manifest. Raises a ManifestException + on failure. + """ + for index, m in enumerate(self._parsed[DOCKER_SCHEMA2_MANIFESTLIST_MANIFESTS_KEY]): + if m[DOCKER_SCHEMA2_MANIFESTLIST_MEDIATYPE_KEY] == DOCKER_SCHEMA1_MANIFEST_CONTENT_TYPE: + platform = m[DOCKER_SCHEMA2_MANIFESTLIST_PLATFORM_KEY] + + # Validate the architecture against the schema 1 architecture defined. + parsed = self.manifests(content_retriever)[index].manifest_obj + assert isinstance(parsed, DockerSchema1Manifest) + if (parsed.architecture and + parsed.architecture != platform[DOCKER_SCHEMA2_MANIFESTLIST_ARCHITECTURE_KEY]): + raise MismatchManifestException('Mismatch in arch for manifest `%s`' % parsed.digest) + def child_manifests(self, content_retriever): return self.manifests(content_retriever) diff --git a/image/docker/schema2/manifest.py b/image/docker/schema2/manifest.py index d3a16d64f..b8d3a9c9d 100644 --- a/image/docker/schema2/manifest.py +++ b/image/docker/schema2/manifest.py @@ -150,6 +150,12 @@ class DockerSchema2Manifest(ManifestInterface): if layer.is_remote and not layer.urls: raise MalformedSchema2Manifest('missing `urls` for remote layer') + def validate(self, content_retriever): + """ Performs validation of required assertions about the manifest. Raises a ManifestException + on failure. + """ + # Nothing to validate. + @property def is_manifest_list(self): return False diff --git a/image/docker/schema2/test/test_list.py b/image/docker/schema2/test/test_list.py index f944c534b..04a321ce0 100644 --- a/image/docker/schema2/test/test_list.py +++ b/image/docker/schema2/test/test_list.py @@ -1,11 +1,12 @@ import json import pytest -from image.docker.schema1 import DockerSchema1Manifest, DOCKER_SCHEMA1_CONTENT_TYPES +from image.docker.schema1 import (DockerSchema1Manifest, DOCKER_SCHEMA1_CONTENT_TYPES, + DockerSchema1ManifestBuilder) from image.docker.schema2 import DOCKER_SCHEMA2_MANIFEST_CONTENT_TYPE from image.docker.schema2.manifest import DockerSchema2Manifest from image.docker.schema2.list import (MalformedSchema2ManifestList, DockerSchema2ManifestList, - DockerSchema2ManifestListBuilder) + DockerSchema2ManifestListBuilder, MismatchManifestException) from image.docker.schema2.test.test_manifest import MANIFEST_BYTES as v22_bytes from image.docker.schemautil import ContentRetrieverForTesting from image.docker.test.test_schema1 import MANIFEST_BYTES as v21_bytes @@ -108,6 +109,9 @@ def test_valid_manifestlist(): assert schema1_manifest.schema_version == 1 assert schema1_manifest.digest == compatible_manifest.digest + # Ensure it validates. + manifestlist.validate(retriever) + def test_get_schema1_manifest_no_matching_list(): manifestlist = DockerSchema2ManifestList(Bytes.for_string_or_unicode(NO_AMD_MANIFESTLIST_BYTES)) @@ -128,3 +132,20 @@ def test_builder(): built = builder.build() assert len(built.manifests(retriever)) == 2 + + +def test_invalid_manifestlist(): + # Build a manifest list with a schema 1 manifest of the wrong architecture. + builder = DockerSchema1ManifestBuilder('foo', 'bar', 'baz') + builder.add_layer('sha:2356', '{"id": "foo"}') + manifest = builder.build().unsigned() + + listbuilder = DockerSchema2ManifestListBuilder() + listbuilder.add_manifest(manifest, 'amd32', 'linux') + manifestlist = listbuilder.build() + + retriever = ContentRetrieverForTesting() + retriever.add_digest(manifest.digest, manifest.bytes.as_encoded_str()) + + with pytest.raises(MismatchManifestException): + manifestlist.validate(retriever) diff --git a/test/registry/protocol_v2.py b/test/registry/protocol_v2.py index 221c3af67..d405db9e1 100644 --- a/test/registry/protocol_v2.py +++ b/test/registry/protocol_v2.py @@ -292,8 +292,8 @@ class V2Protocol(RegistryProtocol): blobs[schema2_config.digest] = schema2_config.bytes.as_encoded_str() return builder.build(ensure_ascii=options.ensure_ascii) - def build_schema1(self, namespace, repo_name, tag_name, images, blobs, options): - builder = DockerSchema1ManifestBuilder(namespace, repo_name, tag_name) + def build_schema1(self, namespace, repo_name, tag_name, images, blobs, options, arch='amd64'): + builder = DockerSchema1ManifestBuilder(namespace, repo_name, tag_name, arch) for image in reversed(images): assert image.urls is None diff --git a/test/registry/registry_tests.py b/test/registry/registry_tests.py index d22d191a0..55c43fe86 100644 --- a/test/registry/registry_tests.py +++ b/test/registry/registry_tests.py @@ -1441,7 +1441,8 @@ def test_push_pull_manifest_list_back_compat(v22_protocol, legacy_puller, basic_ # Build the manifests that will go in the list. blobs = {} - signed = v22_protocol.build_schema1('devtable', 'newrepo', 'latest', basic_images, blobs, options) + signed = v22_protocol.build_schema1('devtable', 'newrepo', 'latest', basic_images, blobs, options, + arch='amd64' if is_amd else 'something') first_manifest = signed.unsigned() if schema_version == 2: first_manifest = v22_protocol.build_schema2(basic_images, blobs, options) @@ -1904,3 +1905,30 @@ def test_push_pull_older_mimetype(pusher, puller, basic_images, liveserver_sessi # Pull the repository to verify. puller.pull(liveserver_session, 'devtable', 'newrepo', 'latest', basic_images, credentials=credentials, options=options) + +def test_attempt_push_mismatched_manifest(v22_protocol, basic_images, liveserver_session, + app_reloader, data_model): + """ Test: Attempt to push a manifest list refering to a schema 1 manifest with a different + architecture than that specified in the manifest list. + """ + if data_model != 'oci_model': + return + + credentials = ('devtable', 'password') + options = ProtocolOptions() + + # Build the manifest that will go in the list. This will be amd64. + blobs = {} + signed = v22_protocol.build_schema1('devtable', 'newrepo', 'latest', basic_images, blobs, options) + manifest = signed.unsigned() + + # Create the manifest list, but refer to the manifest as arm. + builder = DockerSchema2ManifestListBuilder() + builder.add_manifest(manifest, 'arm', 'linux') + manifestlist = builder.build() + + # Attempt to push the manifest, which should fail. + v22_protocol.push_list(liveserver_session, 'devtable', 'newrepo', 'latest', manifestlist, + [manifest], blobs, + credentials=credentials, options=options, + expected_failure=Failures.INVALID_MANIFEST) diff --git a/workers/tagbackfillworker.py b/workers/tagbackfillworker.py index 1e3c12dd5..62b784c28 100644 --- a/workers/tagbackfillworker.py +++ b/workers/tagbackfillworker.py @@ -72,6 +72,9 @@ class BrokenManifest(ManifestInterface): def local_blob_digests(self): return [] + def validate(self, content_retriever): + pass + def child_manifests(self, lookup_manifest_fn): return None