-
Notifications
You must be signed in to change notification settings - Fork 25
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 verify DSSE bundles (after signing) #258
Conversation
When signing, if you optionally supply a trusted root we will attempt to verify the bundle before we return it. Previously we were using the wrong artifact digest for DSSE signing. We could add a way to specify the artifact referred to in DSSE, but we are already trusting the certificate identity in the user-supplied id token. Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
f055bf8
to
5ca2049
Compare
// However, we just signed the DSSE object trusting the user has | ||
// referenced the artifact(s) they intended. | ||
artifactOpts := verify.WithoutArtifactUnsafe() | ||
if bundle.GetMessageSignature() != nil { |
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.
Why is this conditional necessary, and we only skip artifact verification?
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.
sigstore-go requires that some artifact reference is provided when verifying a message signature:
sigstore-go/pkg/verify/signature.go
Line 48 in 6f7e99d
return errors.New("artifact must be provided to verify message signature") |
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.
Oh yea, that makes sense, because you need an artifact for a signature, but not for a DSSE since that's just checking subject claims
Since this is a bugfix, how soon can you do another release? |
Usually we cut release as-needed, so this morning I made https://github.com/sigstore/sigstore-go/releases/tag/v0.6.0 |
Summary
Fixes #257.
When signing, if you optionally supply a trusted root we will attempt to verify the bundle before we return it.
Previously we were using the wrong artifact digest for DSSE signing. We could add a way to specify the artifact referred to in DSSE, but we are already trusting the certificate identity in the user-supplied id token.
You can test with something like this (which previously would fail):
Release Note
NONE
Documentation
N/A