fix(clean): Fix clean cmd for private registries (#3446)

Without this patch, running `cosign clean` on an image in a private
registry uses the wrong object reference and fails to delete artifacts
for the image.

On Dockerhub, it is sufficient to call DELETE directly on an object
name:

```
DELETE /v2/namespace/repo/manifests/sha256-deadbeef.sig
```

On registry version v2.3 and greater, this does not work. Instead, we
need to perform a GET to retrieve the object digest, and call DELETE on
that:

```
GET /v2/namespace/repo/manifests/sha256-deadbeef.sig
=> ... Docker-Content-Digest: sha256:cafeb0ba
DELETE /v2/namespace/repo/manifests/sha256:cafeb0ba
```

Since we can't know what type of registry we're dealing with, we try the
original version first, and the new version as a fallback.

See the GitHub issue[1] which explains the issue, and the
API documentation[2].

This also fixes a minor formatting issue in the error message.

[1] distribution/distribution#1579
[2] https://github.com/distribution/distribution/blob/main/docs/content/spec/api.md#deleting-an-image

Fixes #2265

Signed-off-by: Colleen Murphy <colleenmurphy@google.com>

cmd/cosign/cli/clean.go

@@ -96,12 +96,24 @@ func CleanCmd(ctx context.Context, regOpts options.RegistryOptions, cleanType op
 	for _, t := range cleanTags {
 		if err := remote.Delete(t, remoteOpts...); err != nil {
 			var te *transport.Error
-			if errors.As(err, &te) && te.StatusCode == http.StatusNotFound { //nolint: revive
+			switch {
+			case errors.As(err, &te) && te.StatusCode == http.StatusNotFound:
 				// If the tag doesn't exist, some registries may
 				// respond with a 404, which shouldn't be considered an
 				// error.
-			} else {
-				fmt.Fprintf(os.Stderr, "could not delete %s from %s\n: %v\n", t, imageRef, err)
+			case errors.As(err, &te) && te.StatusCode == http.StatusBadRequest:
+				// Docker registry >=v2.3 requires does not allow deleting the OCI object name directly, must use the digest instead.
+				// See https://github.com/distribution/distribution/blob/main/docs/content/spec/api.md#deleting-an-image
+				if err := deleteByDigest(t, remoteOpts...); err != nil {
+					if errors.As(err, &te) && te.StatusCode == http.StatusNotFound { //nolint: revive
+					} else {
+						fmt.Fprintf(os.Stderr, "could not delete %s by digest from %s:\n%v\n", t, imageRef, err)
+					}
+				} else {
+					fmt.Fprintf(os.Stderr, "Removed %s from %s\n", t, imageRef)
+				}
+			default:
+				fmt.Fprintf(os.Stderr, "could not delete %s from %s:\n%v\n", t, imageRef, err)
 			}
 		} else {
 			fmt.Fprintf(os.Stderr, "Removed %s from %s\n", t, imageRef)
@@ -111,6 +123,14 @@ func CleanCmd(ctx context.Context, regOpts options.RegistryOptions, cleanType op
 	return nil
 }
 
+func deleteByDigest(tag name.Tag, opts ...remote.Option) error {
+	digestTag, err := ociremote.DockerContentDigest(tag, ociremote.WithRemoteOptions(opts...))
+	if err != nil {
+		return err
+	}
+	return remote.Delete(digestTag, opts...)
+}
+
 func prompt(cleanType options.CleanType) string {
 	switch cleanType {
 	case options.CleanTypeSignature:

pkg/oci/remote/remote.go

@@ -133,6 +133,20 @@ func DigestTag(ref name.Reference, opts ...Option) (name.Tag, error) {
 	return suffixTag(ref, "", ":", o)
 }
 
+// DockerContentDigest fetches the Docker-Content-Digest header for the referenced tag,
+// which is required to delete the object in registry API v2.3 and greater.
+// See https://github.com/distribution/distribution/blob/main/docs/content/spec/api.md#deleting-an-image
+// and https://github.com/distribution/distribution/issues/1579
+func DockerContentDigest(ref name.Tag, opts ...Option) (name.Tag, error) {
+	o := makeOptions(ref.Context(), opts...)
+	desc, err := remoteGet(ref, o.ROpt...)
+	if err != nil {
+		return name.Tag{}, err
+	}
+	h := desc.Digest
+	return o.TargetRepository.Tag(normalizeWithSeparator(h, o.TagPrefix, "", ":")), nil
+}
+
 func suffixTag(ref name.Reference, suffix string, algorithmSeparator string, o *options) (name.Tag, error) {
 	var h v1.Hash
 	if digest, ok := ref.(name.Digest); ok {

pkg/oci/remote/remote_test.go

@@ -147,3 +147,59 @@ func TestTagMethodErrors(t *testing.T) {
 		})
 	}
 }
+
+func TestDockercontentDigest(t *testing.T) {
+	rg := remoteGet
+	defer func() {
+		remoteGet = rg
+	}()
+	remoteGet = func(ref name.Reference, options ...remote.Option) (*remote.Descriptor, error) {
+		return &remote.Descriptor{
+			Descriptor: v1.Descriptor{
+				Digest: v1.Hash{
+					Algorithm: "sha256",
+					// As of 2021-09-20:
+					// crane digest gcr.io/distroless/static:nonroot
+					Hex: "be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4",
+				},
+			},
+		}, nil
+	}
+
+	repo, err := name.NewRepository("gcr.io/distroless/static")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	tests := []struct {
+		name    string
+		tag     name.Tag
+		wantTag name.Tag
+	}{
+		{
+			name:    "docker content digest for tag",
+			tag:     name.MustParseReference("gcr.io/distroless/static:sha256-be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4.sig").(name.Tag),
+			wantTag: repo.Tag("sha256:be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4"),
+		},
+		{
+			name:    "docker content digest for attestation",
+			tag:     name.MustParseReference("gcr.io/distroless/static:sha256-be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4.att").(name.Tag),
+			wantTag: repo.Tag("sha256:be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4"),
+		},
+		{
+			name:    "docker content digest for SBOM",
+			tag:     name.MustParseReference("gcr.io/distroless/static:sha256-be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4.sbom").(name.Tag),
+			wantTag: repo.Tag("sha256:be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4"),
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			gotTag, err := DockerContentDigest(test.tag)
+			if err != nil {
+				t.Fatalf("fn() = %v", err)
+			}
+			if gotTag != test.wantTag {
+				t.Errorf("fn() = %s, wanted %s", gotTag.String(), test.wantTag.String())
+			}
+		})
+	}
+}

test/e2e_test.sh

@@ -52,6 +52,16 @@ popd
 go build -o cosign ./cmd/cosign
 go test -tags=e2e -race $(go list ./... | grep -v third_party/)
 
+# Test on a private registry
+echo "testing sign/verify/clean on private registry"
+cleanup() {
+    docker rm -f registry
+}
+trap cleanup EXIT
+docker run -d -p 5000:5000 --restart always -e REGISTRY_STORAGE_DELETE_ENABLED=true --name registry registry:latest
+export COSIGN_TEST_REPO=localhost:5000
+go test -tags=e2e -v ./test/... -run TestSignVerifyClean
+
 # Test `cosign dockerfile verify`
 ./cosign dockerfile verify ./test/testdata/single_stage.Dockerfile --certificate-identity https://github.com/distroless/alpine-base/.github/workflows/release.yaml@refs/heads/main --certificate-oidc-issuer https://token.actions.githubusercontent.com
 if (./cosign dockerfile verify ./test/testdata/unsigned_build_stage.Dockerfile --certificate-identity https://github.com/distroless/alpine-base/.github/workflows/release.yaml@refs/heads/main --certificate-oidc-issuer https://token.actions.githubusercontent.com); then false; fi