rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.63k stars 2.4k forks source link

Cargo appears to leak SSL_CERT_FILE and SSL_CERT_DIR to subprocesses #3676

Open sfackler opened 7 years ago

sfackler commented 7 years ago

I have not personally confirmed that this is the case, but I'm betting it's the cause of https://github.com/sfackler/rust-openssl/issues/575

cc #2888

sfackler commented 7 years ago

Looks like its probably a dependency doing this (maybe libcurl?). Might be a bit awkward to deal with, but we could maybe snapshot the environment on startup and feed that to subprocesses?

sfackler commented 7 years ago

Oh right, it's openssl-probe.

alexcrichton commented 7 years ago

Yeah I'm ~100% sure openssl-probe would be doing this.

I had no idea this could lead to bugs...

@nathanaeljones for background on this there's a very long comment explaining what's going on, but the general gist is that we're shipping a statically linked OpenSSL so it's up to Cargo to find ssl certs for a system (normally this is configured by a distro). In doing so the only way we've found at least so far is to initialize through env vars, which is then causing this to leak into child processes.

jethrogb commented 7 years ago

Got bit by this in #4002

jethrogb commented 7 years ago

Instead of overriding the environment, cargo should use X509_STORE_load_locations instead of X509_STORE_set_default_paths on every X509 store to set the locations found using openssl-probe.

sfackler commented 7 years ago

@jethrogb this is all mediated through curl which AFAIK doesn't expose that.

jethrogb commented 7 years ago

Sure it does, you just need to set CURLOPT_CAINFO and CURLOPT_CAPATH.

alexcrichton commented 7 years ago

PRs are of course always welcome to patch this up! This isn't intentional, it's just a side effect of how things are implemented today.

sfackler commented 7 years ago

Those environment variables will still affect anything downstream using curl.