Issue with isConnected/isAuthenticated after remote terminal restart

[Martellini90]

Description of the Issue:
I am encountering an issue with the phpseclib library regarding the isConnected and isAuthenticated methods. After restarting the remote terminal, these methods continue to return true, even though logically the connection should be lost.

Additional Details:

• phpseclib version in use: [3.0.35]
• Remote operating system: [Windows 10]

Steps to Reproduce the Issue:

• Initialize the SSH connection with the phpseclib library.
• Perform operations on the remote terminal.
• Restart the remote terminal.
• Check the connection status with the isConnected and isAuthenticated methods.

Expected Behavior:
After restarting the remote terminal, it is expected that the isConnected and isAuthenticated methods return false to indicate the loss of connection.

Thank you for your assistance in resolving this problem.

Best regards,

[terrafrost]

In-so-far as socket tests go isConnected() does this:

is_resource($this->fsock) && !feof($this->fsock)

If feof() isn't returning true when a server is restarted then one could argue that that's a PHP problem but tbh idk. https://stackoverflow.com/a/1321716/569976 suggests that an end-of-file usually means that the other side has closed the connection but that doesn't mean that it sole way to verify the closure of a connection. In theory something like ping() could be done. ping() tries to send a channel open request and then immediately closes it. If it's unable to do so it reconnects.

One problem with ping() is that, technically, if a server only supports one channel (eg. some routers), and you're in the middle of an interactive shell or the middle of an SFTP session, then the failure of ping() doesn't necessarily mean that you've been disconnected.

I suppose an SSH_MSG_IGNORE could be sent, instead. That one wouldn't require a response from the server but it's easy to imagine scenarios wherein the server would have still "closed" the connection even if the client was able to successfully send packets (phpseclib's setKeepAlive() sends out SSH_MSG_IGNORE messages with some caveats).

Really, I don't think there's a perfect way to implement isConnected() and, tbh, as is, I think it's roughly equivalent to a wife asking her husband "is the front door locked?". Is the husband supposed to go home, every time she asks, and physically turn the handle to make sure the door is locked, or should he just be able to say it based on the information that he currently has? I mean, technically, even if the husband did lock the door when he left what if, say, that couples 10yo kid decided to unlock the door, check the mail or some such, and then forgot to lock it upon reentry? Expecting the husband to go back to the house to verify the physical condition of the door knob every time he's asked, even if he explicitly does remember locking it, is a bit overkill.

What if I add a sendDummyPacket() method or some such?

[terrafrost]

libssh's ssh_is_connected() method appears to behave as phpseclib's isConnected() method:

https://git.libssh.org/projects/libssh.git/tree/src/session.c?id=3e2bbbc96a81909c0949b89fb14361122248ff28#n581

In fact, phpseclib's isConnected() method appears to be more solid then libssh's because of phpseclib's doing feof($this->fsock) whereas libssh doesn't appear to be doing anything comparable to that.

[terrafrost]

I suppose something else I could do is to give isConnected() a $level option that defaults to 0.

• $level == 0 would mean that phpseclib is performing the test as is.
• $level == 1 would involve phpseclib sending an SSH_MSG_IGNORE packet
• $level == 2 would involve phpseclib sending an SSH_MSG_CHANNEL_OPEN packet and then immediately trying to close that packet

[terrafrost]

How does terrafrost@d27429a look to you?

I'll merge it into the 3.0 branch in a few days unless you (or anyone really lol) reply sooner then that with some reason as to why I shouldn't!

(also, I don't like openChannel / close_channel but I can fix that on a different commit; the goal of this commit is to change the behavior is isConnected - not to make function names more consistent)

[terrafrost]

terrafrost@d27429a has been merged.