tests: add regression test for pam_auth_deinit coroutine safety

Add test-pam-abort, which verifies that pam_auth_deinit() correctly
resumes a suspended PAM coroutine before calling pam_end().

The test posts a username-only HTTP request to trigger SEC_AUTH_INIT,
leaving the PAM coroutine suspended in PAM_S_WAIT_FOR_PASS while the
worker exits without sending a password.  The stale pre-auth entry is
cleaned up by the sec-mod maintenance cycle (driven by the new
sec-mod-db-cleanup-time config knob, set to 3 s in the test config).

Bug detection is provided by pam_abort_test.so, a small PAM module that
registers a pam_set_data() cleanup which calls abort() if pam_end() fires
while conv->conv() has not yet returned.  Without the fix, sec-mod would
abort and the subsequent authentication check would fail.

Relates: #741

Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>

src/cfg.proto

@@ -122,6 +122,7 @@ message ReloadableConfig {
   optional bool   no_udp                 = 54; /* [scope: vhost user] */
   optional bool   expose_iroutes         = 55; /* [scope: vhost] */
   optional uint32 limit_worker_memory   =  56; /* [scope: global] */
+  optional uint32 sec_mod_db_cleanup_time = 57; /* [scope: global] undocumented; 0 = use built-in default (310 s) */
 
   /* ------------------------------------------------------------------ */
   /* Strings — NULL pointer means "not set"; no has_X for proto2 strings */

src/config.c

@@ -1283,6 +1283,10 @@ static int cfg_ini_handler(void *_ctx, const char *section, const char *name,
 		if (error_on_vhost(vhost->name, "limit-worker-memory"))
 			return 0;
 		READ_TF_VC(limit_worker_memory);
+	} else if (strcmp(name, "sec-mod-db-cleanup-time") == 0) {
+		if (error_on_vhost(vhost->name, "sec-mod-db-cleanup-time"))
+			return 0;
+		READ_NUMERIC_VC(sec_mod_db_cleanup_time);
 	} else if (strcmp(name, "predictable-ips") == 0) {
 		READ_TF_VC(predictable_ips);
 	} else if (strcmp(name, "use-utmp") == 0) {

src/sec-mod.c

@@ -627,6 +627,12 @@ static void handle_alarm(int signo)
 	need_maintenance = 1;
 }
 
+static unsigned int maintenance_time(sec_mod_st *sec)
+{
+	unsigned int t = GETRCONFIG(sec)->sec_mod_db_cleanup_time;
+	return (t > 0) ? t : MAINTENANCE_TIME;
+}
+
 static void handle_sigterm(int signo)
 {
 	need_exit = 1;
@@ -718,7 +724,7 @@ static void check_other_work(sec_mod_st *sec)
 		send_stats_to_main(sec);
 		seclog(sec, LOG_DEBUG, "active sessions %d",
 		       sec_mod_client_db_elems(sec));
-		alarm(MAINTENANCE_TIME);
+		alarm(maintenance_time(sec));
 		need_maintenance = 0;
 	}
 }
@@ -1094,7 +1100,7 @@ void sec_mod_server(void *main_pool, void *config_pool,
 	}
 
 	sigprocmask(SIG_BLOCK, &blockset, &sig_default_set);
