[patch] 0001-net-l2tp.sh-Rewrite-to-address-issues-of-POSIX-confo.patch

0001-net-l2tp.sh-Rewrite-to-address-issues-of-POSIX-confo.patch (text/plain), 14.87 KB, created by kfm on 2023-01-14 23:09:46 UTC

From e50623794088858f7bc5ec9261088094f6e0842b Mon Sep 17 00:00:00 2001
From: Kerin Millar <[e-mail]>
Date: Sat, 14 Jan 2023 22:58:41 +0000
Subject: [PATCH 1/2] net/l2tp.sh: Rewrite to address issues of POSIX
 conformance (and more besides)

Ensure that awk(1) is used portably throughout.

Eliminate the use of ${parameter^^} expansion syntax, which is a bashism.

Delegate netfirc parameter parsing to xargs(1) and awk(1). The potential for
code injection is thus eliminated, to the extent that is currently possible in
netifrc. It also eliminates potential issues pertaining to word splitting and
unintentional pathname expansion.

Add additional sanity checks and increase the rigour of those that exist. For
instance, blank values are no longer permitted and the tunnel_id parameter must
match that of l2tpsession_*, in the case that l2tptunnel_* is defined.

Add additional diagnostic messages while improving the clarity of those that
already existed. This is acheived in some instances by being more precise and,
in others, through the use of English that exhibits greater formality and
consistency. At least one grammatical error was rectified.

Simplify and refine the code in terms of both structure and syntax, and greatly
reduce the number of local variables. As a byproduct, all complaints previously
raised by shellcheck have been eliminated, save for the use of local, whose
behaviour is not defined by POSIX. I have not attempted to eliminate the use of
local because, for now, it continues to be used extensively throughout the
netifrc codebase.

Honour the exit status value of ip(8) for the "add" and "del" verbs, rather
than parse STDERR.

Optimise l2tp_post_stop() by refraining from executing ip(8) and awk(8) in the
case that the interface cannot be identifed as a virtual one. Further, do not
attempt to destroy the tunnels associated with an identified session in the
case that the attempt to destroy the session has failed.

Signed-off-by: Kerin Millar <[e-mail]>
Closes: https://bugs.gentoo.org/890238
---
 net/l2tp.sh | 349 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 206 insertions(+), 143 deletions(-)

diff --git a/net/l2tp.sh b/net/l2tp.sh
index 82649b9..51a2331 100644
--- a/net/l2tp.sh
+++ b/net/l2tp.sh
@@ -1,6 +1,11 @@
 # Copyright (c) 2016 Emeric Verschuur <[e-mail]>
+# Copyright (c) 2023 Kerin Millar <[e-mail]>
 # All rights reserved. Released under the 2-clause BSD license.
-# shellcheck shell=sh disable=SC1008
+
+# Don't complain about local, even though POSIX does not define its behaviour.
+# This is unwise but, as things stand, it is being used extensively by netifrc.
+# Also, SC2034 and SC2316 are muted because they produce false-positives.
+# shellcheck shell=sh disable=SC3043,SC2034,SC2316
 
 l2tp_depend()
 {
@@ -8,166 +13,224 @@ l2tp_depend()
 	before bridge interface macchanger
 }
 
