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

Making fulcio and rekor clients independent of reqwest #176

Open
lulf opened this issue Dec 9, 2022 · 5 comments
Open

Making fulcio and rekor clients independent of reqwest #176

lulf opened this issue Dec 9, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@lulf
Copy link
Contributor

lulf commented Dec 9, 2022

I'm interested in package signing support in crates.io + cargo using Sigstore. After looking through some of the earlier discussions from @lukehinds and others, it seems to me that the main obstacle is that crates.io and cargo teams have little time to work on this and/or review. In order to push things forward, I'm exploring an approach that could be used as basis for an RFC, with little changes to the existing crates.io and cargo code bases to reduce friction in adding the support and migrating existing infrastructure.

One of the things I've found is that cargo currently uses the curl (+ curl-sys) crate for HTTP, and does not support async Rust. I think it's going to be hard to justify changing a big chunk of cargo dependencies for this, so I'd like to hear others think.

Could it make sense to modify the fulcio and rekor clients, or alternatively create a set of clients with a lower level API, that could be re-used in places like cargo?

@lulf lulf added the enhancement New feature or request label Dec 9, 2022
@lukehinds
Copy link
Member

lukehinds commented Dec 9, 2022

hey @lulf , awesome that you're interested in this, much support from my side.

I think we could explore two options here, the first would be to enable non-async alternatives alongside the async funcs, some of the code already offers this. It can become a bit of a breach of the DRY principle, but we could at least establish to what extent.

pub fn auth_url(&self) -> Result<(Url, CoreClient, Nonce, PkceCodeVerifier)> {
let issuer = IssuerUrl::new(self.oidc_issuer.to_owned()).expect("Missing the OIDC_ISSUER.");
let provider_metadata =
CoreProviderMetadata::discover(&issuer, http_client).map_err(|err| {
error!("Error is: {:?}", err);
SigstoreError::ClaimsVerificationError
})?;
self.auth_url_internal(provider_metadata)
}
pub async fn auth_url_async(&self) -> Result<(Url, CoreClient, Nonce, PkceCodeVerifier)> {
let issuer = IssuerUrl::new(self.oidc_issuer.to_owned()).expect("Missing the OIDC_ISSUER.");
let provider_metadata = CoreProviderMetadata::discover_async(issuer, async_http_client)
.await
.map_err(|_| SigstoreError::ClaimsVerificationError)?;
self.auth_url_internal(provider_metadata)
}
}

I would prefer that I think, then having another set of clients, but I am malleable if it helps us get cargo / crates onboard.

in regards to the fulcio client, you don't need to use the one in here, its possible to do this directly against the sigstore-rs lower level apis and use whatever is your prepared http client. This should give you some ideas: https://github.com/lukehinds/ferris-sign

Maybe all this could be hashed out in a collaborative google-doc?

cc @flavio / @lkatalin / @Xynnn007

@tpletcher
Copy link

Hi Folks,
I discussed getting a Rust community RFC going with @bobmcwhirter earlier this week. I am interested in supporting/participating in getting the RFC moving so we could proceed to implementation as soon as is practical.

@flavio
Copy link
Member

flavio commented Dec 14, 2022

I'm interested in helping with that.

As for getting rid of the reqwest dependency, keep in mind that this is a transitive dependency of the following dependencies of sigstore-rs:

  • tough: used to fetch fulcio public key and rekor certificates from Sigstore's TUF repository. This crate uses reqwest in blocking mode. I know they had plans to move to reqwest async. I don't know if this is going to replace the current usage of blocking reqwest
  • oci-distribution: this is used to interact with OCI registries. I'm a maintainer over there, I can help make/drive changes
  • openid-connect: this is used to generate keyless signatures via Rust
  • oauth2: this is a transitive dependency of openid-connect

As you can see getting completely rid of reqwest is going to be quite an effort

@lulf
Copy link
Contributor Author

lulf commented Jan 2, 2023

Thanks for the feedback everyone, it was very useful for me to get a better picture of the current state of things.

@lukehinds The ferris-sign approach was what I had in mind, but I'm also fine if we think adding non-blocking variants to sigstore-rs would be useful (I think they would).

@tpletcher Great! It would be good to present some of these possible approaches in the RFC to learn what the community would accept in terms of additional dependencies for cargo for instance.

@flavio Agreed, that's a comprehensive list. The first thing that strikes me is if it would be possible to make the oci-distribution dependency enable through some feature flag? From what I understand, interaction with OCI should not be needed in this case? I'll be happy to draft up a PR for that I you think it makes sense in general.

To get the ball rolling, I'll start a conversation with the Cargo team on this topic to check if there are any (other) concerns.

For crates.io, my impression is that we can more easily reuse sigstore-rs as is, but I think even so it would be good to include that team in the conversation as well before the RFC.

@lulf
Copy link
Contributor Author

lulf commented Jan 4, 2023

The conversation created some interest (https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Cargo.20and.20signing.2Fverification ). Please add to that thread to signal your interest in the work since I don't have the full grasp of sigstore.

The next step I think is to create an RFC and post it to the developer forum for feedback from a wider audience. I've taken the official RFC template and started working on that here https://github.com/trustification/rust-rfcs/blob/sigstore-rfc/text/0000-sigstore-integration.md - but I'm open to move this anywhere that makes it easier to collaborate @tpletcher @flavio .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants