Fix session timeout bypass

- Fixes an issue #599 where the session timeout could be bypassed
  by reconnecting, such as through a laptop lid close/open cycle.
- Adds 'Session started at:' field to 'occtl show user' output.

Signed-off-by: Grigory Trenin <grigory.trenin@gmail.com>

NEWS

@@ -1,5 +1,9 @@
 * Version 1.4.1 (unreleased)
 - The bundled inih was updated to r62.
+- Fixed a bug where session timeout could be bypassed by reconnecting
+  (e.g., closing/opening laptop lid) (#599)
+- occtl: 'show user' command now includes a 'Session started at:' field,
+  indicating when the VPN session was established
 - occtl: Fix column misalignment in ban command outputs
 - Handle dotted client hostnames (e.g., .local) by stripping the domain suffix
 - Renamed `min-reauth-time` configuration option to `ban-time` to better reflect

src/ctl.proto

@@ -53,7 +53,7 @@ message user_info_rep
 	optional string local_ip = 7;
 	optional string remote_ip6 = 8;
 	optional string local_ip6 = 9;
-	required uint32 conn_time = 10;
+	required uint64 conn_time = 10;
 	optional string hostname = 11;
 	optional string user_agent = 12;
 	required uint32 status = 13; /* PS_ */
@@ -80,6 +80,7 @@ message user_info_rep
 
 	required bytes safe_id = 32; /* a value derived from the cookie */
 	required string vhost = 33;
+	required uint64 session_start_time = 34;
 }
 
 message user_list_rep

src/ipc.proto

@@ -83,6 +83,7 @@ message auth_cookie_reply_msg
 	optional string ipv6_local = 10;
 
 	required bytes sid = 11;
+	required uint64 session_start_time = 12;
 	required bytes secmod_addr = 13;
 
 	/* additional config */
@@ -354,6 +355,7 @@ message secm_session_reply_msg
 	optional string user_agent = 12;
 	optional string device_platform = 13;
 	optional string device_type = 14;
+	required uint64 session_start_time = 15;
 }
 
 /* internal struct */

src/main-auth.c

@@ -119,6 +119,7 @@ int send_cookie_auth_reply(main_server_st *s, struct proc_st *proc, AUTHREP r)
 
 		msg.sid.data = proc->sid;
 		msg.sid.len = sizeof(proc->sid);
+		msg.session_start_time = proc->session_start_time;
 
 		msg.vname = proc->tun_lease.name;
 		msg.user_name = proc->username;

src/main-ctl-unix.c

