clue / reactphp-ssh-proxy

Async SSH proxy connector and forwarder, tunnel any TCP/IP-based protocol through an SSH server, built on top of ReactPHP.
https://clue.engineering/2018/introducing-reactphp-ssh-proxy
MIT License
21 stars 7 forks source link

Do not inherit open FDs to SSH child process #2

Closed clue closed 5 years ago

clue commented 5 years ago

This somewhat obscure PR ensures that we do not inherit open file descriptors (FDs) to the SSH child process. This can cause all sorts of errors in long running applications and really is not desired here.

This is implemented by explicitly closing all superfluous FDs in the implicit sh child process before launching the actual ssh binary. PHP does not support FD_CLOEXEC, O_CLOEXEC or SOCK_CLOEXEC and this appears to be the best work around I could find (yes, I should probably write a lengthy, somewhat technical blog post about this). Additionally, this PR includes a test to verify this works on all supported platforms and this could perhaps be used as a starting point for other libraries (YMMV).

See also https://github.com/reactphp/child-process/issues/51

clue commented 5 years ago

For the reference: This PR ensures that we immediately close all inherited file descriptors before executing the actual ssh child process. This means that there is still a (very short) period where these file descriptors are in fact inherited to the intermediary sh process and could potentially cause problems. While I have not seen any actual issues in my application because of this possible race condition, I still agree that this is something to address in a future version, most likely by taking advantage of https://github.com/reactphp/child-process/pull/65.