-
Notifications
You must be signed in to change notification settings - Fork 136
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
fix: Switch to newer DSSE rekor type #3299
Conversation
The intoto v001 type does not persist signatures of the DSSE envelope, as noted in sigstore/rekor#973. We introduced an intoto v002 type shortly after to fix this, but since then, we've introduced another newer type, DSSE v001, which also does not persist the attestation in Rekor (as we discourage using Rekor as storage). I also updated the verifier in slsa-framework/slsa-verifier#742 to search for both Rekor entry types. Signed-off-by: Hayden Blauzvern <[email protected]>
@kpk47 @laurentsimon, a Sigstore community noticed that the signature from the DSSE envelope was missing in Rekor (thread). This was a known issue in the intoto v001 type, and we've since introduced a newer DSSE type that a) fixes this and b) stops persisting the attestation since Rekor shouldn't be attestation storage. Lemme know if this is an issue. This should also get merged with slsa-framework/slsa-verifier#742. |
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.
Thanks @haydentherapper . I think this change works. Shall we consider it a breaking change or not? I think so. Wdut?
nit: Can you update the CHANGELOG.md file?
/cc @ramonpetgrave64 |
It's sort of a breaking change. It's not a breaking change for the generator itself, but it's a breaking change in that it'll require verifiers to be updated. When we made this change in Cosign, we didn't call it a breaking change since Cosign is both the signer and verifier, but given the verification tooling is separate here, probably would be good to have this as a minor rather than patch release. Edit: Also given the attestation is no longer persisted in rekor, calling this a breaking change would be reasonable in case anyone was relying on it. |
Signed-off-by: Hayden B <[email protected]>
Updated changelog |
but it could be possible that a signer uses a more recent version than the verifier (e.g., another team), so I think it probably was a breaking change.
SG, thanks. I'll wait to merge until we're 100% sure we'll bump the major version of the next release. |
There's a linter failing. |
Signed-off-by: Hayden B <[email protected]>
Fixed |
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Thanks for fixing the PR Laurent. Failing test seems like it just wants a rebase? |
Oh it is already. Hm.. |
pre-submit failing is not your fault. I'll send a PR to fix it. |
PR is out from this morning #3454 :) Need to be merged first |
Bocking PR is merged. I've rebased and enabled auto-merge. Thanks @haydentherapper ! |
The intoto v001 type does not persist signatures of the DSSE envelope, as noted in sigstore/rekor#973. We introduced an intoto v002 type shortly after to fix this, but since then, we've introduced another newer type, DSSE v001, which also does not persist the attestation in Rekor (as we discourage using Rekor as storage).
I also updated the verifier in slsa-framework/slsa-verifier#742 to search for both Rekor entry types.
Summary
...
Testing Process
...
Checklist