fix: Move VPN masquerade rules from iptables PostUp to nftables

iptables-nft rules added by WireGuard's PostUp were being flushed by Docker on startup, breaking VPN LAN routing until the next reboot. The nftables inet wild-central table survives Docker restarts.
2026-10-23 13:32:41 +00:00
parent 1176c7715a
commit d35d0854cb
4 changed files with 76 additions and 20 deletions
--- a/internal/api/v1/handlers_nftables.go
+++ b/internal/api/v1/handlers_nftables.go
@@ -50,6 +50,20 @@ func (api *API) vpnAutoUDPPorts() []int {
 	return []int{cfg.ListenPort}
 }

+// vpnCIDR returns the VPN subnet CIDR (e.g. "10.8.0.0/24") when VPN is enabled
+// with LAN routing configured. Used to generate nftables masquerade rules.
+func (api *API) vpnCIDR() string {
+	cfg, err := api.vpn.GetConfig()
+	if err != nil || !cfg.Enabled || cfg.LanCIDR == "" {
+		return ""
+	}
+	_, vpnNet, err := net.ParseCIDR(cfg.Address)
+	if err != nil {
+		return ""
+	}
+	return vpnNet.String()
+}
+
 // syncNftablesOnly regenerates and applies nftables rules from the current global config
 // without touching HAProxy. Called when firewall settings change independently.
 // Runs errors are logged only — the caller does not wait for completion.
