-
Notifications
You must be signed in to change notification settings - Fork 13
Add Proposal F #50
Comments
👋 Before following the full blown template, I thought it'd be quicker to give an outline of a possible solution. It has elements of Proposal C and D, and tackles signing and attestations. Goals:
Non-goals:
The unsigned object that you want to attach metadata to is represented by an index or image manifest. Metadata is stored in a signed index that references the unsigned index in the graph TD;
SignedIndex-->UnsignedIndexOrManifest;
Unsigned index or manifest ( {
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
"schemaVersion": 2,
"manifests": [
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"digest": "sha256:c3c58223e2af75154c4a7852d6924b4cc51a00c821553bbd9b3319481131b2e0",
"size": 528,
"platform": {
"architecture": "arm64",
"os": "linux"
}
},
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"digest": "sha256:4ff3ca91275773af45cb4b0834e12b7eb47d1c18f770a0b151381cd227f4c253",
"size": 528,
"platform": {
"architecture": "amd64",
"os": "linux"
}
}
]
} Signed index pushed with unsigned object's tag ( {
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
"schemaVersion": 2,
"manifests": [
{
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
"digest": "sha256:6444e2ab69f4c3acedad9997be5e3bdc498edec1dc7b3a5dac8faed54899ef16", // Unsigned index (Alpine)
"size": 741,
"annotations": {
"dev.cosignproject.cosign/signature.0": "...",
"dev.sigstore.cosign/bundle.0": "...",
"dev.sigstore.cosign/certificate.0": "...",
"dev.sigstore.cosign/chain.0": "...",
"dev.cosignproject.cosign/signature.1": "...",
"dev.sigstore.cosign/bundle.1": "...",
"dev.sigstore.cosign/certificate.1": "...",
"dev.sigstore.cosign/chain.1": "..."
}
},
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"digest": "sha256:5b0044a1244...", // arm64 SBOM
"size": 1024,
"annotations": {
"org.opencontainers.reference.type": "sbom",
"org.opencontainers.reference": "sha256:c3c58223e2af75154c4a7852d6924b4cc51a00c821553bbd9b3319481131b2e0"
}
},
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"digest": "sha256:5b0044a44d...", // amd64 SBOM
"size": 1024,
"annotations": {
"org.opencontainers.reference.type": "sbom",
"org.opencontainers.reference": "sha256:4ff3ca91275773af45cb4b0834e12b7eb47d1c18f770a0b151381cd227f4c253"
}
}
]
} Note that the signature data is stored as indexed annotations. Signing tools would need to increment [footnote 0] This assumes client support for nested indexes. If this is not a safe assumption, the contents of the subject's |
Wondering if this part will cause some controversy, but looking forward to the full proposal! 🍿🍿🍿 |
@jdolitsky Is there some previous discussion about this that we can read? It is mostly for performance to keep the verification overhead minimal compared to unsigned images. If you are just worried about some fields (eg. |
Edit: I misinterpreted what root object meant. Original text preserved below. I agree we should solve racy edits. I think the incrementing approach can work fine, and is a reasonable tradeoff given the requirement on not adding new fields. A list would obviously be "cleaner", but I'm not a purist. If we go this route we should specify how to handle gaps in the integers and move on. ---BEGIN ORIGINAL TEXT--- |
Overall I like this but need to dig more into compatibility today. The early attempts at cosign were very similar but I think we ran into some compatibly issues with registries. Some cases we should test:
|
This looks really promising!
I don't love it -- I don't think any of us do -- but if this is the biggest wart then I'll take it. 😆 +1 to making OCI specify handling racy updates better, that's needed regardless. If we specify this we should take care when wording the incrementing Re:
Some questions:
When you say they "can" be, do you mean you expect they practically will be? I think we should take a stronger stand on which option we want to encourage or require, instead of specifying that both are possible, since it means clients have to check and handle both behaviors. edit: Rereading this section, I misread this as "clients can do either" and not "if support for nested indexes can't be safely assumed, we can update the proposal to copy the unsigned index's manifests into the signed index." This means we'd change the digest whenever we add a new attachment, which has different understood downsides. Let me know if I'm still misreading or misinterpreting. |
Thanks for the reviews so far!
100 % agree. If there are better ideas for how to handle this, I'd be happy to hear them.
We had the same issue with storing CNABs. We might even have a mapping of which registry supported what somewhere. We solved it by using a fallback to Docker types. I'm the first to admit that this is not ideal but we decided it was the least worst option. Rewriting manifests in a different format is an option but it will change digests as well. I would be curious to hear about other pitfalls that you ran into though.
This is a great summary of how I feel 😬 It really is a case of trying to pick the least worst option. If it's any consolation, it should mostly be us tool builders who're exposed to this not end users.
Yes, we'd likely want some well known types list for interoperability. I think the OCI is an appropriate place to define this.
I was intentionally ambigious here. I know that the OCI have previously raised that they don't like indexes pointing to blobs from the
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"digest": "sha256:5b0044a1244...", // arm64 SBOM
"size": 1024,
"annotations": {
"org.opencontainers.reference.type": "sbom",
"org.opencontainers.reference": "sha256:c3c58223e2af75154c4a7852d6924b4cc51a00c821553bbd9b3319481131b2e0"
}
}, In the example,
I didn't want to distract from the main proposal which is why I added this as a footnote. Today, some runtimes ignore nested indexes and try to find a manifest that meets their needs in the top level index. This would mean with the above design, they would fail to run an image. The workaround is to copy the manifests section from the nexted index into the signed top level index. This would mean that runtimes that don't traverse the nested index would find an image to run and work. When appending signatures, the original object (the unsigned index) would remain the same but the signed index would be mutated with new annotations. This would mean that the original object's digest would remain constant. One possible issue with this is what should client tooling do if the manifests in the signed index do not match those in the nested index? I think this is solvable though. We just need to write down what the behavior should be. |
Looking over this, I've got a couple questions:
|
From above:
From that index client can find the attestation or signature for any inner object.
|
Woops, completely overlooked that line. We should consider a more generic name since this will be used for more than just signing.
Got it. Thanks! Sounds like the big concerns have already been highlighted, race conditions, and existing runtimes that won't follow nested Indexes. The latter is easy enough to work around by not changing the original tag and using digest tags, or pulling up platform specific descriptors. The reason that D opted for unique digest tags was to avoid the race conditions, so we're left deciding if we want to not solve race conditions to simplify the client pull. We should also test how existing runtimes deal with an Index that includes multiple descriptors without any platform specification. |
Just a few ideas we can explore... Top-level key holds a JSON list{
"annotations": {
"org.opencontainers.signature": "[{ \"name\": \"dev.cosignproject.cosign\", \"signature\": \"...\", \"bundle\": \"...\", \"certificate\": \"...\" }, { \"name\": \"dev.sigstore.cosign\", \"chain\": \"...\" }, { \"name\": \"dev.cosignproject.cosign\", \"signature\": \"...\" }, { \"name\": \"dev.sigstore.cosign\", \"bundle\": \"...\" }, { \"name\": \"dev.sigstore.cosign\", \"certificate\": \"...\" }, { \"name\": \"dev.sigstore.cosign\", \"chain\": \"...\" }]"
}
} Hashes in keys and a separate indexThis replaces use of ints in @chris-crone's proposal with hashes of each of the value and adds an explicit index. {
"annotations": {
"dev.cosignproject.cosign/signature.e59879c": "...",
"dev.sigstore.cosign/bundle.6436749": "...",
"dev.sigstore.cosign/certificate.776a3f1": "...",
"dev.sigstore.cosign/chain.8adf60e": "...",
"dev.cosignproject.cosign/signature.de25bf6": "...",
"dev.sigstore.cosign/bundle.37d34ff": "...",
"dev.sigstore.cosign/certificate.e8adf60": "...",
"dev.sigstore.cosign/chain.b1c64a3": "...",
"org.opencontainers.signatures.index":"dev.cosignproject.cosign/signature.e59879c,dev.cosignproject.cosign/signature.de25bf6,dev.sigstore.cosign/bundle.37d34ff,dev.sigstore.cosign/certificate.e8adf60,dev.sigstore.cosign/chain.b1c64a3,dev.sigstore.cosign/bundle.6436749,dev.sigstore.cosign/certificate.776a3f1,dev.sigstore.cosign/chain.8adf60e",
}
} I mostly just thought to put a few ideas foward, so we have something to discuss. I do think it'd best if implementations didn't have to parse to worry about which one is the number to pick and increment. |
Yes, integers has a lot of problems, just using an identifier is much better. Could use the public key hash. |
Fallback would work fine, but don't we then have an issue with the Docker manifest list not having an annotation field? https://docs.docker.com/registry/spec/manifest-v2-2/#example-manifest-list |
My concern with embedding this kind of data in annotations is that OCI annotations are optional and do not follow any set format. So if a client doesn't understand or ignores these annotations, all the reference data will be lost, which is no different from the backwards compatibility issues that were raised in Proposal C (moving manifests from V3 to V2 registries will result in loss of references). |
Reg: racey index: currently, one can GET and PUT any manifest by digest using |
There's a lot of great discussion happening here, but I just want to point out that the proposal template is meant to be succinct and minimal. Check out Proposal B, just 85 lines. Several of the comments here are already longer than that. It also sounds like this new proposal isn't even suggesting changes to the registry API, cutting out an entire section of the template. Please take a few minutes to submit a proposal, it does not need to be perfect. If you want to put a "TODO" for the Requirements section, that's ok for now. Once it is submitted, we can continue to iterate on it. |
So multiple signatures with the same key are not allowed by definition? Given it is a prefix and our scale, we should define behavior for collision case as well.
Can you list more specifically what problem integers have that public key prefix does not?
I'm not sure if fallback to Docker mediatypes even makes sense because this was a long time ago, and afaik OCI mediatypes have been supported for years by everyone(this is different from the OCI artifact requirement). Regarding annotations, they are only parsed on the client side. Only new clients understand how to verify signatures anyway and old ones would ignore them.
Not sure what you mean by "will be lost" in here.
I think a the race people are referring to is the case for parallel |
These collisions would happen on a single image within a repository (not the entire registry). So the space for any two objects having a collision will be small. But you may have a lot more of those spaces than other registries.
The push back we got on opencontainers/distribution-spec#251 was that some storage backends for registries are eventually consistent, which means even the registry can't make this guarantee. We either do a best effort on the registry (with the assumption that not all registries will even support it), require clients to do it themselves, or we make the PUT requests idempotent. |
This would be multiple signatures, from the same signer, of the same image. For that use case, would you want/need to keep the older signature? |
Can we store multiple signatures as
? In theory the same image can work on multiple platforms, so digests don't have to be unique in the list. |
@dmage I think that's a very good idea to consider, if indeed permited by OCI and existing registry implemetnations, which is soemthing I cannot really comment on. |
According to the spec, that should be supported by OCI index spec:
I submitted that text not too long ago thinking of scenarios like this. In this case, if the client is looking for a specific signature, it may continue to search the index until if finds one. But unofficially, I won't be surprised to see existing runtimes break if they encounter that. |
What about the other annotations this descriptor has? Should the process that is adding a signature also make a copy of all the other values it doesn't understand.
In practice, it is more complicated indeed. There is a concept of "which platform matches more". Eg. if you have descriptors for |
That's a good question. We already know the image digest from the tag name, so we don't have to put it again into the manifest. In this case most of annotations can be in the top-level property. {
"schemaVersion": 2,
"mediaType": "application/vnd.oci.image.index.v1+json",
"annotations": {
"org.opencontainers.type": "org.opencontainers.references",
// Simple dynamic attributes:
"com.vendor.lifecycle": "Maintenance support phase",
"com.vendor.end-of-life": "2022-09-30",
"com.vendor.upgradable-to": "3.5-stable",
"com.vendor.annotation": "it can be only one",
},
"manifests": [
// Linked objects:
// 1. Either a simple object (a set of key-value pairs), in this case
// digest, mediaType, and size should match the image, and the linked
// object itself is saved as annotations.
{
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
"digest": "sha256:6444e2ab69f4c3acedad9997be5e3bdc498edec1dc7b3a5dac8faed54899ef16",
"size": 741,
"annotations": {
"org.opencontainers.reference.type": "dev.cosignproject.cosign.embedded",
"dev.cosignproject.cosign/signature": "...",
"dev.sigstore.cosign/bundle": "...",
"dev.sigstore.cosign/certificate": "...",
"dev.sigstore.cosign/chain": "...",
}
},
// 2. Or an arbitrarily complex object that is known to the registry.
{
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"digest": "sha256:5b0044a1244...", // arm64 SBOM
"size": 1024,
"annotations": {
"org.opencontainers.reference.type": "org.opencontainers.sbom",
"org.opencontainers.reference": "sha256:c3c58223e2af75154c4a7852d6924b4cc51a00c821553bbd9b3319481131b2e0"
}
}
]
}
In this case the process either changes the top-level annotation or an entity in "manifests". It should keep untouched all entities that it doesn't understand.
The references index shouldn't have platforms as it has only metadata. Its records aren't platform-specific and it shouldn't be executable. |
I've started to explore some ideas on top of some of these concepts in proposal form here: https://github.com/jdolitsky/wg-reference-types/blob/proposal-g/docs/proposals/PROPOSAL_G.md I wasn't planning to submit this, but some of that content might be useful for the original formation of Proposal F. Please feel free to copy parts out of it. |
closed in #50 |
Considering unfinished discussion in #47 and #48, and also on the weekly call yesterday, the best path forward is to add another proposal (F) that addresses these concerns. Please see the template.
Without an alternative proposal to consider, the WG will continue to explore the feasibility of Proposal E, our "frankenstein / voltron" proposal that, to the best of our abilities, combines various good parts of proposals A-D.
Please let us know if we can expect to see this soon.
cc: Docker friends @chris-crone @tonistiigi @errordeveloper @justincormack
The text was updated successfully, but these errors were encountered: