-
Notifications
You must be signed in to change notification settings - Fork 29
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
sigstore_rekor: clarify inclusion_promise requirement #380
sigstore_rekor: clarify inclusion_promise requirement #380
Conversation
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
protos/sigstore_rekor.proto
Outdated
// Optional for >= v0.2 bundles, and SHOULD be verified when present. | ||
// Also may be used as a signed timestamp. | ||
// Optional for >= v0.2 bundles if another source of signed time | ||
// is present. |
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.
One could interpret "another source of ... time" to be current time, but "current time" wouldn't be signed. We could be very prescriptive and say "When verifying long-lived certificates, the client MAY choose to not require a signed timestamp and instead use the system clock." Thoughts?
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.
That sounds good to me! Updating.
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.
Updated in d9edb63 -- it now talks about a "suitable" source of time, which can be either another signed time source or the current system time when the certs are long-lived. LMKWYT!
Signed-off-by: William Woodruff <[email protected]>
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!
This clarifies the (expected) requirements around
inclusion_promise
slightly. In particular, it clarifies thatinclusion_promise
is optional if and only if another source of signed time is present. If no other source of signed time is present, then aninclusion_promise
is required and MUST be verified.For cross-referencing, this is the part of the Client spec that suggests this behavior:
(NB: Like the other requirements on bundle formats/required fields, this requirement is for short-lived certificate instances of Sigstore, like the Public Good Instance. CC @haydentherapper for thoughts on if/how this can be better communicated -- I'm happy to add additional language here or in the
sigstore_bundle.proto
file!)