Skip to content
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

Support for DSSE signatures over binary payloads #982

Open
laurentsimon opened this issue Apr 24, 2024 · 19 comments
Open

Support for DSSE signatures over binary payloads #982

laurentsimon opened this issue Apr 24, 2024 · 19 comments
Labels
component:api Public APIs component:signing Core signing functionality enhancement New feature or request
Milestone

Comments

@laurentsimon
Copy link
Contributor

The current API supports only intoto statement, and we'd like support for arbitrary DSSE blobs.

We discussed this in the past but the original DSSE issue has been closed, so creating this issue for tracking.

@laurentsimon laurentsimon added the enhancement New feature or request label Apr 24, 2024
@woodruffw woodruffw changed the title Support for DSSE Support for DSSE signatures over binary payloads Apr 24, 2024
@woodruffw
Copy link
Member

Thanks for filing @laurentsimon! I tweaked the issue title slightly so that I don't forget that this is about arbitrary blobs wrapped in a DSSE envelope 🙂

@woodruffw woodruffw added component:api Public APIs component:signing Core signing functionality labels Apr 24, 2024
@laurentsimon
Copy link
Contributor Author

Is there anything I can do to help with this feature?

@woodruffw
Copy link
Member

If you want to take a stab at implementing it, that would be great!

Otherwise, I don't have any immediate bandwidth to try my hand at it 😅

@laurentsimon
Copy link
Contributor Author

laurentsimon commented May 10, 2024

Would the change consist in updating https://github.com/sigstore/sigstore-python/blob/main/sigstore/sign.py#L192 by allowing input_: dsse.Statement | bytes, dsse_type: str (where bytes would be the payload and dsse_type is the type of the payload) + update https://github.com/sigstore/sigstore-python/blob/main/sigstore/dsse.py#L207 accordingly?

For verification, we'd add an optional dsse_type as argument to _verify.

Or do you have another idea?

@woodruffw
Copy link
Member

woodruffw commented May 13, 2024

Would the change consist in updating https://github.com/sigstore/sigstore-python/blob/main/sigstore/sign.py#L192 by allowing input_: dsse.Statement | bytes, dsse_type: str (where bytes would be the payload and dsse_type is the type of the payload) + update https://github.com/sigstore/sigstore-python/blob/main/sigstore/dsse.py#L207 accordingly?

That sounds pretty close to what I was thinking, although I have a (minor) concern that exposing the DSSE payload type like this will be misuse prone (e.g. users marking JSON as binary or vice versa). Do you have a concrete need for a payload type other than application/octet-stream or similar?

