Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle SSH servers that send success response out of spec order #1786

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cabloo
Copy link

@cabloo cabloo commented Apr 23, 2022

There are some Juniper switches that have a (maybe incorrect?) implementation of the SSH spec which send back a SUCCESS response at a time this lib does not expect it. This causes the lib to bail out immediately. This patch ensures that when this unexpected SSH response comes back, we just continue operations as usual.

Here's the last bit of the SSH debug output when this behavior was happening:

SSH Error: Error reading channel data (99)

-> NET_SSH2_MSG_REQUEST_FAILURE (since last: 0.0001, network: 0.0001s)
                                                 

<- NET_SSH2_MSG_CHANNEL_OPEN_CONFIRMATION (since last: 0.2641, network: 0s)
00000000  00:00:00:02:00:00:00:00:00:00:00:00:00:00:80:00  ................

-> NET_SSH2_MSG_CHANNEL_REQUEST (since last: 0.0001, network: 0s)
00000000  00:00:00:00:00:00:00:07:70:74:79:2d:72:65:71:01  ........pty-req.
00000010  00:00:00:05:76:74:31:30:30:00:00:03:e8:00:00:00  ....vt100.......
00000020  18:00:00:00:00:00:00:00:00:00:00:00:01:00        ..............

<- NET_SSH2_MSG_CHANNEL_SUCCESS (since last: 0.2727, network: 0s)
00000000  00:00:00:02                                      ....

-> NET_SSH2_MSG_CHANNEL_REQUEST (since last: 0.0001, network: 0s)
00000000  00:00:00:00:00:00:00:05:73:68:65:6c:6c:01        ........shell.

<- NET_SSH2_MSG_CHANNEL_WINDOW_ADJUST (since last: 0.2672, network: 0s)
00000000  00:00:00:02:00:20:00:00                          ..... ..

-> NET_SSH2_MSG_CHANNEL_DATA (since last: 0.0001, network: 0s)
00000000  00:00:00:00:00:00:00:09:0a:0a:0a:0a:0a:0a:0a:0a  ................
00000010  0a                                               .

<- NET_SSH2_MSG_CHANNEL_SUCCESS (since last: 0.0001, network: 0s)
00000000  00:00:00:02                                      ....

-> NET_SSH2_MSG_DISCONNECT (since last: 0, network: 0s)
00000000  00:00:00:0b:00:00:00:00:00:00:00:00              ............


#0 /var/www/html/vendor/phpseclib/phpseclib/phpseclib/Net/SSH2.php(3138): phpseclib3\Net\SSH2->get_channel_packet(2)

@terrafrost
Copy link
Member

I'm assuming you're doing $ssh->write("\n\n\n\n\n\n\n\n\n");? Can you post the full SSH2 logs?

In theory phpseclib should request a second binary packet when it encounters a window adjust channel packet:

case MessageType::CHANNEL_WINDOW_ADJUST:
Strings::shift($payload, 1);
list($channel, $window_size) = Strings::unpackSSH2('NN', $payload);
$this->window_size_client_to_server[$channel] += $window_size;
$payload = ($this->bitmap & self::MASK_WINDOW_ADJUST) ? true : $this->get_binary_packet($skip_channel_filter);

That's from the filter() method, which is called in get_binary_packet(). The only time $this->bitmap & self::MASK_WINDOW_ADJUST is true is in send_binary_packet():

if (!$this->window_size_client_to_server[$client_channel]) {
$this->bitmap ^= self::MASK_WINDOW_ADJUST;
// using an invalid channel will let the buffers be built up for the valid channels
$this->get_channel_packet(-1);
$this->bitmap ^= self::MASK_WINDOW_ADJUST;
}

Consequently, the change you made shouldn't be necessary. If the code I've mentioned isn't working then that may indicate other problems that this change kinda sweeps under the rug.

@terrafrost
Copy link
Member

I'd still love to see log files!

Like I discussed in my last post the concern I have with your PR is that there's already code in place that should deal with the situation you're describing and if that's not working then that's quite potentially a harbinger for other issues that your fix wouldn't address.

Thanks!

@cabloo
Copy link
Author

cabloo commented May 3, 2022

I'm assuming you're doing $ssh->write("\n\n\n\n\n\n\n\n\n");?

That's correct, I do this to determine the host line (the part that gets output when you put a few empty lines in) and differentiate that from the MOTD/any broadcast messages.

Can you post the full SSH2 logs?

Wasn't sure if there was anything sensitive in there, so I've sent via email.

@terrafrost
Copy link
Member

I tried to "replay" the logs you gave me without success. That's not exactly an exact science however.

In light of my inability to reproduce them... maybe you could try this?:

#
#-----[ OPEN ]------------------------------------------
#
Net/SSH2.php
#
#-----[ FIND ]------------------------------------------
#
                case NET_SSH2_MSG_CHANNEL_WINDOW_ADJUST:
                    Strings::shift($payload, 1);
                    list($channel, $window_size) = Strings::unpackSSH2('NN', $payload);

                    $this->window_size_client_to_server[$channel] += $window_size;

                    $payload = ($this->bitmap & self::MASK_WINDOW_ADJUST) ? true : $this->get_binary_packet($skip_channel_filter);
#
#-----[ REPLACE WITH ]----------------------------------
#
                case NET_SSH2_MSG_CHANNEL_WINDOW_ADJUST:
echo "THIS FAR\n";
                    Strings::shift($payload, 1);
                    list($channel, $window_size) = Strings::unpackSSH2('NN', $payload);

                    $this->window_size_client_to_server[$channel] += $window_size;

                    $payload = ($this->bitmap & self::MASK_WINDOW_ADJUST) ? true : $this->get_binary_packet($skip_channel_filter);
var_dump($payload);

You can turn off logging and just run it with those changes. They won't fix anything but it'll at least tell me if that block of code is being called. We can proceed from there.

Thanks!

@terrafrost
Copy link
Member

Try doing this too:

#
#-----[ OPEN ]------------------------------------------
#
Net/SSH2.php
#
#-----[ FIND ]------------------------------------------
# in initShell
#
        $packet = Strings::packSSH2(
            'CNsb',
            NET_SSH2_MSG_CHANNEL_REQUEST,
            $this->server_channels[self::CHANNEL_SHELL],
            'shell',
            true // want reply
        );
        $this->send_binary_packet($packet);

        $response = $this->get_channel_packet(self::CHANNEL_SHELL);
        if ($response === false) {
            throw new \RuntimeException('Unable to request shell');
        }

        $this->channel_status[self::CHANNEL_SHELL] = NET_SSH2_MSG_CHANNEL_DATA;
#
#-----[ REPLACE WITH ]----------------------------------
#
        $packet = Strings::packSSH2(
            'CNsb',
            NET_SSH2_MSG_CHANNEL_REQUEST,
            $this->server_channels[self::CHANNEL_SHELL],
            'shell',
            true // want reply
        );
        $this->send_binary_packet($packet);
echo "BINARY PACKET SENT\n";

        $response = $this->get_channel_packet(self::CHANNEL_SHELL);
echo "CHANNEL PACKET RCVD\n";
var_dump($response);
        if ($response === false) {
            throw new \RuntimeException('Unable to request shell');
        }

        $this->channel_status[self::CHANNEL_SHELL] = NET_SSH2_MSG_CHANNEL_DATA;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants