[PATCH net-next 5/6] selftests: ovpn: wait for the TCP server key setup before traffic

[Antonio Quartulli <antonio@openvpn.net>]

In TCP mode ovpn_add_peer() starts the server side from a background
subshell that first listens for the incoming connections and then
installs the data key of every peer. The subshell PID is discarded, so
nothing synchronizes the test against the key installation.

The sleep 5 that follows does not cover it: it elapses while "listen" is
still waiting for connections, and the peers only connect in the
subsequent ovpn_add_peer() iterations, so the key loop starts well after
that sleep has expired. Until now the test happened to work because of
the unrelated delays that ran in between.

Record the PID of the background subshell and wait for it once all the
peers have been registered, which is the earliest point at which
"listen" can have returned. A peer that was not given a key yet would
otherwise drop the server-to-client packets and make the first ping
fail.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 tools/testing/selftests/net/ovpn/common.sh    | 61 ++++++++++++++++++-
 .../selftests/net/ovpn/test-close-socket.sh   |  2 +
 tools/testing/selftests/net/ovpn/test.sh      |  2 +
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/ovpn/common.sh b/tools/testing/selftests/net/ovpn/common.sh
index 2f7851b82343..da732c64ffea 100644
--- a/tools/testing/selftests/net/ovpn/common.sh
+++ b/tools/testing/selftests/net/ovpn/common.sh
@@ -256,14 +256,19 @@ ovpn_add_peer() {
 		fi
 	else
 		if [ ${1} -eq 0 ]; then
+			OVPN_TCP_KEYS_LOG=$(mktemp)
 			(ip netns exec "${server_ns}" ${OVPN_CLI} listen tun0 \
 				1 ${M_ID} ${OVPN_TCP_PEERS_FILE} && {
+				rc=0
 				for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 					ip netns exec "${server_ns}" \
 						${OVPN_CLI} new_key tun0 ${p} \
-						1 0 ${OVPN_ALG} 0 data64.key
+						1 0 ${OVPN_ALG} 0 data64.key \
+						|| rc=1
 				done
-			}) &
+				exit ${rc}
+			}) >"${OVPN_TCP_KEYS_LOG}" 2>&1 &
+			OVPN_TCP_KEYS_PID=$!
 			sleep 5
 		else
 			peer_ns="ovpn_peer${1}"
@@ -281,6 +286,58 @@ ovpn_add_peer() {
 	fi
 }
 
+# In TCP mode the server accepts the peers and installs their data keys from a
+# background subshell. "listen" returns once a connection has been accepted for
+# every entry of the peers file, so the key installation runs after the last
+# ovpn_add_peer() call. Wait for that subshell before generating any traffic,
+# otherwise the first packets may race the key installation and get dropped for
+# lack of a key.
+#
+# The wait is bounded: both the accept loop and the subsequent receive in
+# ovpn-cli are unbounded, so a peer that never connects (for instance when
+# OVPN_NUM_PEERS is smaller than the number of entries in the peers file) would
+# otherwise hang the whole test instead of failing.
+OVPN_TCP_KEYS_TIMEOUT=${OVPN_TCP_KEYS_TIMEOUT:-30}
+
+ovpn_wait_tcp_keys() {
+	local deadline rc
+
+	[ -n "${OVPN_TCP_KEYS_PID:-}" ] || return 0
+
+	deadline=$((SECONDS + OVPN_TCP_KEYS_TIMEOUT))
+	while kill -0 "${OVPN_TCP_KEYS_PID}" 2>/dev/null; do
+		if [ "${SECONDS}" -ge "${deadline}" ]; then
+			printf '%s\n' \
+				"server-side key setup still running after ${OVPN_TCP_KEYS_TIMEOUT}s"
+			kill "${OVPN_TCP_KEYS_PID}" 2>/dev/null
+			wait "${OVPN_TCP_KEYS_PID}" 2>/dev/null
+			ovpn_dump_tcp_keys_log
+			return 1
+		fi
+		sleep 0.2
+	done
+
+	wait "${OVPN_TCP_KEYS_PID}"
+	rc=$?
+
+	# ovpn_cmd_run() has already removed the temporary file this subshell
+	# inherited, so without this its output would be lost and a failed
+	# new_key would only show up later as a ping failure.
+	if [ "${rc}" -ne 0 ]; then
+		ovpn_dump_tcp_keys_log
+	fi
+	rm -f "${OVPN_TCP_KEYS_LOG}"
+
+	return "${rc}"
+}
+
+ovpn_dump_tcp_keys_log() {
+	[ -s "${OVPN_TCP_KEYS_LOG:-}" ] || return 0
+
+	printf '%s\n' "--- server-side key setup output ---"
+	cat "${OVPN_TCP_KEYS_LOG}"
+}
+
 ovpn_compare_ntfs() {
 	local diff_rc=0
 	local diff_file
diff --git a/tools/testing/selftests/net/ovpn/test-close-socket.sh b/tools/testing/selftests/net/ovpn/test-close-socket.sh
index ec9a51bbf3c9..142dc14e2606 100755
--- a/tools/testing/selftests/net/ovpn/test-close-socket.sh
+++ b/tools/testing/selftests/net/ovpn/test-close-socket.sh
@@ -37,6 +37,8 @@ ovpn_prepare_network() {
 		ovpn_cmd_ok "register peer${p} in overlay" ovpn_add_peer "${p}"
 	done
 
+	ovpn_cmd_ok "wait for server-side key setup" ovpn_wait_tcp_keys
+
 	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 		peer_ns="ovpn_peer${p}"
 		ovpn_cmd_ok "set peer0 timeout for peer ${p}" \
diff --git a/tools/testing/selftests/net/ovpn/test.sh b/tools/testing/selftests/net/ovpn/test.sh
index e2e6ffdd29bb..0762fa83e4b5 100755
--- a/tools/testing/selftests/net/ovpn/test.sh
+++ b/tools/testing/selftests/net/ovpn/test.sh
@@ -45,6 +45,8 @@ ovpn_prepare_network() {
 		ovpn_cmd_ok "register peer${p} in overlay" ovpn_add_peer "${p}"
 	done
 
+	ovpn_cmd_ok "wait for server-side key setup" ovpn_wait_tcp_keys
+
 	for p in $(seq 1 ${OVPN_NUM_PEERS}); do
 		peer_ns="ovpn_peer${p}"
 		ovpn_cmd_ok "set peer0 timeout for peer ${p}" \
-- 
2.55.0