Merge pull request #2012 from rposky/ssh-keepalive-timeout

Correction to stream timeout for keep alive

phpseclib/Net/SSH2.php

@@ -825,11 +825,11 @@ class SSH2
     private $quiet_mode = false;
 
     /**
-     * Time of first network activity
+     * Time of last read/write network activity
      *
      * @var float
      */
-    private $last_packet;
+    private $last_packet = null;
 
     /**
      * Exit status returned from ssh if any
@@ -3500,9 +3500,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 +3525,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 +3550,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 +3696,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 +4358,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;

tests/Functional/Net/SSH2Test.php

@@ -575,6 +575,27 @@ class SSH2Test extends PhpseclibFunctionalTestCase
         $this->assertSame(0, $ssh->getOpenChannelCount());
     }
 
+    public function testKeepAlive()
+    {
+        $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
      */

tests/PhpseclibTestCase.php

@@ -89,11 +89,41 @@ abstract class PhpseclibTestCase extends TestCase
     protected static function getVar($obj, $var)
     {
         $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);
         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));

tests/Unit/Net/SSH2UnitTest.php

@@ -221,6 +221,56 @@ class SSH2UnitTest extends PhpseclibTestCase
         $this->assertEquals(20, $ssh->getTimeout());
     }
 
+    /**
+     * @requires PHPUnit < 10
+     */
+    public function testGetStreamTimeout()
+    {
+        // 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
      */