-
Notifications
You must be signed in to change notification settings - Fork 559
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
add sha256:...
as alternative way to specify a blob
#4016
base: main
Are you sure you want to change the base?
Conversation
@woodruffw, @kommendorkapten and I had a discussion in sigstore/protobuf-specs#444. There was some concern that providing only the hash of an attestation would lead to a lack of verification of the document itself. |
Sure, but in our case the server application already checks the hash and we don't want to send the file around to the microservice to validate the signature :) Also sigstore-python already supports this. But also fine with me if it isn't accepted! |
Hey @wolfv! Good to see you 🙂 Per sigstore/protobuf-specs#444, I think there are two subtly different cases here: when doing a "bare" verification ( However, in the attestation verification context, just a hash is insufficient (since it binds to the subject's digest, but not the subject's name. So, I think (In |
I agree with William. This PR is for the attestation case, but if this were for the "bare" case, that can be accepted. With that said, I'm realizing that Cosign's attestation verification here is incomplete, missing subject name verification. I'd be open to a change that accepts a digest and a subject name, with the accompanying changes for the latter. |
Just to be clear, the current implementation doesn't check the file name -- it requires a file on disk and just computes its digest. This PR just adds the ability to specify a digest as an input instead of a file. We can discuss validating that the subject name matches, but that is orthogonal to this PR. |
@haydentherapper @woodruffw I'm curious how you'd respond to #4019? It's a very related ask with regard to passing a subject digest as an input for attestation verification, but in the context of verifying git commits (not files on disk) using cosign. I think it's a reasonable ask and I'm not quite sure I understand the argument against providing digests as input for attestation verification. |
Yeah after diving deeper into the whole sigstore / in-toto validation topic I understand that you would want to validate the in-toto statement subject.name against the file name of the artifact that we're verifying. I guess that makes a lot of sense, but this PR seemed to work without providing a subject or alternative filename so I do not think that's currently implemented (as @codysoyland mentions). We'll do these kind of validations on our end either way but yeah, might be nice to (also) add a |
Note that subject.name is not a required field in statements, so if the proposal is to check that value against something it won't work for a number of use cases. |
I guess the basic argument is that it's confusing 🙂 -- it makes sense to allow "bare" verifications to be verified against digests because the signed "statement" (i.e. blob) is fully enclosed by the digest. In contrast, it's not clear what the user is intending when they pass a digest during attestation verification: they might be communicating that they don't care about the subject, or that they don't know the subject, or even an unknown unknown (they don't know the subject even exists as a verifiable piece of state). This could be fixed with UX work, e.g. requiring users to pass something like Alternatively, we could firm up the client spec to make it clear where the division of responsibilities between Sigstore clients and downstreams lies: I would be OK with Sigstore adopting the opinion that it only cares about digests, and that it's the responsibility of downstreams to adequately verify the remainder of the subject/statement modulo their own policies! However, I think we'd need to make that very explicit, because on its own an attestation verification conveys almost none of the "trust" that a normal signal verification does (since the bundle is essentially verifying its own contents). |
Interesting, I hadn't considered that the user's intent might be unclear if we offer a flag to specify a digest. The I'm actually more receptive to the notion of disallowing digest as inputs in "raw" message signature verification (as opposed to attestations)... In sigstore/protobuf-specs#444, @kommendorkapten brought up three scenarios where providing a digest may be considered harmful, and two applied to message signatures: poor crypto library support for verifying signatures with only a message digest, and some algorithms (i.e. ed25519) incapable of verification with just a digest. The third argument was related to api security and could possible apply to DSSE, and I admit I don't understand the threat model aside from some potentially confusing UX like you pointed out.
Hard agree! Sigstore is mainly concerned with tying an identity to a claim/subject, and it seems sensible for the verifier to open the attestation and confirm the subject is present, but the breakdown in responsibilities is definitely unclear to the average user. This is sort of covered in the client spec:
|
I think I can't add much content to the discussion here, most of pitfalls have been mentioned with passing in digests. And as @codysoyland mentions the client spec mentions that
So i think the spec is ok, it's more about getting the cli flags in order. But another thing briefly mentioned here, and something I took up in sigstore/protobuf-specs#444 is the effect this would have in the future for PQC. The current standardized algorithms follows Ed25519 in that they require the complete message. There are pre-hashed versions defined too, but will they be common/supported? My understanding now is that Ed25519ph is not very common/supported, and exposing an API that will break for the common scenarios is not that good (note that his will be a primary concerns for blobs, not in-toto/dsse as the dsse envelope is the message being signed and verified). |
@wolfv I spoke with @kommendorkapten this morning, and we're in agreement that we could accept this PR with some caveats/changes. We want to make sure that the UI is clear and explicit about what is checked when you enter a digest. I would recommend:
|
Agreed with @codysoyland on the suggested changes. You'll also need to update cosign/pkg/cosign/verifiers.go Lines 79 to 87 in accc80a
sha256 .
TIL!
Let's discuss more outside of this PR, but the short answer is that for now, I'd say it's on the verifier to know if providing only a digest is possible for the given algorithm. This is meant to be an optimization, and the common use case is still to provide the blob. |
Hey, looking for some early feedback on this change to make validation of blobs without providing the actual blob easier.
Summary
With this change, one can validate blobs without the blob - just by
sha256:<hash>
.Release Note
sha256:<hash>
.