From c884e5000e881f45e5c2328e219eebd07b0560ca Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Wed, 9 Jul 2014 00:15:20 +0800
Subject: [PATCH] Make -K keepalive behave like OpenSSH's ServerAliveInterval

---
 cli-runopts.c    |  2 ++
 common-session.c | 52 +++++++++++++++++++++++++++++++++++++-----------
 options.h        |  5 +++++
 packet.c         | 25 +++++++++++++----------
 process-packet.c | 14 +++++++++++--
 runopts.h        |  4 ++--
 session.h        | 11 ++++++----
 svr-session.c    |  2 --
 8 files changed, 82 insertions(+), 33 deletions(-)

diff --git a/cli-runopts.c b/cli-runopts.c
index d8b250a..f8e289b 100644
--- a/cli-runopts.c
+++ b/cli-runopts.c
@@ -163,6 +163,8 @@ void cli_getopts(int argc, char ** argv) {
 	opts.ipv6 = 1;
 	*/
 	opts.recv_window = DEFAULT_RECV_WINDOW;
+	opts.keepalive_secs = DEFAULT_KEEPALIVE;
+	opts.idle_timeout_secs = DEFAULT_IDLE_TIMEOUT;
 
 	fill_own_user();
 
diff --git a/common-session.c b/common-session.c
index e23be66..90129b4 100644
--- a/common-session.c
+++ b/common-session.c
@@ -51,6 +51,7 @@ int exitflag = 0; /* GLOBAL */
 
 /* called only at the start of a session, set up initial state */
 void common_session_init(int sock_in, int sock_out) {
+	time_t now;
 
 	TRACE(("enter session_init"))
 
@@ -58,9 +59,12 @@ void common_session_init(int sock_in, int sock_out) {
 	ses.sock_out = sock_out;
 	ses.maxfd = MAX(sock_in, sock_out);
 
-	ses.connect_time = 0;
-	ses.last_trx_packet_time = 0;
-	ses.last_packet_time = 0;
+	now = monotonic_now();
+	ses.connect_time = now;
+	ses.last_packet_time_keepalive_recv = now;
+	ses.last_packet_time_idle = now;
+	ses.last_packet_time_any_sent = 0;
+	ses.last_packet_time_keepalive_sent = 0;
 	
 	if (pipe(ses.signal_pipe) < 0) {
 		dropbear_exit("Signal pipe failed");
@@ -387,11 +391,21 @@ static int ident_readln(int fd, char* buf, int count) {
 	return pos+1;
 }
 
-void send_msg_ignore() {
+static void send_msg_keepalive() {
 	CHECKCLEARTOWRITE();
-	buf_putbyte(ses.writepayload, SSH_MSG_IGNORE);
-	buf_putstring(ses.writepayload, "", 0);
+	time_t old_time_idle = ses.last_packet_time_idle;
+	/* Try to force a response from the other end. Some peers will
+	reply with SSH_MSG_REQUEST_FAILURE, some will reply with SSH_MSG_UNIMPLEMENTED */
+	buf_putbyte(ses.writepayload, SSH_MSG_GLOBAL_REQUEST);
+	/* A short string */
+	buf_putstring(ses.writepayload, "k@dropbear.nl", 0);
+	buf_putbyte(ses.writepayload, 1); /* want_reply */
 	encrypt_packet();
+
+	ses.last_packet_time_keepalive_sent = monotonic_now();
+
+	/* keepalives shouldn't update idle timeout, reset it back */
+	ses.last_packet_time_idle = old_time_idle;
 }
 
 /* Check all timeouts which are required. Currently these are the time for
@@ -401,7 +415,7 @@ static void checktimeouts() {
 	time_t now;
 	now = monotonic_now();
 	
-	if (ses.connect_time != 0 && now - ses.connect_time >= AUTH_TIMEOUT) {
+	if (now - ses.connect_time >= AUTH_TIMEOUT) {
 			dropbear_close("Timeout before auth");
 	}
 
@@ -417,13 +431,27 @@ static void checktimeouts() {
 		send_msg_kexinit();
 	}
 	
-	if (opts.keepalive_secs > 0 
-			&& now - ses.last_trx_packet_time >= opts.keepalive_secs) {
-		send_msg_ignore();
+	if (opts.keepalive_secs > 0) {
+		/* Send keepalives if we've been idle */
+		if (now - ses.last_packet_time_any_sent >= opts.keepalive_secs) {
+			send_msg_keepalive();
+		}
+
+		/* Also send an explicit keepalive message to trigger a response
+		if the remote end hasn't sent us anything */
+		if (now - ses.last_packet_time_keepalive_recv >= opts.keepalive_secs
+			&& now - ses.last_packet_time_keepalive_sent >= opts.keepalive_secs) {
+			send_msg_keepalive();
+		}
+
+		if (now - ses.last_packet_time_keepalive_recv 
+			>= opts.keepalive_secs * DEFAULT_KEEPALIVE_LIMIT) {
+			dropbear_exit("Keepalive timeout");
+		}
 	}
 
-	if (opts.idle_timeout_secs > 0 && ses.last_packet_time > 0
-			&& now - ses.last_packet_time >= opts.idle_timeout_secs) {
+	if (opts.idle_timeout_secs > 0 
+			&& now - ses.last_packet_time_idle >= opts.idle_timeout_secs) {
 		dropbear_close("Idle timeout");
 	}
 }
diff --git a/options.h b/options.h
index 44d6d23..ab6abea 100644
--- a/options.h
+++ b/options.h
@@ -308,6 +308,11 @@ much traffic. */
 be overridden at runtime with -K. 0 disables keepalives */
 #define DEFAULT_KEEPALIVE 0
 
+/* If this many KEEPALIVES are sent with no packets received from the
+other side, exit. Not run-time configurable - if you have a need
+for runtime configuration please mail the Dropbear list */
+#define DEFAULT_KEEPALIVE_LIMIT 3
+
 /* Ensure that data is received within IDLE_TIMEOUT seconds. This can
 be overridden at runtime with -I. 0 disables idle timeouts */
 #define DEFAULT_IDLE_TIMEOUT 0
diff --git a/packet.c b/packet.c
index 5a279f2..42d4229 100644
--- a/packet.c
+++ b/packet.c
@@ -57,9 +57,7 @@ void write_packet() {
 
 	int len, written;
 	buffer * writebuf = NULL;
-	time_t now;
 	unsigned packet_type;
-	int all_ignore = 1;
 #ifdef HAVE_WRITEV
 	struct iovec *iov = NULL;
 	int i;
@@ -90,7 +88,6 @@ void write_packet() {
 		packet_type = writebuf->data[writebuf->len-1];
 		len = writebuf->len - 1 - writebuf->pos;
 		dropbear_assert(len > 0);
-		all_ignore &= (packet_type == SSH_MSG_IGNORE);
 		TRACE2(("write_packet writev #%d  type %d len %d/%d", i, packet_type,
 				len, writebuf->len-1))
 		iov[i].iov_base = buf_getptr(writebuf, len);
@@ -146,7 +143,6 @@ void write_packet() {
 			dropbear_exit("Error writing: %s", strerror(errno));
 		}
 	} 
-	all_ignore = (packet_type == SSH_MSG_IGNORE);
 
 	if (written == 0) {
 		ses.remoteclosed();
@@ -163,13 +159,6 @@ void write_packet() {
 	}
 #endif /* writev */
 
-	now = monotonic_now();
-	ses.last_trx_packet_time = now;
-
-	if (!all_ignore) {
-		ses.last_packet_time = now;
-	}
-
 	TRACE2(("leave write_packet"))
 }
 
@@ -515,6 +504,8 @@ void encrypt_packet() {
 	unsigned char packet_type;
 	unsigned int len, encrypt_buf_size;
 	unsigned char mac_bytes[MAX_MAC_LEN];
+
+	time_t now;
 	
 	TRACE2(("enter encrypt_packet()"))
 
@@ -622,6 +613,18 @@ void encrypt_packet() {
 	ses.kexstate.datatrans += writebuf->len;
 	ses.transseq++;
 
+	now = monotonic_now();
+	ses.last_packet_time_any_sent = now;
+	/* idle timeout shouldn't be affected by responses to keepalives.
+	send_msg_keepalive() itself also does tricks with 
+	ses.last_packet_idle_time - read that if modifying this code */
+	if (packet_type != SSH_MSG_REQUEST_FAILURE
+		&& packet_type != SSH_MSG_UNIMPLEMENTED
+		&& packet_type != SSH_MSG_IGNORE) {
+		ses.last_packet_time_idle = now;
+
+	}
+
 	TRACE2(("leave encrypt_packet()"))
 }
 
diff --git a/process-packet.c b/process-packet.c
index 6cbcfc7..ddeb9ce 100644
--- a/process-packet.c
+++ b/process-packet.c
@@ -44,6 +44,7 @@ void process_packet() {
 
 	unsigned char type;
 	unsigned int i;
+	time_t now;
 
 	TRACE2(("enter process_packet"))
 
@@ -52,7 +53,8 @@ void process_packet() {
 
 	ses.lastpacket = type;
 
-	ses.last_packet_time = monotonic_now();
+	now = monotonic_now();
+	ses.last_packet_time_keepalive_recv = now;
 
 	/* These packets we can receive at any time */
 	switch(type) {
@@ -65,13 +67,21 @@ void process_packet() {
 		case SSH_MSG_UNIMPLEMENTED:
 			/* debugging XXX */
 			TRACE(("SSH_MSG_UNIMPLEMENTED"))
-			dropbear_exit("Received SSH_MSG_UNIMPLEMENTED");
+			goto out;
 			
 		case SSH_MSG_DISCONNECT:
 			/* TODO cleanup? */
 			dropbear_close("Disconnect received");
 	}
 
+	/* Ignore these packet types so that keepalives don't interfere with
+	idle detection. This is slightly incorrect since a tcp forwarded
+	global request with failure won't trigger the idle timeout,
+	but that's probably acceptable */
+	if (!(type == SSH_MSG_GLOBAL_REQUEST || type == SSH_MSG_REQUEST_FAILURE)) {
+		ses.last_packet_time_idle = now;
+	}
+
 	/* This applies for KEX, where the spec says the next packet MUST be
 	 * NEWKEYS */
 	if (ses.requirenext != 0) {
diff --git a/runopts.h b/runopts.h
index 21fc8e5..f3d784c 100644
--- a/runopts.h
+++ b/runopts.h
@@ -37,8 +37,8 @@ typedef struct runopts {
 	int listen_fwd_all;
 #endif
 	unsigned int recv_window;
-	time_t keepalive_secs;
-	time_t idle_timeout_secs;
+	time_t keepalive_secs; /* Time between sending keepalives. 0 is off */
+	time_t idle_timeout_secs; /* Exit if no traffic is sent/received in this time */
 
 #ifndef DISABLE_ZLIB
 	/* TODO: add a commandline flag. Currently this is on by default if compression
diff --git a/session.h b/session.h
index e66fc39..16a6e37 100644
--- a/session.h
+++ b/session.h
@@ -147,11 +147,14 @@ struct sshsession {
 	int signal_pipe[2]; /* stores endpoints of a self-pipe used for
 						   race-free signal handling */
 						
-	time_t last_trx_packet_time; /* time of the last packet transmission, for
-							keepalive purposes. Not real-world clock */
+	/* time of the last packet send/receive, for keepalive. Not real-world clock */
+	time_t last_packet_time_keepalive_sent;
+	time_t last_packet_time_keepalive_recv;
+	time_t last_packet_time_any_sent;
 
-	time_t last_packet_time; /* time of the last packet transmission or receive, for
-								idle timeout purposes. Not real-world clock */
+	time_t last_packet_time_idle; /* time of the last packet transmission or receive, for
+								idle timeout purposes so ignores SSH_MSG_IGNORE
+								or responses to keepalives. Not real-world clock */
 
 
 	/* KEX/encryption related */
diff --git a/svr-session.c b/svr-session.c
index 2adcb01..90d3414 100644
--- a/svr-session.c
+++ b/svr-session.c
@@ -95,8 +95,6 @@ void svr_session(int sock, int childpipe) {
 	chaninitialise(svr_chantypes);
 	svr_chansessinitialise();
 
-	ses.connect_time = monotonic_now();
-
 	/* for logging the remote address */
 	get_socket_address(ses.sock_in, NULL, NULL, &host, &port, 0);
 	len = strlen(host) + strlen(port) + 2;