Discourage signing references to images that aren't digests

[znewman01]

If you run cosign sign container.registry.io/foo:tag, then you're vulnerable to:

1. Race conditions, where :tag changed. Immutable tags help with this, but it's hard for cosign to know whether a registry's tags are immutable.
2. Funny business, where the registry itself changes the image.

It's much more meaningful to sign the digests, which are immutable.

In Slack discusssion, @imjasonh writes:

I think we could just recommend signing by digest more, maybe show some warning about signing by tag, or requiring confirmation?

I think this is a good idea.

[imjasonh]

It looks like one place we could put the warning is here:

cosign/cmd/cosign/cli/sign/sign.go

Lines 150 to 160 in dbd430f

	if digest, ok := ref.(name.Digest); ok && !recursive {
	se, err := ociremote.SignedEntity(ref, opts...)
	if err != nil {
	return fmt.Errorf("accessing image: %w", err)
	}
	err = signDigest(ctx, digest, staticPayload, ko, regOpts, annotations, upload, outputSignature, outputCertificate, force, recursive, dd, sv, se)
	if err != nil {
	return fmt.Errorf("signing digest: %w", err)
	}
	continue
	}

with an

if _, ok := ref.(name.Tag); ok {
  log.Println("hey don't sign tags pls")
}

[ChristianCiach]

Don't cosign attach sbom and cosign attest have the same issue? When used with tags, you may attach sboms or attestations to the wrong image.

[ianlewis]

@ChristianCiach Yes, I think whenever you reference a remote image you could potentially have this issue.

[znewman01]

Before resolving, make sure to:

• do this for other commands (cosign attach *, cosign attest, cosign generate)
  • can we do this more systematically? make it hard to sign something by reference (make the signing APIs take only digests)
• ensure all docs are up to date
• actually finish the deprecation (will take a couple of releases)

[ChristianCiach]

Please also make sure that references like repo:tag@digest continue to work. This means that the warning should not be emitted when a tag is present, but only when a digest is absent.

[znewman01]

FYI #2313 just merged. It only covers cosign sign (it does add a test for repo:tag@digest @ChristianCiach 😄 ).

There's still a lot to do. Next steps:

• Do cosign generate
• Do other commands (I think we may want to distinguish between commands where tag references are a security issue, like sign and generate, and ones where they're just a potential race condition. I haven't sorted those all out in my head yet.)
• Make a draft PR with the warnings turned into errors and see what fails (e.g., in our release process). There's a bunch of scripts to worry about.
• Fix Fix Counter-Signing demo in README #2333
• Update docs everywhere (docs.sigstore.dev, old blog posts, etc.)
• Wait a long time.
• Turn warnings into errors (this might have to be in a major version release).

[Namanl2001]

I would like to do this for cosign generate :)

can we do this more systematically? make it hard to sign something by reference (make the signing APIs take only digests)

Any plans for this?

[znewman01]

I would like to do this for cosign generate :)

Excellent, go for it!

FYI you can just make the breaking change if you do it before the Cosign 2.0 release, no need to bother with a deprecation period. Let us know if you'd like any guidance.

can we do this more systematically? make it hard to sign something by reference (make the signing APIs take only digests)

Any plans for this?

Long-term, yes. But I think this is blocked on a few other things:

• Versioning plan for the Cosign library API that will allow big refactors like this
• https://github.com/sigstore/sigstore-go pulling out the core signing/verification functionality

[mtrmac]

Discourage signing references to images that aren't digests

FWIW, as the original author of the c/image “simple signing” payload format, I think this should be exactly the opposite: discourage signing digest references. Or, to be more precise, the signing operation must point at an image by digest[1] — but the signed payload should name the image, in critical.identity.docker-reference, using both a repo name and a tag_, not just a repo (like Cosign currently does), and not replacing the tag in that field with a digest.

---

First, a technical note: the signature payload, both as originally specified and as created by cosign sign, already contains a separate digest field: https://github.com/containers/image/blob/main/docs/containers-signature.5.md#criticalimagedocker-manifest-digest . So putting a digest in the other field, critical.identity.docker-reference, is at best redundant.