(Or as a related question: is there a fixed DSSE payload type for binary data already that we should use? I couldn't find one with some searching, but I might have missed it 😅)

For verification, we'd add an optional dsse_type as argument to _verify.

IIUC, this shouldn't be necessary: _verify takes an Envelope, which will have the payload_type you specify.


To take a step back a second: I realized that I'm not actually sure the rest of the Sigstore ecosystem even supports non-JSON DSSE bundles yet.

CC @haydentherapper: do you know if Rekor would accept these? I seem to recall there being a requirement that DSSE entries contain in-toto statements within Rekor itself.

Edit: this is the check I was thinking of: https://github.com/sigstore/rekor/blob/92cf4d5d3682ffcbf4138341f1bc5f7a66766549/pkg/types/dsse/v0.0.1/entry.go#L150-L156

@laurentsimon
Copy link
Contributor Author

laurentsimon commented May 13, 2024

application/octet-stream

Our use case is model signing for multiple files in folder, so would be something like application/vnd.ai-model-manifest+json or maybe more generic application/vnd.path-manifest+json @mihaimaruseac to comment

@mihaimaruseac
Copy link

Oh, I haven't thought at all about this :(. Yes, I think something like application/...+json is what we'd need to have, but I'll have to think a little bit more about this

@haydentherapper
Copy link
Contributor

Yep, json is all that Rekor supports currently, though we could support more than that. It means that other clients have to understand how to canonicalize non-JSON DSSEs, which I'm not sure if any would currently.

@laurentsimon
Copy link
Contributor Author

laurentsimon commented May 13, 2024

Thanks all. For model signing, it will be json format.

CC @haydentherapper: do you know if Rekor would accept these? I seem to recall there being a requirement that DSSE entries contain in-toto statements within Rekor itself.

@haydentherapper can you confirm non-intoto DSSE payloads work with rekor?

@haydentherapper
Copy link
Contributor

No, they don't currently. Rekor only understands intoto payloads - https://github.com/sigstore/rekor/blob/main/pkg/types/dsse/v0.0.1/entry.go#L113 and https://github.com/sigstore/rekor/blob/main/pkg/types/dsse/v0.0.1/entry.go#L159

@woodruffw woodruffw added this to the 3.1 milestone May 14, 2024
@laurentsimon
Copy link
Contributor Author

We had an offline chat with @haydentherapper and @mihaimaruseac to brainstorm. What came out is that Sigstore would like to not be in the business of litigating addition of new hashes that users may want to use. This mean (Hayden, Mihai please correct me if wrong) that #1018 can be closed in favor of this issue?

@haydentherapper mentioned that hashrekord (already supported in this library) could be a way forward, and we'd update https://github.com/sigstore/rekor/blob/92cf4d5d3682ffcbf4138341f1bc5f7a66766549/pkg/types/dsse/v0.0.1/entry.go#L150-L156 to accept a new DSSE payload type.

@laurentsimon
Copy link
Contributor Author

/cc @susperius

@laurentsimon
Copy link
Contributor Author

@haydentherapper can you explain why rekor needs to be aware of the hash type used in intoto subject? Why is the hash type used for DSSE signing not enough? I chatted with @susperius and this question came up.
Thanks

@haydentherapper
Copy link
Contributor

(catching up from being out) It's part of the canonicalized structure. The service computes the payload hash based on the provided payload - https://github.com/sigstore/rekor/blob/main/pkg/types/dsse/v0.0.1/entry.go#L247-L251

@susperius
Copy link

Sorry if I've missed the point.

So the service computes the hash based on the provided payload that's fine but does not explain the need to restrict the in-toto Subject's DigestSet to the predefined hashes. AIU rekor only sees the already encoded payload so the digest inside of the subject can be anything and wouldn't have an adverse effect on rekor.

PLMK what I'm missing here

@woodruffw
Copy link
Member

I think Rekor's restriction might be better suited for discussion on their issue tracker, but for the sigstore-python side: my justification for restricting the DigestSet to a fixed set of (well-known, strong) digests is that a signature is only as strong as the image it's been bound to. So, for sigstore-python's part, restricting the digests to things that are at least as strong as SHA2/256 is a way to ensure that users don't ascribe strength to a signature over a weaker hash function.

(That being said, I think the hash function discussion is unrelated to the top-level issue here, which is DSSE over a binary payload, rather than an in-toto statement. DigestSet is only relevant for the latter I believe, not the former.)

@haydentherapper
Copy link
Contributor

For Rekor, I don't believe we have restrictions on the DigestSet. I've been looking over the code and don't see where this restriction is present. Do you have an error message from calling the API with an intoto statement with non-sha256 digests?

@susperius
Copy link

It was just something that came up while talking to @laurentsimon about using in-toto attestation for model signing. The imposed checks on the sigstore side make it difficult to add "custom" hashing information (chunked sha256 for example) as the digest.
Overall, that is the reason for DSSE signatures over binary payloads. Perhaps we could add functionality to disable the digest check?

@laurentsimon
Copy link
Contributor Author

Was discussed in #1018

@woodruffw woodruffw added this to the 3.4 milestone Sep 18, 2024
@woodruffw woodruffw modified the milestones: 3.4, 3.6 Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api Public APIs component:signing Core signing functionality enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants