Skip to content

Fix verify DSSE bundles (after signing)#258

Merged
steiza merged 2 commits intomainfrom
verifying-signed-dsse
Aug 6, 2024
Merged

Fix verify DSSE bundles (after signing)#258
steiza merged 2 commits intomainfrom
verifying-signed-dsse

Conversation

@steiza
Copy link
Copy Markdown
Member

@steiza steiza commented Aug 6, 2024

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):

$ go run examples/sigstore-go-signing/main.go -id-token "..." -in-toto -rekor examples/sigstore-go-signing/intoto.txt 

Release Note

NONE

Documentation

N/A

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>
@steiza steiza requested a review from a team August 6, 2024 15:18
Comment thread pkg/sign/signer.go
Signed-off-by: Zach Steindler <steiza@github.com>
Comment thread pkg/sign/signer.go
// However, we just signed the DSSE object trusting the user has
// referenced the artifact(s) they intended.
artifactOpts := verify.WithoutArtifactUnsafe()
if bundle.GetMessageSignature() != nil {
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Member Author

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:

return errors.New("artifact must be provided to verify message signature")

Copy link
Copy Markdown
Contributor

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

@steiza steiza merged commit 004c425 into main Aug 6, 2024
@steiza steiza deleted the verifying-signed-dsse branch August 6, 2024 20:41
@ramonpetgrave64
Copy link
Copy Markdown

Since this is a bugfix, how soon can you do another release?

@steiza
Copy link
Copy Markdown
Member Author

steiza commented Aug 16, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bundle signer does not correctly verify artifact if bundle content is DSSE

3 participants