-
Notifications
You must be signed in to change notification settings - Fork 547
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
refactor bundle validation code, add support for DSSE rekor type #3016
Conversation
Signed-off-by: Bob Callaway <[email protected]>
Signed-off-by: Bob Callaway <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #3016 +/- ##
==========================================
- Coverage 31.06% 31.00% -0.07%
==========================================
Files 155 155
Lines 9744 9712 -32
==========================================
- Hits 3027 3011 -16
+ Misses 6255 6250 -5
+ Partials 462 451 -11
|
case *rekord_v001.V001Entry: | ||
return entry.RekordObj.Signature.Content.String(), nil | ||
default: | ||
return "", errors.New("unsupported 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.
What about intoto types? I see we didn't try to extract signatures previously, but isn't that a gap?
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.
in compareSigs
if an actual attestation is stored in OCI, the signature from the DSSE envelope is not stored as a separate file (as it would be with other types) so the verification is done elsewhere.
Signed-off-by: Bob Callaway <[email protected]>
Signed-off-by: Bob Callaway <[email protected]>
needs rebase |
LGTM to me, just one comment on a test. @bobcallaway do you want to also update this PR to switch the default type to dsse too since the Rekor update is now in prod? |
Signed-off-by: Bob Callaway <[email protected]>
Signed-off-by: Bob Callaway <[email protected]>
@bobcallaway is this ready to go? |
@haydentherapper @bobcallaway, after this can we cut a release? |
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
@bobcallaway need rebase |
Signed-off-by: Bob Callaway <[email protected]>
eb62e59
Summary
This refactors the bundle verification code to only parse entry JSON once, and use reflection to determine the appropriate way to extract the Rekor-type specific information.
Also adds support for the newly added DSSE type in Rekor (currently only deployed to staging, not production). I will propose a new PR to switch over the default behavior to use
dsse
instead ofintoto
Rekor type once this goes live into production.Release Note
NONE
Documentation