Skip to content

Commit

Permalink
t_server_null: multiple improvements and fixes
Browse files Browse the repository at this point in the history
- 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 <[email protected]>
Acked-by: Frank Lichtenheld <[email protected]>
Message-Id: <[email protected]>
URL: https://www.mail-archive.com/[email protected]/msg28871.html
Signed-off-by: Gert Doering <[email protected]>
  • Loading branch information
mattock authored and cron2 committed Jul 4, 2024
1 parent c535fa7 commit f8f4771
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 40 deletions.
13 changes: 6 additions & 7 deletions doc/t_server_null.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
------------------------

Expand Down Expand Up @@ -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
-------------

Expand Down
3 changes: 1 addition & 2 deletions tests/t_server_null.rc-sample
Original file line number Diff line number Diff line change
@@ -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"

Expand Down
9 changes: 2 additions & 7 deletions tests/t_server_null.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
#!/bin/sh
#
TSERVER_NULL_SKIP_RC="${TSERVER_NULL_SKIP_RC:-77}"

Expand Down Expand Up @@ -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=$?

Expand Down
16 changes: 10 additions & 6 deletions tests/t_server_null_client.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
#!/bin/sh

launch_client() {
test_name=$1
Expand Down Expand Up @@ -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
Expand All @@ -101,6 +104,7 @@ while [ $count -lt $server_max_wait ]; do

if [ $count -eq $server_max_wait ]; then
retval=1
exit $retval
fi
done

Expand Down
8 changes: 5 additions & 3 deletions tests/t_server_null_default.rc
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,24 @@ 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}"

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"
Expand Down
53 changes: 39 additions & 14 deletions tests/t_server_null_server.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
#!/bin/sh

launch_server() {
server_name=$1
Expand All @@ -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

Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion tests/t_server_null_stress.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
#!/bin/sh
#
# Run this stress test as root to avoid sudo authorization from timing out.

Expand Down

0 comments on commit f8f4771

Please sign in to comment.