@@ -76,7 +90,8 @@ func (api *API) syncNftablesOnly(globalCfg *config.State) {
 	extraTCP, extraUDP := config.SplitExtraPorts(nftCfg.ExtraPorts)
 	extraUDP = append(extraUDP, api.vpnAutoUDPPorts()...)
 	ports := api.haproxy.GetListenPorts(customRoutes)
-	nftContent := api.nftables.Generate(ports, extraTCP, extraUDP, nftCfg.WANInterface)
+	vpnCIDR := api.vpnCIDR()
+	nftContent := api.nftables.Generate(ports, extraTCP, extraUDP, nftCfg.WANInterface, vpnCIDR)
 	if err := api.nftables.WriteRules(nftContent); err != nil {
 		slog.Error("failed to write nftables rules", "component", "nftables-sync", "error", err)
 		return
--- a/internal/nftables/manager.go
+++ b/internal/nftables/manager.go
@@ -35,7 +35,8 @@ func (m *Manager) GetRulesPath() string {
 // Policy is always "accept" — the allowed_tcp_ports set documents what is in use
 // and enables strict WAN filtering when wanInterface is configured.
 // extraTCPPorts and extraUDPPorts are user-specified ports to allow beyond the HAProxy set.
-func (m *Manager) Generate(haproxyPorts []int, extraTCPPorts []int, extraUDPPorts []int, wanInterface string) string {
+// vpnCIDR, when non-empty (e.g. "10.8.0.0/24"), adds forward and masquerade rules for VPN traffic.
+func (m *Manager) Generate(haproxyPorts []int, extraTCPPorts []int, extraUDPPorts []int, wanInterface string, vpnCIDR string) string {
 	// Merge and deduplicate TCP ports (HAProxy + extra TCP)
 	seen := make(map[int]bool)
 	var tcpPorts []int
@@ -108,6 +109,23 @@ func (m *Manager) Generate(haproxyPorts []int, extraTCPPorts []int, extraUDPPort
 	}

 	sb.WriteString("    }\n")
+
+	// VPN forwarding and masquerade: when vpnCIDR is set, allow forwarding between
+	// wg0 and LAN, and masquerade VPN traffic leaving the VPN subnet.
+	// These rules live in the nftables table (not iptables PostUp) so they survive
+	// Docker restarts, which flush the iptables-nft compatibility tables.
+	if vpnCIDR != "" {
+		sb.WriteString("\n    chain vpn-forward {\n")
+		sb.WriteString("        type filter hook forward priority filter; policy accept;\n")
+		sb.WriteString("        iif \"wg0\" accept\n")
+		sb.WriteString("        oif \"wg0\" ct state { established, related } accept\n")
+		sb.WriteString("    }\n")
+		sb.WriteString("\n    chain vpn-postrouting {\n")
+		sb.WriteString("        type nat hook postrouting priority srcnat; policy accept;\n")
+		fmt.Fprintf(&sb, "        ip saddr %s ip daddr != %s masquerade\n", vpnCIDR, vpnCIDR)
+		sb.WriteString("    }\n")
+	}
+
 	sb.WriteString("}\n")

 	return sb.String()
--- a/internal/nftables/manager_test.go
+++ b/internal/nftables/manager_test.go
@@ -37,7 +37,7 @@ func TestPortsToStr(t *testing.T) {

 func TestGenerate_AlwaysIncludesStructure(t *testing.T) {
 	m := NewManager("")
-	out := m.Generate([]int{80, 443}, nil, nil, "")
+	out := m.Generate([]int{80, 443}, nil, nil, "", "")

 	for _, want := range []string{
 		"table inet wild-central",
@@ -55,7 +55,7 @@ func TestGenerate_AlwaysIncludesStructure(t *testing.T) {

 func TestGenerate_WithPorts_IncludesElements(t *testing.T) {
 	m := NewManager("")
-	out := m.Generate([]int{80, 443, 8404}, nil, nil, "")
+	out := m.Generate([]int{80, 443, 8404}, nil, nil, "", "")

 	if !strings.Contains(out, "elements = { 80, 443, 8404 }") {
 		t.Errorf("expected elements line with ports, got:\n%s", out)
@@ -64,7 +64,7 @@ func TestGenerate_WithPorts_IncludesElements(t *testing.T) {

 func TestGenerate_NoPorts_TCPSetHasNoElements(t *testing.T) {
 	m := NewManager("")
-	out := m.Generate([]int{}, nil, nil, "")
+	out := m.Generate([]int{}, nil, nil, "", "")

 	// TCP set should have no elements when no ports given
 	if strings.Contains(out, "allowed_tcp_ports") && strings.Contains(out, "elements") {
@@ -83,7 +83,7 @@ func TestGenerate_NoPorts_TCPSetHasNoElements(t *testing.T) {

 func TestGenerate_ExtraPortsMerged(t *testing.T) {
 	m := NewManager("")
-	out := m.Generate([]int{80, 443}, []int{22, 443}, nil, "") // 443 deduplicated
+	out := m.Generate([]int{80, 443}, []int{22, 443}, nil, "", "") // 443 deduplicated

 	if !strings.Contains(out, "22") {
 		t.Errorf("expected extra port 22 in output, got:\n%s", out)
@@ -97,7 +97,7 @@ func TestGenerate_ExtraPortsMerged(t *testing.T) {

 func TestGenerate_NoWANInterface_NoDropRules(t *testing.T) {
 	m := NewManager("")
-	out := m.Generate([]int{80, 443}, nil, nil, "")
+	out := m.Generate([]int{80, 443}, nil, nil, "", "")

 	if strings.Contains(out, "drop") {
 		t.Errorf("expected no drop rules without WAN interface, got:\n%s", out)
@@ -106,7 +106,7 @@ func TestGenerate_NoWANInterface_NoDropRules(t *testing.T) {

 func TestGenerate_WithWANInterface_IncludesDropRules(t *testing.T) {
 	m := NewManager("")
-	out := m.Generate([]int{80, 443}, nil, nil, "eth0")
+	out := m.Generate([]int{80, 443}, nil, nil, "eth0", "")

 	if !strings.Contains(out, `iif "eth0"`) {
 		t.Errorf("expected WAN interface in rules, got:\n%s", out)
@@ -129,7 +129,7 @@ func TestGenerate_WithWANInterface_IncludesDropRules(t *testing.T) {
 func TestGenerate_AlwaysIncludesUDPSet(t *testing.T) {
 	m := NewManager("")
 	// No WAN interface — permissive mode, but UDP set should still be visible
-	out := m.Generate([]int{80, 443}, nil, []int{51820}, "")
+	out := m.Generate([]int{80, 443}, nil, []int{51820}, "", "")

 	if !strings.Contains(out, "set allowed_udp_ports") {
 		t.Errorf("expected allowed_udp_ports set even without WAN interface, got:\n%s", out)
@@ -143,9 +143,39 @@ func TestGenerate_AlwaysIncludesUDPSet(t *testing.T) {
 	}
 }

+func TestGenerate_WithVPNCIDR_IncludesMasquerade(t *testing.T) {
+	m := NewManager("")
+	out := m.Generate([]int{80, 443}, nil, nil, "", "10.8.0.0/24")
+
+	if !strings.Contains(out, "chain vpn-forward") {
+		t.Errorf("expected vpn-forward chain, got:\n%s", out)
+	}
+	if !strings.Contains(out, `iif "wg0" accept`) {
+		t.Errorf("expected wg0 accept rule, got:\n%s", out)
+	}
+	if !strings.Contains(out, "chain vpn-postrouting") {
+		t.Errorf("expected vpn-postrouting chain, got:\n%s", out)
+	}
+	if !strings.Contains(out, "ip saddr 10.8.0.0/24 ip daddr != 10.8.0.0/24 masquerade") {
+		t.Errorf("expected masquerade rule with VPN CIDR, got:\n%s", out)
+	}
+}
+
+func TestGenerate_NoVPNCIDR_NoMasquerade(t *testing.T) {
+	m := NewManager("")
+	out := m.Generate([]int{80, 443}, nil, nil, "", "")
+
+	if strings.Contains(out, "masquerade") {
+		t.Errorf("expected no masquerade without VPN CIDR, got:\n%s", out)
+	}
+	if strings.Contains(out, "vpn-forward") {
+		t.Errorf("expected no vpn-forward chain without VPN CIDR, got:\n%s", out)
+	}
+}
+
 func TestGenerate_ExtraUDPPorts(t *testing.T) {
 	m := NewManager("")
-	out := m.Generate([]int{80, 443}, nil, []int{5353}, "eth0")
+	out := m.Generate([]int{80, 443}, nil, []int{5353}, "eth0", "")

 	// User UDP port should appear in the UDP exception list
 	if !strings.Contains(out, "5353") {
--- a/internal/wireguard/manager.go
+++ b/internal/wireguard/manager.go
@@ -460,16 +460,9 @@ func (m *Manager) renderWgConfig(cfg *Config, srv *secrets, peers []*Peer) strin
 	// DNS is intentionally omitted from the server config — it belongs only in
 	// the client peer config (GeneratePeerConfigText). Setting it here would
 	// cause wg-quick to reconfigure the server's own DNS resolver.
-	// When LAN CIDR is configured: allow forwarding between wg0 and LAN, and masquerade.
-	// Uses iptables (iptables-nft on modern systems) for reliable PostUp/PreDown syntax.
-	if cfg.LanCIDR != "" {
-		_, vpnNet, _ := net.ParseCIDR(cfg.Address)
-		if vpnNet != nil {
-			vpnCIDR := vpnNet.String()
-			fmt.Fprintf(&sb, "PostUp = iptables -I FORWARD -i wg0 -j ACCEPT; iptables -I FORWARD -o wg0 -m state --state RELATED,ESTABLISHED -j ACCEPT; iptables -t nat -A POSTROUTING -s %s ! -d %s -j MASQUERADE\n", vpnCIDR, vpnCIDR)
-			fmt.Fprintf(&sb, "PreDown = iptables -D FORWARD -i wg0 -j ACCEPT 2>/dev/null; iptables -D FORWARD -o wg0 -m state --state RELATED,ESTABLISHED -j ACCEPT 2>/dev/null; iptables -t nat -D POSTROUTING -s %s ! -d %s -j MASQUERADE 2>/dev/null; true\n", vpnCIDR, vpnCIDR)
-		}
-	}
+	// VPN forwarding and masquerade rules are managed in the nftables table
+	// (inet wild-central) rather than iptables PostUp/PreDown, so they survive
+	// Docker restarts which flush the iptables-nft compatibility tables.
 	for _, p := range peers {
 		sb.WriteString("\n[Peer]\n")
 		fmt.Fprintf(&sb, "# %s\n", p.Name)