From c7e7b60f01b69f1daa399fabd2e9230eff14f68d Mon Sep 17 00:00:00 2001 From: Kroese Date: Sat, 18 Jul 2026 10:54:20 +0200 Subject: [PATCH] fix: Improve IP tables cleanup verification (#1298) --- src/network.sh | 84 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 69 insertions(+), 15 deletions(-) diff --git a/src/network.sh b/src/network.sh index bd463da..286793c 100644 --- a/src/network.sh +++ b/src/network.sh @@ -1136,6 +1136,19 @@ tableError() { return 1 } +showTableCleanupError() { + + local command="$1" + local message="$2" + + enabled "$DEBUG" || return 0 + + printf "Failed IP tables cleanup command:\n\n%s\n\n" "$command" >&2 + [ -n "$message" ] && printf "%s\n\n" "$message" >&2 + + return 0 +} + applyTables() { local ip="$1" @@ -1259,21 +1272,28 @@ applyTables() { clearTables() { - local table="" line="" - local rules="" failed="N" + local table="" line="" chain="" + local rules="" remaining="" message="" local dnat_chain="QEMU_DNAT" local rule_tag="$dnat_chain" - local re="--comment[[:space:]]+\"?$rule_tag\"?([[:space:]]|\$)" + local own_rule="--comment[[:space:]]+\"?$rule_tag\"?([[:space:]]|\$)" + local remaining_rule="^:${dnat_chain}[[:space:]]|$own_rule" # Return 2 when the currently selected backend cannot be accessed. - # This lets configureTables() distinguish it from an actual rule-cleanup failure. + # This lets configureTables() distinguish it from an actual cleanup failure. if ! rules=$(iptables-save 2> /dev/null); then + + if enabled "$DEBUG"; then + message=$(iptables-save 2>&1 > /dev/null || true) + showTableCleanupError "iptables-save" "$message" + fi + return 2 fi if [ -n "$rules" ]; then - # Delete every rule tagged with our unique identifier, + # Delete tagged rules outside the dedicated DNAT chain, # leaving all other rules intact. while IFS= read -r line; do @@ -1284,34 +1304,68 @@ clearTables() { \*raw ) table="raw" ;; esac - if [[ "$line" == -A* ]] && [[ "$line" =~ $re ]]; then + if [[ "$line" == -A* ]] && [[ "$line" =~ $own_rule ]]; then + + chain="${line#-A }" + chain="${chain%% *}" + + # Rules inside this chain are removed together by the flush below. + if [[ "$table" == "nat" && "$chain" == "$dnat_chain" ]]; then + continue + fi + line="${line/-A /-D }" # Parse the quoting produced by iptables-save before deleting the rule. - if ! printf '%s\n' "$line" | - xargs -r iptables -t "$table" > /dev/null 2>&1; then - failed="Y" + if ! message=$( + printf '%s\n' "$line" | + xargs -r iptables -t "$table" 2>&1 + ); then + showTableCleanupError "iptables -t $table $line" "$message" fi + fi done <<< "$rules" fi - # Remove the dedicated DNAT chain after deleting its rules and references. + # Remove the dedicated DNAT chain after deleting its references. if iptables -t nat -S "$dnat_chain" > /dev/null 2>&1; then - if ! iptables -t nat -F "$dnat_chain" > /dev/null 2>&1; then - failed="Y" + if ! message=$(iptables -t nat -F "$dnat_chain" 2>&1); then + showTableCleanupError "iptables -t nat -F $dnat_chain" "$message" fi - if ! iptables -t nat -X "$dnat_chain" > /dev/null 2>&1; then - failed="Y" + if ! message=$(iptables -t nat -X "$dnat_chain" 2>&1); then + showTableCleanupError "iptables -t nat -X $dnat_chain" "$message" fi fi - enabled "$failed" && return 1 + # Base the result on the final ruleset instead of intermediate errors. + if ! rules=$(iptables-save 2> /dev/null); then + + if enabled "$DEBUG"; then + message=$(iptables-save 2>&1 > /dev/null || true) + showTableCleanupError "iptables-save" "$message" + fi + + return 1 + fi + + remaining=$(grep -E -- "$remaining_rule" <<< "$rules" || true) + + if [ -n "$remaining" ]; then + + if enabled "$DEBUG"; then + warn "IP tables cleanup left the following rules or chains behind:" + echo "$remaining" >&2 + fi + + return 1 + fi + return 0 }