-	alarm(MAINTENANCE_TIME);
+	alarm(maintenance_time(sec));
 	seclog(sec, LOG_INFO, "sec-mod initialized (socket: %s)", SOCKET_FILE);
 
 	for (;;) {

tests/check-config-scope.py

@@ -282,9 +282,10 @@ def main():
     # Deprecated aliases that map to a canonical global option and call
     # error_on_vhost() but intentionally have no sample.config entry.
     DEPRECATED_GLOBAL_ALIASES = {
-        "use-seccomp",       # replaced by isolate-workers
-        "min-reauth-time",   # replaced by ban-time
-        "use-dbus",          # replaced by use-occtl
+        "use-seccomp",            # replaced by isolate-workers
+        "min-reauth-time",        # replaced by ban-time
+        "use-dbus",               # replaced by use-occtl
+        "sec-mod-db-cleanup-time", # internal/undocumented; test-only knob
     }
 
     # (c) Every error_on_vhost option must be annotated [... global] in sample.config

tests/data/pam/meson.build

@@ -35,3 +35,10 @@ configure_file(
   configuration: pam_conf,
   install:       false,
 )
+
+configure_file(
+  input:         'ocserv-abort.in',
+  output:        'ocserv-abort',
+  configuration: pam_conf,
+  install:       false,
+)

tests/data/pam/ocserv-abort.in

@@ -0,0 +1,4 @@
+#%PAM-1.0
+auth       required	@PAM_ABORT_MOD@
+account    sufficient	pam_permit.so
+session    sufficient	pam_permit.so

tests/data/test-pam-abort.config

@@ -0,0 +1,53 @@
+# Minimal ocserv config for test-pam-abort.
+# Uses the ocserv-abort PAM service backed by pam_abort_test.so, which
+# aborts the process if pam_end() is called while the conversation
+# coroutine is still live (regression test for issue #657).
+
+auth = "pam[service=ocserv-abort]"
+
+isolate-workers = @ISOLATE_WORKERS@
+
+use-dbus = no
+
+max-clients = 16
+max-same-clients = 2
+
+tcp-port = @PORT@
+udp-port = @PORT@
+
+keepalive = 32400
+dpd = 440
+try-mtu-discovery = false
+
+server-cert = @SRCDIR@/certs/server-cert.pem
+server-key = @SRCDIR@/certs/server-key.pem
+
+tls-priorities = "PERFORMANCE:%SERVER_PRECEDENCE:%COMPAT"
+
+auth-timeout = 5
+
+# Short cookie-timeout so stale pre-auth entries expire quickly
+# (exptime = cookie_timeout + AUTH_SLACK_TIME = 0 + 15 = 15 s).
+cookie-timeout = 0
+
+# Short maintenance interval so cleanup_client_entries() fires within
+# seconds of entry expiry.  Undocumented; 0 = use built-in default.
+sec-mod-db-cleanup-time = 3
+
+pid-file = ./ocserv.pid
+socket-file = ./ocserv-socket
+
+run-as-user = root
+run-as-group = root
+
+device = vpns
+
+default-domain = example.com
+
+ipv4-network = 192.168.1.0
+ipv4-netmask = 255.255.255.0
+ipv4-dns = 192.168.1.1
+
+ping-leases = false
+
+route = 192.168.1.0/255.255.255.0

tests/meson.build

@@ -173,13 +173,21 @@ if have_cwrap
   endforeach
 
   if have_cwrap_pam
+    # Detection module for the pam_auth_deinit coroutine-safety test (#657).
+    pam_abort_mod = shared_module('pam_abort_test', 'pam-abort-mod.c',
+      dependencies: [pam_dep],
+      name_prefix:  '',
+      install:      false,
+    )
+
     # Generate PAM test data files from templates into builddir/data/pam/
     # common.sh expects: ${builddir}/data/pam/{nss-passwd,nss-group,ocserv}
     pam_conf = configuration_data({
-      'ROOTUID':    run_command('id', '-u', check: true).stdout().strip(),
-      'ROOTGID':    run_command('id', '-g', check: true).stdout().strip(),
-      'PAMWRAPDIR': cwrap_pam_dep.get_variable(pkgconfig: 'modules',
-                                               default_value: '/usr/lib/pam_wrapper'),
+      'ROOTUID':       run_command('id', '-u', check: true).stdout().strip(),
+      'ROOTGID':       run_command('id', '-g', check: true).stdout().strip(),
+      'PAMWRAPDIR':    cwrap_pam_dep.get_variable(pkgconfig: 'modules',
+                                                  default_value: '/usr/lib/pam_wrapper'),
+      'PAM_ABORT_MOD': pam_abort_mod.full_path(),
     })
     subdir('data/pam')
 
@@ -191,6 +199,12 @@ if have_cwrap
       )
     endforeach
 
