Merge branch '3.0'

phpseclib/Exception/InvalidPacketLengthException.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace phpseclib3\Exception;
+
+/**
+ * Indicates an absent or malformed packet length header
+ */
+class InvalidPacketLengthException extends ConnectionClosedException
+{
+}

phpseclib/Net/SFTP.php

@@ -2921,9 +2921,9 @@ class SFTP extends SSH2
     /**
      * Resets a connection for re-use
      */
-    protected function reset_connection(int $reason): void
+    protected function reset_connection(): void
     {
-        parent::reset_connection($reason);
+        parent::reset_connection();
         $this->reset_sftp();
     }
 

phpseclib/Net/SSH2.php

@@ -66,6 +66,7 @@ use phpseclib3\Crypt\Twofish;
 use phpseclib3\Exception\ConnectionClosedException;
 use phpseclib3\Exception\InsufficientSetupException;
 use phpseclib3\Exception\InvalidArgumentException;
+use phpseclib3\Exception\InvalidPacketLengthException;
 use phpseclib3\Exception\LengthException;
 use phpseclib3\Exception\LogicException;
 use phpseclib3\Exception\NoSupportedAlgorithmsException;
@@ -674,6 +675,13 @@ class SSH2
      */
     private int|null $keepAlive = null;
 
+    /**
+     * Timestamp for the last sent keep alive message
+     *
+     * @see self::send_keep_alive()
+     */
+    private float|null $keep_alive_sent = null;
+
     /**
      * Real-time log file pointer
      *
@@ -863,7 +871,7 @@ class SSH2
     /**
      * Binary Packet Buffer
      */