-# Extract parameter list to shell vars
-#   1. variable prefix
-#   2. string to parse
-_l2tp_eval_props() {
-	local prop_pref=$1
-	local prop_list=$2
-	eval set -- "$3"
-	while [ -n "$1" ]; do
-		eval "case $1 in
-			$prop_list)
-				$prop_pref$1=\"$2\"
-				shift
-				shift
-				;;
-			*)
-				l2tp_err=\"invalid property $1\"
-				return 1
-				;;
-			
-		esac" || return 1
-	done
-	return 0
+_l2tp_parse_opts()
+{
+	# Parses lt2psession or l2tptunnel options using xargs(1), conveying
+	# them as arguments to awk(1). The awk program interprets the arguments
+	# as a series of key/value pairs and safely prints those specified as
+	# being required as variable declarations for evaluation by sh(1).
+	# Other keys are handled similarly, only in a way that renders them a
+	# no-op. For the program to exit successfully, all key names must be
+	# well-formed, all required keys must be seen, and all values must be
+	# non-blank. Note that assigning 1 to ARGC prevents awk from treating
+	# its arguments as the names of files to be opened.
+	printf %s "$1" \
+	| LC_CTYPE=C xargs -E '' awk -v q="'" -v required_keys="$2" -v other_keys="$3" '
+		function shquote(str) {
+			gsub(q, q "\\" q q, str)
+			return q str q
+		}
+		BEGIN {
+			argc = ARGC
+			ARGC = 1
+			gsub(" ", "|", required_keys)
+			gsub(" ", "|", other_keys)
+			re = "^(" required_keys "|" other_keys ")$"
+			sorter = "sort"
+			for (i = 1; i < argc; i += 2) {
+				key = ARGV[i]
+				val = ARGV[i + 1]
+				if (key !~ /^[[:alpha:]][_[:alnum:]]+$/) {
+					system("ewarn " shquote("Skipping malformed parameter: " key))
+				} else if (key ~ re) {
+					print key "=" shquote(val) | sorter
+					val_by[key] = val
+				} else {
+					print ": " key "=" shquote(val) | sorter
+				}
+			}
+			close(sorter)
+			split(required_keys, keys, "|")
+			missing = 0
+			for (i in keys) {
+				key = keys[i]
+				if (! (key in val_by)) {
+					system("eerror " shquote("The \"" key "\" parameter is missing"))
+					missing += 1
+				} else if (val_by[key] ~ /^[[:blank:]]*$/) {
+					system("eerror " shquote("The \"" key "\" parameter has a blank value"))
+					missing += 1
+				}
+			}
+			exit(!!missing)
+		}
+	'
 }
 
