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

Allow using rustls instead of OpenSSL #88

Open
Shnatsel opened this issue Nov 3, 2022 · 13 comments · Fixed by #129
Open

Allow using rustls instead of OpenSSL #88

Shnatsel opened this issue Nov 3, 2022 · 13 comments · Fixed by #129
Labels

Comments

@Shnatsel
Copy link

Shnatsel commented Nov 3, 2022

In cargo audit we're looking to get rid of OpenSSL because of its security track record. So we need crates-index to allow using rustls instead of OpenSSL.

There is a number of way to accomplish this:

  1. Allow using the out-of-tree hyper transport for libgit2: https://github.com/henry40408/git2-hyper
  2. Switch from libgit2 to gitoxide: Replace git2 with gitoxide #82
  3. Sparse registry support #87 (still being tested in Cargo)

Which option do you prefer, as the maintainer?

@kornelski
Copy link
Collaborator

My preferred solution would be 2.

The git2-hyper solution seems technically reasonable, but the crate seems very young and already has an out-of-date git2 dependency, so this would need a more mature, actively-maintained implementation.

I don't mind adding a sparse registry support separately, but this only covers fetch of individual crates by name, and not listing them all and (currently WIP) learning about newly-published crates.

@Shnatsel
Copy link
Author

Shnatsel commented Nov 4, 2022

Is migrating to gitoxide and completely dropping support for libgit2 acceptable?

It's technically possible to support both backends, but would create additional complexity and maintenance burden, so I'd prefer to avoid that.

@kornelski
Copy link
Collaborator

Yes

@BlackHoleFox
Copy link

Gonna give this a try and see if git-repository can fill in everything git2 was doing.

@Shnatsel
Copy link
Author

The month-old progress update for gitoxide details the ongoing integration into Cargo and the fixes that resulted from it. Those progress updates are intentionally delayed and the actual status is even better now.

@BlackHoleFox did you make any progress on migrating this crate to gitoxide? Do you think you could get it over the finish line in the next month or so?

@BlackHoleFox
Copy link

@Shnatsel Yeah, i'll pick this up again this week, sorry. I got thrown off by the amount of boilerplate you need to wrap gitoxide in to do common stuff (but maybe I'm going about it wrong, we'll see :) ) and then got distracted elsewhere.

@Shnatsel
Copy link
Author

Alright! Perhaps you could check with Byron, the author of gitoxide, if you're on the right track and using the right APIs. Or perhaps this is something that could be improved in gitoxide!

@Shnatsel
Copy link
Author

The integration into Cargo might also be useful as a reference point: rust-lang/cargo#11448

@Byron
Copy link
Collaborator

Byron commented Jan 24, 2023

Alright! Perhaps you could check with Byron, the author of gitoxide, if you're on the right track and using the right APIs. Or perhaps this is something that could be improved in gitoxide!

Definitely! A cumbersome API is something to improve, especially if git2 allows the same in an easier fashion. It's also advisable to track the latest git-repository releases as they typically come with improvements here and there.

@obi1kenobi
Copy link
Contributor

Just a little +1 for this idea from the maintainers of the cargo-semver-checks crate. We've been dealing with obnoxious OpenSSL-related errors both when vendoring it in (e.g. obi1kenobi/cargo-semver-checks#272) and when relying on it being dynamically-linked (the installed OpenSSL isn't always found, different systems require different solutions, and the whole thing just doesn't scale from an end user support perspective).

If/when gitoxide and rustls are able to be used instead of git2 and OpenSSL, we'll probably switch to them quite quickly.

@Byron Byron mentioned this issue Jul 18, 2023
9 tasks
@Byron
Copy link
Collaborator

Byron commented Aug 1, 2023

Apologies, this issue was auto-closed which wasn't actually my intention nor my doing even though GitHub claims it was 😅. Could you recheck to see if the issue is fixed with the latest version 2.0 or above? Thank you.

@Byron Byron reopened this Aug 1, 2023
@Byron Byron added the question label Aug 1, 2023
@complexspaces
Copy link

I don't believe that this is fully addressed yet. The new gix integration-based implementation is a start but due to this line unconditionally selecting the "default" curl backend will pull in OpenSSL and build it.

I think another feature would need to be added which exposes the http-client-reqwest-rust-tls flag in gix-transport in order to remove the OpenSSL dependency. http-client-curl-rust-tls might work as well but I imagine people looking for a "pure Rust" implementation don't want curl either, but I could be wrong!

@Byron
Copy link
Collaborator

Byron commented Feb 21, 2025

It's true - to make this work one would have to prevent ending up with the curl feature in the first place, so the respective cargo features would have to be exposed in this crate to give a choice. A PR would definitely be welcome.

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

Successfully merging a pull request may close this issue.

6 participants