-
Notifications
You must be signed in to change notification settings - Fork 53
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
Route away from experimental to 1.0 #274
Comments
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:
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
Rekor API
Overall APIDoes 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 FeaturesLog proofs:
|
Thanks for your feedback @vembacher! I'm in favor of stabilizing the API before moving forward with the 1.0 release.
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 sigstore/rekor#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! |
Will do as soon as I get around to it!
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. 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.
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.
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.
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. |
This makes sense to me -- I'll add it to our list of tasks. Edit: Updated the list above. |
FYI this is actually implemented in |
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 don't believe there's currently a firm timeline here, unfortunately -- we've recently merged some upstream work (sigstore/protobuf-specs#118) that'll unblock bundle signing and verification here, but I can't offer a firm timeline on when that'll get integrated here. |
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 |
Revisiting this:
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 #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 - #274 (comment) mentioned the use of the AWS TUF library, I don't think this is a blocker unless AWS is not actively supporting the library. |
To tack on another action item, we @trailofbits have done some work around autogenerating the Fulcio/Rekor API layer. It'd be nice to see this integrated, if only to deprecate the pile of hacks I had to add for Fulcio API v2. |
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
The text was updated successfully, but these errors were encountered: