From e48e9079bdf461dba8beef15d6e693b678bdc525 Mon Sep 17 00:00:00 2001
From: unkin-agent <unkin-agent@unkin.net>
Date: Sun, 4 Oct 2026 15:06:11 +1100
Subject: [PATCH] Accept DHCP on dhcp interfaces as shorewall does

---
 internal/nftables/compiler.go      | 50 +++++++++-----------------
 internal/nftables/compiler_test.go | 56 +++++++++++++++++++++---------
 2 files changed, 55 insertions(+), 51 deletions(-)

diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go
index a86c5b5..42fcd65 100644
--- a/internal/nftables/compiler.go
+++ b/internal/nftables/compiler.go
@@ -126,40 +126,22 @@ func (c *Compiler) compileDHCP(state *FirewallState) {
 			continue
 		}
 		name := iface.PhysicalName()
-		// Allow DHCPv4 client traffic (bootpc:68 → bootps:67)
-		state.Rules["input"] = append(state.Rules["input"], ManagedRule{
-			Chain: "input",
-			Exprs: append(append(append(
-				matchIfaceName(true, name),
-				matchProtoNum(unix.IPPROTO_UDP)...),
-				matchSPort(68)...),
-				matchDPort(67)...,
-			),
-			Tag: fmt.Sprintf("dhcp:in:%s", iface.Interface),
-		})
-		// Allow DHCPv4 server → client replies
-		state.Rules["input"] = append(state.Rules["input"], ManagedRule{
-			Chain: "input",
-			Exprs: append(append(append(append(
-				matchIfaceName(true, name),
-				matchProtoNum(unix.IPPROTO_UDP)...),
-				matchSPort(67)...),
-				matchDPort(68)...),
-				&expr.Verdict{Kind: expr.VerdictAccept},
-			),
-			Tag: fmt.Sprintf("dhcp:reply:%s", iface.Interface),
-		})
-		state.Rules["output"] = append(state.Rules["output"], ManagedRule{
-			Chain: "output",
-			Exprs: append(append(append(append(
-				matchIfaceName(false, name),
-				matchProtoNum(unix.IPPROTO_UDP)...),
-				matchSPort(68)...),
-				matchDPort(67)...),
-				&expr.Verdict{Kind: expr.VerdictAccept},
-			),
-			Tag: fmt.Sprintf("dhcp:out:%s", iface.Interface),
-		})
+		dhcp := func(chain, dir string, ifaceMatch []expr.Any) {
+			state.Rules[chain] = append(state.Rules[chain], ManagedRule{
+				Chain: chain,
+				Exprs: append(append(append(ifaceMatch,
+					matchProtoNum(unix.IPPROTO_UDP)...),
+					matchDPortRange(67, 68)...),
+					&expr.Verdict{Kind: expr.VerdictAccept}),
+				Tag: fmt.Sprintf("dhcp:%s:%s", dir, iface.Interface),
+			})
+		}
+		// shorewall: udp dport 67:68 both ways between fw and iface, forwarded back out a bridge
+		dhcp("input", "in", matchIfaceName(true, name))
+		dhcp("output", "out", matchIfaceName(false, name))
+		if iface.Options.Bridge {
+			dhcp("forward", "fwd", append(matchIfaceName(true, name), matchIfaceName(false, name)...))
+		}
 	}
 }
 
diff --git a/internal/nftables/compiler_test.go b/internal/nftables/compiler_test.go
index 7be75de..31df18d 100644
--- a/internal/nftables/compiler_test.go
+++ b/internal/nftables/compiler_test.go
@@ -1013,38 +1013,60 @@ func TestCompile_DHCP(t *testing.T) {
 		Zones: map[string]config.Zone{
 			"fw":  {Type: config.ZoneFirewall},
 			"net": {Type: config.ZoneIP},
+			"loc": {Type: config.ZoneIP},
 		},
 		Interfaces: []config.Interface{
 			{Zone: "net", Interface: "eth0", Options: config.InterfaceOptions{DHCP: true}},
+			{Zone: "loc", Interface: "br0", Options: config.InterfaceOptions{DHCP: true, Bridge: true}},
 		},
 		Policy: []config.Policy{
 			{Source: "all", Dest: "all", Action: config.PolicyDrop},
 		},
 		PortGroups: make(map[string]config.PortGroup),
 	}
-	c := NewCompiler(cfg)
-	state, err := c.Compile()
+	state, err := NewCompiler(cfg).Compile()
 	if err != nil {
 		t.Fatalf("Compile() error: %v", err)
 	}
 
-	foundIn := false
-	foundOut := false
-	for _, r := range state.Rules["input"] {
-		if r.Tag == "dhcp:in:eth0" || r.Tag == "dhcp:reply:eth0" {
-			foundIn = true
+	find := func(chain, tag string) *ManagedRule {
+		for i, r := range state.Rules[chain] {
+			if r.Tag == tag {
+				return &state.Rules[chain][i]
+			}
+		}
+		return nil
+	}
+	for _, want := range []struct{ chain, tag string }{
+		{"input", "dhcp:in:eth0"},
+		{"output", "dhcp:out:eth0"},
+		{"input", "dhcp:in:br0"},
+		{"output", "dhcp:out:br0"},
+		{"forward", "dhcp:fwd:br0"},
+	} {
+		r := find(want.chain, want.tag)
+		if r == nil {
+			t.Errorf("%s: no rule %s", want.chain, want.tag)
+			continue
+		}
+		v, ok := r.Exprs[len(r.Exprs)-1].(*expr.Verdict)
+		if !ok || v.Kind != expr.VerdictAccept {
+			t.Errorf("%s: last expr %#v, want accept verdict", want.tag, r.Exprs[len(r.Exprs)-1])
+		}
+		var lo, hi []byte
+		for _, e := range r.Exprs {
+			if c, ok := e.(*expr.Cmp); ok && c.Op == expr.CmpOpGte {
+				lo = c.Data
+			} else if ok && c.Op == expr.CmpOpLte {
+				hi = c.Data
+			}
+		}
+		if !bytes.Equal(lo, []byte{0, 67}) || !bytes.Equal(hi, []byte{0, 68}) {
+			t.Errorf("%s: dport range %v-%v, want 67-68", want.tag, lo, hi)
 		}
 	}
-	for _, r := range state.Rules["output"] {
-		if r.Tag == "dhcp:out:eth0" {
-			foundOut = true
-		}
-	}
-	if !foundIn {
-		t.Error("no DHCP input rule found for eth0")
-	}
-	if !foundOut {
-		t.Error("no DHCP output rule found for eth0")
+	if find("forward", "dhcp:fwd:eth0") != nil {
+		t.Error("non-bridge eth0 must not forward DHCP")
 	}
 }