t_server_null: multiple improvements and fixes

- exit after a timeout if unable to kill servers
- use sudo or equivalent only for server stop/start
- use /bin/sh directly instead of through /usr/bin/env
- simplify sudo call in the sample rc file
- remove misleading and outdated documentation
- make it work on OpenBSD 7.5
- make it work on NetBSD 10.0
- make server logs readable by normal users

Change-Id: I2cce8ad4e0d262e1404ab1eb6ff673d8590b6b3a
Signed-off-by: Samuli Seppänen <samuli.seppanen@gmail.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20240704133337.26595-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28871.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/doc/t_server_null.rst b/doc/t_server_null.rst
index e3a098a1d3234c7c2f381bf779ea858db1e971b7..5fe9080ae7d4a4ebe65e1fda9b53b87e6e6d0b69 100644 (file)
--- a/doc/t_server_null.rst
+++ b/doc/t_server_null.rst
@@ -43,6 +43,12 @@ Running the test suite requires the following:
   * run as root
   * a privilege escalation tool (sudo, doas, su) and the permission to become root
 
+If you use "doas" you should enable nopass feature in */etc/doas.conf*. For
+example to allow users in the *wheel* group to run commands without a password
+prompt::
+
+    permit nopass keepenv :wheel
+
 Technical implementation
 ------------------------
 
@@ -73,13 +79,6 @@ The tests suite is launched via "make check":
 
       * Waits until servers have launched. Then launch all clients, wait for them to exit and then check test results by parsing the client log files. Each client kills itself after some delay using an "--up" script.
 
-Note that "make check" moves on once *t_server_null_client.sh* has exited. At
-that point *t_server_null_server.sh* is still running, because it exists only
-after waiting a few seconds for more client connections to potentially appear.
-This is a feature and not a bug, but means that launching "make check" runs too
-quickly might cause test failures or unexpected behavior such as leftover
-OpenVPN server processes.
-
 Configuration
 -------------
 

diff --git a/tests/t_server_null.rc-sample b/tests/t_server_null.rc-sample
index 28c37735d2f3b115d762297e8be30ec207b4670d..98d78695df339382a3117f38a5b1bd99f4bfd907 100644 (file)
--- a/tests/t_server_null.rc-sample
+++ b/tests/t_server_null.rc-sample
@@ -1,6 +1,5 @@
 # Uncomment to run tests with sudo
-#SUDO_EXEC=`which sudo`
-#RUN_SUDO="${SUDO_EXEC} -E"
+#RUN_SUDO="sudo -E"
 
 TEST_RUN_LIST="1 2 3 10 11"
 

diff --git a/tests/t_server_null.sh b/tests/t_server_null.sh
index 0e53ba458f82eebd0064eb73411fc7fb02be122e..7627edf605721ac7537891ee4de5e88c505623d9 100755 (executable)
--- a/tests/t_server_null.sh
+++ b/tests/t_server_null.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env sh
+#!/bin/sh
 #
 TSERVER_NULL_SKIP_RC="${TSERVER_NULL_SKIP_RC:-77}"
 
@@ -57,12 +57,7 @@ fi
 
 srcdir="${srcdir:-.}"
 
-if [ -z "${RUN_SUDO}" ]; then
-    "${srcdir}/t_server_null_server.sh" &
-else
-    $RUN_SUDO "${srcdir}/t_server_null_server.sh" &
-fi
-
+"${srcdir}/t_server_null_server.sh" &
 "${srcdir}/t_server_null_client.sh"
 retval=$?
 