@@ -405,6 +405,7 @@ static int append_user_info(method_ctx *ctx, UserListRep *list,
 	rep->remote_ip6 = strtmp;
 
 	rep->conn_time = ctmp->conn_time;
+	rep->session_start_time = ctmp->session_start_time;
 	rep->hostname = ctmp->hostname;
 	rep->user_agent = ctmp->user_agent;
 

src/main-sec-mod-cmd.c

@@ -562,6 +562,8 @@ int session_open(sec_mod_instance_st *sec_mod_instance, struct proc_st *proc,
 		return -1;
 	}
 
+	proc->session_start_time = (time_t)msg->session_start_time;
+
 	if (msg->username == NULL) {
 		mslog(s, proc, LOG_INFO,
 		      "no username present in session reply");

src/main.h

@@ -128,6 +128,8 @@ typedef struct proc_st {
 	uint8_t sid[SID_SIZE];
 	unsigned int active_sid;
 
+	time_t session_start_time;
+
 	/* non zero if the sid has been invalidated and must not be allowed
 	 * to reconnect. */
 	unsigned int invalidated;

src/occtl/unix.c

@@ -1210,7 +1210,8 @@ static int common_info_cmd(UserListRep *args, FILE *out, cmd_params_st *params)
 {
 	char *username;
 	char *groupname;
-	char str_since[64];
+	char str_last_connect[64];
+	char str_session_start[64];
 	char tmpbuf[MAX_TMPSTR_SIZE];
 	char tmpbuf2[MAX_TMPSTR_SIZE];
 	struct tm *tm, _tm;
@@ -1238,7 +1239,13 @@ static int common_info_cmd(UserListRep *args, FILE *out, cmd_params_st *params)
 
 		t = args->user[i]->conn_time;
 		tm = localtime_r(&t, &_tm);
-		strftime(str_since, sizeof(str_since), DATE_TIME_FMT, tm);
+		strftime(str_last_connect, sizeof(str_last_connect),
+			 DATE_TIME_FMT, tm);
+
+		t = args->user[i]->session_start_time;
+		tm = localtime_r(&t, &_tm);
+		strftime(str_session_start, sizeof(str_session_start),
+			 DATE_TIME_FMT, tm);
 
 		username = args->user[i]->username;
 		if (username == NULL || username[0] == 0)
@@ -1330,13 +1337,20 @@ static int common_info_cmd(UserListRep *args, FILE *out, cmd_params_st *params)
 		print_single_value(out, params, "Hostname",
 				   args->user[i]->hostname, 1);
 
-		print_time_ival7(tmpbuf, time(NULL), t);
-		print_single_value_ex(out, params, "Connected at", str_since,
-				      tmpbuf, 1);
+		print_time_ival7(tmpbuf, time(NULL), args->user[i]->conn_time);
+		print_single_value_ex(out, params, "Last connected at",
+				      str_last_connect, tmpbuf, 1);
+		print_time_ival7(tmpbuf, time(NULL),
+				 args->user[i]->session_start_time);
+		print_single_value_ex(out, params, "Session started at",
+				      str_session_start, tmpbuf, 1);
 
 		if (HAVE_JSON(params)) {
 			print_single_value_int(out, params, "raw_connected_at",
-					       t, 1);
+					       args->user[i]->conn_time, 1);
+			print_single_value_int(
+				out, params, "raw_session_started_at",
+				args->user[i]->session_start_time, 1);
 			print_single_value(out, params, "Full session",
 					   shorten(args->user[i]->safe_id.data,
 						   args->user[i]->safe_id.len,

src/sec-mod-auth.c

@@ -572,6 +572,7 @@ int handle_secm_session_open_cmd(sec_mod_st *sec, int fd,
 
 	rep.sid.data = e->sid;
 	rep.sid.len = sizeof(e->sid);
+	rep.session_start_time = e->created;
 
 	rep.reply = AUTH__REP__OK;
 

src/worker-auth.c

@@ -713,6 +713,8 @@ static int recv_cookie_auth_reply(worker_st *ws)
 			/* update our sid */
 			memcpy(ws->sid, msg->sid.data, sizeof(ws->sid));
 			ws->sid_set = 1;
+			ws->session_start_time =
+				(time_t)msg->session_start_time;
 
 			if (msg->secmod_addr.len > sizeof(ws->secmod_addr)) {
 				oclog(ws, LOG_ERR,

tests/session-timeout

@@ -58,10 +58,10 @@ if [ -z "$COOKIE" ];then
 fi
 
 #echo "Cookie: $COOKIE"
-sleep 1
+sleep 10
 echo ""
 echo "Connecting with cookie... "
-${CMDNS1} ${OPENCONNECT} ${ADDRESS}:${PORT} -u test -C "$COOKIE" --servercert=pin-sha256:xp3scfzy3rOQsv9NcOve/8YVVv+pHr4qNCXEXrNl5s8= --background --pid-file "${CPIDFILE}"
+${CMDNS1} ${OPENCONNECT} ${ADDRESS}:${PORT} -u test -C "$COOKIE" --servercert=pin-sha256:xp3scfzy3rOQsv9NcOve/8YVVv+pHr4qNCXEXrNl5s8= --background --pid-file "${CPIDFILE}" --verbose >${OUTFILE}
 
 sleep 4
 
@@ -69,13 +69,34 @@ if [ ! -f "${CPIDFILE}" ];then
 	fail $PID "It was not possible to establish session!"
 fi
 
+TIME_LEFT=$(awk '/X-CSTP-Session-Timeout-Remaining/ {print $2}' ${OUTFILE})
+if [ -z "${TIME_LEFT}" ];then
+	fail $PID "No session timeout advertised by server!"
+fi
+
+# session-timeout is 25 seconds, and we have already waited 10s before reconnecting.
+# So the remaining time should be no more than 15 seconds (25 - 10 = 15).
+# If it is greater than this, it indicates the timeout was reset
+# back to 25 seconds on the new connection (ref: issue #599)
+
+if [ "${TIME_LEFT}" -gt 20 ];then
+	fail $PID "Session timeout was reset to ${TIME_LEFT}s after reconnection!"
+fi
+
 echo "ping remote address"
 
 set -e
 ${CMDNS1} ping -c 3 ${VPNADDR}
-${CMDNS2} ${OCCTL} -s ${OCCTL_SOCKET} show user test
+UPTIME=$(${CMDNS2} ${OCCTL} -s ${OCCTL_SOCKET} -j show user test | jq 'now - .[].raw_session_started_at | floor')
 set +e
 
+if [ -z "${UPTIME}" ];then
+	fail $PID "Failed to retrieve session start time from occtl show user!"
+fi
+if [ "${UPTIME}" -lt 10 -o "${UPTIME}" -gt 60 ];then
+	fail $PID "Session uptime ${UPTIME}s is outside the expected range (10–60 seconds)!"
+fi
+
 # We wait more than the configured time as session timeout is enforced every
 # a couple of seconds.
 echo "Waiting for session timeout... "