check_multiple_users: do not account disconnected ones

When max-same-clients is set to 1 and a user re-using a cookie
connects, check_multiple_users() would prevent the user from
reconnecting. This corrects the issue by taking into account
only valid sessions that have not yet been disconnected.

Resolves: #223

Signed-off-by: Nikos Mavrogiannopoulos <nmav@gnutls.org>

NEWS

@@ -1,7 +1,8 @@
 * Version 0.12.6 (unreleased)
 - Improved IPv6 support for anyconnect clients. Patch by Leendert van Doorn.
 - The 'split-dns' configuration directive can be used per-user (#229).
-
+- The max-same-clients=1 configuration option no longer refuses the
+  reconnection of an already connected user (#223).
 
 * Version 0.12.5 (released 2019-10-16)
 - Fixed issue with FreeBSD tun devices closing (#213).

src/main-auth.c

@@ -205,8 +205,9 @@ struct proc_st *old_proc;
 		/* steal its leases */
 		steal_ip_leases(old_proc, proc);
 
-		if (old_proc->pid > 0)
-			kill(old_proc->pid, SIGTERM);
+		if (old_proc->pid > 0) {
+			kill_proc(old_proc);
+		}
 		mslog(s, proc, LOG_DEBUG, "re-using session");
 	} else {
 		mslog(s, proc, LOG_INFO, "new user session");
@@ -237,9 +238,9 @@ struct proc_st *old_proc;
  */
 int check_multiple_users(main_server_st *s, struct proc_st* proc)
 {
-struct proc_st *ctmp = NULL, *cpos;
-unsigned int entries = 1; /* that one */
-unsigned max;
+	struct proc_st *ctmp = NULL, *cpos;
+	unsigned int entries = 1; /* that one */
+	unsigned max;
 
 	max = proc->config->max_same_clients;
 
@@ -248,7 +249,7 @@ unsigned max;
 
 	list_for_each_safe(&s->proc_list.head, ctmp, cpos, list) {
 		if (ctmp != proc && ctmp->pid != -1) {
-			if (strcmp(proc->username, ctmp->username) == 0) {
+			if (!ctmp->pid_killed && strcmp(proc->username, ctmp->username) == 0) {
 				entries++;
 
 				if (entries > max)

src/main-worker-cmd.c

@@ -381,8 +381,9 @@ int handle_worker_commands(main_server_st * s, struct proc_st *proc)
 
 					/* If the address is in the BAN list, terminate it */
 					if (check_if_banned(s, &proc->remote_addr, proc->remote_addr_len) != 0) {
-						if (proc->pid != -1 && proc->pid != 0)
-							kill(proc->pid, SIGTERM);
+						if (proc->pid != -1 && proc->pid != 0) {
+							kill_proc(proc);
+						}
 					}
 				}
 

src/main.h

@@ -32,6 +32,7 @@
 #include <common.h>
 #include <sys/un.h>
 #include <sys/uio.h>
+#include <signal.h>
 #include <ev.h>
 
 #include "vhost.h"
@@ -90,6 +91,8 @@ typedef struct proc_st {
 	struct list_node list;
 	int fd; /* the command file descriptor */
 	pid_t pid;
+	unsigned pid_killed; /* if explicitly disconnected */
+
 	time_t udp_fd_receive_time; /* when the corresponding process has received a UDP fd */
 	
 	time_t conn_time; /* the time the user connected */
@@ -160,6 +163,12 @@ typedef struct proc_st {
 	vhost_cfg_st *vhost;
 } proc_st;
 
+inline static void kill_proc(proc_st *proc)
+{
+	kill(proc->pid, SIGTERM);
+	proc->pid_killed = 1;
+}
+
 struct ip_lease_db_st {
 	struct htable ht;
 };

tests/Makefile.am

@@ -34,7 +34,8 @@ EXTRA_DIST = certs/ca-key.pem certs/ca.pem ns.sh common.sh certs/server-cert.pem
 	data/raddb/clients.conf data/raddb/radiusd.conf data/raddb/users \
 	data/radiusclient/dictionary data/radiusclient/radiusclient.conf \
 	data/radiusclient/servers data/radius.config data/radius-group.config data/radius-otp.config \
-	data/test-udp-listen-host.config data/pam-kerberos/passdb.templ
+	data/test-udp-listen-host.config data/pam-kerberos/passdb.templ \
+	data/test-max-same-1.config
 
 SUBDIRS = docker-ocserv
 
@@ -58,7 +59,8 @@ endif
 dist_check_SCRIPTS += test-iroute test-multi-cookie test-pass-script \
 	test-cookie-timeout test-cookie-timeout-2 test-explicit-ip \
 	test-cookie-invalidation test-user-config test-append-routes test-ban \
-	multiple-routes haproxy-connect radius-group radius-otp json test-udp-listen-host
+	multiple-routes haproxy-connect radius-group radius-otp json test-udp-listen-host \
+	test-max-same-1
 
 #other tests requiring nuttcp for traffic
 if ENABLE_NUTTCP_TESTS

tests/data/test-max-same-1.config

@@ -0,0 +1,186 @@
+# User authentication method. Could be set multiple times and in that case
+# all should succeed.
+# Options: certificate, pam. 
+#auth = "certificate"
+auth = "plain[./data/test1.passwd]"
+#auth = "pam"
+
+isolate-workers = false
+
+# A banner to be displayed on clients
+#banner = "Welcome"
+
+# Use listen-host to limit to specific IPs or to the IPs of a provided hostname.
+#listen-host = [IP|HOSTNAME]
+
+use-dbus = no
+
+# Limit the number of clients. Unset or set to zero for unlimited.
+#max-clients = 1024
+max-clients = 16
+
+# Limit the number of client connections to one every X milliseconds 
+# (X is the provided value). Set to zero for no limit.
+#rate-limit-ms = 100
+
+# Limit the number of identical clients (i.e., users connecting multiple times)
+# Unset or set to zero for unlimited.
+max-same-clients = 1
+
+# TCP and UDP port number
+tcp-port = 4500
+udp-port = 4500
+
+# Keepalive in seconds
+keepalive = 32400
+
+# Dead peer detection in seconds
+dpd = 440
+
+# MTU discovery (DPD must be enabled)
+try-mtu-discovery = false
+
+# The key and the certificates of the server
+# The key may be a file, or any URL supported by GnuTLS (e.g., 
+# tpmkey:uuid=xxxxxxx-xxxx-xxxx-xxxx-xxxxxxxx;storage=user
+# or pkcs11:object=my-vpn-key;object-type=private)
+#
+# There may be multiple certificate and key pairs and each key
+# should correspond to the preceding certificate.
+server-cert = ./certs/server-cert.pem
+server-key = ./certs/server-key.pem
+
+# Diffie-Hellman parameters. Only needed if you require support
+# for the DHE ciphersuites (by default this server supports ECDHE).
+# Can be generated using:
+# certtool --generate-dh-params --outfile /path/to/dh.pem
+#dh-params = /path/to/dh.pem
+
+# If you have a certificate from a CA that provides an OCSP
+# service you may provide a fresh OCSP status response within
+# the TLS handshake. That will prevent the client from connecting
+# independently on the OCSP server.
+# You can update this response periodically using:
+# ocsptool --ask --load-cert=your_cert --load-issuer=your_ca --outfile response
+# Make sure that you replace the following file in an atomic way.
+#ocsp-response = /path/to/ocsp.der
+
+# In case PKCS #11 or TPM keys are used the PINs should be available
+# in files. The srk-pin-file is applicable to TPM keys only (It's the storage
+# root key).
+#pin-file = /path/to/pin.txt
+#srk-pin-file = /path/to/srkpin.txt
+
+# The Certificate Authority that will be used
+# to verify clients if certificate authentication
+# is set.
+#ca-cert = /path/to/ca.pem
+
+# The object identifier that will be used to read the user ID in the client certificate.
+# The object identifier should be part of the certificate's DN
+# Useful OIDs are: 
+#  CN = 2.5.4.3, UID = 0.9.2342.19200300.100.1.1
+#cert-user-oid = 0.9.2342.19200300.100.1.1
+
+# The object identifier that will be used to read the user group in the client 
+# certificate. The object identifier should be part of the certificate's DN
+# Useful OIDs are: 
+#  OU (organizational unit) = 2.5.4.11 
+#cert-group-oid = 2.5.4.11
+
+# A revocation list of ca-cert is set
+#crl = /path/to/crl.pem
+
+# GnuTLS priority string
+tls-priorities = "PERFORMANCE:%SERVER_PRECEDENCE:%COMPAT"
+
+# To enforce perfect forward secrecy (PFS) on the main channel.
+#tls-priorities = "NORMAL:%SERVER_PRECEDENCE:%COMPAT:-RSA"
+
+# The time (in seconds) that a client is allowed to stay connected prior
+# to authentication
+auth-timeout = 40
+
+# The time (in seconds) that a client is not allowed to reconnect after 
+# a failed authentication attempt.
+#min-reauth-time = 2
+
+# Cookie validity time (in seconds)
+# Once a client is authenticated he's provided a cookie with
+# which he can reconnect. This option sets the maximum lifetime
+# of that cookie.
+cookie-validity = 172800
+
+# Script to call when a client connects and obtains an IP
+# Parameters are passed on the environment.
+# REASON, USERNAME, GROUPNAME, HOSTNAME (the hostname selected by client), 
+# DEVICE, IP_REAL (the real IP of the client), IP_LOCAL (the local IP
+# in the P-t-P connection), IP_REMOTE (the VPN IP of the client). REASON
+# may be "connect" or "disconnect".
+#connect-script = /usr/bin/myscript
+#disconnect-script = /usr/bin/myscript
+
+# UTMP
+use-utmp = true
+
+# PID file
+pid-file = /var/run/ocserv.pid
+
+# The default server directory. Does not require any devices present.
+#chroot-dir = /path/to/chroot
+
+# socket file used for IPC, will be appended with .PID
+# It must be accessible within the chroot environment (if any)
+socket-file = /var/run/ocserv-socket
+
+# The user the worker processes will be run as. It should be
+# unique (no other services run as this user).
+run-as-user = nobody
+run-as-group = daemon
+
+# Network settings
+
+device = vpns
+
+# The default domain to be advertised
+default-domain = example.com
+
+ipv4-network = 192.168.1.0
+ipv4-netmask = 255.255.255.0
+# Use the keywork local to advertize the local P-t-P address as DNS server
+dns = 192.168.1.1
+
+# The NBNS server (if any)
+#ipv4-nbns = 192.168.2.3
+
+ipv6-network = fe80::
+ipv6-prefix = 16
+#ipv6-dns = 
+
+# Prior to leasing any IP from the pool ping it to verify that
+# it is not in use by another (unrelated to this server) host.
+ping-leases = false
+
+# Leave empty to assign the default MTU of the device
+# mtu = 
+
+route = 192.168.1.0/255.255.255.0
+#route = 192.168.5.0/255.255.255.0
+
+#
+# The following options are for (experimental) AnyConnect client 
+# compatibility. They are only available if the server is built 
+# with --enable-anyconnect
+#
+
+# Client profile xml. A sample file exists in doc/profile.xml.
+# This file must be accessible from inside the worker's chroot. 
+# The profile is ignored by the openconnect client.
+#user-profile = profile.xml
+
+# Unless set to false it is required for clients to present their
+# certificate even if they are authenticating via a previously granted
+# cookie. Legacy CISCO clients do not do that, and thus this option
+# should be set for them.
+#always-require-cert = false
+

tests/test-max-same-1

@@ -0,0 +1,75 @@
+#!/bin/sh
+#
+# Copyright (C) 2019 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.
+
+SERV="${SERV:-../src/ocserv}"
+srcdir=${srcdir:-.}
+PORT=4500
+
+. `dirname $0`/common.sh
+
+echo "Testing whether max-same-clients=1 allows cookie re-use... "
+
+PIDFILE1="${srcdir}/ci$$-1.pid.tmp"
+PIDFILE2="${srcdir}/ci$$-2.pid.tmp"
+rm -f "${PIDFILE1}" "${PIDFILE2}"
+
+launch_server -d 1 -f -c "${srcdir}/data/test-max-same-1.config" & PID=$!
+wait_server $PID
+
+echo "Connecting to obtain cookie... "
+eval `echo "test" | $OPENCONNECT -q localhost:$PORT -u test --authenticate --servercert=d66b507ae074d03b02eafca40d35f87dd81049d3`
+
+if [ -z "$COOKIE" ];then
+	fail $PID "Could not obtain cookie"
+fi
+
+#echo "Cookie: $COOKIE"
+
+echo "Connecting with cookie... "
+echo "test" | $OPENCONNECT -q localhost:$PORT -u test -C "$COOKIE" --servercert=d66b507ae074d03b02eafca40d35f87dd81049d3 --script=/bin/true --verbose --pid-file "${PIDFILE1}" --background
+CPID=$!
+
+sleep 4
+
+echo "Connecting again with same cookie... "
+echo "test" | $OPENCONNECT -q localhost:$PORT -b -u test -C "$COOKIE" --servercert=d66b507ae074d03b02eafca40d35f87dd81049d3 --script=/bin/true --verbose --pid-file "${PIDFILE2}" --background
+C2PID=$!
+
+sleep 4
+
+if [ ! -f "${PIDFILE2}" ];then
+	fail $PID "It was not possible to establish session!"
+fi
+
+# checking whether server has already terminated $CPID
+sleep 1
+
+if [ -f "${PIDFILE1}" ];then
+	fail $PID "Initial connection was still running!"
+fi
+
+rm -f "${PIDFILE1}" "${PIDFILE2}"
+
+kill $C2PID >/dev/null 2>&1
+kill $CPID >/dev/null 2>&1
+kill $PID
+wait
+
+exit 0