DO NOT MERGE IRRESPONSIBLE: Cosign integration

[mtrmac]

This is a single integration branch of the Cosign prototypes.

💀💀💀DO NOT MERGE THIS WITHOUT ADDING TEST COVERAGE

See #1598.

---

(Outstanding) merge/review order:

• Three independent parts:
  • Add support for reading and writing Cosign attachments to c/image/docker #1595
  • Cosign sign #1597
  • Add Cosign verification support #1598

[vrothberg]

Suggested change

	SignByCosignPrivateKeyPassphrase []byte // Passphare to use when signing with `SignByCosignPrivateKeyFile`.
	SignCosignPrivateKeyPassphrase []byte // Passphare to use when signing with `SignByCosignPrivateKeyFile`.

Just to be consistent with SignBy and SignPassphrase

[mtrmac]

Done in #1597 .

[vrothberg]

I think it could be useful to add some (debug) logs and status on the report writer.

[mtrmac]

Debug logs added in #1595 .

We can add a report writer, or progress bar updates, with the new private interfaces; that would require a bit of new interfaces / infrastructure.

[vrothberg]

I think that using sig is dangerous. If there are multiple signatures, wouldn't all users of UntrustedAnnotations point to the field of the last item in the slice?

[vrothberg]

Fresher eyes after my lunch break are not concerned anymore. Only pointers to members of the loop variable are dangerous.

[mtrmac]

Nevertheless, doing more explicit copies when creating/consuming signatures would be worth the peace of mind.

[mtrmac]

As of updated #1595 , signature objects are only accessible via copying constructors and copying getters.

[vrothberg]

@mtrmac, do you think the blob cache could speed things up?

[mtrmac]

Those FIXMEs are because I just wasn’t ready to deal with that interface change at that time either way. We could pass that.

Looking at it more, the info cache (which is what the above is) is useful for:

• compressed/uncompressed correspondence. Irrelevant because we are not changing that for signatures, and a signature payload is not likely to match a “real” layer.
• TryReusingBlob*, where we are consuming the cache (the call from PutBlob* is write only). And it doesn’t even make much sense — I’d naively, without measurement, expect most of the cost of dealing with these small blobs to be the round-trips, so any extra round-trips checking for blobs to mount instead of just writing them are likely to make things worse.

So none.NoCache might be the right thing to do here anyway: At least with the current attachment implementation, we would just be adding entries (which we never delete) and growing the database for no benefit.

---

WRT the blob data cache, note that it is write-through; if attached to a destination, it writes the data to local storage in addition to writing them to a destination. So that’s not making writes any faster. The blob cache helps on reads, and that might be somewhat significant here if network reads are slower than disk reads (which is reportedly not always the case…). So… how often does one pull an image after pushing it? If it happens frequently, I’d expect the image to already be in c/storage (… and, true, we do a copy with most of the round-trips anyway…) In that case I think teaching the copy code not to copy an already-existing image (or just already-existing signatures?) would be better than the blob cache, because c/storage has delete operations and is under better user control.

---

In general, it is a bit of a concern that working with the cosign attachments is round-trip heavy (though not much heavier than a https:// look-side). We might eventually want to worry about optimizing out individual redundant attachment copies, especially if some of them could grow quite large (SBOMs? I have never seen one in the wild). And, potentially much worse, we might need to stream the attachment payloads instead of just reading them into memory.

I think we can deal later with that if it ever shows up as an user-reported issue. The new …WithFormat API shows we have the conceptual flexibility.

[mtrmac]

Replaced the FIXMEs with comments in #1595 .

[vrothberg]

Nit: can we rename found to something similar to alreadyOnRegistry? It adds some more context that would help me brain-parse the code.

[mtrmac]

Done in #1595 .

[vrothberg]

Are the diff IDs needed? I couldn't find something in the cosign spec but recall that cosign just ships an empty config.

[mtrmac]

I don’t know.

It’s quite possible they aren’t needed. I also thought cosign ships a completely empty config, but when looking what to write into that config, I found DiffID entries in a Cosign-generated file. That might be a go-containerregistry artifact, the API has concepts like “add a layer” which adds it both to the manifest and to DiffIDs.

At least distribution/distribution seems not to care, except if the client is schema1-only.

Historically Quay has been the registry with the strictest validation we’ve been hearing about, I have no idea what they do.

[vrothberg]

Thanks for checking!

I also suspect go-containerregistry to be the source. I am totally fine to add the DiffIDs.

[mtrmac]

It’s a good thing to point out and to track — if we don’t need to add DiffIDs, we would eliminate a blob write for the config on every copy that adds signatures to an existing image with signatures.

[mtrmac]

There is now just one outstanding PR - #1598 , so closing this.

Outstanding unfinished tasks and the like are tracked in #1601.