Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Specify only leaf cert in bundle #190

Closed
haydentherapper opened this issue Jan 9, 2024 · 6 comments · Fixed by #191
Closed

Proposal: Specify only leaf cert in bundle #190

haydentherapper opened this issue Jan 9, 2024 · 6 comments · Fixed by #191

Comments

@haydentherapper
Copy link
Collaborator

Background: The Sigstore bundle includes the verification material for a signature. For a signature generated with a key, it includes a public key fingerprint. When generated with a certificate, it currently can include either just the leaf certificate, or the leaf and its chain. The chain should only include intermediates, although there are bundles that were generated with roots too (resulting in this comment.

The X509CertificateChain message is currently used in two places: In the verification material for a bundle, or in the trust root. The comments on the message are mostly relevant to the former case.

As discussed in sig-clients, we would like to propose removing support for specifying a chain and require that the chain be distributed out of band or in the trust root. Including the chain in the bundle is useful for classical PKI cases where an intermediate is untrusted and only used for chain building. However the bundle wasn't designed with private deployments in mind. We do still want to support those cases, but want to be intentional in doing so.

One way we could this is to avoid breaking changes is to add an additional field to the oneof in VerificationMaterial, specifying a single certificate. Clients could then choose whether to support a BYO PKI case using x509_certificate_chain, or only support a single certificate. We could also do this in a breaking way and drop x509_certificate_chain.

Clients for thoughts: @loosebazooka @woodruffw @kommendorkapten @bdehamer @codysoyland

@woodruffw
Copy link
Member

Big +1 from me -- my 0.02c preference would be a breaking change, but a new oneof variant also seems like a reasonable compromise between highlighting the "right" way and preserving compatibility 🙂

@kommendorkapten
Copy link
Member

I'm currently leaning @haydentherapper's proposal of adding new filed to the oneof, with a clarification that for PGI, certificate field has to be used for bundles over version 0.3+. I believe this would provide a nice tradeoff with the different verification options:

  1. Key pair
  2. X.509 Certificate with untrusted intermediates (i.e non PGI use-cases)
  3. X.509 Certificate where intermediates are delivered OOB (i.e via the trust root)

If we perform this change, we should move the comment from X509CertificateChain on how to use the chain during verification to the bundle message, IMHO it's a bit misplaced to discuss verification semantics in the X509CertificateChain message, as that can be used for other purposes, such as delivering a trust root.

@woodruffw
Copy link
Member

with a clarification that for PGI, certificate field has to be used for bundles over version 0.3+. I believe this would provide a nice tradeoff with the different verification options:

This sounds good to me!

we should move the comment from X509CertificateChain on how to use the chain during verification to the bundle message, IMHO it's a bit misplaced to discuss verification semantics in the X509CertificateChain message, as that can be used for other purposes, such as delivering a trust root.

Agreed.

@woodruffw
Copy link
Member

(I'm happy to submit a PR for these changes later today, unless @haydentherapper or @kommendorkapten you'd prefer to do it.)

@kommendorkapten
Copy link
Member

@woodruffw please do, I'm stuck in meetings the rest of my day ❤️

@woodruffw
Copy link
Member

Opened #191 for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants