-
Couldn't load subscription status.
- Fork 44
Add witnessing configuration and other changes for TrustedRoot v0.2 #783
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
base: main
Are you sure you want to change the base?
Conversation
This introduces three changes for a v0.2 TrustedRoot file: * A witness configuration for a transparency log. Witnesses have two levels of quorum. A group of witnesses in the same trust domain can specify a quorum policy, and then the list of groups specifies another. This allows for more complex policies where witnesses have different availability. A client that wants to specify their own witnesses or different quorum policies will need to construct their own TrustedRoot. * The log's final checkpoint once the log is frozen, as an additional way that a client can check that it hasn't received a proof from a log that should have been turned down, in additional to validity windows. * Additional public keys for when a log generates multiple signatures for a checkpoint, e.g. using an Ed25519 key and an ML-DSA key. Following the versioning scheme we're using for the public good instance, we'll introduce a trusted_root.v0.2.json target in a later TUF root signing. Signed-off-by: Hayden <[email protected]>
| // a log uses multiple signing algorithms to generate checkpoint signatures. | ||
| // Only supported for TrustedRoot media types matching or greater than | ||
| // application/vnd.dev.sigstore.trustedroot.v0.2+json | ||
| repeated dev.sigstore.common.v1.PublicKey additional_public_keys = 9; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For matching against the checkpoint key id, would it be convenient to compute the for the additional keys? As written in the comment for the checkpoint key id, they can easily be computed. But for consistency it may make sense to include them.
And this also begs the question if there is any need to have "public key" and "additional keys". Is one key primary or could this just be simplified to two arrays, one for the public keys and one for the checkpoint key ids?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, we'd need an array of checkpoint key IDs as well.
And this also begs the question if there is any need to have "public key" and "additional keys". Is one key primary or could this just be simplified to two arrays, one for the public keys and one for the checkpoint key ids?
I wanted to avoid making a breaking change, even if this is just adding fields. It's a fair point that this is a little confusing to have this rather than just have an array. Lemme see about specifying a oneof of either public_key or public_keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead, how about have all keys in the additional_public_keys (and maybe come up with a better name). Then we can update clients to rely on that and over time deprecate the old field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for checkpoint ids of course.
| // requiring M-of-N, 2) any from a set of regionalized witnesses run by a | ||
| // single operator, and 3) a single witness instance. | ||
| // Inspired by https://git.glasklar.is/sigsum/core/sigsum-go/-/blob/main/doc/policy.md | ||
| message WitnessConfiguration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of grouping. But I'm not so sure about the witness trust policy, if it should be in the trusted root file or not. (I get that witness verification can be seen as a preliminary step to trust the entry or not, before even using the entry for artifact verification, but it feels like we are mixing concerns here).
For artifact verification we are tracking the policy (require X instances of tlog entries, x signed timestamps etc( in a separate file https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_verification.proto#L149
My gut feeling is that for witness policies, we should do the same. Offer a format on how to define the policy, but not ship the policy via the trusted root. This is something that the client should be in control over.
What I think we should do is similar to the artifact verification policy; define the standard sigstore client policy in the client spec, and implement that policy as the default in each clients (cosign, npm etc).
Not related to this, but what I think we should do for the future is to actually ship the complete PGI policy via TUF, and so we can avoid the clients to hard code the default policy in the binary, and consume that file to initialize the verifier. This would simplify operations for clients that implement support for different verification options (e.g. like the gh cli). That is a larger step, but moving the witness policy definition to the ArtifactVerificatonOptions would be a good step in that direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is different than the policy specified in sigstore_verification.proto because the log itself is configured with a witness policy to dictate how many witnesses it needs to get cosignatures from before publicizing a checkpoint. What's specified here will match the log's configuration.
One option could be to move WitnessConfiguration and the other messages to sigstore_verification.proto under ArtifactVerificatonOptions, so that users can specify their own witness policies, but also have a witness configuration for each tlog instance.
The witness configuration on the tlog instance is like a validity window by specifying when a proof should be trusted (e.g. proof was issued when the log was valid and the checkpoint's cosignatures matches the log's witness policy). Specifying this policy in the trust root mitigates the risk of the log operator changing its policy silently, say to a single witness which is controlled by the log operator. To change the policy would require a quorum of TUF root keyholders and the policy would be public.
Not related to this, but what I think we should do for the future is to actually ship the complete PGI policy via TUF
I very much agree with this, to have a default policy standardized across clients.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To add onto this, if we do "ship the complete PGI policy", then we should replace the witness configuration in the trust root to avoid duplicating it.
As I poke around sigstore_verification.proto, lemme see how straightforward it would be to knock this out at the same time. Otherwise, I'd suggest shipping the per-instance witness config in the trust root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thanks. Having the trust root manage only verification materials feels like the better option in the long run IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the log itself is configured with a witness policy to dictate how many witnesses it needs to get cosignatures from before publicizing a checkpoint. What's specified here will match the log's configuration.
It seems a little strange to me to define the quorum policy in trusted_root.json if this is intended to also be consumed the by the log, which doesn't use trusted_root.json currently.
I had a section in the design document about a client quorum policy that I ended up removing after a discussion with @mhutchinson - basically letting the client have its own policy that could be different from Rekor's policy seemed overly complex. So if we're continuing to follow that decision from the design doc then I don't think trusted_root.json is the place to define this.
|
This is great work @haydentherapper , thanks! |
| // application/vnd.dev.sigstore.trustedroot.v0.2+json | ||
| dev.sigstore.rekor.v1.Checkpoint frozen_log_checkpoint = 8; | ||
| // Additional public keys used to verify log checkpoint signatures for when | ||
| // a log uses multiple signing algorithms to generate checkpoint signatures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a link handy to any discussion of rekor using multiple signing algorithms so I can understand the context here?
| // requiring M-of-N, 2) any from a set of regionalized witnesses run by a | ||
| // single operator, and 3) a single witness instance. | ||
| // Inspired by https://git.glasklar.is/sigsum/core/sigsum-go/-/blob/main/doc/policy.md | ||
| message WitnessConfiguration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the log itself is configured with a witness policy to dictate how many witnesses it needs to get cosignatures from before publicizing a checkpoint. What's specified here will match the log's configuration.
It seems a little strange to me to define the quorum policy in trusted_root.json if this is intended to also be consumed the by the log, which doesn't use trusted_root.json currently.
I had a section in the design document about a client quorum policy that I ended up removing after a discussion with @mhutchinson - basically letting the client have its own policy that could be different from Rekor's policy seemed overly complex. So if we're continuing to follow that decision from the design doc then I don't think trusted_root.json is the place to define this.
| // The list of witness groups the log will be witnessed by. | ||
| repeated WitnessGroup witness_groups = 1 [(google.api.field_behavior) = REQUIRED]; | ||
| // Specifies how many witness groups are required for proving consistency. | ||
| ServiceConfiguration service_config = 2 [(google.api.field_behavior) = REQUIRED]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might also be worthwhile to allow specifying a witness group name or witness name (or list of such) as they do in the sigsum policy.
|
Offline chat from TDays conf:
|
This introduces three changes for a v0.2 TrustedRoot file:
Following the versioning scheme we're using for the public good instance, we'll introduce a trusted_root.v0.2.json target in a later TUF root signing.
Summary
Release Note
Documentation