-_is_l2tp() {
-	# Check for L2TP support in kernel
-	ip l2tp show session 2>/dev/null 1>/dev/null || return 1
-
-	eval "$(ip l2tp show session | \
-		awk "match(\$0, /^Session ([0-9]+) in tunnel ([0-9]+)\$/, ret) {sid=ret[1]; tid=ret[2]} 
-		match(\$0, /^[ ]*interface name: ${IFACE}\$/) {print \"session_id=\"sid\";tunnel_id=\"tid; exit}")"
-	test -n "$session_id"
+_l2tp_parse_existing_session() {
+	ip l2tp show session \
+	| LC_CTYPE=C awk -v iface="${IFACE:?}" '
+		BEGIN { found = 0 }
+		/^Session [0-9]+ in tunnel [0-9]+$/ {
+			session_id = $2
+			tunnel_id = $5
+		}
+		/^[[:blank:]]*interface name:/ && "" $NF == "" iface {
+			print "session_id=" session_id
+			print "tunnel_id=" tunnel_id
+			found = 1
+			exit
+		}
+		END { exit(!found) }
+	'
 }
 
-# Get tunnel info
-#    1. Output variable prefix
-#    2. Tunnel ID to find
-_l2tp_get_tunnel_info() {
-	local found
-	eval "$(ip l2tp show tunnel | \
-		awk -v id=$2 -v prefix=$1 '
-		match($0, /^Tunnel ([0-9]+), encap (IP|UDP)$/, ret) {
-			if (found == "1") exit;
-			if (ret[1] == id) {
-				print "found=1;"
-				print prefix "tunnel_id=" ret[1] ";"
-				print prefix "encap=" ret[2] ";";
-				found="1"
-			}
-		} 
-		match($0, /^[ ]*From ([^ ]+) to ([^ ]+)$/, ret) {
-			if (found == "1") {
-				print prefix "local=" ret[1] ";"; 
-				print prefix "remote=" ret[2] ";"; 
-			}
+
+_l2tp_parse_existing_tunnel() {
+	ip l2tp show tunnel \
+	| LC_CTYPE=C awk -v q="'" -v id="$1" '
+		function shquote(str) {
+			gsub(q, q "\\" q q, str)
+			return q str q
 		}
-		match($0, /^[ ]*Peer tunnel ([0-9]+)$/, ret) {
-			if (found == "1") {
-				print prefix "peer_tunnel_id=" ret[1] ";"; 
-			}
+		BEGIN {
+			found = 0
+			sorter = "sort"
 		}
-		match($0, /^[ ]*UDP source \/ dest ports: ([0-9]+)\/([0-9]+)$/, ret) {
-			if (found == "1") {
-				print prefix "udp_sport=" ret[1] ";"; 
-				print prefix "udp_dport=" ret[2] ";"; 
+		/^Tunnel [0-9]+, encap (IP|UDP)$/ {
+			if (found) exit
+			tunnel_id = substr($2, 0, length($2) - 1)
+			if (tunnel_id == id) {
+				found = 1
+				print "tunnel_id=" shquote(tunnel_id) | sorter
+				print "encap=" shquote(tolower($4)) | sorter
 			}
-		}')"
-	test -n "$found"
+		}
+		found && /^[[:blank:]]*From [^[:blank:]]+ to [^[:blank:]]+$/ {
+			print "local=" shquote($2) | sorter
+			print "remote=" shquote($4) | sorter
+		}
+		found && /^[[:blank:]]*Peer tunnel [0-9]+$/ {
+			print "peer_tunnel_id=" shquote($NF) | sorter
+		}
+		found && /^[[:blank:]]*UDP source \/ dest ports: [0-9]+\/[0-9]+$/ {
+			split($NF, ports, "/")
+			print ": udp_sport=" shquote(ports[1]) | sorter
+			print ": udp_dport=" shquote(ports[2]) | sorter
+		}
+		END {
+			close(sorter)
+			exit(!found)
+		}
+	'
 }
 
-_ip_l2tp_add() {
-	local e
-	e="$(LC_ALL=C ip l2tp add "$@" 2>&1 1>/dev/null)"
-	case $e in
-		"")
-			return 0
-			;;
-		"RTNETLINK answers: No such process")
-			# seems to not be a fatal error but I don't know why I have this error... hmmm
-			ewarn "ip l2tp add $2 error: $e"
-			return 0
-			;;
-		*)
-			eend 1 "ip l2tp add $2 error: $e"
-			return 1
-			;;
-	esac
-	
+_l2tp_should_add_tunnel() {
+	local existing_tunnel
+
+	if ! existing_tunnel=$(_l2tp_parse_existing_tunnel "$1"); then
+		return 0
+	elif [ "$2" = "${existing_tunnel}" ]; then
+		return 1
+	else
+		return 2
+	fi
 }
 
+_l2tp_has_tunnel() {
+	_l2tp_parse_existing_tunnel "$1" >/dev/null
+}
+
+_l2tp_in_session() {
+	ip l2tp show session | {
+		LC_CTYPE=C
+		while read -r line; do
+			case ${line} in
+				"Session "*" in tunnel $1") return 0
+			esac
+		done
+	}
+	return 1
+}
+
+_is_blank() (
+	LC_CTYPE=C
+	case $1 in
+		*[![:blank:]]*) return 1
+	esac
+)
+
 l2tp_pre_start()
 {
-	local l2tpsession=
-	eval l2tpsession=\$l2tpsession_${IFVAR}
-	test -n "${l2tpsession}" || return 0
-	
-	ebegin "Creating L2TPv3 link ${IFVAR}"
-	local l2tp_err s_name s_tunnel_id s_session_id s_peer_session_id s_cookie s_peer_cookie s_offset s_peer_offset s_l2spec_type
-	if ! _l2tp_eval_props s_ "name|tunnel_id|session_id|peer_session_id|cookie|peer_cookie|offset|peer_offset|l2spec_type" "${l2tpsession}"; then
-		eend 1 "l2tpsession_${IFVAR} syntax error: $l2tp_err"
-		return 1
-	fi
-	if [ -n "$s_name" ]; then
-		eend 1 "l2tpsession_${IFVAR} error: please remove the \"name\" parameter (this parameter is managed by the system)"
-		return 1
-	fi
-	# Try to load mendatory l2tp_eth kernel module
-	if ! modprobe l2tp_eth; then
-		eend 1 "l2tp_eth module not present in your kernel (please enable CONFIG_L2TP_ETH option in your kernel config)"
-		return 1
-	fi
-	local l2tptunnel=
-	eval l2tptunnel=\$l2tptunnel_${IFVAR}
-	if [ -n "${l2tptunnel}" ]; then
-		local t_tunnel_id t_encap t_local t_remote t_peer_tunnel_id t_udp_sport t_udp_dport
-		_l2tp_eval_props t_ "remote|local|encap|tunnel_id|peer_tunnel_id|encap|udp_sport|udp_dport" "${l2tptunnel}"
-		# if encap=ip we need l2tp_ip kernel module
-		if [ "${t_encap^^}" = "IP" ] && ! modprobe l2tp_ip; then
-			eend 1 "l2tp_ip module not present in your kernel (please enable CONFIG_L2TP_IP option in your kernel config)"
-			return 1
-		fi
-		# Search for an existing tunnel with the same ID
-		local f_tunnel_id f_encap f_local f_remote f_peer_tunnel_id f_udp_sport f_udp_dport
-		if _l2tp_get_tunnel_info f_ $t_tunnel_id; then
-			# check if the existing tunnel has the same property than expected
-			if [ "tunnel_id:$f_tunnel_id;encap:$f_encap;local:$f_local;remote:$f_remote;
-			peer_tunnel_id:$f_peer_tunnel_id;udp_sport:$f_udp_sport;udp_dport:$f_udp_dport" \
-			!= "tunnel_id:$t_tunnel_id;encap:${t_encap^^};local:$t_local;remote:$t_remote;
-			peer_tunnel_id:$t_peer_tunnel_id;udp_sport:$t_udp_sport;udp_dport:$t_udp_dport" ]; then
-				eend 1 "There are an existing tunnel with id=$s_tunnel_id, but the properties mismatch with the one you want to create"
-				return 1
-			fi
+	local declared_session declared_tunnel l2tpsession l2tptunnel
+	local name peer_session_id session_id tunnel_id
+	local encap local peer_tunnel_id remote
+	local key
+
+	if key="l2tpsession_${IFVAR:?}"; ! eval "[ \${${key}+set} ]"; then
+		return
+	elif eval "l2tpsession=\$${key}"; _is_blank "${l2tpsession}"; then
+		eend 1 "${key} is defined but its value is blank"
+	elif ! declared_session=$(_l2tp_parse_opts "${l2tpsession}" "peer_session_id session_id tunnel_id" "name"); then
+		eend 1 "${key} is missing at least one required parameter"
+	elif eval "${declared_session}"; [ "${name+set}" ]; then
+		eend 1 "${key} defines a \"name\" parameter, which is forbidden by netifrc"
+	elif ! modprobe l2tp_eth; then
+		eend 1 "Couldn't load the l2tp_eth module (perhaps the CONFIG_L2TP_ETH kernel option is disabled)"
+	elif key="l2tptunnel_${IFVAR}"; eval "[ \${${key}+set} ]"; then
+		if eval "l2tptunnel=\$${key}"; _is_blank "${l2tptunnel}"; then
+			eend 1 "${key} is defined but its value is blank"
+		elif ! declared_tunnel=$(_l2tp_parse_opts "${l2tptunnel}" "local peer_tunnel_id remote tunnel_id" "encap"); then
+			eend 1 "${key} is missing at least one required parameter"
+		elif set -- "${tunnel_id}"; eval "${declared_tunnel}"; [ "$1" != "${tunnel_id}" ]; then
+			eend 1 "${key} defines a \"tunnel_id\" parameter that contradicts l2tpsession_${IFVAR}"
+		elif _l2tp_should_add_tunnel "${tunnel_id}" "${declared_tunnel}"; set -- $?; [ "$1" -eq 2 ]; then
+			eend 1 "Tunnel #${tunnel_id} exists but its properties mismatch those defined by ${key}"
+		elif [ "$1" -eq 1 ]; then
+			# The config matches an existing tunnel.
+			true
+		elif [ "${encap}" = ip ] && ! modprobe l2tp_ip; then
+			eend 1 "Couldn't load the l2tp_ip module (perhaps the CONFIG_L2TP_IP kernel option is disabled)"
 		else
-			veinfo ip l2tp add tunnel ${l2tptunnel}
-			_ip_l2tp_add tunnel ${l2tptunnel} || return 1
+			ebegin "Creating L2TPv3 tunnel (tunnel_id ${tunnel_id})"
+			printf %s "l2tp add tunnel ${l2tptunnel}" \
+			| xargs -E '' ip
+			eend $?
 		fi
-	elif ! ip l2tp show tunnel | grep -Eq "^Tunnel $s_tunnel_id,"; then
-		# no l2tptunnel_<INTF> declaration, assume that the tunnel is already present
-		# checking if tunnel_id exists otherwise raise an error
-		eend 1 "Tunnel id=$s_tunnel_id no found (you may have to set l2tptunnel_${IFVAR})"
-		return 1
-	fi
-	veinfo ip l2tp add session ${l2tpsession} name "${IFACE}"
-	_ip_l2tp_add session ${l2tpsession} name "${IFACE}" || return 1
-	_up
-}
+	elif ! _l2tp_has_tunnel "${tunnel_id}"; then
+		# A tunnel may incorporate more than one session (link). This
+		# module allows for the user not to define a tunnel for a given
+		# session. In that case, it will be expected that the required
+		# tunnel has already been created to satisfy some other session.
+		eend 1 "Tunnel #${tunnel_id} not found (defining ${key} may be required)"
+	fi || return
 
+	ebegin "Creating L2TPv3 session (session_id ${session_id} tunnel_id ${tunnel_id})"
+	printf %s "l2tp add session ${l2tpsession} name ${IFACE:?}" \
+	| xargs -E '' ip && _up
+	eend $?
+}
 
 l2tp_post_stop()
 {
-	local session_id tunnel_id
-	_is_l2tp || return 0
-	
-	ebegin "Destroying L2TPv3 link ${IFACE}"
-	veinfo ip l2tp del session tunnel_id $tunnel_id session_id $session_id
-	ip l2tp del session tunnel_id $tunnel_id session_id $session_id
-	if ! ip l2tp show session | grep -Eq "^Session [0-9]+ in tunnel $tunnel_id\$"; then
-		#tunnel $tunnel_id no longer used, destoying it...
-		veinfo ip l2tp del tunnel tunnel_id $tunnel_id
-		ip l2tp del tunnel tunnel_id $tunnel_id
+	local existing_session session_id tunnel_id
+
+	# This function may be invoked for every interface. If not a virtual
+	# interface, it can't possibly be one that's managed by this module, in
+	# which case running ip(8) and awk(1) would be a needless expense.
+	[ -e /sys/devices/virtual/net/"${IFACE:?}" ] \
+	&& existing_session=$(_l2tp_parse_existing_session 2>/dev/null) \
+	|| return 0
+
+	eval "${existing_session}"
+	set -- session_id "${session_id}" tunnel_id "${tunnel_id}"
+	ebegin "Destroying L2TPv3 session ($*)"
+	ip l2tp del session "$@"
+	eend $? &&
+	if ! _l2tp_in_session "${tunnel_id}"; then
+		shift 2
+		ebegin "Destroying L2TPv3 tunnel ($*)"
+		ip l2tp del tunnel "$@"
+		eend $?
 	fi
-	eend $?
 }
-- 
2.39.0