Image manifest lists/indexes with invalid references

Context

In #404 (closed), we learned that it's currently possible to push manifest lists/indexes that reference not manifests but layers. This causes problems with, for example, offline garbage collection.

According to the Docker Image Spec and OCI Image Spec, a manifest list/index should reference manifests and manifests only. Nevertheless, the lack of validation has allowed an unknown number of invalid images to be uploaded to the GitLab Container Registry, both for GitLab.com and self-managed.

To be able to identify the scale and impact of this bug, we'll add a custom validation to the registry API which will log a warning message whenever an invalid reference is found.

Purpose

This issue is meant to collect evidence and reports from GitLab engineers, the community, and our customers whenever a possible occurrence of this issue surfaces.

added Category:Container Registry backend devopspackage golang grouppackage registry workflowscheduling labels

mentioned in issue #407 (closed)

changed milestone to %14.2

@jdrpereira do you think we should try to get this out in %14.1 along with !640 (merged)?

CC: @trizzi

@hswimelar, you mean adding the warning log to the API? If so, yes, that's trivial so it won't impact the higher priority work we have in hands.

Changing the milestone back, so we can discuss this in %14.2 even though the logging will be in for %14.1

added sectionops label

assigned to @hswimelar

mentioned in merge request !643 (merged)

changed milestone to %14.1

added workflowin review label and removed workflowscheduling label

changed milestone to %14.2

mentioned in issue #411 (closed)

mentioned in issue gitlab-com/gl-infra/production#5149 (closed)

marked this issue as related to #411 (closed)

marked this issue as related to gitlab-com/gl-infra/production#5149 (closed)

There was a related production incident today (gitlab-com/gl-infra/production#5149 (closed)). It was fixed in #411 (closed).

We've requested some feedback from the corresponding user (https://gitlab.com/gitlab-com/support/internal-requests/-/issues/8024 - internal).

marked this issue as related to #407 (closed)

We deployed the MR to log these a couple of days ago. Here's the kibana link for these entries: https://log.gprd.gitlab.net/goto/d67adf170ab1feab27656a0f4efdd7c6

We'll let these collect data collect for at least a couple of weeks before performing an analysis.

Looking over the data we've gathered over the past week (as far as records seem to go back) for these manifests, we have:

Gets	Puts	Total
47,117	8,490	55,597

All of which are likely to be buildx cache manifests, which is good is it appears that this issue is not widespread amount various types of manifest lists.

If we want to keep support for these, what I would recommend would be to merge in this PR from upstream: https://github.com/distribution/distribution/pull/3365. This will prevent manifests list with blobs from being pushed. Afterward, we can check if an incoming manifest list is a buildx cache manifest, and properly verify their references and allow only them to be pushed here: https://gitlab.com/gitlab-org/container-registry/blob/master/registry/storage/validation/manifestlist.go#L31

We'll need to make changes to the database to accommodate them if we do this, and ensure that the importer can handle these appropriately (no matter what we choose) before starting phase 1.

I'm on the side of allowing only buildx cache manifests through, they're being pushed up by users using official tooling appropriately, even if the tooling is misbehaving, but there are probably few enough events compared to our total traffic to justify dropping support.

@jdrpereira @trizzi WDYT?

@hswimelar, thanks for the data. The vast majority of these pushes/pulls come from a handful of customers, so it's not as widespread (in terms of user base) as we feared. I would propose that we follow the solution described in #407#solution (which is similar to what you describe here). I created a new thread there for approval: #407 (comment 637394495).

Responded in the thread here: #407 (comment 637394495)

Since we've invested this and addressing the issue in #407 (closed) this may be closed.

closed

added workflowproduction label and removed workflowin review label

mentioned in incident gitlab-com/gl-infra/production#18715 (closed)