-    private string|false $binary_packet_buffer = false;
+    private object|null $binary_packet_buffer = null;
 
     /**
      * Authentication Credentials
@@ -1378,15 +1386,13 @@ class SSH2
         // we don't initialize any crypto-objects, yet - we do that, later. for now, we need the lengths to make the
         // diffie-hellman key exchange as fast as possible
         $decrypt = self::array_intersect_first($s2c_encryption_algorithms, $this->encryption_algorithms_server_to_client);
-        $decryptKeyLength = $this->encryption_algorithm_to_key_size($decrypt);
+        if (!$decrypt || ($decryptKeyLength = $this->encryption_algorithm_to_key_size($decrypt)) === null) {
-        if ($decryptKeyLength === null) {
             $this->disconnect_helper(DisconnectReason::KEY_EXCHANGE_FAILED);
             throw new NoSupportedAlgorithmsException('No compatible server to client encryption algorithms found');
         }
 
         $encrypt = self::array_intersect_first($c2s_encryption_algorithms, $this->encryption_algorithms_client_to_server);
-        $encryptKeyLength = $this->encryption_algorithm_to_key_size($encrypt);
+        if (!$encrypt || ($encryptKeyLength = $this->encryption_algorithm_to_key_size($encrypt)) === null) {
-        if ($encryptKeyLength === null) {
             $this->disconnect_helper(DisconnectReason::KEY_EXCHANGE_FAILED);
             throw new NoSupportedAlgorithmsException('No compatible client to server encryption algorithms found');
         }
@@ -1908,7 +1914,7 @@ class SSH2
         }
     }
 
-    /*
+    /**
      * Tests whether or not proposed algorithm has a potential for issues
      *
      * @link https://www.chiark.greenend.org.uk/~sgtatham/putty/wishlist/ssh2-aesctr-openssh.html
@@ -2070,17 +2076,18 @@ class SSH2
             $this->send_binary_packet($packet);
 
             try {
-                $bad_key_size_fix = $this->bad_key_size_fix;
                 $response = $this->get_binary_packet();
-            } catch (\Exception $e) {
+            } catch (InvalidPacketLengthException $e) {
-                // bad_key_size_fix is only ever re-assigned to true
+                // the first opportunity to encounter the "bad key size" error
-                // under certain conditions. when it's newly set we'll
+                if (!$this->bad_key_size_fix && $this->decryptName != null && self::bad_algorithm_candidate($this->decryptName)) {
+                    // bad_key_size_fix is only ever re-assigned to true here
                     // retry the connection with that new setting but we'll
                     // only try it once.
-                if ($bad_key_size_fix != $this->bad_key_size_fix) {
+                    $this->bad_key_size_fix = true;
-                    $this->connect();
+                    return $this->reconnect();
-                    return $this->login_helper($username, $password);
                 }
+                throw $e;
+            } catch (\Exception $e) {
                 $this->disconnect_helper(DisconnectReason::CONNECTION_LOST);
                 throw $e;
             }
@@ -3123,7 +3130,7 @@ class SSH2
      */
     private function reconnect(): bool
     {
-        $this->reset_connection(DisconnectReason::CONNECTION_LOST);
+        $this->disconnect_helper(DisconnectReason::BY_APPLICATION);
         $this->connect();
         foreach ($this->auth as $auth) {
             $result = $this->login(...$auth);
@@ -3134,19 +3141,46 @@ class SSH2
     /**
      * Resets a connection for re-use
      */
-    protected function reset_connection(int $reason): void
+    protected function reset_connection(): void
     {
-        $this->disconnect_helper($reason);
+        if (is_resource($this->fsock) && get_resource_type($this->fsock) === 'stream') {
+            fclose($this->fsock);
+        }
+        $this->fsock = null;
+        $this->bitmap = 0;
+        $this->binary_packet_buffer = null;
         $this->decrypt = $this->encrypt = false;
         $this->decrypt_block_size = $this->encrypt_block_size = 8;
         $this->hmac_check = $this->hmac_create = false;
         $this->hmac_size = false;
         $this->session_id = false;
+        $this->keep_alive_sent = null;
         $this->get_seq_no = $this->send_seq_no = 0;
         $this->channel_status = [];
         $this->channel_id_last_interactive = 0;
     }
 
+    /**
+     * @return int[] second and microsecond stream timeout options based on user-requested timeout and keep-alive, 0 by default
+     */
+    private function get_stream_timeout()
+    {
+        $sec = 0;
+        $usec = 0;
+        if ($this->curTimeout > 0) {
+            $sec = (int) floor($this->curTimeout);
+            $usec = (int) (1000000 * ($this->curTimeout - $sec));
+        }
+        if ($this->keepAlive > 0) {
+            $elapsed = microtime(true) - $this->keep_alive_sent;
+            if ($elapsed < $this->curTimeout) {
+                $sec = (int) floor($elapsed);
+                $usec = (int) (1000000 * ($elapsed - $sec));
+            }
+        }
+        return [$sec, $usec];
+    }
+
     /**
      * Gets Binary Packets
      *
@@ -3155,78 +3189,64 @@ class SSH2
      * @return bool|string
      * @see self::_send_binary_packet()
      */
-    private function get_binary_packet(bool $skip_channel_filter = false)
+    private function get_binary_packet()
     {
-        if ($skip_channel_filter) {
         if (!is_resource($this->fsock)) {
             throw new InvalidArgumentException('fsock is not a resource.');
         }
-            $read = [$this->fsock];
+        if ($this->binary_packet_buffer == null) {
-            $write = $except = null;
+            // buffer the packet to permit continued reads across timeouts
-
+            $this->binary_packet_buffer = (object) [
-            if (!$this->curTimeout) {
+                'raw' => '', // the raw payload read from the socket
-                if ($this->keepAlive <= 0) {
+                'plain' => '', // the packet in plain text, excluding packet_length header
-                    static::stream_select($read, $write, $except, null);
+                'packet_length' => null, // the packet_length value pulled from the payload
-                } else {
+                'size' => $this->decrypt_block_size, // the total size of this packet to be read from the socket
-                    if (!static::stream_select($read, $write, $except, $this->keepAlive)) {
+                                                     // initialize to read single block until packet_length is available
-                        $this->send_binary_packet(pack('CN', MessageType::IGNORE, 0));
+            ];
-                        return $this->get_binary_packet(true);
         }
+        $packet = $this->binary_packet_buffer;
+        while (strlen($packet->raw) < $packet->size) {
+            if (feof($this->fsock)) {
+                $this->disconnect_helper(DisconnectReason::CONNECTION_LOST);
+                throw new ConnectionClosedException('Connection closed by server');
             }
-            } else {
             if ($this->curTimeout < 0) {
                 $this->is_timeout = true;
                 return true;
             }
+            $this->send_keep_alive();
 
-                $start = microtime(true);
+            [$sec, $usec] = $this->get_stream_timeout();
-
-                if ($this->keepAlive > 0 && $this->keepAlive < $this->curTimeout) {
-                    if (!static::stream_select($read, $write, $except, $this->keepAlive)) {
-                        $this->send_binary_packet(pack('CN', MessageType::IGNORE, 0));
-                        $elapsed = microtime(true) - $start;
-                        $this->curTimeout -= $elapsed;
-                        return $this->get_binary_packet(true);
-                    }
-                    $elapsed = microtime(true) - $start;
-                    $this->curTimeout -= $elapsed;
-                }
-
-                $sec = (int) floor($this->curTimeout);
-                $usec = (int) (1000000 * ($this->curTimeout - $sec));
-
-                // this can return a "stream_select(): unable to select [4]: Interrupted system call" error
-                if (!static::stream_select($read, $write, $except, $sec, $usec)) {
-                    $this->is_timeout = true;
-                    return true;
-                }
-                $elapsed = microtime(true) - $start;
-                $this->curTimeout -= $elapsed;
-            }
-        }
-
-        if (!is_resource($this->fsock) || feof($this->fsock)) {
-            $this->bitmap = 0;
-            $str = 'Connection closed (by server) prematurely';
-            if (isset($elapsed)) {
-                $str .= ' ' . $elapsed . 's';
-            }
-            throw new ConnectionClosedException($str);
-        }
-
-        $start = microtime(true);
-        if ($this->curTimeout) {
-            $sec = (int) floor($this->curTimeout);
-            $usec = (int) (1000000 * ($this->curTimeout - $sec));
             stream_set_timeout($this->fsock, $sec, $usec);
+            $start = microtime(true);
+            $raw = stream_get_contents($this->fsock, $packet->size - strlen($packet->raw));
+            $elapsed = microtime(true) - $start;
+            if ($this->curTimeout > 0) {
+                $this->curTimeout -= $elapsed;
             }
-        $raw = stream_get_contents($this->fsock, $this->decrypt_block_size);
+            if ($raw === false) {
-
+                $this->disconnect_helper(DisconnectReason::CONNECTION_LOST);
-        if (!strlen($raw)) {
+                throw new ConnectionClosedException('Connection closed by server');
-            $this->bitmap = 0;
+            } elseif (!strlen($raw)) {
-            throw new ConnectionClosedException('No data received from server');
+                continue;
             }
+            $packet->raw .= $raw;
+            if (!$packet->packet_length) {
+                $this->get_binary_packet_size($packet);
+            }
+        };
 
+        if (strlen($packet->raw) != $packet->size) {
+            throw new \RuntimeException('Size of packet was not expected length');
+        }
+        // destroy buffer as packet represents the entire payload and should be processed in full
+        $this->binary_packet_buffer = null;
+        // copy the raw payload, so as not to destroy original
+        $raw = $packet->raw;
+        if ($this->hmac_check instanceof Hash) {
+            $hmac = Strings::pop($raw, $this->hmac_size);
+        }
+        $packet_length_header_size = 4;
         if ($this->decrypt) {
             switch ($this->decryptName) {
                 case 'aes128-gcm@openssh.com':
@@ -3236,40 +3256,16 @@ class SSH2
                         $this->decryptInvocationCounter
                     );
                     Strings::increment_str($this->decryptInvocationCounter);
-                    $this->decrypt->setAAD($temp = Strings::shift($raw, 4));
+                    $this->decrypt->setAAD(Strings::shift($raw, $packet_length_header_size));
-                    extract(unpack('Npacket_length', $temp));
+                    $this->decrypt->setTag(Strings::pop($raw, $this->decrypt_block_size));
-                    /**
+                    $packet->plain = $this->decrypt->decrypt($raw);
-                     * @var integer $packet_length
-                     */
-
-                    $raw .= $this->read_remaining_bytes($packet_length - $this->decrypt_block_size + 4);
-                    $stop = microtime(true);
-                    $tag = stream_get_contents($this->fsock, $this->decrypt_block_size);
-                    $this->decrypt->setTag($tag);
-                    $raw = $this->decrypt->decrypt($raw);
-                    $raw = $temp . $raw;
-                    $remaining_length = 0;
                     break;
                 case 'chacha20-poly1305@openssh.com':
                     // This should be impossible, but we are checking anyway to narrow the type for Psalm.
                     if (!($this->decrypt instanceof ChaCha20)) {
                         throw new LogicException('$this->decrypt is not a ' . ChaCha20::class);
                     }
-
+                    $this->decrypt->setNonce(pack('N2', 0, $this->get_seq_no));
-                    $nonce = pack('N2', 0, $this->get_seq_no);
-
-                    $this->lengthDecrypt->setNonce($nonce);
-                    $temp = $this->lengthDecrypt->decrypt($aad = Strings::shift($raw, 4));
-                    extract(unpack('Npacket_length', $temp));
-                    /**
-                     * @var integer $packet_length
-                     */
-
-                    $raw .= $this->read_remaining_bytes($packet_length - $this->decrypt_block_size + 4);
-                    $stop = microtime(true);
-                    $tag = stream_get_contents($this->fsock, 16);
-
-                    $this->decrypt->setNonce($nonce);
                     $this->decrypt->setCounter(0);
                     // this is the same approach that's implemented in Salsa20::createPoly1305Key()
                     // but we don't want to use the same AEAD construction that RFC8439 describes
@@ -3277,79 +3273,55 @@ class SSH2
                     $this->decrypt->setPoly1305Key(
                         $this->decrypt->encrypt(str_repeat("\0", 32))
                     );
-                    $this->decrypt->setAAD($aad);
+                    $this->decrypt->setAAD(Strings::shift($raw, $packet_length_header_size));
                     $this->decrypt->setCounter(1);
-                    $this->decrypt->setTag($tag);
+                    $this->decrypt->setTag(Strings::pop($raw, 16));
-                    $raw = $this->decrypt->decrypt($raw);
+                    $packet->plain = $this->decrypt->decrypt($raw);
-                    $raw = $temp . $raw;
-                    $remaining_length = 0;
                     break;
                 default:
                     if (!$this->hmac_check instanceof Hash || !$this->hmac_check_etm) {
-                        $raw = $this->decrypt->decrypt($raw);
+                        // first block was already decrypted for contained packet_length header
+                        Strings::shift($raw, $this->decrypt_block_size);
+                        if (strlen($raw) > 0) {
+                            $packet->plain .= $this->decrypt->decrypt($raw);
+                        }
+                    } else {
+                        Strings::shift($raw, $packet_length_header_size);
+                        $packet->plain = $this->decrypt->decrypt($raw);
+                    }
                     break;
             }
-                    extract(unpack('Npacket_length', $temp = Strings::shift($raw, 4)));
+        } else {
-                    /**
+            Strings::shift($raw, $packet_length_header_size);
-                     * @var integer $packet_length
+            $packet->plain = $raw;
-                     */
-                    $raw .= $this->read_remaining_bytes($packet_length - $this->decrypt_block_size + 4);
-                    $stop = microtime(true);
-                    $encrypted = $temp . $raw;
-                    $raw = $temp . $this->decrypt->decrypt($raw);
-                    $remaining_length = 0;
         }
-        }
-
-        if (strlen($raw) < 5) {
-            $this->bitmap = 0;
-            throw new RuntimeException('Plaintext is too short');
-        }
-        extract(unpack('Npacket_length/Cpadding_length', Strings::shift($raw, 5)));
-        /**
-         * @var integer $packet_length
-         * @var integer $padding_length
-         */
-
-        if (!isset($remaining_length)) {
-            $remaining_length = $packet_length + 4 - $this->decrypt_block_size;
-        }
-
-        $buffer = $this->read_remaining_bytes($remaining_length);
-
-        if (!isset($stop)) {
-            $stop = microtime(true);
-        }
-        if (strlen($buffer)) {
-            $raw .= $this->decrypt ? $this->decrypt->decrypt($buffer) : $buffer;
-        }
-
-        $payload = Strings::shift($raw, $packet_length - $padding_length - 1);
-        $padding = Strings::shift($raw, $padding_length); // should leave $raw empty
-
         if ($this->hmac_check instanceof Hash) {
-            $hmac = stream_get_contents($this->fsock, $this->hmac_size);
-            if ($hmac === false || strlen($hmac) != $this->hmac_size) {
-                $this->disconnect_helper(DisconnectReason::MAC_ERROR);
-                throw new RuntimeException('Error reading socket');
-            }
-
             $reconstructed = !$this->hmac_check_etm ?
-                pack('NCa*', $packet_length, $padding_length, $payload . $padding) :
+                pack('Na*', $packet->packet_length, $packet->plain) :
-                $encrypted;
+                substr($packet->raw, 0, -$this->hmac_size);
             if (($this->hmac_check->getHash() & "\xFF\xFF\xFF\xFF") == 'umac') {
                 $this->hmac_check->setNonce("\0\0\0\0" . pack('N', $this->get_seq_no));
                 if ($hmac != $this->hmac_check->hash($reconstructed)) {
                     $this->disconnect_helper(DisconnectReason::MAC_ERROR);
-                    throw new RuntimeException('Invalid UMAC');
+                    throw new ConnectionClosedException('Invalid UMAC');
                 }
             } else {
                 if ($hmac != $this->hmac_check->hash(pack('Na*', $this->get_seq_no, $reconstructed))) {
                     $this->disconnect_helper(DisconnectReason::MAC_ERROR);
-                    throw new RuntimeException('Invalid HMAC');
+                    throw new ConnectionClosedException('Invalid HMAC');
                 }
             }
         }
+        $padding_length = 0;
+        $payload = $packet->plain;
+        extract(unpack('Cpadding_length', Strings::shift($payload, 1)));
+        if ($padding_length > 0) {
+            Strings::pop($payload, $padding_length);
+        }
+        if (empty($payload)) {
+            $this->disconnect_helper(DisconnectReason::PROTOCOL_ERROR);
+            throw new ConnectionClosedException('Plaintext is too short');
+        }
 
         switch ($this->decompress) {
             case self::NET_SSH2_COMPRESSION_ZLIB_AT_OPENSSH:
@@ -3404,62 +3376,69 @@ class SSH2
             $this->last_packet = $current;
         }
 
-        return $this->filter($payload, $skip_channel_filter);
+        return $this->filter($payload);
     }
 
     /**
-     * Read Remaining Bytes
+     * @param object $packet The packet object being constructed, passed by reference
-     *
+     *        The size, packet_length, and plain properties of this object may be modified in processing
-     * @return string
+     * @throws InvalidPacketLengthException if the packet length header is invalid
-     * @see self::get_binary_packet()
      */
-    private function read_remaining_bytes(int $remaining_length)
+    private function get_binary_packet_size(object &$packet): void
     {
-        if (!$remaining_length) {
+        $packet_length_header_size = 4;
-            return '';
+        if (strlen($packet->raw) < $packet_length_header_size) {
+            return;
         }
-
+        $packet_length = 0;
-        $adjustLength = false;
+        $added_validation_length = 0; // indicates when the packet length header is included when validating packet length against block size
         if ($this->decrypt) {
-            switch (true) {
+            switch ($this->decryptName) {
-                case $this->decryptName == 'aes128-gcm@openssh.com':
+                case 'aes128-gcm@openssh.com':
-                case $this->decryptName == 'aes256-gcm@openssh.com':
+                case 'aes256-gcm@openssh.com':
-                case $this->decryptName == 'chacha20-poly1305@openssh.com':
+                    extract(unpack('Npacket_length', substr($packet->raw, 0, $packet_length_header_size)));
-                case $this->hmac_check instanceof Hash && $this->hmac_check_etm:
+                    $packet->size = $packet_length_header_size + $packet_length + $this->decrypt_block_size; // expect tag
-                    $remaining_length += $this->decrypt_block_size - 4;
+                    break;
-                    $adjustLength = true;
+                case 'chacha20-poly1305@openssh.com':
+                    $this->lengthDecrypt->setNonce(pack('N2', 0, $this->get_seq_no));
+                    $packet_length_header = $this->lengthDecrypt->decrypt(substr($packet->raw, 0, $packet_length_header_size));
+                    extract(unpack('Npacket_length', $packet_length_header));
+                    $packet->size = $packet_length_header_size + $packet_length + 16; // expect tag
+                    break;
+                default:
+                    if (!$this->hmac_check instanceof Hash || !$this->hmac_check_etm) {
+                        if (strlen($packet->raw) < $this->decrypt_block_size) {
+                            return;
                         }
+                        $packet->plain = $this->decrypt->decrypt(substr($packet->raw, 0, $this->decrypt_block_size));
+                        extract(unpack('Npacket_length', Strings::shift($packet->plain, $packet_length_header_size)));
+                        $packet->size = $packet_length_header_size + $packet_length;
+                        $added_validation_length = $packet_length_header_size;
+                    } else {
+                        extract(unpack('Npacket_length', substr($packet->raw, 0, $packet_length_header_size)));
+                        $packet->size = $packet_length_header_size + $packet_length;
+                    }
+                    break;
+            }
+        } else {
+            extract(unpack('Npacket_length', substr($packet->raw, 0, $packet_length_header_size)));
+            $packet->size = $packet_length_header_size + $packet_length;
+            $added_validation_length = $packet_length_header_size;
         }
-
         // quoting <http://tools.ietf.org/html/rfc4253#section-6.1>,
         // "implementations SHOULD check that the packet length is reasonable"
         // PuTTY uses 0x9000 as the actual max packet size and so to shall we
-        // don't do this when GCM mode is used since GCM mode doesn't encrypt the length
+        if (
-        if ($remaining_length < -$this->decrypt_block_size || $remaining_length > 0x9000 || $remaining_length % $this->decrypt_block_size != 0) {
+            $packet_length <= 0 || $packet_length > 0x9000
-            if (!$this->bad_key_size_fix && self::bad_algorithm_candidate($this->decrypt ? $this->decryptName : '') && !($this->bitmap & SSH2::MASK_LOGIN)) {
+            || ($packet_length + $added_validation_length) % $this->decrypt_block_size != 0
-                $this->bad_key_size_fix = true;
+        ) {
-                $this->reset_connection(DisconnectReason::KEY_EXCHANGE_FAILED);
+            $this->disconnect_helper(DisconnectReason::PROTOCOL_ERROR);
-                return false;
+            throw new InvalidPacketLengthException('Invalid packet length');
         }
-            throw new RuntimeException('Invalid size');
+        if ($this->hmac_check instanceof Hash) {
+            $packet->size += $this->hmac_size;
         }
-
+        $packet->packet_length = $packet_length;
-        if ($adjustLength) {
-            $remaining_length -= $this->decrypt_block_size - 4;
-        }
-
-        $buffer = '';
-        while ($remaining_length > 0) {
-            $temp = stream_get_contents($this->fsock, $remaining_length);
-            if ($temp === false || feof($this->fsock)) {
-                $this->disconnect_helper(DisconnectReason::CONNECTION_LOST);
-                throw new RuntimeException('Error reading from socket');
-            }
-            $buffer .= $temp;
-            $remaining_length -= strlen($temp);
-        }
-
-        return $buffer;
     }
 
     /**
@@ -3470,7 +3449,7 @@ class SSH2
      * @return string|bool
      * @see self::_get_binary_packet()
      */
-    private function filter(string $payload, bool $skip_channel_filter)
+    private function filter(string $payload)
     {
         switch (ord($payload[0])) {
             case MessageType::DISCONNECT:
@@ -3481,14 +3460,14 @@ class SSH2
                 return false;
             case MessageType::IGNORE:
                 $this->extra_packets++;
-                $payload = $this->get_binary_packet($skip_channel_filter);
+                $payload = $this->get_binary_packet();
                 break;
             case MessageType::DEBUG:
                 $this->extra_packets++;
                 Strings::shift($payload, 2); // second byte is "always_display"
                 [$message] = Strings::unpackSSH2('s', $payload);
                 $this->errors[] = "SSH_MSG_DEBUG: $message";
-                $payload = $this->get_binary_packet($skip_channel_filter);
+                $payload = $this->get_binary_packet();
                 break;
             case MessageType::UNIMPLEMENTED:
                 return false;
@@ -3499,7 +3478,7 @@ class SSH2
                         $this->bitmap = 0;
                         return false;
                     }
-                    $payload = $this->get_binary_packet($skip_channel_filter);
+                    $payload = $this->get_binary_packet();
                 }
                 break;
             case MessageType::EXT_INFO:
@@ -3535,19 +3514,9 @@ class SSH2
                             if (ord(substr($payload, 9 + $length))) { // want reply
                                 $this->send_binary_packet(pack('CN', MessageType::CHANNEL_SUCCESS, $this->server_channels[$channel]));
                             }
-                            $payload = $this->get_binary_packet($skip_channel_filter);
-                        }
-                    }
-                    break;
-                case MessageType::CHANNEL_DATA:
-                case MessageType::CHANNEL_EXTENDED_DATA:
-                case MessageType::CHANNEL_CLOSE:
-                case MessageType::CHANNEL_EOF:
-                    if (!$skip_channel_filter && !empty($this->server_channels)) {
-                        $this->binary_packet_buffer = $payload;
-                        $this->get_channel_packet(true);
                             $payload = $this->get_binary_packet();
                         }
+                    }
                     break;
                 case MessageType::GLOBAL_REQUEST: // see http://tools.ietf.org/html/rfc4254#section-4
                     Strings::shift($payload, 1);
@@ -3560,7 +3529,7 @@ class SSH2
                         return $this->disconnect_helper(DisconnectReason::BY_APPLICATION);
                     }
 
-                    $payload = $this->get_binary_packet($skip_channel_filter);
+                    $payload = $this->get_binary_packet();
                     break;
                 case MessageType::CHANNEL_OPEN: // see http://tools.ietf.org/html/rfc4254#section-5.1
                     Strings::shift($payload, 1);
@@ -3613,7 +3582,7 @@ class SSH2
                             }
                     }
 
-                    $payload = $this->get_binary_packet($skip_channel_filter);
+                    $payload = $this->get_binary_packet();
                     break;
                 case MessageType::CHANNEL_WINDOW_ADJUST:
                     Strings::shift($payload, 1);
@@ -3621,7 +3590,7 @@ class SSH2
 
                     $this->window_size_client_to_server[$channel] += $window_size;
 
-                    $payload = ($this->bitmap & self::MASK_WINDOW_ADJUST) ? true : $this->get_binary_packet($skip_channel_filter);
+                    $payload = ($this->bitmap & self::MASK_WINDOW_ADJUST) ? true : $this->get_binary_packet();
             }
         }
 
