Merge pull request #1904 from terrafrost/3.0-openchannel

(3.0 branch) SSH/SFTP: create new openChannel() method to eliminate dupe code

phpseclib/Net/SFTP.php

@@ -545,22 +545,7 @@ class SFTP extends SSH2
      */
     private function partial_init_sftp_connection()
     {
-        $this->window_size_server_to_client[self::CHANNEL] = $this->window_size;
+        $response = $this->openChannel(self::CHANNEL, true);
-
-        $packet = Strings::packSSH2(
-            'CsN3',
-            NET_SSH2_MSG_CHANNEL_OPEN,
-            'session',
-            self::CHANNEL,
-            $this->window_size,
-            0x4000
-        );
-
-        $this->send_binary_packet($packet);
-
-        $this->channel_status[self::CHANNEL] = NET_SSH2_MSG_CHANNEL_OPEN;
-
-        $response = $this->get_channel_packet(self::CHANNEL, true);
         if ($response === true && $this->isTimeout()) {
             return false;
         }

phpseclib/Net/SSH2.php

@@ -1087,6 +1087,21 @@ class SSH2
      */
     private $smartMFA = true;
 
+    /**
+     * How many channels are currently opened
+     *
+     * @var int
+     */
+    private $channelCount = 0;
+
+    /**
+     * Does the server support multiple channels? If not then error out
+     * when multiple channels are attempted to be opened
+     *
+     * @var bool
+     */
+    private $errorOnMultipleChannels;
+
     /**
      * Default Constructor.
      *
@@ -1384,6 +1399,18 @@ class SSH2
             throw new UnableToConnectException("Cannot connect to SSH $matches[3] servers");
         }
 
+        // Ubuntu's OpenSSH from 5.8 to 6.9 didn't work with multiple channels. see
+        // https://bugs.launchpad.net/ubuntu/+source/openssh/+bug/1334916 for more info.
+        // https://lists.ubuntu.com/archives/oneiric-changes/2011-July/005772.html discusses
+        // when consolekit was incorporated.
+        // https://marc.info/?l=openssh-unix-dev&m=163409903417589&w=2 discusses some of the
+        // issues with how Ubuntu incorporated consolekit
+        $pattern = '#^SSH-2\.0-OpenSSH_([\d.]+)[^ ]* Ubuntu-.*$#';
+        $match = preg_match($pattern, $this->server_identifier, $matches);
+        $match = $match && version_compare('5.8', $matches[1], '<=');
+        $match = $match && version_compare('6.9', $matches[1], '>=');
+        $this->errorOnMultipleChannels = $match;
+
         if (!$this->send_id_string_first) {
             fputs($this->fsock, $this->identifier . "\r\n");
         }
@@ -2725,32 +2752,11 @@ class SSH2
             return false;
         }
 
-        if ($this->isPTYOpen()) {
+        //if ($this->isPTYOpen()) {
-            throw new \RuntimeException('If you want to run multiple exec()\'s you will need to disable (and re-enable if appropriate) a PTY for each one.');
+        //    throw new \RuntimeException('If you want to run multiple exec()\'s you will need to disable (and re-enable if appropriate) a PTY for each one.');
-        }
+        //}
 
-        // RFC4254 defines the (client) window size as "bytes the other party can send before it must wait for the window to
+        $this->openChannel(self::CHANNEL_EXEC);
-        // be adjusted".  0x7FFFFFFF is, at 2GB, the max size.  technically, it should probably be decremented, but,
-        // honestly, if you're transferring more than 2GB, you probably shouldn't be using phpseclib, anyway.
-        // see http://tools.ietf.org/html/rfc4254#section-5.2 for more info
-        $this->window_size_server_to_client[self::CHANNEL_EXEC] = $this->window_size;
-        // 0x8000 is the maximum max packet size, per http://tools.ietf.org/html/rfc4253#section-6.1, although since PuTTy
-        // uses 0x4000, that's what will be used here, as well.
-        $packet_size = 0x4000;
-
-        $packet = Strings::packSSH2(
-            'CsN3',
-            NET_SSH2_MSG_CHANNEL_OPEN,
-            'session',
-            self::CHANNEL_EXEC,
-            $this->window_size_server_to_client[self::CHANNEL_EXEC],
-            $packet_size
-        );
-        $this->send_binary_packet($packet);
-
-        $this->channel_status[self::CHANNEL_EXEC] = NET_SSH2_MSG_CHANNEL_OPEN;
-
-        $this->get_channel_packet(self::CHANNEL_EXEC);
 
         if ($this->request_pty === true) {
             $terminal_modes = pack('C', NET_SSH2_TTY_OP_END);
@@ -2830,6 +2836,60 @@ class SSH2
         }
     }
 
+    /**
+     * How many channels are currently open?
+     *
+     * @return int
+     */
+    public function getOpenChannelCount()
+    {
+        return $this->channelCount;
+    }
+
+    /**
+     * Opens a channel
+     *
+     * @param string $channel
+     * @param bool $skip_extended
+     * @return bool
+     */
+    protected function openChannel($channel, $skip_extended = false)
+    {
+        if (isset($this->channel_status[$channel]) && $this->channel_status[$channel] != NET_SSH2_MSG_CHANNEL_CLOSE) {
+            throw new \RuntimeException('Please close the channel (' . $channel . ') before trying to open it again');
+        }
+
+        $this->channelCount++;
+
+        if ($this->channelCount > 1 && $this->errorOnMultipleChannels) {
+            throw new \RuntimeException("Ubuntu's OpenSSH from 5.8 to 6.9 doesn't work with multiple channels");
+        }
+
+        // RFC4254 defines the (client) window size as "bytes the other party can send before it must wait for the window to
+        // be adjusted".  0x7FFFFFFF is, at 2GB, the max size.  technically, it should probably be decremented, but,
+        // honestly, if you're transferring more than 2GB, you probably shouldn't be using phpseclib, anyway.
+        // see http://tools.ietf.org/html/rfc4254#section-5.2 for more info
+        $this->window_size_server_to_client[$channel] = $this->window_size;
+        // 0x8000 is the maximum max packet size, per http://tools.ietf.org/html/rfc4253#section-6.1, although since PuTTy
+        // uses 0x4000, that's what will be used here, as well.
+        $packet_size = 0x4000;
+
+        $packet = Strings::packSSH2(
+            'CsN3',
+            NET_SSH2_MSG_CHANNEL_OPEN,
+            'session',
+            $channel,
+            $this->window_size_server_to_client[$channel],
+            $packet_size
+        );
+
+        $this->send_binary_packet($packet);
+
+        $this->channel_status[$channel] = NET_SSH2_MSG_CHANNEL_OPEN;
+
+        return $this->get_channel_packet($channel, $skip_extended);
+    }
+
     /**
      * Creates an interactive shell
      *
@@ -2846,31 +2906,11 @@ class SSH2
      */
     public function openShell()
     {
-        if ($this->isShellOpen()) {
-            return false;
-        }
-
         if (!$this->isAuthenticated()) {
             throw new InsufficientSetupException('Operation disallowed prior to login()');
         }
 
-        $this->window_size_server_to_client[self::CHANNEL_SHELL] = $this->window_size;
+        $this->openChannel(self::CHANNEL_SHELL);
-        $packet_size = 0x4000;
-
-        $packet = Strings::packSSH2(
-            'CsN3',
-            NET_SSH2_MSG_CHANNEL_OPEN,
-            'session',
-            self::CHANNEL_SHELL,
-            $this->window_size_server_to_client[self::CHANNEL_SHELL],
-            $packet_size
-        );
-
-        $this->send_binary_packet($packet);
-
-        $this->channel_status[self::CHANNEL_SHELL] = NET_SSH2_MSG_CHANNEL_OPEN;
-
-        $this->get_channel_packet(self::CHANNEL_SHELL);
 
         $terminal_modes = pack('C', NET_SSH2_TTY_OP_END);
         $packet = Strings::packSSH2(
@@ -3023,6 +3063,10 @@ class SSH2
      */
     public function read($expect = '', $mode = self::READ_SIMPLE, $channel = null)
     {
+        if (!$this->isAuthenticated()) {
+            throw new InsufficientSetupException('Operation disallowed prior to login()');
+        }
+
         $this->curTimeout = $this->timeout;
         $this->is_timeout = false;
 
@@ -3030,7 +3074,7 @@ class SSH2
             $channel = $this->get_interactive_channel();
         }
 
-        if (!$this->isInteractiveChannelOpen($channel)) {
+        if (!$this->is_channel_status_data($channel) && empty($this->channel_buffers[$channel])) {
             if ($channel != self::CHANNEL_SHELL) {
                 throw new InsufficientSetupException('Data is not available on channel');
             } elseif (!$this->openShell()) {
@@ -3080,11 +3124,15 @@ class SSH2
      */
     public function write($cmd, $channel = null)
     {
+        if (!$this->isAuthenticated()) {
+            throw new InsufficientSetupException('Operation disallowed prior to login()');
+        }
+
         if ($channel === null) {
             $channel = $this->get_interactive_channel();
         }
 
-        if (!$this->isInteractiveChannelOpen($channel)) {
+        if (!$this->is_channel_status_data($channel)) {
             if ($channel != self::CHANNEL_SHELL) {
                 throw new InsufficientSetupException('Data is not available on channel');
             } elseif (!$this->openShell()) {
@@ -3110,22 +3158,7 @@ class SSH2
      */
     public function startSubsystem($subsystem)
     {
-        $this->window_size_server_to_client[self::CHANNEL_SUBSYSTEM] = $this->window_size;
+        $this->openChannel(self::CHANNEL_SUBSYSTEM);
-
-        $packet = Strings::packSSH2(
-            'CsN3',
-            NET_SSH2_MSG_CHANNEL_OPEN,
-            'session',
-            self::CHANNEL_SUBSYSTEM,
-            $this->window_size,
-            0x4000
-        );
-
-        $this->send_binary_packet($packet);
-
-        $this->channel_status[self::CHANNEL_SUBSYSTEM] = NET_SSH2_MSG_CHANNEL_OPEN;
-
-        $this->get_channel_packet(self::CHANNEL_SUBSYSTEM);
 
         $packet = Strings::packSSH2(
             'CNsCs',
@@ -3303,23 +3336,8 @@ class SSH2
             return false;
         }
 
-        $this->window_size_server_to_client[self::CHANNEL_KEEP_ALIVE] = $this->window_size;
-        $packet_size = 0x4000;
-        $packet = Strings::packSSH2(
-            'CsN3',
-            NET_SSH2_MSG_CHANNEL_OPEN,
-            'session',
-            self::CHANNEL_KEEP_ALIVE,
-            $this->window_size_server_to_client[self::CHANNEL_KEEP_ALIVE],
-            $packet_size
-        );
-
         try {
-            $this->send_binary_packet($packet);
+            $this->openChannel(self::CHANNEL_KEEP_ALIVE);
-
-            $this->channel_status[self::CHANNEL_KEEP_ALIVE] = NET_SSH2_MSG_CHANNEL_OPEN;
-
-            $response = $this->get_channel_packet(self::CHANNEL_KEEP_ALIVE);
         } catch (\RuntimeException $e) {
             return $this->reconnect();
         }
@@ -4111,6 +4129,8 @@ class SSH2
                     }
 
                     $this->channel_status[$channel] = NET_SSH2_MSG_CHANNEL_CLOSE;
+                    $this->channelCount--;
+
                     if ($client_channel == $channel) {
                         return true;
                     }
@@ -4445,6 +4465,7 @@ class SSH2
         }
 
         $this->channel_status[$client_channel] = NET_SSH2_MSG_CHANNEL_CLOSE;
+        $this->channelCount--;
 
         $this->curTimeout = 5;
 
@@ -4916,6 +4937,14 @@ class SSH2
         ];
     }
 
+    /**
+     * Force multiple channels (even if phpseclib has decided to disable them)
+     */
+    public function forceMultipleChannels()
+    {
+        $this->errorOnMultipleChannels = false;
+    }
+
     /**
      * Allows you to set the terminal
      *

tests/Functional/Net/SSH2Test.php

@@ -416,7 +416,7 @@ class SSH2Test extends PhpseclibFunctionalTestCase
     public function testMultipleExecPty()
     {
         $this->expectException(\RuntimeException::class);
-        $this->expectExceptionMessage('If you want to run multiple exec()\'s you will need to disable (and re-enable if appropriate) a PTY for each one.');
+        $this->expectExceptionMessage('Please close the channel (1) before trying to open it again');
 
         $ssh = $this->getSSH2Login();
 
@@ -542,4 +542,21 @@ class SSH2Test extends PhpseclibFunctionalTestCase
             'Failed asserting that exec channel identifier is maintained as last opened channel.'
         );
     }
+
+    public function testReadingOfClosedChannel()
+    {
+        $ssh = $this->getSSH2Login();
+        $this->assertSame(0, $ssh->getOpenChannelCount());
+        $ssh->enablePTY();
+        $ssh->exec('ping -c 3 127.0.0.1; exit');
+        $ssh->write("ping 127.0.0.2\n", SSH2::CHANNEL_SHELL);
+        $ssh->setTimeout(3);
+        $output = $ssh->read('', SSH2::READ_SIMPLE, SSH2::CHANNEL_SHELL);
+        $this->assertStringContainsString('PING 127.0.0.2', $output);
+        $output = $ssh->read('', SSH2::READ_SIMPLE, SSH2::CHANNEL_EXEC);
+        $this->assertStringContainsString('PING 127.0.0.1', $output);
+        $this->assertSame(1, $ssh->getOpenChannelCount());
+        $ssh->reset(SSH2::CHANNEL_SHELL);
+        $this->assertSame(0, $ssh->getOpenChannelCount());
+    }
 }

tests/Unit/Net/SSH2UnitTest.php

@@ -202,12 +202,11 @@ class SSH2UnitTest extends PhpseclibTestCase
     {
         $ssh = $this->getMockBuilder(SSH2::class)
             ->disableOriginalConstructor()
-            ->setMethods(['__destruct', 'isShellOpen'])
+            ->setMethods(['__destruct'])
             ->getMock();
 
-        $ssh->expects($this->once())
+        $this->expectException(InsufficientSetupException::class);
-            ->method('isShellOpen')
+        $this->expectExceptionMessage('Operation disallowed prior to login()');
-            ->willReturn(true);
 
         $this->assertFalse($ssh->openShell());
     }