-
Notifications
You must be signed in to change notification settings - Fork 55
sigstore: prep verify APIs for DSSE #904
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
Conversation
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]>
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]>
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 like a solid improvement to me. Left one test suggestion/question
(_, a_materials) = signing_materials("a.txt") | ||
(file, offline_rekor_materials) = signing_bundle("bundle.txt") | ||
|
||
with file.open(mode="rb", buffering=0) as input_: |
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 wonder if signing_materials()
should just open the file and yield (input_, materials)
instead so we can avoid doing this in the tests?
- Not every test needs input_ of course... but I would think
open()
is not expensive in context. - some tests want to read the file twice so would then probably need
input_.seek(0)
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 works, although to go one step further: I think signing_materials
/signing_bundle
should probably just return bytes
/Hashed
directly, since the plan is to have both sign
and verify
take those types rather than do their own I/O 🙂
WIP.This is the first step towards DSSE verification support, breaking apart some of our internal design mistakes around the state carried in
VerificationMaterials
. Merging this will allow us to be more flexible about the inputs passed toverify()
, including DSSE statements and prehashed inputs asHashed
objects.