-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add checkpoint verification #660
Conversation
Signed-off-by: Appu Goundan <[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.
Looks great!
var keyHash = Hashing.sha256().hashBytes(tlog.getPublicKey().getRawBytes()).asBytes(); | ||
// checkpoint 0 is always the log, not any of the cross signing verifiers/monitors | ||
var sig = checkpoint.getSignatures().get(0); | ||
for (int i = 0; i < 4; i++) { |
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.
what is the 4
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.
Key ID is 4 bytes, https://github.com/sigstore/rekor/blob/main/pkg/util/signed_note.go#L140, truncated public key hash
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.
Would be good to have a link to that or https://github.com/C2SP/C2SP/blob/main/signed-note.md#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.
that should be added in a comment at least IMO
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's defined in the spec and the spec is linked in the checkpoint type
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.
negative test cases are here: sigstore/sigstore-conformance#139
it's always a bit weird to mirror those test cases in sigstore-java, but I guess I can?