diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go index 0308049..f7dcfea 100644 --- a/internal/nftables/compiler.go +++ b/internal/nftables/compiler.go @@ -126,40 +126,23 @@ 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(append(ifaceMatch, + matchNFProto(unix.NFPROTO_IPV4)...), + 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)...)) + } } } @@ -1595,10 +1578,14 @@ func matchOrigDest(addr string) ([]expr.Any, error) { if err != nil { return nil, err } - return append([]expr.Any{ + return append(matchNFProto(proto), dst...), nil +} + +func matchNFProto(proto byte) []expr.Any { + return []expr.Any{ &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{proto}}, - }, dst...), nil + } } func matchAddrCIDR(cidr string, isSrc bool) ([]expr.Any, error) { diff --git a/internal/nftables/compiler_test.go b/internal/nftables/compiler_test.go index a83cd22..3fc2b3b 100644 --- a/internal/nftables/compiler_test.go +++ b/internal/nftables/compiler_test.go @@ -1013,38 +1013,84 @@ 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, iif, oif string }{ + {"input", "dhcp:in:eth0", "eth0", ""}, + {"output", "dhcp:out:eth0", "", "eth0"}, + {"input", "dhcp:in:br0", "br0", ""}, + {"output", "dhcp:out:br0", "", "br0"}, + {"forward", "dhcp:fwd:br0", "br0", "br0"}, + } { + r := find(want.chain, want.tag) + if r == nil { + t.Errorf("%s: no rule %s", want.chain, want.tag) + continue + } + metas := map[expr.MetaKey][]byte{} + for i := 0; i+1 < len(r.Exprs); i++ { + if m, ok := r.Exprs[i].(*expr.Meta); ok { + if c, ok := r.Exprs[i+1].(*expr.Cmp); ok && c.Op == expr.CmpOpEq { + metas[m.Key] = c.Data + } + } + } + if got := metas[expr.MetaKeyNFPROTO]; !bytes.Equal(got, []byte{unix.NFPROTO_IPV4}) { + t.Errorf("%s: nfproto %v, want ipv4 guard", want.tag, got) + } + if got := metas[expr.MetaKeyL4PROTO]; !bytes.Equal(got, []byte{unix.IPPROTO_UDP}) { + t.Errorf("%s: l4proto %v, want udp", want.tag, got) + } + for key, name := range map[expr.MetaKey]string{expr.MetaKeyIIFNAME: want.iif, expr.MetaKeyOIFNAME: want.oif} { + got, ok := metas[key] + if name == "" { + if ok { + t.Errorf("%s: unexpected meta %v match %q", want.tag, key, got) + } + } else if string(got) != name+"\x00" { + t.Errorf("%s: meta %v %q, want %q", want.tag, key, got, name) + } + } + 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") } }