diff --git a/tests/t_server_null_client.sh b/tests/t_server_null_client.sh
index 88900071bd4a3dfadbd0c2f8e644c343d25d22c6..e7dd332423c750a2fe14dce270fdb6b670879823 100755 (executable)
--- a/tests/t_server_null_client.sh
+++ b/tests/t_server_null_client.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env sh
+#!/bin/sh
 
 launch_client() {
     test_name=$1
@@ -76,19 +76,22 @@ export retval=0
 count=0
 server_max_wait=15
 while [ $count -lt $server_max_wait ]; do
-    server_pids=""
-    server_count=$(set|grep 'SERVER_NAME_'|wc -l)
+    servers_up=0
+    server_count=$(echo $TEST_SERVER_LIST|wc -w)
 
     # We need to trim single-quotes because some shells return quoted values
     # and some don't. Using "set -o posix" which would resolve this problem is
     # not supported in all shells.
+    #
+    # While inactive server configurations may get checked they won't increase
+    # the active server count as the processes won't be running.
     for i in `set|grep 'SERVER_NAME_'|cut -d "=" -f 2|tr -d "[\']"`; do
         server_pid=$(cat $i.pid 2> /dev/null)
-        server_pids="${server_pids} ${server_pid}"
+        if ps -p $server_pid > /dev/null 2>&1; then
+            servers_up=$(( $servers_up + 1 ))
+        fi
     done
 
-    servers_up=$(ps -p $server_pids 2>/dev/null|sed '1d'|wc -l)
-
     echo "OpenVPN test servers up: ${servers_up}/${server_count}"
 
     if [ $servers_up -ge $server_count ]; then
@@ -101,6 +104,7 @@ while [ $count -lt $server_max_wait ]; do
 
     if [ $count -eq $server_max_wait ]; then
         retval=1
+        exit $retval
     fi
 done
 

diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index 63b6bcdb10d0957c4fe82f087c44bdd8e40d8ddc..825bb529994cef1fe81ff2779b95ae1d455fdf40 100755 (executable)
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -24,7 +24,7 @@ TA="${sample_keys}/ta.key"
 MAX_CLIENTS="10"
 CLIENT_MATCH="Test-Client"
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
-SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --server 10.29.41.0 255.255.255.0 --max-clients $MAX_CLIENTS --persist-tun --verb 3"
+SERVER_BASE_OPTS="--daemon --local 127.0.0.1 --dev tun --topology subnet --max-clients $MAX_CLIENTS --persist-tun --verb 3"
 SERVER_CIPHER_OPTS=""
 SERVER_CERT_OPTS="--ca ${CA} --dh ${DH} --cert ${SERVER_CERT} --key ${SERVER_KEY} --tls-auth ${TA} 0"
 SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
@@ -32,14 +32,16 @@ SERVER_CONF_BASE="${SERVER_BASE_OPTS} ${SERVER_CIPHER_OPTS} ${SERVER_CERT_OPTS}"
 TEST_SERVER_LIST="1 2"
 
 SERVER_NAME_1="t_server_null_server-1194_udp"
+SERVER_SERVER_1="--server 10.29.41.0 255.255.255.0"
 SERVER_MGMT_PORT_1="11194"
 SERVER_EXEC_1="${SERVER_EXEC}"
-SERVER_CONF_1="${SERVER_CONF_BASE} --lport 1194 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_1}"
+SERVER_CONF_1="${SERVER_CONF_BASE} ${SERVER_SERVER_1} --lport 1194 --proto udp --management 127.0.0.1 ${SERVER_MGMT_PORT_1}"
 
 SERVER_NAME_2="t_server_null_server-1195_tcp"
+SERVER_SERVER_2="--server 10.29.42.0 255.255.255.0"
 SERVER_MGMT_PORT_2="11195"
 SERVER_EXEC_2="${SERVER_EXEC}"
-SERVER_CONF_2="${SERVER_CONF_BASE} --lport 1195 --proto tcp --management 127.0.0.1 ${SERVER_MGMT_PORT_2}"
+SERVER_CONF_2="${SERVER_CONF_BASE} ${SERVER_SERVER_2} --lport 1195 --proto tcp --management 127.0.0.1 ${SERVER_MGMT_PORT_2}"
 
 # Test client configurations
 CLIENT_EXEC="${top_builddir}/src/openvpn/openvpn"

diff --git a/tests/t_server_null_server.sh b/tests/t_server_null_server.sh
index 9bc0c882bdb4bf0e4f2044d289ceb13d6b9b3522..e5906eec3b6eb67f530cec81c3026d5a29640c09 100755 (executable)
--- a/tests/t_server_null_server.sh
+++ b/tests/t_server_null_server.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env sh
+#!/bin/sh
 
 launch_server() {
     server_name=$1
@@ -8,18 +8,28 @@ launch_server() {
     status="${server_name}.status"
     pid="${server_name}.pid"
 
-    # Ensure that old status, log and pid files are gone
-    rm -f "${status}" "${log}" "${pid}"
-
-    "${server_exec}" \
-        $server_conf \
-        --status "${status}" 1 \
-        --log "${log}" \
-        --writepid "${pid}" \
-        --explicit-exit-notify 3
-
+    if [ -z "${RUN_SUDO}" ]; then
+        rm -f "${status}" "${log}" "${pid}"
+        "${server_exec}" \
+         $server_conf \
+         --status "${status}" 1 \
+         --log "${log}" \
+         --writepid "${pid}" \
+         --explicit-exit-notify 3
+    else
+        $RUN_SUDO rm -f "${status}" "${log}" "${pid}"
+        $RUN_SUDO "${server_exec}" \
+                   $server_conf \
+                   --status "${status}" 1 \
+                   --log "${log}" \
+                   --writepid "${pid}" \
+                   --explicit-exit-notify 3
+    fi
 }
 
+# Make server log files readable by normal users
+umask 022
+
 # Load base/default configuration
 . "${srcdir}/t_server_null_default.rc" || exit 1
 
@@ -64,15 +74,30 @@ done
 
 echo "All clients have disconnected from all servers"
 
+# Make sure that the server processes are truly dead before exiting.  If a
+# server process does not exit in 15 seconds assume it never will, move on and
+# hope for the best.
+echo "Waiting for servers to exit"
 for PID_FILE in $server_pid_files
 do
     SERVER_PID=$(cat "${PID_FILE}")
-    $KILL_EXEC "${SERVER_PID}"
 
-    # Make sure that the server processes are truly dead before exiting
-    while :
+    if [ -z "${RUN_SUDO}" ]; then
+        $KILL_EXEC "${SERVER_PID}"
+    else
+        $RUN_SUDO $KILL_EXEC "${SERVER_PID}"
+    fi
+
+    count=0
+    maxcount=75
+    while [ $count -le $maxcount ]
     do
         ps -p "${SERVER_PID}" > /dev/null || break
+        count=$(( count + 1))
         sleep 0.2
     done
+
+    if [ $count -ge $maxcount ]; then
+        echo "WARNING: could not kill server with pid ${SERVER_PID}!"
+    fi
 done

diff --git a/tests/t_server_null_stress.sh b/tests/t_server_null_stress.sh
index 12813978066c2ca970c36e8a7fddd3cf44f6e502..0bb9452780bee1bc2be1a081e67e60c44b426670 100755 (executable)
--- a/tests/t_server_null_stress.sh
+++ b/tests/t_server_null_stress.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env sh
+#!/bin/sh
 #
 # Run this stress test as root to avoid sudo authorization from timing out.