Configuration file TCPKeepAlive option does not work reliably

[Björn Grönvall]

On request from John Hawkinson I'm reposting this with additional information and not relying on attachments.

Hi!

There are configuration knobs (TCPKeepAlive) to enable/disable the use of TCP keepalives both in the ssh client and server. Unfortunately some UNIX systems default to SO_KEEPALIVE=on and some to =off. This may even be settable on a per host basis (OpenBSD default net.inet.tcp.always_keepalive=1 ???).

For the TCPKeepAlive configuration knob I would like to propose changes along the lines showed in the attached patch (there is perhaps more elegant ways to do this). The patch will always set SO_KEEPALIVE according to the ssh configuration files regardless of the local UNIX TCP defaults.

If there are questions or comments I'm available on this email address bg at sics.se (I'm not on any ssh email list).

Additional information comes here:

> You don't explain *why* you propose this change.

I am a mobile user and at all times have a ssh login on a particular machine, TCP keepalives drops the TCP connection if my laptop is not connected for some 2h.

Therefore, in the server /etc/ssh/sshd_config I have (amongst other things):

TCPKeepAlive no
ClientAliveInterval 90000

This turns off TCP keepalives and enables a ClientAlive probe after 25h of inactivity.

Unfortunately the code implicitly ASSUMES that all systems default to SO_KEEPALIVE=off which is not true. E.g Free and OpenBSD has sysctls net.inet.tcp.always_keepalive that defaults to SO_KEEPALIVE=on. If I write 'TCPKeepAlive no' in a configuration file SO_KEEPALIVE will not be changed to off because the relevant call to setsockopt will never happen. The code is written to change  SO_KEEPALIVE to on, never to change SO_KEEPALIVE to off.

> (Your patch is also missing.)

Guess the MTA removed attachments :-(, anyways I paste the diff inline but that may make whitespace changes and that's why I always attach patches (I also try it as an attachment again).

--- sshconnect.c~       2011-11-30 13:47:36.000000000 +0100
+++ sshconnect.c        2012-01-06 18:21:50.138571927 +0100
@@ -431,10 +431,11 @@
       debug("Connection established.");

       /* Set SO_KEEPALIVE if requested. */
-       if (want_keepalive &&
-           setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, (void *)&on,
+       on = want_keepalive ? 1 : 0;
+       if (setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, (void *)&on,
           sizeof(on)) < 0)
               error("setsockopt SO_KEEPALIVE: %.100s", strerror(errno));
+       on = 1;

       /* Set the connection. */
       packet_set_connection(sock, sock);
--- sshd.c~     2011-11-30 13:47:36.000000000 +0100
+++ sshd.c      2012-01-06 18:21:37.312969797 +0100
@@ -1910,9 +1910,11 @@
       packet_set_server();

       /* Set SO_KEEPALIVE if requested. */
-       if (options.tcp_keep_alive && packet_connection_is_on_socket() &&
+       on = options.tcp_keep_alive ? 1 : 0;
+       if (packet_connection_is_on_socket() &&
           setsockopt(sock_in, SOL_SOCKET, SO_KEEPALIVE, &on, sizeof(on)) < 0)
               error("setsockopt SO_KEEPALIVE: %.100s", strerror(errno));
+       on = 1;

       if ((remote_port = get_remote_port()) < 0) {
               debug("get_remote_port failed");




ssh-sample-keepalive.patch (attached again):
-------------- next part --------------