Add architecture validation to manifest lists that contain schema 1 manifests

Fixes https://jira.coreos.com/browse/QUAY-1266

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 = []

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

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

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)
 

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

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)

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

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)

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