-
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
Require inclusion proofs, make promises optional #84
Require inclusion proofs, make promises optional #84
Conversation
The log always generates inclusion proofs, so we will make it a requirement that clients verify the proof. Promises will be deprecated over time, but for now, we'll make them optional. Fixes sigstore#82 Ref sigstore/rekor#1566 Signed-off-by: Hayden Blauzvern <[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.
Just a quick drive by comment, could we also do a major version bump of the mime type version? Clients could then choose to key a verification strategy off this.
@feelepxyz, can you check the last commit to make sure that's correct? Haven't done a version bump before. |
aa034e3
to
7379d27
Compare
Signed-off-by: Hayden Blauzvern <[email protected]>
7379d27
to
a0861a3
Compare
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.
LGTM. A little weird that we're requiring clients to verify inclusion promises if present, but intending to totally get rid of them. I understand why though.
+1 to bumping the minor version.
Probably should mention how to handle older bundles. Maybe something like: "Older bundles will contain only an inclusion promise, which must be verified if present." We could also mark the field as deprecated, or hold off for a little before doing so. Thoughts? |
We have to be a little careful about "downgrade attacks" here (in scare quotes because the inclusion promise should still not be possible to get for an attacker). Maybe something like:
|
Signed-off-by: Hayden Blauzvern <[email protected]>
Added a comment in the bundle, and updated the comment for |
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.
LGTM!
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.
LGTM
The log always generates inclusion proofs, so we will make it a requirement that clients verify the proof. Promises will be deprecated over time, but for now, we'll make them optional.
Fixes #82
Ref sigstore/rekor#1566
Summary
Release Note
Documentation