cargo test leaves file descriptors to /dev/{u,}random open when exec'ing a binary on Linux

[alecmocatta]

Problem
cargo test leaves file descriptors to /dev/{u,}random open when execing a binary on Linux.

Steps
See @alexcrichton's comment below.

Notes

cargo 1.32.0-nightly (241fac0e3 2018-11-09)

[lukaslueg]

Tested here; on nightly, file descriptors are open for /dev/random and target/debug/deps/main-... when main() executes under --test.

[alexcrichton]

Thanks for the report!

I can reproduce what @lukaslueg is seeing with this main program:

fn main() {
    std::process::Command::new("ls")
        .arg("-lah")
        .arg("/proc/self/fd")
        .status()
        .unwrap();
}

which for me reports:

$ cargo run
total 0                                                   
dr-x------ 2 alex alex  0 Nov 19 15:23 .                  
dr-xr-xr-x 9 alex alex  0 Nov 19 15:23 ..                 
lrwx------ 1 alex alex 64 Nov 19 15:23 0 -> /dev/pts/1    
lrwx------ 1 alex alex 64 Nov 19 15:23 1 -> /dev/pts/1    
lrwx------ 1 alex alex 64 Nov 19 15:23 2 -> /dev/pts/1    
lr-x------ 1 alex alex 64 Nov 19 15:23 3 -> /dev/urandom  
lr-x------ 1 alex alex 64 Nov 19 15:23 4 -> /dev/random   
lr-x------ 1 alex alex 64 Nov 19 15:23 5 -> /proc/18694/fd

It looks like both /dev/random and /dev/urandom are opened up by OpenSSL, and this is presumably regressing because we updated to OpenSSL 1.1.0 recently which must be doing something different.

cc @sfackler, is this something you've run into before with OpenSSL?

[alecmocatta]

@lukaslueg @alexcrichton Apologies, I mixed up the issue which I'd seen for ~4 months which affects Mac, with some observations that in hindsight were actually the /dev/{u,}random issue you've identified on Linux. I updated this issue to reflect the /dev/{u,}random issue, and I've re-posted the original issue here with a link to the code and travis log that prompted this.

[lukaslueg]

sfackler/rust-openssl#1020 should allow us to tell OpenSSL to close /dev/{u,}random and keep them closed until further need.

Yet I'm unsure: AFAICS openssl is currently purely a transient dependency. Turning the introduced knob would require promoting it to a hard dependency, so there is actually a knob to turn.

[alexcrichton]

Interesting! @lukaslueg would you be interested in adding that to Cargo to see if we can solve this?

[lukaslueg]

Can do. Are we Ok with adding openssl as a hard dependency?

[alexcrichton]

We can't pick up the dependency on platforms that don't already use it, but on platforms that already use it thisd be good to fix!

[lukaslueg]

@alexcrichton maybe I'm missing a simple point but where is the optional openssl-feature actually activated? I can't find it being activated in rust-lang/rust's bootstrapping. The point being that we need a #[cfg(feature="openssl")]

[alexcrichton]

@lukaslueg it's probably sufficient to gate it on the vendored-openssl feature for now in Cargo perhaps?

[lukaslueg]

It seems this will require further investigation: openssl was recently bumped, exposing RAND_keep_random_devices_open(). Even with the random sources closed, urandom and random are still open for the test-executable.

[alexcrichton]

Hm ok, want to send a patch with RAND_keep_random_devices_open being called and we can investigate from there?

[lukaslueg]

The change that introduced RAND_keep_random_devices_open() did not work as planned. A fix has been pushed in openssl 1.1.1a, which was released in November. I'll get back to this once the dust settles and 1.1.1a appears in distributions; this does not seem like a pressing issue.

[alexcrichton]

FWIW we're using 1.1.1a in rust-lang/rust, which is used to distribute Cargo/rustc/etc binaries, so we'd see the benefit there at least!