Respect git conventions when validating certs #2408
Conversation
Git supports the `http.sslVerify` option as well as the `GIT_SSL_NO_VERIFY` environment variable for disabling validation of SSL certificates. This was removed from libgit2 in libgit2/libgit2@41698f22 citing bad security practice and how applications now had enough information to decide for themselves. There seems to be enough corporate environments (as mentioned in rust-lang#1180) where certificate validation won't work that this seems prudent for Cargo to support. Closes rust-lang#1180
|
r? @wycats (rust_highfive has picked a reviewer for you, use r? to override) |
|
r? @brson |
|
vWhen testing the pull request on an untrusted https git source, it seems that nor the GIT_SSL_NO_VERIFY, nor the http.sslVerify git config work. I've never built Cargo before, though it may be a mistake of mine: Reproduce with: Set cargo PATH to the fresh build Make a fake Cargo.toml that refers an untrusted https git repository (actually it's not a rust repo, just for testing the git clone): Test cargo build that should attempt to fetch the git repo (should fail as no ssl verify bypass is set): Test again with gitconfig, unexpectedly fails on my side: Test also with GIT_SSL_NO_VERIFY=1, unexpectedly fails also on my side: Note that a bare git clone works in both cases and fails with no config: $ git config --global http.sslVerify false $ git config --global --unset-all http.sslVerify |
|
@guillon thanks for giving this a spin! The first time you used gitconfig, the option was spelled "http.sslVerfify", but was that just a copy-paste typo? Also, do you have an HTTP proxy set? Cargo will use libcurl to fetch git repositories when a proxy is set, but I'm not sure if the certificate callback is handled in that case in libgit2. |
|
Inded for the "sslVerfify" it was a typo. I rechecked with same conclusion (updated comment). |
|
Indeed I have a http proxy set. Probably means thus that libcurl2 will also have to honor this. I will recheck my use case on a machine without a proxy to complete the report. |
|
Ok, it's probably the case that git2-curl isn't respecting the certificate callback in that case. As a data point, I verified this manually to ensure that it worked as expected, and it does work without git2-curl in play |
|
Right. Thus for the HTTP proxy case, it's less obvious than expected...
I guess thus that for supporting curl insecure configuration the choices are:
|
|
For https protocol the libcurl option CURLOPT_SSL_VERIFYPEER is sufficient I guess, CURLOPT_SSL_VERIFYHOST is, I suppose, required only for the the ssh/scp protocols. My guess is that you'd better propose a cargo specific option and/or env. var. that deactivates the cert check callback in rust libgit and unset the CURLOPT_SSL_VERIFYPEER in rust libcurl instead of trying to parse .gitconfig and .curlrc. |
|
This is really sad. Its good that Rust takes the security thing seriously but not allowing this, even after legitimate use case, is disheartening. If the user is already specifying that he is not validating SSL for git, for example, why cant Cargo consider users to be adults and allow them to use it, albeit with a warning that this is a _very_ bad security practice. |
|
Ok, been talking with @wycats and we may not want to do this just yet, so closing. |
|
This PR was for disabling cert verification. What about the options for passing a local cert store to the underlying libraries? When you're in an environment where certificates are not installed in the default location on the system, there doesn't seem to be a way to get around this. You can't disable checking and you can't pass it the proper certs to verify against. (e.g. the git |
|
@apaprocki the problem might be that we're using libgit2 and curl which have different configurations, but if there's standard methods to configure where certificates are located then we should definitely respect that! Do you know if there's documentation about these options that we could follow? |
|
@alexcrichton |
|
|
|
Awesome, thanks! I'll add that to the issue. |
Git supports the
http.sslVerifyoption as well as theGIT_SSL_NO_VERIFYenvironment variable for disabling validation of SSL certificates. This was
removed from libgit2 in libgit2/libgit2@41698f22 citing bad security practice
and how applications now had enough information to decide for themselves.
There seems to be enough corporate environments (as mentioned in #1180) where
certificate validation won't work that this seems prudent for Cargo to support.
Closes #1180