-
Notifications
You must be signed in to change notification settings - Fork 49
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
pyproject: bump sigstore-protobuf-specs #705
Conversation
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
...but only if inclusion_proof is not None. Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
To summarize what's happening here:
I need to think a bit more about (2) and ensure that we're doing what's stated above, but this should be consistent with both bundle formats. |
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
One edge case that (I think) still needs to be covered here is around checkpoints. Here are the different states we can be in (limited to just inclusion proofs and checkpoints, not considering inclusion promises):
When in online mode, all of these variants are handle-able: (1) and (3) should cause us to perform an online entry lookup, while (2) and (4) will be handled using the supplied inclusion proof. When in offline mode, (1) and (3) are murky: it's unclear whether they should be error cases, whether we should verify the inclusion promise instead (if present), or something else. My intuition is that they should be error cases unless the inclusion promise is present, but it'd be nice to have some guidance/normative behavior here. CC @znewman01 for opinions on the client behavior side + @haydentherapper for behavioral opinions as well 🙂 |
Signed-off-by: William Woodruff <[email protected]>
Apparently Pydantic needs this now. Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
I agree with this, but we should decide at what point we deprecate this behavior and mandate inclusion proofs. Do we have a deprecation policy in place yet? For justification, an inclusion proof without a checkpoint should be considered invalid since there's no link to the log. If you can't query online or do not have a promise (both checkpoints and promises are signed with the same key, so while each commits to something different, they're still both commitments from the log), the client must return an error. |
Signed-off-by: William Woodruff <[email protected]>
Agreed! I don't think we have a deprecation policy in place; CC @znewman01 as clients czar 🙂 |
I think deprecation is up to each individual client. For instance, Go mandates a strict adherence to semver. Other ecosystems may have other conventions. You can look at the Cosign versioning policy for an example: https://github.com/sigstore/cosign/blob/main/VERSIONING.md My preference is probably:
+1 to offline-mode being totally offline and erroring out if additional information is needed. |
Signed-off-by: William Woodruff <[email protected]>
@jku Do you want to take a look too? |
# 6) Verify the Signed Entry Timestamp (SET) supplied by Rekor for this artifact | ||
try: | ||
verify_set(self._rekor, entry) | ||
except InvalidSETError as inval_set: | ||
return VerificationFailure(reason=f"invalid Rekor entry SET: {inval_set}") | ||
if entry.inclusion_promise: | ||
try: | ||
verify_set(self._rekor, entry) | ||
except InvalidSETError as inval_set: | ||
return VerificationFailure( | ||
reason=f"invalid Rekor entry SET: {inval_set}" | ||
) |
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.
Is the inclusion promise/SET optional for 0.2 bundles?
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.
Yep. 0.2 inverts the behavior here:
- With 0.1, the SET is mandatory and the inclusion proof is optional;
- With 0.2, the SET is optional and the inclusion proof is mandatory.
Signed-off-by: William Woodruff <[email protected]>
CC @di for review, if you have the time 🙂 |
This bumps
sigstore-protobuf-specs
to the0.2.x
series which, among other things, changes the required status of a few fields.Draft until I figure out what needs to change on our client side.