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

Rustls transport? #623

Open
jsha opened this issue Sep 20, 2020 · 16 comments
Open

Rustls transport? #623

jsha opened this issue Sep 20, 2020 · 16 comments

Comments

@jsha
Copy link

jsha commented Sep 20, 2020

Hi,

I've been looking at the security of the cargo / crates.io ecosystem, and I've noticed that it depends heavily on the security of TLS. The index of crates is downloaded over HTTPS from GitHub, and serves as the root of trust for subsequent crate downloads, because it contains the sha256 hashes of the crates.

Given that, I think it makes sense to use the strongest TLS implementation. Fortunately, rustls is a very high quality implementation, and is in Rust! Besides the memory safety benefits, it has a couple of other benefits: It does not implement older TLS versions (1.0 and 1.1), which have some vulnerabilities. Similarly it does not implement weaker ciphersuites. Also, it ships by default with webpki-certs, which is beneficial because it tends to be more up-to-date than the native root store on Debian-derived operating systems. When there's a CA that's been removed from the Mozilla root program (and others), but not removed yet from a root store, that's a big risk. In that situation there aren't (necessarily) any audits or controls happening any longer to ensure the CA is doing the right thing.

So I was thinking it would be useful to implement a git transport for HTTPS that uses Rustls. Probably the most straightforward way to do that would be to incorporate a Rust HTTP client. I see git2-rs already has a small HTTP client in git2-curl, but that it's considered less battle-tested (and has an issue with pulling a whole repository into memory), so isn't used as the default in cargo, but is only used when there are special HTTP setting, like proxies or custom root stores.

Is that something you think would be useful? Since my goal would be for cargo to (eventually) adopt this as the default instead of libgit's built-in HTTP transport, what quality and testing bar would it need to meet?

Thanks,
Jacob

@alexcrichton
Copy link
Member

Seems reasonable to me! There's the git2-curl crate which implements all HTTP crate fetches through the curl crate, so presumably there could be a git2-hyper crate (or something like that) which disables all of libgit2's internal networking and provides it via hyper+rustls?

@gilescope
Copy link

gilescope commented Oct 19, 2020

Being able to use a pure rust ssl makes using a lot of rust tools a lot easier where finding openssl or building ssl from source is tricky. Building cargo, cargo-audit, cargo-local-registry etc. It would make a big chunk of the ecosystem easier for people to use.

@weihanglo
Copy link
Member

Hi. I am wondering about how to integrate an async HTTP backend like hyper into git2. And it seems that spawning threads for hyper in the Read/Write trait is the most feasible solution which I can come up with. If there is other possible approach, please share it here. I am happy to do some experiments.

@kim
Copy link
Contributor

kim commented Mar 2, 2021

@weihanglo We’re doing something like this. You can also pass around an executor.

@henry40408
Copy link

Hi, I just implemented a HTTP back-end and send a pull request.

@brooksmtownsend
Copy link

Checking back in on this, is there still a desire to create a feature that will let users of git2 enable rustls instead of openssl?

@jsha
Copy link
Author

jsha commented May 26, 2022

Yes, I'm still very interested in using git2-rs with a rustls backend. I wrote a couple of starts at this in #624 and #625. What I found was that what's needed is not just a TLS transport but also an HTTP implementation. I think it should be quite possible to dust off one of those PRs and proceed either with the ureq HTTP client or with a hyper HTTP client (or both).

@Shifulor
Copy link

Shifulor commented Jun 5, 2022

Just wanted to add a heavy interest in a rustls usage because openssl crate does not support libressl or modern openssl, while rustls does. Openssl-vendored kinda bloats the binaries so i would greatly appreciate a rustls version

@gilescope
Copy link

Two years on and at a different place and I find things still failing to install due to openssl dependencies. Pure rust might be more secure by now as well as work first time. It would help rust's roll out if we could ween ourselves off openssl.

@jsha
Copy link
Author

jsha commented Sep 19, 2022

@gilescope @Shifulor have you tried @henry40408's git2-hyper crate? https://github.com/henry40408/git2-hyper

@Shifulor
Copy link

I Did not, it seems like a nice project tho. Sadly it does not help in my specific case, because it would require remodeling a lot of code. Also his example puts the code into unsafe brackets, which honestly worries me (No idea if this crate maybe also does so don't pin me on that 😰 )

@Shnatsel
Copy link
Member

Shnatsel commented Nov 3, 2022

I'm about to port crates-index and cargo-audit from git2 to gitoxide specifically because gitoxide supports rustls.

I'd be very happy to see this issue fixed, and save myself and the ecosystem the migration.

@orhun
Copy link

orhun commented Aug 18, 2023

I would like to express my interest as well. It would be absolutely great to have rustls support in git2!

@extrawurst
Copy link
Contributor

Would also like to see this for gitui

@SIGSTACKFAULT
Copy link

also means you don't have to scrounge around to find exactly what the openssl dev package is named in your distro.

@KisaragiEffective
Copy link

How about depend on reqwest and its support of rustls?
I think it has advantages over past PRs:

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

No branches or pull requests