util/linuxfw: fix nil pointer panic in connmark rules without IPv6 (#18946)

When IPv6 is unavailable on a system, AddConnmarkSaveRule() and
DelConnmarkSaveRule() would panic with a nil pointer dereference.
Both methods directly iterated over []iptablesInterface{i.ipt4, i.ipt6}
without checking if ipt6 was nil.

Use `getTables()` instead to properly retrieve the available tables
on a given system

Fixes #3310

Signed-off-by: Mike O'Driscoll <mikeo@tailscale.com>

util/linuxfw/fake.go

@@ -26,13 +26,15 @@ type fakeRule struct {
 func newFakeIPTables() *fakeIPTables {
 	return &fakeIPTables{
 		n: map[string][]string{
-			"filter/INPUT":    nil,
-			"filter/OUTPUT":   nil,
-			"filter/FORWARD":  nil,
-			"nat/PREROUTING":  nil,
-			"nat/OUTPUT":      nil,
-			"nat/POSTROUTING": nil,
-			"mangle/FORWARD":  nil,
+			"filter/INPUT":      nil,
+			"filter/OUTPUT":     nil,
+			"filter/FORWARD":    nil,
+			"nat/PREROUTING":    nil,
+			"nat/OUTPUT":        nil,
+			"nat/POSTROUTING":   nil,
+			"mangle/FORWARD":    nil,
+			"mangle/PREROUTING": nil,
+			"mangle/OUTPUT":     nil,
 		},
 	}
 }
@@ -80,7 +82,7 @@ func (n *fakeIPTables) Delete(table, chain string, args ...string) error {
 				return nil
 			}
 		}
-		return fmt.Errorf("delete of unknown rule %q from %s", strings.Join(args, " "), k)
+		return errors.New("exitcode:1")
 	} else {
 		return fmt.Errorf("unknown table/chain %s", k)
 	}

util/linuxfw/iptables_runner.go

