Only reset the serveralive check when we receive traffic from the server

and ignore traffic from a port forwarding client, preventing a client from
keeping a connection alive when it should be terminated.  Based on a patch
from jxraynor at gmail.com via openssh-unix-dev and bz#2265, ok djm@

1 files changed, 22 insertions, 14 deletions

diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c
index d0598da0fab..61e48afd6b6 100644
--- a/usr.bin/ssh/clientloop.c
+++ b/usr.bin/ssh/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.344 2020/04/24 02:19:40 dtucker Exp $ */
+/* $OpenBSD: clientloop.c,v 1.345 2020/07/03 05:09:06 dtucker Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -155,6 +155,7 @@ static int connection_out;	/* Connection to server (output). */
 static int need_rekeying;	/* Set to non-zero if rekeying is requested. */
 static int session_closed;	/* In SSH2: login session closed. */
 static u_int x11_refuse_time;	/* If >0, refuse x11 opens after this time. */
+static time_t server_alive_time;	/* Time to do server_alive_check */
 
 static void client_init_dispatch(struct ssh *ssh);
 int	session_ident = -1;
@@ -460,6 +461,13 @@ client_global_request_reply(int type, u_int32_t seq, struct ssh *ssh)
 }
 
 static void
+schedule_server_alive_check(void)
+{
+	if (options.server_alive_interval > 0)
+		server_alive_time = monotime() + options.server_alive_interval;
+}
+
+static void
 server_alive_check(struct ssh *ssh)
 {
 	int r;
@@ -475,6 +483,7 @@ server_alive_check(struct ssh *ssh)
 		fatal("%s: send packet: %s", __func__, ssh_err(r));
 	/* Insert an empty placeholder to maintain ordering */
 	client_register_global_confirm(NULL, NULL);
+	schedule_server_alive_check();
 }
 
 /*
@@ -488,7 +497,7 @@ client_wait_until_can_do_something(struct ssh *ssh,
 {
 	struct timeval tv, *tvp;
 	int timeout_secs;
-	time_t minwait_secs = 0, server_alive_time = 0, now = monotime();
+	time_t minwait_secs = 0, now = monotime();
 	int r, ret;
 
 	/* Add any selections by the channel mechanism. */
@@ -517,10 +526,8 @@ client_wait_until_can_do_something(struct ssh *ssh,
 	 */
 
 	timeout_secs = INT_MAX; /* we use INT_MAX to mean no timeout */
-	if (options.server_alive_interval > 0) {
-		timeout_secs = options.server_alive_interval;
-		server_alive_time = now + options.server_alive_interval;
-	}
+	if (options.server_alive_interval > 0)
+		timeout_secs = MAXIMUM(server_alive_time - now, 0);
 	if (options.rekey_interval > 0 && !rekeying)
 		timeout_secs = MINIMUM(timeout_secs,
 		    ssh_packet_get_rekey_timeout(ssh));
@@ -550,7 +557,6 @@ client_wait_until_can_do_something(struct ssh *ssh,
 		 */
 		memset(*readsetp, 0, *nallocp);
 		memset(*writesetp, 0, *nallocp);
-
 		if (errno == EINTR)
 			return;
 		/* Note: we might still have data in the buffers. */
@@ -558,15 +564,14 @@ client_wait_until_can_do_something(struct ssh *ssh,
 		    "select: %s\r\n", strerror(errno))) != 0)
 			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 		quit_pending = 1;
-	} else if (ret == 0) {
+	} else if (options.server_alive_interval > 0 && !FD_ISSET(connection_in,
+	     *readsetp) && monotime() >= server_alive_time)
 		/*
-		 * Timeout.  Could have been either keepalive or rekeying.
-		 * Keepalive we check here, rekeying is checked in clientloop.
+		 * ServerAlive check is needed. We can't rely on the select
+		 * timing out since traffic on the client side such as port
+		 * forwards can keep waking it up.
 		 */
-		if (server_alive_time != 0 && server_alive_time <= monotime())
-			server_alive_check(ssh);
-	}
-
+		server_alive_check(ssh);
 }
 
 static void
@@ -606,6 +611,7 @@ client_process_net_input(struct ssh *ssh, fd_set *readset)
 	 * the packet subsystem.
 	 */
 	if (FD_ISSET(connection_in, readset)) {
+		schedule_server_alive_check();
 		/* Read as much as possible. */
 		len = read(connection_in, buf, sizeof(buf));
 		if (len == 0) {
@@ -1306,6 +1312,8 @@ client_loop(struct ssh *ssh, int have_pty, int escape_char_arg,
 		    client_channel_closed, 0);
 	}
 
+	schedule_server_alive_check();
+
 	/* Main loop of the client for the interactive session mode. */
 	while (!quit_pending) {