Merge pull request #2 from clue-labs/fds

Do not inherit open FDs to SSH child process

Author: clue

src/SshProcessConnector.php

@@ -66,7 +66,27 @@ public function connect($uri)
             return \React\Promise\reject(new \InvalidArgumentException('Invalid target URI'));
         }
 
-        $process = new Process($this->cmd . ' -W ' . \escapeshellarg($parts['host'] . ':' . $parts['port']));
+        $command = $this->cmd . ' -W ' . \escapeshellarg($parts['host'] . ':' . $parts['port']);
+
+        // try to get list of all open FDs (Linux only) or simply assume range 3-1024 (FD_SETSIZE)
+        $fds = @scandir('/proc/self/fd');
+        if ($fds === false) {
+            $fds = range(3, 1024); // @codeCoverageIgnore
+        }
+
+        // do not inherit open FDs by explicitly closing all of them
+        foreach ($fds as $fd) {
+            if ($fd > 2) {
+                $command .= ' ' . $fd . '>&-';
+            }
+        }
+
+        // default `sh` only accepts single-digit FDs, so run in bash if needed
+        if ($fds && max($fds) > 9) {
+            $command = 'exec bash -c ' . escapeshellarg($command);
+        }
+
+        $process = new Process($command);
         $process->start($this->loop);
 
         $deferred = new Deferred(function () use ($process, $uri) {
@@ -110,9 +130,6 @@ public function connect($uri)
             }
 
             $connection = new CompositeConnection($process->stdout, $process->stdin);
-            $connection->on('close', function() use ($process) {
-                //$process->terminate();
-            });
             $deferred->resolve($connection);
         });
 

tests/FunctionalSshProcessConnectorTest.php

@@ -68,4 +68,32 @@ public function testConnectValidTargetWillReturnPromiseWhichResolvesToConnection
         $this->assertTrue($connection->isWritable());
         $connection->close();
     }
+
+    public function testConnectValidTargetWillNotInheritActiveFileDescriptors()
+    {
+        $server = stream_socket_server('tcp://127.0.0.1:0');
+        $address = stream_socket_get_name($server, false);
+
+        // ensure that we can not listen on the same address twice
+        $copy = @stream_socket_server('tcp://' . $address);
+        if ($copy !== false) {
+            fclose($server);
+            fclose($copy);
+
+            $this->markTestSkipped('Platform does not prevent binding to same address (Windows?)');
+        }
+
+        // wait for successful connection
+        $promise = $this->connector->connect('example.com:80');
+        $connection = \Clue\React\Block\await($promise, $this->loop, self::TIMEOUT);
+
+        // close server and ensure we can start a new server on the previous address
+        // the open SSH connection process should not inherit the existing server socket
+        fclose($server);
+        $server = stream_socket_server('tcp://' . $address);
+        $this->assertTrue(is_resource($server));
+
+        fclose($server);
+        $connection->close();
+    }
 }