sigstore-rs
sigstore-rs copied to clipboard
Route away from experimental to 1.0
I would like us to consider what would it look like where we are no longer experimental and can cut a release a 1.0.
Aspects to consider...
Do we have API stability? Are we performing conformance tests? Are we aligned with the protobuf client spec?
cc the codeowners:
@flavio @arsa @viccuad @Xynnn007
cc'ing correct @asraa
+1 to integrating with the conformance suite - https://github.com/sigstore/sigstore-conformance#usage. Right now, there aren't many tests, but as we add more tests, this makes it easy to detect if a breaking change occurs for the client.
Also I would recommend producing and consuming bundles from https://github.com/sigstore/protobuf-specs. sigstore-python and sigstore-js should have examples of doing this.
Would it also make sense to add FFI so other languages can interface?
For the protobuf-specs: it looks like https://docs.rs/protobuf/latest/protobuf/ is the most mature protobuf implementation layer for Rust.
To summarize discussions in the community meeting and Slack, here's my understanding of the current blockers for a 1.0 release of the Rust client:
- [ ] Some degree of integration with the conformance testing suite
- [ ] Support for the
protobuf-specsformats, including the Sigstore bundle format - [ ] Inclusion proof verification, per https://github.com/sigstore/sigstore-rs/issues/274#issuecomment-1614939716
Are there other things I'm missing here? If not (and assuming it's okay with @lukehinds and the other maintainers here) I can begin assigning some people to work on these 🙂
I have used the library quite a bit and did some internal modifications over the last months, and I have some (opinionated) feedback. I think the API can still be improved before being stabilized. I found workaround for most of my issues, but I think others might be less willing to do so.
Cosign API
- I feel like some parts of the Cosign API are too "low level" and unclear about what is being verified.
trusted_signature_layers: I think this function could be removed from the public API, because it is somewhat confusing and it is not obvious what it does.verify_constraints: I think this function could be moved into the theCosignCapabilitiestrait and replacetrusted_signature_layersas part of the public API. The parameters could be a mixture of both. The name could be changed to something similar to theverify_blob*methods.- Is it the user's/OCI client's task to verify that image data and signature are consistent? The way I understand it, I can only assume the actual image data is authentic, if I'm fetching it with a reference that contains the digest and the OCI client ensures image data is consistent with the manifest.
- Blob verification
- I think the
verify_blob*methods could use a variant that supports Fulcio based certificates + verification constraints. - Does it make sense to have
verify_blob*methods for both bundle and raw signatures? - Does it make to use an enum for the 'verification key source' for public key, certs and Fulcio, instead of having functions for each?
- I think the
Rekor API
- I think adding a
RekorClientcould be useful to store the Rekor configuration, and as a wrapper for the API calls. This would not require a breaking change. - I think the API module structure could be flattened because the Rekor API isn't too big anyways. A
RekorClientcould be used to address this. - A lot of the Rekor data is encoded in some way during in the JSON models. In my opinion this is kind of annoying to work with. It is possible to use the
serde_withcrate to automatically decode a lot of these. E.g. decoding B64 signatures directly into aVec<u8>. - In general, a lot of the structs can be modified to provide a better DX by using more serde features related to enum tagging an such. Though I'm not sure if this is desired because it would require some changes and be quite a change from the generated API bindings. Also I'm not sure if this is compatible with the protobuf specifications, as I'm not familiar with them.
Overall API
Does it make sense to split the crate into several subcrates? The way I understand it, this could improve development compile times, because it avoids having to recompile all the serde models for each minor. Though I'm unsure about this.
Future Features
Log proofs:
- The way I understand it for now there's no support for inclusion proof, consistency proof and checkpoint/STH verification. Will these be added? I think adding them might be a breaking change depending on where they are added.
- Thought, I'm not sure if running an inclusion proof in addition to verifying the SET would give a security advantage, as there is no gossiping.
- Note: I have already implemented these for myself, and plan to create a PR for this.
Thanks for your feedback @vembacher!
I'm in favor of stabilizing the API before moving forward with the 1.0 release.
- Cosign changes: please, file some issues describing the changes to the API that you described. I'm in favor of them, but I need to see how that would impact the code I know about that is already using them
- Rekor changes: I haven't been too much involved with this part of the code. I have the impression that maintaining the models is time expensive, see this comment and this issue
As for splitting the crate into subcrates, I'm not 100% sure about that. I think the changes above have a higher priority
@vembacher I would like to see inclusion proof verification be a requirement of clients. The bundle format defines this as optional currently, but I've proposed making this a requirement in https://github.com/sigstore/rekor/issues/1566.
For consistency proofs and gossiping, this is not something that has been implemented in any Sigstore client yet. We could implement consistency proof verification in clients, but this requires clients to keep state (the previous checkpoints) (Note that I'm not opposed to implementing this as an optional feature for those who do want to, but I don't think it should be a requirement).
If we instead rely on a network of witnesses that compute consistency proofs, we get two benefits: Trusted witnesses verify consistency and handle state, and witnesses can distribute co-signed checkpoints to prevent split-view attacks. Witnesses would push co-signed checkpoints to a set of "distributors" - This is similar to gossiping, except witnesses do not gossip with one another, which makes witnesses far more lightweight. The operational burden is just on distributors, which Sigstore and community members can operate. See https://github.com/transparency-dev/witness/tree/main/cmd/omniwitness for an example.
The end goal is a strong offline proof of inclusion that bundles co-signed checkpoints. For now, for a 1.0 for any client, I'd like to just see inclusion proof verification implemented.
This has been my area of focus, happy to chat more over in a Rekor discussion/issue or Slack if you'd like!
@flavio
Cosign changes: please, file some issues describing the changes to the API that you described. I'm in favor of them, but I need to see how that would impact the code I know about that is already using them
Will do as soon as I get around to it!
Rekor changes: I haven't been too much involved with this part of the code. I have the impression that maintaining the models is time expensive, see this comment and this issue
I agree, it definitely would increase required maintenance effort. I have already tried a lot of the changes for internal development, and if I remember correctly It took me a few hours to modify the models. However, in my opinion it would be worth the reduced effort on the user side, but I can see why you might want to avoid this.
Maybe this can be avoided with the protobuf specifications? Do they require Hex/B64 (de/en)coding? The way I understand it this is mostly necessary because of JSON compatibility issues.
@haydentherapper
@vembacher I would like to see inclusion proof verification be a requirement of clients. The bundle format defines this as optional currently, but I've proposed making this a requirement in sigstore/rekor#1566.
I will look into this proposal, but if there is an ongoing effort to require inclusion proofs it definitely makes a lot of sense to contribute my implementation.
For consistency proofs and gossiping, this is not something that has been implemented in any Sigstore client yet. We could implement consistency proof verification in clients, but this requires clients to keep state (the previous checkpoints) (Note that I'm not opposed to implementing this as an optional feature for those who do want to, but I don't think it should be a requirement).
If we instead rely on a network of witnesses that compute consistency proofs, we get two benefits: Trusted witnesses verify consistency and handle state, and witnesses can distribute co-signed checkpoints to prevent split-view attacks. Witnesses would push co-signed checkpoints to a set of "distributors" - This is similar to gossiping, except witnesses do not gossip with one another, which makes witnesses far more lightweight. The operational burden is just on distributors, which Sigstore and community members can operate. See https://github.com/transparency-dev/witness/tree/main/cmd/omniwitness for an example.
The end goal is a strong offline proof of inclusion that bundles co-signed checkpoints. For now, for a 1.0 for any client, I'd like to just see inclusion proof verification implemented.
I will need to look more into this concept. I think this is somewhat similar to what I implemented, but I need to check this in more-depth as soon as I have the time to do so.
My approach used what I call "federated gossiping", basically monitors can operate a gossiping endpoint to which other monitors and auditors can send checkpoints bundled with inclusion and consistency proofs. On the active gossiping side you can choose the monitors you want to gossip with. Effectively building a network that propagates all checkpoints to all federated monitors. The protocol itself however is only a proof-of-concept for now, but I can definitely at least try to contribute the consistency proof implementation.
But I think my use case is slightly different to the "default" one, because I focused on Operational Technology use cases. So scenarios where logs are operated by the publisher and potentially private needed to be covered. So there's a much higher risk of split-tree-view attacks compared to using the public instance.
This has been my area of focus, happy to chat more over in a Rekor discussion/issue or Slack if you'd like!
I'd be glad to! Should I just ping you on the Sigstore Slack as soon I get around to it?
The design of witnesses and distributors differs in that witnesses are not addressable (distributors are) and there's not a O(N^2) fanout requiring every witnesses to gossip with other witnesses, which has been one of the major blockers for adoption of gossiping in CT.
Feel free to ping me on the Sigstore slack!
To summarize, for sigstore-rs 1.0, I'd propose inclusion proof verification be a requirement as per the about-to-be-updated bundle spec.
To summarize, for sigstore-rs 1.0, I'd propose inclusion proof verification be a requirement as per the about-to-be-updated bundle spec.
This makes sense to me -- I'll add it to our list of tasks.
Edit: Updated the list above.
For consistency proofs and gossiping, this is not something that has been implemented in any Sigstore client yet. We could implement consistency proof verification in clients, but this requires clients to keep state (the previous checkpoints) (Note that I'm not opposed to implementing this as an optional feature for those who do want to, but I don't think it should be a requirement).
FYI this is actually implemented in rekor-cli
Would https://github.com/theupdateframework/rust-tuf be usable for the TUF integration rather than https://github.com/sigstore/sigstore-rs/tree/main/src/tuf?
I was just curious if there's an estimated timeline for this? It's something I'm interested in using in production in the somewhat near future. Specifically looking to verify a signature bundle, including the Rekor inclusion.
I was just curious if there's an estimated timeline for this? It's something I'm interested in using in production in the somewhat near future. Specifically looking to verify a signature bundle, including the Rekor inclusion.
I don't believe there's currently a firm timeline here, unfortunately -- we've recently merged some upstream work (https://github.com/sigstore/protobuf-specs/pull/118) that'll unblock bundle signing and verification here, but I can't offer a firm timeline on when that'll get integrated here.
Would https://github.com/theupdateframework/rust-tuf be usable for the TUF integration rather than https://github.com/sigstore/sigstore-rs/tree/main/src/tuf?
This crate didn't exist when I added the TUF integration. The code is currently using https://github.com/awslabs/tough - which is a comparable client written and maintained by AWS (they use that as part of bottlerocket and maybe other production code).
I don't see any particular advantage in moving from the tough crate to rust-tuf
Revisiting this:
To summarize discussions in the community meeting and Slack, here's my understanding of the current blockers for a 1.0 release of the Rust client:
- [ ] Some degree of integration with the conformance testing suite
- [ ] Support for the
protobuf-specsformats, including the Sigstore bundle format- [ ] Inclusion proof verification, per Route away from experimental to 1.0 #274 (comment)
Are there other things I'm missing here? If not (and assuming it's okay with @lukehinds and the other maintainers here) I can begin assigning some people to work on these 🙂
We've made progress towards (1) and (2) in #310, now #311, and the small soon-to-be follow-up #315. These are the features we're tracking for a 1.0 from Trail of Bits' side.
Is there anything else being tracked as a requisite feature for cutting a 1.0 release?
I think it would be nice to have https://github.com/sigstore/sigstore-rs/issues/307 done.
Is there DSSE support? At a quick glance, I see references to the intoto type. Ideally we would migrate to uploading DSSE rekor entries.
@jleightcap - https://github.com/sigstore/sigstore-rs/issues/274#issuecomment-1698701976 mentioned the use of the AWS TUF library, I don't think this is a blocker unless AWS is not actively supporting the library.