+    test('test-pam-abort', find_program('test-pam-abort'),
+      env:     test_env,
+      timeout: 90,
+      workdir: test_workdir,
+    )
+
     if get_option('kerberos-tests')
       test('kerberos', find_program('kerberos'),
         env:     test_env,

tests/pam-abort-mod.c

@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2026 Nikos Mavrogiannopoulos
+ *
+ * This program 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.
+ *
+ * This program 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/>.
+ */
+
+/*
+ * PAM module for regression-testing issue #741 (and #657).
+ *
+ * Detects whether pam_end() is called while the conversation function has
+ * not yet returned (i.e. the PAM coroutine is still live in ocserv_conv()).
+ *
+ * Mechanism:
+ *   pam_sm_authenticate() obtains the conversation function, allocates a
+ *   flag, and registers it with pam_set_data().  The pam_set_data() cleanup
+ *   runs when pam_end() fires; if the flag is still set at that point the
+ *   coroutine is live and we call abort().
+ *
+ *   Before calling conv->conv() the flag is set; after conv->conv() returns
+ *   (normally or via PAM_CONV_ERR) it is cleared.
+ *
+ * With the fix (commit af108817 / pam_auth_deinit co_call):
+ *   pam_auth_deinit() resumes the coroutine before calling pam_end(), so
+ *   conv->conv() returns, the flag is cleared, and pam_end() sees flag == 0.
+ *
+ * Without the fix:
+ *   pam_end() fires while the coroutine is suspended inside conv->conv(),
+ *   the cleanup sees flag == 1, and calls abort().
+ */
+
+#define PAM_SM_AUTH
+#include <security/pam_modules.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+struct abort_data {
+	int in_progress;
+};
+
+static void conv_cleanup(pam_handle_t *pamh, void *data, int error_status)
+{
+	struct abort_data *d = data;
+
+	if (d->in_progress) {
+		fprintf(stderr, "pam_abort_test: pam_end() called while "
+				"conversation is active (issue #657)\n");
+		abort();
+	}
+	free(d);
+}
+
+PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc,
+				   const char **argv)
+{
+	const struct pam_conv *conv;
+	struct pam_message msg;
+	const struct pam_message *msgp = &msg;
+	struct pam_response *resp = NULL;
+	struct abort_data *d;
+	int ret;
+
+	ret = pam_get_item(pamh, PAM_CONV, (const void **)&conv);
+	if (ret != PAM_SUCCESS || conv == NULL || conv->conv == NULL)
+		return PAM_AUTH_ERR;
+
+	d = malloc(sizeof(*d));
+	if (d == NULL)
+		return PAM_BUF_ERR;
+	d->in_progress = 0;
+
+	ret = pam_set_data(pamh, "ocserv_abort_in_progress", d, conv_cleanup);
+	if (ret != PAM_SUCCESS) {
+		free(d);
+		return ret;
+	}
+
+	msg.msg_style = PAM_PROMPT_ECHO_OFF;
+	msg.msg = "Password: ";
+
+	d->in_progress = 1;
+	ret = conv->conv(1, &msgp, &resp, conv->appdata_ptr);
+	d->in_progress = 0;
+
+	if (resp != NULL) {
+		if (resp[0].resp != NULL)
+			free(resp[0].resp);
+		free(resp);
+	}
+
+	return (ret == PAM_SUCCESS) ? PAM_SUCCESS : PAM_AUTH_ERR;
+}
+
+PAM_EXTERN int pam_sm_setcred(pam_handle_t *pamh, int flags, int argc,
+			      const char **argv)
+{
+	return PAM_SUCCESS;
+}

