[ssh] Implement SSH keepalive instead of infinite timeout - #5238
Conversation
Waits for channel data with an explicit timeout, independently of the session timeout.
Sends the keepalive@openssh.com global request and waits for the reply. Declared in libssh/server.h, but works on client sessions too.
Default the poll to "data available" and the keepalive to SSH_OK, so tests with mocked channels keep reading as before.
stop() runs on a different thread than run(), which reads the flag.
run() blocked in sftp_get_client_message(), whose read is bounded by the session timeout. That is why the timeout is infinite today: an idle mount can be silent for hours, and a timed-out read looks like a dead sshfs. Poll the channel with an explicit 30s interval and only read once data is available. When the interval elapses, send a keepalive instead, so the connection carries traffic and a dead peer shows up as a poll error. Poll errors take the existing null-message path.
The infinite value only existed so the SFTP server's blocking reads would never give up on an idle mount. The server now polls and sends keepalives, so bound every blocking libssh call instead of letting it hang forever on an unresponsive peer.
xmkg
changed the title
|
@jimporter, the auto-assignment picked you -- feel free to reassign if you feel like there are too many reviews pending on you already. |
|
I noticed that @tobe2098 already filed an issue for this. I think it'd make sense to ask him for a review :) So I stumbled upon this issue today, and I know the history behind the infinite SSH established timeout. It was basically there to prevent inactive SFTP sessions from being dropped, but on the other hand, it causes other bugs. I've implemented a keep-alive system and switched to bounded poll reads. Let me know what your impressions are. |
|
Also, #5237 is complimentary. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5238 +/- ##
==========================================
+ Coverage 72.62% 72.63% +0.02%
==========================================
Files 337 337
Lines 18190 18202 +12
==========================================
+ Hits 13208 13220 +12
Misses 4982 4982 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Description
The SSH session timeout (
SSH_OPTIONS_TIMEOUT) has been infinite since 67bb888, so that the SFTP server backing sshfs mounts never gives up its blockingsftp_get_client_message()read while a mount sits idle. The side effect is that every other blocking libssh call (auth, channel open/exec, sftp round trips, writes) can hang forever on an unresponsive peer.This PR makes the SFTP server independent of the session timeout and then bounds the timeout.
Related Issue(s)
Relates to #5080
Testing
Manual testing steps (Linux, QEMU backend):
sshfs_serverfrom this branch against a fresh instance (withmultipass-sshfsinstalled) and mount a directory; write and read files from both sides.LogLevel DEBUG1in the instance,journalctl _PID=<sshd user child>showsserver_input_global_request: rtype keepalive@openssh.comevery 30s; TCP counters on the connection change only at those points.sshfs_server: it stops within a second with exit code 0 and the instance unmounts.multipass stopthe instance while mounted:sshfs_serverexits with failure through the existing "SFTP server thread stopped unexpectedly" path.Screenshots (if applicable)
N/A
Checklist
Additional Notes
ServerAliveCountMax) is not implemented; a silently vanished peer is detected once TCP retransmissions of the unacked keepalive give up. Can be a follow-up if wanted.