Merge branch 'tmp-drain-ms-repro' into 'master'

Stop listening on ocserv-sm socket on error to prevent looping.

Closes #356

See merge request openconnect/ocserv!219

NEWS

@@ -1,3 +1,8 @@
+* Version 1.1.2 (unreleased)
+- Fixed an infinite loop on sec-mod crash when server-drain-ms is set
+  (#356)
+
+
 * Version 1.1.1 (released 2020-09-21)
 - Fixed compatibility with OpenBSD that lacks procfs (#312)
 - Improved rate-limit-ms and made it dependent on secmod backlog. This

src/main.c

@@ -894,7 +894,6 @@ static void sec_mod_child_watcher_cb(struct ev_loop *loop, ev_child *w, int reve
 	ev_child_stop(loop, w);
 	mslog(s, NULL, LOG_ERR, "ocserv-secmod died unexpectedly");
 	ev_feed_signal_event (loop, SIGTERM);
-
 }
 
 void script_child_watcher_cb(struct ev_loop *loop, ev_child *w, int revents)
@@ -1007,18 +1006,20 @@ static void term_sig_watcher_cb(struct ev_loop *loop, ev_signal *w, int revents)
 	}
 	else 
 	{
-		mslog(s, NULL, LOG_INFO, "termination request received; stopping new connections");
-		graceful_shutdown_watcher.repeat = ((ev_tstamp)(server_drain_ms)) / 1000.;
-		mslog(s, NULL, LOG_INFO, "termination request received; waiting %d ms", server_drain_ms);
-		ev_timer_again(loop, &graceful_shutdown_watcher);
+		if (!ev_is_active(&graceful_shutdown_watcher)) {
+			mslog(s, NULL, LOG_INFO, "termination request received; stopping new connections");
+			graceful_shutdown_watcher.repeat = ((ev_tstamp)(server_drain_ms)) / 1000.;
+			mslog(s, NULL, LOG_INFO, "termination request received; waiting %d ms", server_drain_ms);
+			ev_timer_again(loop, &graceful_shutdown_watcher);
 
-		// Close the listening ports and stop the IO
-		list_for_each_safe(&s->listen_list.head, ltmp, lpos, list) {
-			ev_io_stop(loop, &ltmp->io);
-			close(ltmp->fd);
-			list_del(&ltmp->list);
-			talloc_free(ltmp);
-			s->listen_list.total--;
+			// Close the listening ports and stop the IO
+			list_for_each_safe(&s->listen_list.head, ltmp, lpos, list) {
+				ev_io_stop(loop, &ltmp->io);
+				close(ltmp->fd);
+				list_del(&ltmp->list);
+				talloc_free(ltmp);
+				s->listen_list.total--;
+			}
 		}
 	}
 }
@@ -1283,6 +1284,7 @@ static void sec_mod_watcher_cb (EV_P_ ev_io *w, int revents)
 	if (ret < 0) { /* bad commands from sec-mod are unacceptable */
 		mslog(s, NULL, LOG_ERR,
 		       "error in command from sec-mod");
+		ev_io_stop(loop, w);
 		ev_feed_signal_event (loop, SIGTERM);
 	}
 }

tests/Makefile.am

@@ -91,7 +91,7 @@ dist_check_SCRIPTS += test-pass test-pass-cert test-cert test-group-pass \
 	test-gssapi test-pass-opt-cert test-cert-opt-pass test-gssapi-opt-pass \
 	test-gssapi-opt-cert haproxy-auth test-maintenance resumption \
 	test-group-name flowcontrol banner invalid-configs haproxy-proxyproto \
-	haproxy-proxyproto-v1 test-drain-server
+	haproxy-proxyproto-v1 drain-server drain-server-fail
 
 if HAVE_CWRAP_PAM
 dist_check_SCRIPTS += test-pam test-pam-noauth

tests/drain-server-fail

@@ -0,0 +1,91 @@
+#!/bin/bash
+#
+# Copyright (C) 2020 Nikos Mavrogiannopoulos
+#
+# This file is part of ocserv.
+#
+# ocserv is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by the
+# Free Software Foundation; either version 2 of the License, or (at
+# your option) any later version.
+#
+# ocserv is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+SERV="${SERV:-../src/ocserv}"
+OCCTL="${OCCTL:-../src/occtl/occtl}"
+srcdir=${srcdir:-.}
+NO_NEED_ROOT=1
+PIDFILE=ocserv-pid.$$.tmp
+OCCTL_SOCKET=./occtl-drain-$$.socket
+
+. `dirname $0`/common.sh
+
+eval "${GETPORT}"
+
+echo "Testing server-drain-ms when sec-mod abruptively fails..."
+
+function finish {
+  set +e
+  echo " * Cleaning up..."
+  test -n "${CONFIG}" && rm -f "${CONFIG}" >/dev/null 2>&1
+  test -n "${PID}" && kill "${PID}" >/dev/null 2>&1
+}
+trap finish EXIT
+
+update_config test1.config
+echo server-drain-ms=15000 >> ${CONFIG}
+echo "occtl-socket-file = $OCCTL_SOCKET" >> ${CONFIG}
+echo "use-occtl = true" >> ${CONFIG}
+
+launch_simple_sr_server -d 3 -p ${PIDFILE} -f -c ${CONFIG} & PID=$!
+wait_server $PID
+
+echo "Connecting to obtain cookie... "
+( echo "test" | LD_PRELOAD=libsocket_wrapper.so $OPENCONNECT -q $ADDRESS:$PORT -u test --servercert=d66b507ae074d03b02eafca40d35f87dd81049d3 --cookieonly ) ||
+	fail $PID "Could not receive cookie from server"
+
+if ! test -f ${PIDFILE};then
+	fail $PID "Could not find pid file ${PIDFILE}"
+fi
+
+
+SPID=$(${OCCTL} -s ${OCCTL_SOCKET} show status|grep -i "Sec-mod PID"|cut -d ':' -f 2)
+if test -z "${SPID}";then
+	echo "Could not detect sec-mod PID"
+	${OCCTL} -s ${OCCTL_SOCKET} show status
+	exit 1
+fi
+
+echo "Killing sec-mod"
+kill -15 ${SPID}
+
+function wait_ocserv {
+  local max_time=$1
+  local time=0
+
+  while [ ${time} -lt ${max_time} ]
+  do
+    sleep 5
+    test -e ${OCCTL_SOCKET}
+    if ! test $? = 0;then
+      echo "ocserv is down"
+      return 0
+    fi
+    let time+=5
+  done
+
+  #timeout
+  echo "ocserv did not get offline after ${time} secs"
+  kill -9 $(cat $PIDFILE)
+  exit 1
+}
+
+wait_ocserv 30
+
+exit 0