tests/test-pam-abort

@@ -0,0 +1,95 @@
+#!/bin/bash
+#
+# Copyright (C) 2026 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 GnuTLS; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+# Regression test for the bug fixed in commit af108817 (issue #657).
+#
+# When the PAM conversation coroutine is suspended in PAM_S_WAIT_FOR_PASS
+# and pam_auth_deinit() is called (e.g. during maintenance cleanup of a
+# stale pre-auth entry), the fix must resume the coroutine so it can
+# return PAM_CONV_ERR before pam_end() is called.
+#
+# Bug detection:
+#   pam_abort_test.so registers a pam_set_data() cleanup that calls
+#   abort() if pam_end() fires while the conversation flag is still set.
+#   The flag is cleared only after the conversation function returns.
+#   Without the fix, pam_end() fires with the coroutine still live,
+#   the cleanup aborts the sec-mod process, and subsequent
+#   authentication fails.  With the fix, the coroutine is resumed
+#   cleanly before pam_end() and the server survives.
+#
+# Trigger (PAM_S_WAIT_FOR_PASS):
+#   curl sends POST /auth with only "username=testuser" (no password
+#   field).  The worker parses the username, sends SEC_AUTH_INIT to
+#   sec-mod which starts the PAM coroutine; the coroutine calls the
+#   conversation function and suspends.  The worker then fails to
+#   extract a password from the POST body, returns 401, and exits.
+#   Because proc->active_sid is 0 for unauthenticated workers, main
+#   does not send SECM_SESSION_CLOSE to sec-mod.  The stale pre-auth
+#   entry (in_use == 0) stays in sec-mod's database.
+#
+# Cleanup:
+#   exptime = now + cookie_timeout(0) + AUTH_SLACK_TIME(15) = now + 15 s
+#   sec-mod-db-cleanup-time = 3 s  →  maintenance fires every 3 s
+#   pam_auth_deinit() is called within ~18 s of the POST.
+#   Sleep 25 s to be safe.
+
+srcdir=${srcdir:-.}
+NO_NEED_ROOT=1
+
+. "$(dirname "$0")"/common.sh
+
+eval "${GETPORT}"
+
+echo "Testing PAM cleanup when client aborts during conversation (issue #657)..."
+
+export TEST_PAMDIR=data/pam
+update_config test-pam-abort.config
+
+launch_sr_pam_server -d 1 -f -c ${CONFIG} & PID=$!
+wait_server $PID
+
+echo ""
+echo "Sending username-only POST to trigger PAM_S_WAIT_FOR_PASS..."
+
+# POST with only username= and no password field.
+# Worker sends SEC_AUTH_INIT → PAM coroutine starts, calls conv(), suspends.
+# Worker fails to extract password from body → 401 → exits.
+# The pre-auth entry remains in sec-mod (in_use=0, expires in 15 s).
+LD_PRELOAD=libsocket_wrapper.so curl -sk \
+	--max-time 10 \
+	--data 'username=testuser' \
+	"https://$ADDRESS:$PORT/auth" >/dev/null 2>&1 || true
+
+# exptime ≈ now + 15 s; maintenance fires every 3 s → cleanup within 18 s.
+echo ""
+echo "Waiting for maintenance cycle to call pam_auth_deinit() (~15 s)..."
+sleep 25
+
+echo ""
+echo "Verifying server is still functional after abort..."
+( echo "anypass" | LD_PRELOAD=libsocket_wrapper.so $OPENCONNECT -q \
+	"$ADDRESS:$PORT" -u testuser \
+	--servercert=pin-sha256:xp3scfzy3rOQsv9NcOve/8YVVv+pHr4qNCXEXrNl5s8= \
+	--cookieonly 2>&1 ) ||
+	fail $PID "Server not functional after client abort during PAM conversation"
+
+cleanup
+
+exit 0