Actually, Cosign enforces that the docker-manifest-digest value matches the image, in

cosign/pkg/cosign/verifiers.go

Line 44 in 0081e1a

foundDgst := ss.Critical.Image.DockerManifestDigest

; it does AFACS does nothing with a digest value in critical.identity.docker-reference, so putting a digest in the identity field would be not just redundant, but possibly also misleading.

---

Now, as to the main reason: There are basically two reasons to sign an image: nonrepudiation (for which just the pair of critical.image.docker-manifest-digest + a signature by the expected key is sufficient, and doesn’t matter for this argument), and end-to-end integrity in an untrusted environment (using third-party or untrusted registries, mirrors, caches).

One of the ways an untrusted transport can attack users is to substitute different “legitimate” images, correctly and intentionally signed by the intended author — causing the user to use a different image than the one that was intended.

For example, a malicious registry might, when asked for app:10.2, serve app:10.0-prerelease.1 instead, which has known vulnerabilities. The self-contained and isolated nature of container images makes this kind of attack fairly likely to work, in that the victim might be able to run the substituted image without noticing any obvious breakage.

The protection against this should be effortless, based only on existing data. Users are unlikely to carry around an extra version number around, in addition to the minimal required deployment configuration (e.g. a k8s Pod spec), and users can not be reasonably expected to actively do an extra step (and to never forget) in order not to be vulnerable

AFAICS, about the only practical UI for this is to match the deployment instruction (image name in podman pull $image / k8s pod.spec.containers[].image), which already expresses users’ intent, against a name inside the signature. Anything else could work technically but is unlikely to be used.

And that’s why the critical.identity.docker-reference field exists in “simple signing” at all, and why signature enforcement in the “simple signing” format requires that the whole name:tag of the users’ image request matches this field inside the signature (with configurable exceptions, mapping and the like). System administrators can then configure a system-wide policy.json configuration with the keys expected to sign various images, and users can ideally just use podman pull without worrying about signature enforcement or configurations or transport integrity at all — just like they rely on the automatic matching “subject” field of certificates used in TLS to protect HTTPS connections, without having to worry about substitution attacks.

Now, Cosign might or might not share the view that this kind of protection against substitution attacks is important, or practical enough to enforce; so Cosign might or might not decide to enforce name:tag verification by default. But I’d very much like for cosign sign to create signatures that allow this kind of name:tag matching to be done.

---

The end-user UI for cosign sign could then be something like

cosign sign --digest sha256:… registry.example.com/namespace/name:tag

where the --digest sha256:… option ensures that the correct image is signed, and the registry.example.com/namespace/name:tag argument both specifies what image to sign, and what value to put into critical.identity.docker-reference.

The above is the basic use case; some users may require more control, with

cosign sign --digest sha256:… --sign-identity registry.example.com/production-namespace/name:released-tag \
     registry-staging.example.com/staging-namespace/name:released-tag-staging

to decouple the signed identity from the physical location of the image that is being signed

---

[1] (which is best exposed by not having a separate signing operation, but by integrating signing into a podman push-like operation directly, but that’s a whole another matter)

[ianlewis]

For example, a malicious registry might, when asked for app:10.2, serve app:10.0-prerelease.1 instead, which has known vulnerabilities. The self-contained and isolated nature of container images makes this kind of attack fairly likely to work, in that the victim might be able to run the substituted image without noticing any obvious breakage.

Yes. I think the argument was more that a malicious (or even a non-malicious) registry or user could simply change the 10.2 tag to a different digest after the original push, but it basically results in the same thing you're describing.

The end-user UI for cosign sign could then be something like

cosign sign --digest sha256:… registry.example.com/namespace/name:tag

where the --digest sha256:… option ensures that the correct image is signed, and the registry.example.com/namespace/name:tag argument both specifies what image to sign, and what value to put into critical.identity.docker-reference.

I think that's fine. The spirit of this issue is more around verifying the digest and was perhaps just making the assumption about the UI. For example, if this solution is adopted a warning could be printed to users encouraging them to use the --digest option.