verifier: Ensure that bundle digest and real digest match#1652
Conversation
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 <jkukkonen@google.com>
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 <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
woodruffw
left a comment
There was a problem hiding this comment.
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.
|
(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.) |
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.
I don't really have an opinion, I just keep forgetting. I'll add it here. |
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
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