From cecabb1feaad05ad9990eb27cf7ecae102341e21 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Mon, 27 Mar 2023 17:46:46 -0500 Subject: [PATCH 1/7] SSH/SFTP: create new openChannel() method to eliminate dupe code --- phpseclib/Net/SFTP.php | 15 +----- phpseclib/Net/SSH2.php | 107 ++++++++++++++--------------------------- 2 files changed, 37 insertions(+), 85 deletions(-) diff --git a/phpseclib/Net/SFTP.php b/phpseclib/Net/SFTP.php index a157b665..7015d904 100644 --- a/phpseclib/Net/SFTP.php +++ b/phpseclib/Net/SFTP.php @@ -547,20 +547,7 @@ class SFTP extends SSH2 { $this->window_size_server_to_client[self::CHANNEL] = $this->window_size; - $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); + $response = $this->openChannel(self::CHANNEL, true); if ($response === true && $this->isTimeout()) { return false; } diff --git a/phpseclib/Net/SSH2.php b/phpseclib/Net/SSH2.php index 19d52576..b554759b 100644 --- a/phpseclib/Net/SSH2.php +++ b/phpseclib/Net/SSH2.php @@ -2729,28 +2729,7 @@ class SSH2 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 - // 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); + $this->openChannel(self::CHANNEL_EXEC); if ($this->request_pty === true) { $terminal_modes = pack('C', NET_SSH2_TTY_OP_END); @@ -2830,6 +2809,38 @@ class SSH2 } } + /** + * Opens a channel + * + * @param string $channel + */ + protected function openChannel($channel, $skip_extended = false) + { + // 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 * @@ -2854,23 +2865,7 @@ class SSH2 throw new InsufficientSetupException('Operation disallowed prior to login()'); } - $this->window_size_server_to_client[self::CHANNEL_SHELL] = $this->window_size; - $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); + $this->openChannel(self::CHANNEL_SHELL); $terminal_modes = pack('C', NET_SSH2_TTY_OP_END); $packet = Strings::packSSH2( @@ -3110,22 +3105,7 @@ class SSH2 */ public function startSubsystem($subsystem) { - $this->window_size_server_to_client[self::CHANNEL_SUBSYSTEM] = $this->window_size; - - $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); + $this->openChannel(self::CHANNEL_SUBSYSTEM); $packet = Strings::packSSH2( 'CNsCs', @@ -3303,23 +3283,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->channel_status[self::CHANNEL_KEEP_ALIVE] = NET_SSH2_MSG_CHANNEL_OPEN; - - $response = $this->get_channel_packet(self::CHANNEL_KEEP_ALIVE); + $this->openChannel(self::CHANNEL_KEEP_ALIVE); } catch (\RuntimeException $e) { return $this->reconnect(); } From 5fb084b04ca8a95b5c4b14df20ea168f0f7f9e20 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Wed, 29 Mar 2023 03:52:00 -0500 Subject: [PATCH 2/7] SSH2: if the server doesn't support multiple channels error out --- phpseclib/Net/SSH2.php | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/phpseclib/Net/SSH2.php b/phpseclib/Net/SSH2.php index b554759b..42f1f4e4 100644 --- a/phpseclib/Net/SSH2.php +++ b/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"); } @@ -2813,9 +2840,17 @@ class SSH2 * Opens a channel * * @param string $channel + * @param bool $skip_extended + * @return bool */ protected function openChannel($channel, $skip_extended = false) { + $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. From 184a984e97ff60446c3adfa006919a4bda1d246e Mon Sep 17 00:00:00 2001 From: terrafrost Date: Fri, 14 Apr 2023 11:24:31 -0500 Subject: [PATCH 3/7] SSH2: updates to openchannel refactoring --- phpseclib/Net/SSH2.php | 37 ++++++++++++++++++++++++------- tests/Functional/Net/SSH2Test.php | 2 +- tests/Unit/Net/SSH2UnitTest.php | 7 +++--- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/phpseclib/Net/SSH2.php b/phpseclib/Net/SSH2.php index 42f1f4e4..0dd3a2ee 100644 --- a/phpseclib/Net/SSH2.php +++ b/phpseclib/Net/SSH2.php @@ -2752,9 +2752,9 @@ class SSH2 return false; } - 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.'); - } + //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.'); + //} $this->openChannel(self::CHANNEL_EXEC); @@ -2836,6 +2836,16 @@ class SSH2 } } + /** + * How many channels are currently open? + * + * @return int + */ + public function getOpenChannelCount() + { + return $this->channelCount; + } + /** * Opens a channel * @@ -2845,6 +2855,10 @@ class SSH2 */ 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) { @@ -2892,10 +2906,6 @@ class SSH2 */ public function openShell() { - if ($this->isShellOpen()) { - return false; - } - if (!$this->isAuthenticated()) { throw new InsufficientSetupException('Operation disallowed prior to login()'); } @@ -3060,7 +3070,7 @@ class SSH2 $channel = $this->get_interactive_channel(); } - if (!$this->isInteractiveChannelOpen($channel)) { + if (!$this->isInteractiveChannelOpen($channel) && empty($this->channel_buffers[$channel])) { if ($channel != self::CHANNEL_SHELL) { throw new InsufficientSetupException('Data is not available on channel'); } elseif (!$this->openShell()) { @@ -4111,6 +4121,8 @@ class SSH2 } $this->channel_status[$channel] = NET_SSH2_MSG_CHANNEL_CLOSE; + $this->channelCount--; + if ($client_channel == $channel) { return true; } @@ -4442,6 +4454,7 @@ class SSH2 } $this->channel_status[$client_channel] = NET_SSH2_MSG_CHANNEL_CLOSE; + $this->channelCount--; $this->curTimeout = 5; @@ -4913,6 +4926,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 * diff --git a/tests/Functional/Net/SSH2Test.php b/tests/Functional/Net/SSH2Test.php index 83d35fc3..55df076f 100644 --- a/tests/Functional/Net/SSH2Test.php +++ b/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(); diff --git a/tests/Unit/Net/SSH2UnitTest.php b/tests/Unit/Net/SSH2UnitTest.php index c6b8fa15..42497c06 100644 --- a/tests/Unit/Net/SSH2UnitTest.php +++ b/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()) - ->method('isShellOpen') - ->willReturn(true); + $this->expectException(InsufficientSetupException::class); + $this->expectExceptionMessage('Operation disallowed prior to login()'); $this->assertFalse($ssh->openShell()); } From 06f45881f92270f6d5ff00fde75958d54ce24a6e Mon Sep 17 00:00:00 2001 From: terrafrost Date: Fri, 14 Apr 2023 19:11:32 -0500 Subject: [PATCH 4/7] Tests/SSH2: add more expansive unit test --- tests/Functional/Net/SSH2Test.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/Functional/Net/SSH2Test.php b/tests/Functional/Net/SSH2Test.php index 55df076f..8603e13f 100644 --- a/tests/Functional/Net/SSH2Test.php +++ b/tests/Functional/Net/SSH2Test.php @@ -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()); + } } From f664ccb521819c075f727ffa9953cb4e7050e2e9 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Sun, 7 May 2023 11:07:07 -0500 Subject: [PATCH 5/7] SSH2: make exceptions more useful for read() / write() --- phpseclib/Net/SSH2.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/phpseclib/Net/SSH2.php b/phpseclib/Net/SSH2.php index 0dd3a2ee..fa3ed4d6 100644 --- a/phpseclib/Net/SSH2.php +++ b/phpseclib/Net/SSH2.php @@ -3063,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; @@ -3120,6 +3124,10 @@ 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(); } From 89d8e6ecbbd5a2ae1b55a610280bf1127c8dd472 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Sun, 7 May 2023 11:07:38 -0500 Subject: [PATCH 6/7] SFTP: rm redundant code --- phpseclib/Net/SFTP.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/phpseclib/Net/SFTP.php b/phpseclib/Net/SFTP.php index 7015d904..3d190ada 100644 --- a/phpseclib/Net/SFTP.php +++ b/phpseclib/Net/SFTP.php @@ -545,8 +545,6 @@ 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); if ($response === true && $this->isTimeout()) { return false; From 3dd777993949f5e9467c0d48bc0338effa63ca08 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Sun, 7 May 2023 11:24:33 -0500 Subject: [PATCH 7/7] SSH2: rm redundant isAuthenticated() call --- phpseclib/Net/SSH2.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpseclib/Net/SSH2.php b/phpseclib/Net/SSH2.php index fa3ed4d6..ac6bf3b9 100644 --- a/phpseclib/Net/SSH2.php +++ b/phpseclib/Net/SSH2.php @@ -3074,7 +3074,7 @@ class SSH2 $channel = $this->get_interactive_channel(); } - if (!$this->isInteractiveChannelOpen($channel) && empty($this->channel_buffers[$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()) { @@ -3132,7 +3132,7 @@ class SSH2 $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()) {