@@ -3726,11 +3695,7 @@ class SSH2
         }
 
         while (true) {
-            if ($this->binary_packet_buffer !== false) {
+            $response = $this->get_binary_packet();
-                $response = $this->binary_packet_buffer;
-                $this->binary_packet_buffer = false;
-            } else {
-                $response = $this->get_binary_packet(true);
             if ($response === true && $this->is_timeout) {
                 if ($client_channel == self::CHANNEL_EXEC && !$this->request_pty) {
                     $this->close_channel($client_channel);
@@ -3741,8 +3706,6 @@ class SSH2
                 $this->disconnect_helper(DisconnectReason::CONNECTION_LOST);
                 throw new ConnectionClosedException('Connection closed by server');
             }
-            }
-
             if ($client_channel == -1 && $response === true) {
                 return true;
             }
@@ -4074,6 +4037,18 @@ class SSH2
         }
     }
 
+    /**
+     * Sends a keep-alive message, if keep-alive is enabled and interval is met
+     */
+    private function send_keep_alive()
+    {
+        $elapsed = microtime(true) - $this->keep_alive_sent;
+        if ($this->keepAlive > 0 && $elapsed >= $this->keepAlive) {
+            $this->keep_alive_sent = microtime(true);
+            $this->send_binary_packet(pack('CN', NET_SSH2_MSG_IGNORE, 0));
+        }
+    }
+
     /**
      * Logs data packets
      *
@@ -4272,10 +4247,7 @@ class SSH2
             }
         }
 
-        $this->bitmap = 0;
+        $this->reset_connection();
-        if (is_resource($this->fsock) && get_resource_type($this->fsock) === 'stream') {
-            fclose($this->fsock);
-        }
 
         return false;
     }

tests/Functional/Net/SSH2Test.php

@@ -10,6 +10,7 @@ declare(strict_types=1);
 
 namespace phpseclib3\Tests\Functional\Net;
 
+use phpseclib3\Exception\NoSupportedAlgorithmsException;
 use phpseclib3\Net\SSH2;
 use phpseclib3\Tests\PhpseclibFunctionalTestCase;
 
@@ -555,4 +556,96 @@ class SSH2Test extends PhpseclibFunctionalTestCase
         $ssh->reset(SSH2::CHANNEL_SHELL);
         $this->assertSame(0, $ssh->getOpenChannelCount());
     }
+
+    public function testPing()
+    {
+        $ssh = $this->getSSH2();
+        // assert on unauthenticated ssh2
+        $this->assertNotEmpty($ssh->getServerIdentification());
+        $this->assertFalse($ssh->ping());
+        $this->assertTrue($ssh->isConnected());
+        $this->assertSame(0, $ssh->getOpenChannelCount());
+
+        $ssh = $this->getSSH2Login();
+        $this->assertTrue($ssh->ping());
+        $this->assertSame(0, $ssh->getOpenChannelCount());
+    }
+
+    /**
+     * @return array
+     */
+    public static function getCryptoAlgorithms()
+    {
+        $map = [
+            'kex' => SSH2::getSupportedKEXAlgorithms(),
+            'hostkey' => SSH2::getSupportedHostKeyAlgorithms(),
+            'comp' => SSH2::getSupportedCompressionAlgorithms(),
+            'crypt' => SSH2::getSupportedEncryptionAlgorithms(),
+            'mac' => SSH2::getSupportedMACAlgorithms(),
+        ];
+        $tests = [];
+        foreach ($map as $type => $algorithms) {
+            foreach ($algorithms as $algorithm) {
+                $tests[] = [$type, $algorithm];
+            }
+        }
+        return $tests;
+    }
+
+    /**
+     * @dataProvider getCryptoAlgorithms
+     * @param string $type
+     * @param string $algorithm
+     */
+    public function testCryptoAlgorithms($type, $algorithm)
+    {
+        $ssh = $this->getSSH2();
+        try {
+            switch ($type) {
+                case 'kex':
+                case 'hostkey':
+                    $ssh->setPreferredAlgorithms([$type => [$algorithm]]);
+                    $this->assertEquals($algorithm, $ssh->getAlgorithmsNegotiated()[$type]);
+                    break;
+                case 'comp':
+                case 'crypt':
+                    $ssh->setPreferredAlgorithms([
+                        'client_to_server' => [$type => [$algorithm]],
+                        'server_to_client' => [$type => [$algorithm]],
+                    ]);
+                    $this->assertEquals($algorithm, $ssh->getAlgorithmsNegotiated()['client_to_server'][$type]);
+                    $this->assertEquals($algorithm, $ssh->getAlgorithmsNegotiated()['server_to_client'][$type]);
+                    break;
+                case 'mac':
+                    $macCryptAlgorithms = array_filter(
+                        SSH2::getSupportedEncryptionAlgorithms(),
+                        function ($algorithm) use ($ssh) {
+                            return !self::callFunc($ssh, 'encryption_algorithm_to_crypt_instance', [$algorithm])
+                                ->usesNonce();
+                        }
+                    );
+                    $ssh->setPreferredAlgorithms([
+                        'client_to_server' => ['crypt' => $macCryptAlgorithms, 'mac' => [$algorithm]],
+                        'server_to_client' => ['crypt' => $macCryptAlgorithms, 'mac' => [$algorithm]],
+                    ]);
+                    $this->assertEquals($algorithm, $ssh->getAlgorithmsNegotiated()['client_to_server']['mac']);
+                    $this->assertEquals($algorithm, $ssh->getAlgorithmsNegotiated()['server_to_client']['mac']);
+                    break;
+            }
+        } catch (NoSupportedAlgorithmsException $e) {
+            self::markTestSkipped("{$type} algorithm {$algorithm} is not supported by server");
+        }
+
+        $username = $this->getEnv('SSH_USERNAME');
+        $password = $this->getEnv('SSH_PASSWORD');
+        $this->assertTrue(
+            $ssh->login($username, $password),
+            "SSH2 login using {$type} {$algorithm} failed."
+        );
+
+        $ssh->setTimeout(1);
+        $ssh->write("pwd\n");
+        $this->assertNotEmpty($ssh->read('', SSH2::READ_NEXT));
+        $ssh->disconnect();
+    }
 }