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

trustroot: initial client config messages #277

Merged
merged 9 commits into from
Apr 2, 2024

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Mar 26, 2024

WIP; needs workshopping 🙂. Closes #259.

CC @haydentherapper @loosebazooka @kommendorkapten

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw self-assigned this Mar 26, 2024
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
protos/sigstore_trustroot.proto Show resolved Hide resolved
protos/sigstore_trustroot.proto Show resolved Hide resolved
protos/sigstore_trustroot.proto Outdated Show resolved Hide resolved
Signed-off-by: William Woodruff <william@trailofbits.com>
Co-authored-by: Fredrik Skogman <kommendorkapten@github.com>
Signed-off-by: William Woodruff <william@yossarian.net>
@haydentherapper
Copy link
Collaborator

LGTM, I think this just needs a regeneration

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Member Author

LGTM, I think this just needs a regeneration

Done!

@haydentherapper
Copy link
Collaborator

@woodruffw Do you want to get this in before 0.3.1?

@woodruffw
Copy link
Member Author

@haydentherapper Yep! I'll rework on top of #279 now.

Signed-off-by: William Woodruff <william@trailofbits.com>
Copy link
Member

@kommendorkapten kommendorkapten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Before merging, I would just like to make sure we are agreeing on how this will be distributed via PGI TUF root (i.e how this should be documented in the Sigstore Client Spec).

We now have:

  • SigningConfig
  • TrustedRoot

And then ClientTrustConfig = {SigningConfig, TrustedRoot}

The TUF repository already ships TrustedRoot as single artifact, trusted_root.json.
How would we ship the SigningConfig? My recommendation is to add a new artifact to the TUF root called signing_config.json. Why? Do not break the clients. Adding a new target is trivial.

The alternative is to add a new target client_trust_config that contains both messages. The downside is that we would need to duplicate the trusted_root.

Shipping two files from the TUF repo is not the end of the world IMHO. We can still allow clients to accept a single file (client_trust_config) to simplify the UX when e.g. invoked via the terminal.

As we are in the process of adding support for trusted root to a lot of components, withdrawing that from the TUF repository would be very disruptive 😅

@woodruffw
Copy link
Member Author

woodruffw commented Apr 2, 2024

This might be a minority opinion, but IMO it's okay for the PGI TUF repo to not ship SigningConfig at all -- the PGI and staging instances both have "well-known" signing configurations already, so a client can (continue to) hardcode them.

In other words, IMO it's okay if SigningConfig and ClientTrustConfig exist entirely as message definitions for checking user (i.e. non-TUF) JSON inputs against, with the idea being that they establish the interface for clients that want to support CLIs like --client-trust-config trust.json.

Regardless of the above, I 100% agree about not removing the current trusted_root.json target -- IMO if we want to put SigningConfig or ClientTrustConfig in the TUF repo, then a separate file (either just the signing config or the combined client trust) would be perfectly cromulent 🙂

@kommendorkapten
Copy link
Member

Sounds good, I think we are in agreement!

@woodruffw
Copy link
Member Author

Merging!

@woodruffw woodruffw merged commit 58ba3ec into sigstore:main Apr 2, 2024
24 checks passed
@woodruffw woodruffw deleted the ww/client-trust-config branch April 2, 2024 14:52
@haydentherapper
Copy link
Collaborator

@woodruffw @kommendorkapten I think a primary motivation would be if we move Rekor to yearly sharding, since the URL would change periodically and clients would need to discover it without breaking old clients. I don't think we need to immediately ship a SigningConfig though since this isn't the case currently, but I do think we should start making plans for adding an additional file distributed through TUF.

@kommendorkapten
Copy link
Member

@haydentherapper yes, I proposed in the client meeting to today that we add the signing_config.json to the TUF repo during next signing.

kommendorkapten added a commit to sigstore/root-signing-staging that referenced this pull request Aug 21, 2024
Added `signing_config.json` (see
sigstore/protobuf-specs#277 for details on
the name of the file).

Signed-off-by: Fredrik Skogman <kommendorkapten@github.com>
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 this pull request may close these issues.

Add infrastructure information to trusted root
4 participants