@@ -533,7 +533,7 @@ func (i *iptablesRunner) DelStatefulRule(tunname string) error {
 // proper routing table lookups for exit nodes and subnet routers.
 func (i *iptablesRunner) AddConnmarkSaveRule() error {
 	// Check if rules already exist (idempotency)
-	for _, ipt := range []iptablesInterface{i.ipt4, i.ipt6} {
+	for _, ipt := range i.getTables() {
 		rules, err := ipt.List("mangle", "PREROUTING")
 		if err != nil {
 			continue
@@ -551,7 +551,7 @@ func (i *iptablesRunner) AddConnmarkSaveRule() error {
 
 	// mangle/PREROUTING: Restore mark from conntrack for ESTABLISHED/RELATED connections
 	// This runs BEFORE routing decision and rp_filter check
-	for _, ipt := range []iptablesInterface{i.ipt4, i.ipt6} {
+	for _, ipt := range i.getTables() {
 		args := []string{
 			"-m", "conntrack",
 			"--ctstate", "ESTABLISHED,RELATED",
@@ -566,7 +566,7 @@ func (i *iptablesRunner) AddConnmarkSaveRule() error {
 	}
 
 	// mangle/OUTPUT: Save mark to conntrack for NEW connections with non-zero marks
-	for _, ipt := range []iptablesInterface{i.ipt4, i.ipt6} {
+	for _, ipt := range i.getTables() {
 		args := []string{
 			"-m", "conntrack",
 			"--ctstate", "NEW",
@@ -587,7 +587,7 @@ func (i *iptablesRunner) AddConnmarkSaveRule() error {
 
 // DelConnmarkSaveRule removes conntrack marking rules added by AddConnmarkSaveRule.
 func (i *iptablesRunner) DelConnmarkSaveRule() error {
-	for _, ipt := range []iptablesInterface{i.ipt4, i.ipt6} {
+	for _, ipt := range i.getTables() {
 		// Delete PREROUTING rule
 		args := []string{
 			"-m", "conntrack",

util/linuxfw/iptables_runner_test.go

@@ -364,3 +364,143 @@ func checkSNATRuleCount(t *testing.T, iptr *iptablesRunner, ip netip.Addr, wants
 		t.Fatalf("wants %d rules, got %d", wantsRules, len(rules))
 	}
 }
+
+func TestAddAndDelConnmarkSaveRule(t *testing.T) {
+	preroutingArgs := []string{
+		"-m", "conntrack",
+		"--ctstate", "ESTABLISHED,RELATED",
+		"-j", "CONNMARK",
+		"--restore-mark",
+		"--nfmask", "0xff0000",
+		"--ctmask", "0xff0000",
+	}
+
+	outputArgs := []string{
+		"-m", "conntrack",
+		"--ctstate", "NEW",
+		"-m", "mark",
+		"!", "--mark", "0x0/0xff0000",
+		"-j", "CONNMARK",
+		"--save-mark",
+		"--nfmask", "0xff0000",
+		"--ctmask", "0xff0000",
+	}
+
+	t.Run("with_ipv6", func(t *testing.T) {
+		iptr := newFakeIPTablesRunner()
+
+		// Add connmark rules
+		if err := iptr.AddConnmarkSaveRule(); err != nil {
+			t.Fatalf("AddConnmarkSaveRule failed: %v", err)
+		}
+
+		// Verify rules exist in both IPv4 and IPv6
+		for _, proto := range []iptablesInterface{iptr.ipt4, iptr.ipt6} {
+			if exists, err := proto.Exists("mangle", "PREROUTING", preroutingArgs...); err != nil {
+				t.Fatalf("error checking PREROUTING rule: %v", err)
+			} else if !exists {
+				t.Errorf("PREROUTING connmark rule doesn't exist")
+			}
+
+			if exists, err := proto.Exists("mangle", "OUTPUT", outputArgs...); err != nil {
+				t.Fatalf("error checking OUTPUT rule: %v", err)
+			} else if !exists {
+				t.Errorf("OUTPUT connmark rule doesn't exist")
+			}
+		}
+
+		// Test idempotency - calling AddConnmarkSaveRule again should not fail or duplicate
+		if err := iptr.AddConnmarkSaveRule(); err != nil {
+			t.Fatalf("AddConnmarkSaveRule (second call) failed: %v", err)
+		}
+
+		// Verify rules still exist and weren't duplicated
+		for _, proto := range []iptablesInterface{iptr.ipt4, iptr.ipt6} {
+			preroutingRules, err := proto.List("mangle", "PREROUTING")
+			if err != nil {
+				t.Fatalf("error listing PREROUTING rules: %v", err)
+			}
+			connmarkCount := 0
+			for _, rule := range preroutingRules {
+				if strings.Contains(rule, "CONNMARK") && strings.Contains(rule, "restore-mark") {
+					connmarkCount++
+				}
+			}
+			if connmarkCount != 1 {
+				t.Errorf("expected 1 PREROUTING connmark rule, got %d", connmarkCount)
+			}
+		}
+
+		// Delete connmark rules
+		if err := iptr.DelConnmarkSaveRule(); err != nil {
+			t.Fatalf("DelConnmarkSaveRule failed: %v", err)
+		}
+
+		// Verify rules are deleted
+		for _, proto := range []iptablesInterface{iptr.ipt4, iptr.ipt6} {
+			if exists, err := proto.Exists("mangle", "PREROUTING", preroutingArgs...); err != nil {
+				t.Fatalf("error checking PREROUTING rule: %v", err)
+			} else if exists {
+				t.Errorf("PREROUTING connmark rule still exists after deletion")
+			}
+
+			if exists, err := proto.Exists("mangle", "OUTPUT", outputArgs...); err != nil {
+				t.Fatalf("error checking OUTPUT rule: %v", err)
+			} else if exists {
+				t.Errorf("OUTPUT connmark rule still exists after deletion")
+			}
+		}
+
+		// Test idempotency of deletion
+		if err := iptr.DelConnmarkSaveRule(); err != nil {
+			t.Fatalf("DelConnmarkSaveRule (second call) failed: %v", err)
+		}
+	})
+
+	t.Run("without_ipv6", func(t *testing.T) {
+		// Create an iptables runner with only IPv4 (simulating system without IPv6)
+		iptr := &iptablesRunner{
+			ipt4:              newFakeIPTables(),
+			ipt6:              nil, // IPv6 not available
+			v6Available:       false,
+			v6NATAvailable:    false,
+			v6FilterAvailable: false,
+		}
+
+		// Add connmark rules should NOT panic with nil ipt6
+		if err := iptr.AddConnmarkSaveRule(); err != nil {
+			t.Fatalf("AddConnmarkSaveRule failed with IPv6 disabled: %v", err)
+		}
+
+		// Verify rules exist ONLY in IPv4
+		if exists, err := iptr.ipt4.Exists("mangle", "PREROUTING", preroutingArgs...); err != nil {
+			t.Fatalf("error checking IPv4 PREROUTING rule: %v", err)
+		} else if !exists {
+			t.Errorf("IPv4 PREROUTING connmark rule doesn't exist")
+		}
+
+		if exists, err := iptr.ipt4.Exists("mangle", "OUTPUT", outputArgs...); err != nil {
+			t.Fatalf("error checking IPv4 OUTPUT rule: %v", err)
+		} else if !exists {
+			t.Errorf("IPv4 OUTPUT connmark rule doesn't exist")
+		}
+
+		// Delete connmark rules should NOT panic with nil ipt6
+		if err := iptr.DelConnmarkSaveRule(); err != nil {
+			t.Fatalf("DelConnmarkSaveRule failed with IPv6 disabled: %v", err)
+		}
+
+		// Verify rules are deleted from IPv4
+		if exists, err := iptr.ipt4.Exists("mangle", "PREROUTING", preroutingArgs...); err != nil {
+			t.Fatalf("error checking IPv4 PREROUTING rule: %v", err)
+		} else if exists {
+			t.Errorf("IPv4 PREROUTING connmark rule still exists after deletion")
+		}
+
+		if exists, err := iptr.ipt4.Exists("mangle", "OUTPUT", outputArgs...); err != nil {
+			t.Fatalf("error checking IPv4 OUTPUT rule: %v", err)
+		} else if exists {
+			t.Errorf("IPv4 OUTPUT connmark rule still exists after deletion")
+		}
+	})
+}