From 961897fd1bcebd6d0265707187131149abb0f498 Mon Sep 17 00:00:00 2001
From: Grische <2787581+grische@users.noreply.github.com>
Date: Wed, 12 Jun 2024 21:00:26 +0200
Subject: [PATCH] ffmuc-mesh-vpn-wireguard: improve nslookup calls

- add logging logging in case nslookup failed and
- unify the two nslookup implementations with subtle differences. Improves
  the error handling for nslookup
- enable pipefail: with the above improvementsfixes, pipefail works again
---
 .../gluon-mesh-wireguard-vxlan/checkuplink    | 60 ++++++++++---------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/ffmuc-mesh-vpn-wireguard-vxlan/files/lib/gluon/gluon-mesh-wireguard-vxlan/checkuplink b/ffmuc-mesh-vpn-wireguard-vxlan/files/lib/gluon/gluon-mesh-wireguard-vxlan/checkuplink
index ec37c00d..f022cd49 100755
--- a/ffmuc-mesh-vpn-wireguard-vxlan/files/lib/gluon/gluon-mesh-wireguard-vxlan/checkuplink
+++ b/ffmuc-mesh-vpn-wireguard-vxlan/files/lib/gluon/gluon-mesh-wireguard-vxlan/checkuplink
@@ -2,7 +2,7 @@
 
 # fail fast and abort early
 set -eu
-# set -o pipefail # TODO: pipefail needs more rework in the script
+set -o pipefail
 
 mesh_vpn_enabled="$(uci get wireguard.mesh_vpn.enabled)"
 
@@ -62,17 +62,37 @@ combine_ip_port() {
 }
 
 resolve_host() {
-	local gateway="$1"
+	local host_to_resolve="$1"
+
+	# older versions of nslookup use "Address 1:" with increasing numbers, newer just use "Address:"
+	if ! all_ips="$(gluon-wan nslookup "$host_to_resolve" | grep '^Address \?[0-9]*:\? ' | sed 's/^Address \?[0-9]*:\? //')"; then
+		logger -p err -t checkuplink "nslookup failed. Unable to get addresses of $host_to_resolve."
+		return 5
+	fi
+
 	# Check if we have a default route for v6 if not fallback to v4
 	if ip -6 route show table 1 | grep -q 'default via'
 	then
-		local ipv6
-		ipv6="$(gluon-wan nslookup "$gateway" | grep 'Address:\? [0-9]' | grep -oE '([a-f0-9:]+:+)+[a-f0-9]+')"
-		echo "$ipv6"
+		local ipv6s
+		# We need to match a few special cases for IPv6 here:
+		# - IPs with trailing "::", like 2003:a:87f:c37c::
+		# - IPs with leading "::", like ::1
+		# - IPs not starting with a digit, like fd62:f45c:4d09:180:22b3:ff::
+		# - IPs containing a zone identifier ("%"), like fe80::abcd%enp5s0
+		# As all incoming IPs are already valid IPs, we just grep for all not-IPv4s
+		if ! ipv6s="$(echo "${all_ips}" | grep -vE '\b([0-9]{1,3}\.){3}[0-9]{1,3}\b')"; then
+			logger -p err -t checkuplink "Unable to get any IPv6 from $host_to_resolve (${all_ips})."
+			return 6
+		fi
+		echo "$ipv6s"
 	else
-		local ipv4
-		ipv4="$(gluon-wan nslookup "$gateway" | grep 'Address:\? [0-9]' | grep -oE '\b([0-9]{1,3}\.){3}[0-9]{1,3}\b')"
-		echo "$ipv4"
+		local ipv4s
+		# We want to match IPv4s and not match RFC2765 2.1) IPs like "::ffff:255.255.255.255"
+		if ! ipv4s="$(echo "${all_ips}" | grep -oE '\b([0-9]{1,3}\.){3}[0-9]{1,3}\b')"; then
+			logger -p err -t checkuplink "Unable to get any IPv4 from $host_to_resolve (${all_ips})."
+			return 7
+		fi
+		echo "$ipv4s"
 	fi
 }
 
