Skip to content

Conversation

@jku
Copy link
Member

@jku jku commented Jan 12, 2026

We verify that the signature is over the actual artifact digest but we did not verify that the actual digest matches the digest documented in the bundle.

Ensure that the digests match during verify.


Fixes #1651

We verify that the signature is over the real digest but we did not
verify that the digest matches the digest documented in the bundle.

Ensure that the digests match during verify.

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku jku marked this pull request as draft January 12, 2026 09:50
jku added 2 commits January 12, 2026 12:01
Our tests still use the "separate materials" case (see signing_materials() fixture)
so there may not be a digest in the bundle

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku jku marked this pull request as ready for review January 12, 2026 10:16
woodruffw
woodruffw previously approved these changes Jan 12, 2026
Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jku, makes sense to me!

This is ancient history at this point, but I remember this being one of the reasons why I didn't want the hash in the bundle at all 😅 -- in this case it's a no-op field that adds extra cross-checking work, and in the worst case it's misleading (since users might think it does do something other than serve as an unauthenticated content hint).

I'd love it if a future bundle version removed it entirely, but it also seems like a pretty low priority thing to address.

@woodruffw
Copy link
Member

(We should have a CHANGELOG entry for this too, in the unlikely event this snares someone. But if you prefer to do those at release time, I'll stop bugging on each PR about them.)

@jku
Copy link
Member Author

jku commented Jan 12, 2026

This is ancient history at this point, but I remember this being one of the reasons why I didn't want the hash in the bundle at all 😅 -- in this case it's a no-op field that adds extra cross-checking work, and in the worst case it's misleading (since users might think it does do something other than serve as an unauthenticated content hint).

Yeah, I 100% agree. But now that it is in there, we should not accept a bundle with inconsistent info for exactly the reason in your parenthesis.

(We should have a CHANGELOG entry for this too, in the unlikely event this snares someone. But if you prefer to do those at release time, I'll stop bugging on each PR about them.)

I don't really have an opinion, I just keep forgetting. I'll add it here.

Signed-off-by: Jussi Kukkonen <[email protected]>
@woodruffw woodruffw merged commit 379054c into sigstore:main Jan 13, 2026
41 checks passed
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.

digest in bundle not checked during verify

2 participants