Bug 2917

Created attachment 3193 [details]
Update last_client_time on timeout

I've discovered a bug in serverloop.c(function=wait_until_can_do_something)!
With latest openssh (7.8p1 as well as current master) sshd disconnects a non-responding client after approximately: ((ClientAliveCountMax / 2) * ClientAliveInterval) seconds

I did a bisect which showed that the fix introduced for bz#2756 causes this behavior: https://bugzilla.mindrot.org/show_bug.cgi?id=2756

How to reproduce:
 1. server #> /sbin/sshd -p 2020 -ddd -f ${sshd_config} 2>&1 | ts
 2. client $> ssh $IP -p2020
 3. close e.g. notebook of the client connection and wait for the timeout to happen (maybe killing the client process is already enough, didn't test it).

${sshd_config}
----
TCPKeepAlive no
ClientAliveInterval 15
ClientAliveCountMax 8
----

The debug log of sshd shows:
----
...
[2018-04-26 11:59:35] debug3: /tmp/sshd_config:94 setting TCPKeepAlive no
[2018-04-26 11:59:35] debug3: /tmp/sshd_config:98 setting ClientAliveInterval 15
[2018-04-26 11:59:35] debug3: /tmp/sshd_config:99 setting ClientAliveCountMax 8
...
[2018-04-26 12:00:16] debug2: channel 0: request keepalive@openssh.com confirm 1
[2018-04-26 12:00:16] debug2: channel 0: request keepalive@openssh.com confirm 1
[2018-04-26 12:00:31] debug2: channel 0: request keepalive@openssh.com confirm 1
[2018-04-26 12:00:31] debug2: channel 0: request keepalive@openssh.com confirm 1
[2018-04-26 12:00:46] debug2: channel 0: request keepalive@openssh.com confirm 1
[2018-04-26 12:00:46] debug2: channel 0: request keepalive@openssh.com confirm 1
[2018-04-26 12:01:01] debug2: channel 0: request keepalive@openssh.com confirm 1
[2018-04-26 12:01:01] debug2: channel 0: request keepalive@openssh.com confirm 1
[2018-04-26 12:01:16] Timeout, client not responding from user $USER x.x.x.x port xxxxx
----

keepalive packets are sent twice on every interval. I think the problem is that if a timeout of the select call in function=wait_until_can_do_something happens the next select call will return immediately with data contained in 'writesetp' which causes to send another keepalive packet since variable=last_client_time wasn't updated.

A very naive patch which solves the problem for me is attached. However, there might a better solution for this problem!