@@ -150,7 +170,7 @@ use_api_v1() {
 	PEER_PORT="$(extract_port "$PEER_HOSTPORT")"
 	PEER_PUBLICKEY="$(uci get wireguard.peer_"$PEER".publickey)"
 	PEER_LINKADDRESS="$(uci get wireguard.peer_"$PEER".link_address)"
-	PEER_ADDRESS="$(resolve_host "$PEER_HOST")"
+	PEER_ADDRESS="$(resolve_host "$PEER_HOST" | head -n 1)" # use the first resolved address of the peer
 	PEER_ENDPOINT="$(combine_ip_port "$PEER_ADDRESS" "$PEER_PORT")"
 }
 
@@ -166,12 +186,12 @@ use_api_v2() {
 	fi
 
 	logger -p debug -t checkuplink "Successfully parsed wgkex broker data"
-	
+
 	PEER_HOST="$(echo "$data" | sed -n 1p)"
 	PEER_PORT="$(echo "$data" | sed -n 2p)"
 	PEER_PUBLICKEY="$(echo "$data" | sed -n 3p)"
 	PEER_LINKADDRESS=$(echo "$data" | sed -n 4p)
-	PEER_ADDRESS="$(resolve_host "$PEER_HOST")"
+	PEER_ADDRESS="$(resolve_host "$PEER_HOST" | head -n 1)" # use the first resolved address of the peer
 	PEER_ENDPOINT="$(combine_ip_port "$PEER_ADDRESS" "$PEER_PORT")"
 }
 
@@ -214,26 +234,10 @@ logger -t checkuplink "Reconnecting ..."
 NTP_SERVERS=$(uci get system.ntp.server)
 NTP_SERVERS_ADDRS=""
 
-set -o pipefail # Enable pipefail: this script does not fully support pipefail yet, but required below
 for NTP_SERVER in $NTP_SERVERS; do
-	# older versions of nslookup use "Address 1:" with increasing numbers, newer just use "Address:"
-	all_ntp_ips="$(gluon-wan nslookup "$NTP_SERVER" | grep '^Address \?[0-9]*:\? ' | sed 's/^Address \?[0-9]*:\? //')"
-	if ip -6 route show table 1 | grep -q 'default via'
-	then
-		# We need to match a few special cases for IPv6 here:
-		# - IPs with trailing "::", like 2003:a:87f:c37c::
-		# - IPs with leading "::", like ::1
-		# - IPs not starting with a digit, like fd62:f45c:4d09:180:22b3:ff::
-		# - IPs containing a zone identifier ("%"), like fe80::abcd%enp5s0
-		# As all incoming IPs are already valid IPs, we just grep for all not-IPv4s
-		selected_ntp_ips="$(echo "${all_ntp_ips}" | grep -vE '\b([0-9]{1,3}\.){3}[0-9]{1,3}\b')"
-	else
-		# We want to match IPv4s and not match RFC2765 2.1) IPs like "::ffff:255.255.255.255"
-		selected_ntp_ips="$(echo "${all_ntp_ips}" | grep -oE '\b([0-9]{1,3}\.){3}[0-9]{1,3}\b')"
-	fi
+	selected_ntp_ips=$(resolve_host "$NTP_SERVER")
 	NTP_SERVERS_ADDRS="$(for ip in $selected_ntp_ips; do echo -n "-p $ip "; done)${NTP_SERVERS_ADDRS}"
 done
-set +o pipefail # Disable pipefail: this script does not fully support pipefail yet
 
 # shellcheck disable=SC2086 # otherwise ntpd cries
 if ! force_wan_connection /usr/sbin/ntpd -n -N -S /usr/sbin/ntpd-hotplug ${NTP_SERVERS_ADDRS} -q