From 71a9fc7915476634123e6c3a9ec6d7d6e8380e5c Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 28 Jun 2024 09:39:06 -0400 Subject: [PATCH] Correction to stream timeout for keep alive, wait for time remaining in interval. Reflect total wait time on packet in logging. --- phpseclib/Net/SSH2.php | 21 +++++++------- tests/Functional/Net/SSH2Test.php | 21 ++++++++++++++ tests/PhpseclibTestCase.php | 32 ++++++++++++++++++++- tests/Unit/Net/SSH2UnitTest.php | 47 +++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 11 deletions(-) diff --git a/phpseclib/Net/SSH2.php b/phpseclib/Net/SSH2.php index 49774629..8089af31 100644 --- a/phpseclib/Net/SSH2.php +++ b/phpseclib/Net/SSH2.php @@ -825,11 +825,9 @@ class SSH2 private $quiet_mode = false; /** - * Time of first network activity - * - * @var float + * Time of last read/write network activity */ - private $last_packet; + private $last_packet = null; /** * Exit status returned from ssh if any @@ -3500,9 +3498,10 @@ class SSH2 } if ($this->keepAlive > 0) { $elapsed = microtime(true) - $this->last_packet; - if ($elapsed < $this->curTimeout) { - $sec = (int) floor($elapsed); - $usec = (int) (1000000 * ($elapsed - $sec)); + $timeout = max($this->keepAlive - $elapsed, 0); + if (!$this->curTimeout || $timeout < $this->curTimeout) { + $sec = (int) floor($timeout); + $usec = (int) (1000000 * ($timeout - $sec)); } } return [$sec, $usec]; @@ -3524,6 +3523,7 @@ class SSH2 if ($this->binary_packet_buffer == null) { // buffer the packet to permit continued reads across timeouts $this->binary_packet_buffer = (object) [ + 'read_time' => 0, // the time to read the packet from the socket 'raw' => '', // the raw payload read from the socket 'plain' => '', // the packet in plain text, excluding packet_length header 'packet_length' => null, // the packet_length value pulled from the payload @@ -3548,6 +3548,7 @@ class SSH2 $start = microtime(true); $raw = stream_get_contents($this->fsock, $packet->size - strlen($packet->raw)); $elapsed = microtime(true) - $start; + $packet->read_time += $elapsed; if ($this->curTimeout > 0) { $this->curTimeout -= $elapsed; } @@ -3693,10 +3694,10 @@ class SSH2 $current = microtime(true); $message_number = isset(self::$message_numbers[ord($payload[0])]) ? self::$message_numbers[ord($payload[0])] : 'UNKNOWN (' . ord($payload[0]) . ')'; $message_number = '<- ' . $message_number . - ' (since last: ' . round($current - $this->last_packet, 4) . ', network: ' . round($elapsed, 4) . 's)'; + ' (since last: ' . round($current - $this->last_packet, 4) . ', network: ' . round($packet->read_time, 4) . 's)'; $this->append_log($message_number, $payload); - $this->last_packet = $current; } + $this->last_packet = microtime(true); return $this->filter($payload); } @@ -4355,8 +4356,8 @@ class SSH2 $message_number = '-> ' . $message_number . ' (since last: ' . round($current - $this->last_packet, 4) . ', network: ' . round($stop - $start, 4) . 's)'; $this->append_log($message_number, $logged); - $this->last_packet = $current; } + $this->last_packet = microtime(true); if (strlen($packet) != $sent) { $this->bitmap = 0; diff --git a/tests/Functional/Net/SSH2Test.php b/tests/Functional/Net/SSH2Test.php index 0f2bd8b5..9733b864 100644 --- a/tests/Functional/Net/SSH2Test.php +++ b/tests/Functional/Net/SSH2Test.php @@ -575,6 +575,27 @@ class SSH2Test extends PhpseclibFunctionalTestCase $this->assertSame(0, $ssh->getOpenChannelCount()); } + public function testKeepAlive(): void + { + $ssh = $this->getSSH2(); + $username = $this->getEnv('SSH_USERNAME'); + $password = $this->getEnv('SSH_PASSWORD'); + + $ssh->setKeepAlive(1); + $ssh->setTimeout(1); + + $this->assertNotEmpty($ssh->getServerIdentification()); + $this->assertTrue( + $ssh->login($username, $password), + 'SSH2 login using password failed.' + ); + + $ssh->write("pwd\n"); + sleep(1); // permit keep alive to proc on next read + $this->assertNotEmpty($ssh->read('', SSH2::READ_NEXT)); + $ssh->disconnect(); + } + /** * @return array */ diff --git a/tests/PhpseclibTestCase.php b/tests/PhpseclibTestCase.php index 68108791..6d710969 100644 --- a/tests/PhpseclibTestCase.php +++ b/tests/PhpseclibTestCase.php @@ -89,11 +89,41 @@ abstract class PhpseclibTestCase extends TestCase protected static function getVar($obj, $var) { $reflection = new \ReflectionClass(get_class($obj)); - $prop = $reflection->getProperty($var); + // private variables are not inherited, climb hierarchy until located + while (true) { + try { + $prop = $reflection->getProperty($var); + break; + } catch (\ReflectionException $e) { + $reflection = $reflection->getParentClass(); + if (!$reflection) { + throw $e; + } + } + } $prop->setAccessible(true); return $prop->getValue($obj); } + protected static function setVar($obj, $var, $value) + { + $reflection = new \ReflectionClass(get_class($obj)); + // private variables are not inherited, climb hierarchy until located + while (true) { + try { + $prop = $reflection->getProperty($var); + break; + } catch (\ReflectionException $e) { + $reflection = $reflection->getParentClass(); + if (!$reflection) { + throw $e; + } + } + } + $prop->setAccessible(true); + $prop->setValue($obj, $value); + } + public static function callFunc($obj, $func, $params = []) { $reflection = new \ReflectionClass(get_class($obj)); diff --git a/tests/Unit/Net/SSH2UnitTest.php b/tests/Unit/Net/SSH2UnitTest.php index 0a67b10d..427ae512 100644 --- a/tests/Unit/Net/SSH2UnitTest.php +++ b/tests/Unit/Net/SSH2UnitTest.php @@ -221,6 +221,53 @@ class SSH2UnitTest extends PhpseclibTestCase $this->assertEquals(20, $ssh->getTimeout()); } + public function testGetStreamTimeout(): void + { + // no curTimeout, no keepAlive + $ssh = $this->createSSHMock(); + $this->assertEquals([0, 0], self::callFunc($ssh, 'get_stream_timeout')); + + // curTimeout, no keepAlive + $ssh = $this->createSSHMock(); + $ssh->setTimeout(1); + $this->assertEquals([1, 0], self::callFunc($ssh, 'get_stream_timeout')); + + // no curTimeout, keepAlive + $ssh = $this->createSSHMock(); + $ssh->setKeepAlive(2); + self::setVar($ssh, 'last_packet', microtime(true)); + list($sec, $usec) = self::callFunc($ssh, 'get_stream_timeout'); + $this->assertGreaterThanOrEqual(1, $sec); + $this->assertLessThanOrEqual(2, $sec); + + // smaller curTimeout, keepAlive + $ssh = $this->createSSHMock(); + $ssh->setTimeout(1); + $ssh->setKeepAlive(2); + self::setVar($ssh, 'last_packet', microtime(true)); + $this->assertEquals([1, 0], self::callFunc($ssh, 'get_stream_timeout')); + + // curTimeout, smaller keepAlive + $ssh = $this->createSSHMock(); + $ssh->setTimeout(5); + $ssh->setKeepAlive(2); + self::setVar($ssh, 'last_packet', microtime(true)); + list($sec, $usec) = self::callFunc($ssh, 'get_stream_timeout'); + $this->assertGreaterThanOrEqual(1, $sec); + $this->assertLessThanOrEqual(2, $sec); + + // no curTimeout, keepAlive, no last_packet + $ssh = $this->createSSHMock(); + $ssh->setKeepAlive(2); + $this->assertEquals([0, 0], self::callFunc($ssh, 'get_stream_timeout')); + + // no curTimeout, keepAlive, last_packet exceeds keepAlive + $ssh = $this->createSSHMock(); + $ssh->setKeepAlive(2); + self::setVar($ssh, 'last_packet', microtime(true) - 2); + $this->assertEquals([0, 0], self::callFunc($ssh, 'get_stream_timeout')); + } + /** * @return \phpseclib3\Net\SSH2 */