From e48e9079bdf461dba8beef15d6e693b678bdc525 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 4 Oct 2026 15:06:11 +1100 Subject: [PATCH 1/5] 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") } } From ff4c9b63e88a915fce09594e3dc42329de3bcb91 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 4 Oct 2026 15:06:24 +1100 Subject: [PATCH 2/5] Include firewall zone in all/any rule expansion --- internal/nftables/compiler.go | 28 ++++++++++++++++-- internal/nftables/compiler_test.go | 46 ++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go index a86c5b5..cfdbff5 100644 --- a/internal/nftables/compiler.go +++ b/internal/nftables/compiler.go @@ -467,7 +467,11 @@ func (c *Compiler) compileOneRule(state *FirewallState, tag, srcSpec, dstSpec, p dports, sports config.PortSpec, action config.RuleAction, logLevel string, dnatDest, origDest string, fwZone string, section config.RuleSection) error { - for _, src := range c.zoneSpecs(srcSpec) { + srcs := c.zoneSpecs(srcSpec) + if action != config.RuleDNAT && action != config.RuleRedirect { + srcs = withFirewall(srcs, fwZone) + } + for _, src := range srcs { for _, srcAddr := range splitAddrs(src.Addr) { for _, od := range splitAddrs(origDest) { if action == config.RuleDNAT || action == config.RuleRedirect { @@ -476,7 +480,10 @@ func (c *Compiler) compileOneRule(state *FirewallState, tag, srcSpec, dstSpec, p } continue } - for _, dst := range c.zoneSpecs(dstSpec) { + for _, dst := range withFirewall(c.zoneSpecs(dstSpec), fwZone) { + if src.Zone == fwZone && dst.Zone == fwZone && (isGlobalZone(srcSpec) || isGlobalZone(dstSpec)) { + continue + } // Exclusion expansion never pairs fw with itself, and pairs a zone with itself only for "all+". if src.Zone == dst.Zone && (isZoneExclusion(srcSpec) || isZoneExclusion(dstSpec)) && (src.Zone == fwZone || !strings.Contains(srcSpec, "+!") && !strings.Contains(dstSpec, "+!")) { @@ -526,6 +533,23 @@ func (c *Compiler) zoneSpecs(spec string) []config.ZoneSpec { return out } +// withFirewall adds the firewall zone beside a global all/any spec, which otherwise only reaches forward. +func withFirewall(specs []config.ZoneSpec, fwZone string) []config.ZoneSpec { + out := specs + for _, s := range specs { + if fwZone != "" && isGlobalZone(s.Zone) { + out = append(out, config.ZoneSpec{Zone: fwZone, Addr: s.Addr}) + } + } + return out +} + +func isGlobalZone(spec string) bool { + zone, _ := splitZoneSpec(spec) + base := strings.TrimSuffix(zone, "+") + return base == "all" || base == "any" +} + func isZoneExclusion(spec string) bool { base, _, ok := strings.Cut(spec, "!") base = strings.TrimSuffix(base, "+") diff --git a/internal/nftables/compiler_test.go b/internal/nftables/compiler_test.go index 7be75de..7fdbc39 100644 --- a/internal/nftables/compiler_test.go +++ b/internal/nftables/compiler_test.go @@ -2799,3 +2799,49 @@ func TestCompile_ConntrackHelperZones(t *testing.T) { }) } } + +func TestCompile_AllIncludesFirewall(t *testing.T) { + cases := []struct { + src, dst string + want map[string]int + }{ + {"all", "all", map[string]int{"input": 1, "output": 1, "forward": 1}}, + {"net", "all", map[string]int{"input": 1, "output": 0, "forward": 1}}, + {"all", "net", map[string]int{"input": 0, "output": 1, "forward": 1}}, + {"all", "fw", map[string]int{"input": 1, "output": 0, "forward": 0}}, + {"all:192.0.2.0/24", "fw", map[string]int{"input": 1, "output": 0, "forward": 0}}, + {"all!fw", "all!fw", map[string]int{"input": 0, "output": 0, "forward": 2}}, + } + for _, tc := range cases { + t.Run(tc.src+"->"+tc.dst, func(t *testing.T) { + cfg := &config.Config{ + Settings: config.Settings{TableName: "test", AddressFamily: config.FamilyINET}, + Zones: map[string]config.Zone{ + "fw": {Type: config.ZoneFirewall}, + "net": {Type: config.ZoneIP}, + "loc": {Type: config.ZoneIP}, + }, + Interfaces: []config.Interface{{Zone: "net", Interface: "eth0"}, {Zone: "loc", Interface: "eth1"}}, + Rules: []config.Rule{ + {Action: config.RuleAccept, Source: tc.src, Dest: tc.dst, Proto: "icmp", DPort: config.PortSpec{"8"}}, + }, + PortGroups: map[string]config.PortGroup{}, + } + state, err := NewCompiler(cfg).Compile() + if err != nil { + t.Fatalf("Compile() error: %v", err) + } + for chain, want := range tc.want { + got := 0 + for _, r := range state.Rules[chain] { + if r.Tag == "rule:0" { + got++ + } + } + if got != want { + t.Errorf("%s: got %d rule:0 entries, want %d", chain, got, want) + } + } + }) + } +} From 0551120eecd41c8fb846b8f9cc205738c748dd90 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 4 Oct 2026 15:07:41 +1100 Subject: [PATCH 3/5] Scope DHCP accept rules to IPv4 --- internal/nftables/compiler.go | 11 ++++++--- internal/nftables/compiler_test.go | 36 +++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go index 42fcd65..6a9cf97 100644 --- a/internal/nftables/compiler.go +++ b/internal/nftables/compiler.go @@ -129,7 +129,8 @@ func (c *Compiler) compileDHCP(state *FirewallState) { dhcp := func(chain, dir string, ifaceMatch []expr.Any) { state.Rules[chain] = append(state.Rules[chain], ManagedRule{ Chain: chain, - Exprs: append(append(append(ifaceMatch, + Exprs: append(append(append(append(ifaceMatch, + matchNFProto(unix.NFPROTO_IPV4)...), matchProtoNum(unix.IPPROTO_UDP)...), matchDPortRange(67, 68)...), &expr.Verdict{Kind: expr.VerdictAccept}), @@ -1536,10 +1537,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 31df18d..982df5f 100644 --- a/internal/nftables/compiler_test.go +++ b/internal/nftables/compiler_test.go @@ -1037,18 +1037,42 @@ func TestCompile_DHCP(t *testing.T) { } 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"}, + 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]) From df9326330a0a4466e1bac9dcb6ac3bdfc36130b3 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 4 Oct 2026 15:08:48 +1100 Subject: [PATCH 4/5] Count firewall-expanded all/any specs for limiter guard --- internal/nftables/compiler.go | 43 +++++++++++++++++------------ internal/nftables/compiler_test.go | 44 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go index cfdbff5..12d2940 100644 --- a/internal/nftables/compiler.go +++ b/internal/nftables/compiler.go @@ -381,7 +381,7 @@ func (c *Compiler) compileRules(state *FirewallState) error { if err != nil { return fmt.Errorf("rule[%d]: %w", i, err) } - if len(matches)*c.specCount(rule.Source, rule.Dest, rule.OrigDest, rule.Action) > 1 { + if len(matches)*c.specCount(rule.Source, rule.Dest, rule.OrigDest, fwZone, rule.Action) > 1 { return fmt.Errorf("rule[%d]: ratelimit/connlimit cannot be combined with proto, port, zone or address lists (each expanded rule would get its own limiter)", i) } } @@ -481,12 +481,7 @@ func (c *Compiler) compileOneRule(state *FirewallState, tag, srcSpec, dstSpec, p continue } for _, dst := range withFirewall(c.zoneSpecs(dstSpec), fwZone) { - if src.Zone == fwZone && dst.Zone == fwZone && (isGlobalZone(srcSpec) || isGlobalZone(dstSpec)) { - continue - } - // Exclusion expansion never pairs fw with itself, and pairs a zone with itself only for "all+". - if src.Zone == dst.Zone && (isZoneExclusion(srcSpec) || isZoneExclusion(dstSpec)) && - (src.Zone == fwZone || !strings.Contains(srcSpec, "+!") && !strings.Contains(dstSpec, "+!")) { + if skipPair(srcSpec, dstSpec, src.Zone, dst.Zone, fwZone) { continue } for _, dstAddr := range splitAddrs(dst.Addr) { @@ -503,18 +498,32 @@ func (c *Compiler) compileOneRule(state *FirewallState, tag, srcSpec, dstSpec, p } // specCount is how many zone/address combinations compileOneRule expands src and dst into. -func (c *Compiler) specCount(srcSpec, dstSpec, origDest string, action config.RuleAction) int { - count := func(spec string) (n int) { - for _, z := range c.zoneSpecs(spec) { - n += len(splitAddrs(z.Addr)) - } - return n - } - n := count(srcSpec) * len(splitAddrs(origDest)) +func (c *Compiler) specCount(srcSpec, dstSpec, origDest, fwZone string, action config.RuleAction) int { + n := 0 if action == config.RuleDNAT || action == config.RuleRedirect { - return n + for _, src := range c.zoneSpecs(srcSpec) { + n += len(splitAddrs(src.Addr)) + } + return n * len(splitAddrs(origDest)) } - return n * count(dstSpec) + for _, src := range withFirewall(c.zoneSpecs(srcSpec), fwZone) { + for _, dst := range withFirewall(c.zoneSpecs(dstSpec), fwZone) { + if !skipPair(srcSpec, dstSpec, src.Zone, dst.Zone, fwZone) { + n += len(splitAddrs(src.Addr)) * len(splitAddrs(dst.Addr)) + } + } + } + return n * len(splitAddrs(origDest)) +} + +// skipPair reports whether compileOneRule drops a src/dst zone pair from the expansion. +func skipPair(srcSpec, dstSpec, srcZone, dstZone, fwZone string) bool { + if srcZone == fwZone && dstZone == fwZone && (isGlobalZone(srcSpec) || isGlobalZone(dstSpec)) { + return true + } + // Exclusion expansion never pairs fw with itself, and pairs a zone with itself only for "all+". + return srcZone == dstZone && (isZoneExclusion(srcSpec) || isZoneExclusion(dstSpec)) && + (srcZone == fwZone || !strings.Contains(srcSpec, "+!") && !strings.Contains(dstSpec, "+!")) } // zoneSpecs expands a comma zone list; "all"/"any" stay global and "all!x,y" becomes every zone but x and y. diff --git a/internal/nftables/compiler_test.go b/internal/nftables/compiler_test.go index 7fdbc39..776e1c5 100644 --- a/internal/nftables/compiler_test.go +++ b/internal/nftables/compiler_test.go @@ -2222,6 +2222,9 @@ func TestCompile_CommaZoneListLimitErrors(t *testing.T) { {Action: config.RuleAccept, Source: "net", Dest: "fw,lan", RateLimit: "10/sec:5"}, {Action: config.RuleAccept, Source: "net,lan", Dest: "fw", ConnLimit: "10"}, {Action: config.RuleAccept, Source: "net", Dest: "fw:192.0.2.1,198.51.100.1", RateLimit: "10/sec"}, + {Action: config.RuleAccept, Source: "all", Dest: "all", RateLimit: "10/sec"}, + {Action: config.RuleAccept, Source: "net", Dest: "all", ConnLimit: "10"}, + {Action: config.RuleAccept, Source: "all", Dest: "net", RateLimit: "10/sec"}, } { t.Run(r.Source+">"+r.Dest, func(t *testing.T) { cfg := &config.Config{ @@ -2800,6 +2803,46 @@ func TestCompile_ConntrackHelperZones(t *testing.T) { } } +func TestCompile_AllIncludesFirewallMatches(t *testing.T) { + cases := []struct { + name string + rule config.Rule + want map[string][]string + }{ + { + name: "all address kept on added fw rules", + rule: config.Rule{Action: config.RuleAccept, Source: "all:192.0.2.5", Dest: "all"}, + want: map[string][]string{"input": {"saddr=192.0.2.5"}, "output": {"saddr=192.0.2.5"}, "forward": {"saddr=192.0.2.5"}}, + }, + { + name: "dnat with all source unchanged", + rule: config.Rule{Action: config.RuleDNAT, Source: "all", Dest: "net:192.0.2.10", Proto: "tcp", DPort: config.PortSpec{"80"}}, + want: map[string][]string{"prerouting": {""}}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg := &config.Config{ + Settings: config.Settings{TableName: "test", AddressFamily: config.FamilyINET}, + Zones: map[string]config.Zone{"fw": {Type: config.ZoneFirewall}, "net": {Type: config.ZoneIP}}, + Interfaces: []config.Interface{{Zone: "net", Interface: "eth0"}}, + Rules: []config.Rule{tc.rule}, + PortGroups: map[string]config.PortGroup{}, + } + state := mustCompile(t, cfg) + got := map[string][]string{} + for _, chain := range []string{"prerouting", "input", "output", "forward"} { + for _, r := range taggedRules(state, chain, "rule:0") { + got[chain] = append(got[chain], describeRule(r)) + } + } + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("rules = %q, want %q", got, tc.want) + } + }) + } +} + func TestCompile_AllIncludesFirewall(t *testing.T) { cases := []struct { src, dst string @@ -2811,6 +2854,7 @@ func TestCompile_AllIncludesFirewall(t *testing.T) { {"all", "fw", map[string]int{"input": 1, "output": 0, "forward": 0}}, {"all:192.0.2.0/24", "fw", map[string]int{"input": 1, "output": 0, "forward": 0}}, {"all!fw", "all!fw", map[string]int{"input": 0, "output": 0, "forward": 2}}, + {"all+", "all", map[string]int{"input": 1, "output": 1, "forward": 1}}, } for _, tc := range cases { t.Run(tc.src+"->"+tc.dst, func(t *testing.T) { From e3130b6c3b132026de3340dc3bc2bd9bd7542d96 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 4 Oct 2026 15:10:49 +1100 Subject: [PATCH 5/5] Expand all/any firewall pairs per zone and dedupe fw --- internal/nftables/compiler.go | 78 ++++++++++++++++-------------- internal/nftables/compiler_test.go | 20 ++++++++ 2 files changed, 63 insertions(+), 35 deletions(-) diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go index 12d2940..0308049 100644 --- a/internal/nftables/compiler.go +++ b/internal/nftables/compiler.go @@ -467,28 +467,26 @@ func (c *Compiler) compileOneRule(state *FirewallState, tag, srcSpec, dstSpec, p dports, sports config.PortSpec, action config.RuleAction, logLevel string, dnatDest, origDest string, fwZone string, section config.RuleSection) error { - srcs := c.zoneSpecs(srcSpec) - if action != config.RuleDNAT && action != config.RuleRedirect { - srcs = withFirewall(srcs, fwZone) - } - for _, src := range srcs { - for _, srcAddr := range splitAddrs(src.Addr) { - for _, od := range splitAddrs(origDest) { - if action == config.RuleDNAT || action == config.RuleRedirect { + if action == config.RuleDNAT || action == config.RuleRedirect { + for _, src := range c.zoneSpecs(srcSpec) { + for _, srcAddr := range splitAddrs(src.Addr) { + for _, od := range splitAddrs(origDest) { if err := c.compileDNATRule(state, tag, src.Zone, srcAddr, od, dstSpec, proto, dports, action, logLevel); err != nil { return err } - continue } - for _, dst := range withFirewall(c.zoneSpecs(dstSpec), fwZone) { - if skipPair(srcSpec, dstSpec, src.Zone, dst.Zone, fwZone) { - continue - } - for _, dstAddr := range splitAddrs(dst.Addr) { - if err := c.compileZonePair(state, tag, src.Zone, srcAddr, dst.Zone, dstAddr, od, proto, - dports, sports, action, logLevel, fwZone, section); err != nil { - return err - } + } + } + return nil + } + for _, p := range c.zonePairs(srcSpec, dstSpec, fwZone) { + src, dst := p[0], p[1] + for _, srcAddr := range splitAddrs(src.Addr) { + for _, od := range splitAddrs(origDest) { + for _, dstAddr := range splitAddrs(dst.Addr) { + if err := c.compileZonePair(state, tag, src.Zone, srcAddr, dst.Zone, dstAddr, od, proto, + dports, sports, action, logLevel, fwZone, section); err != nil { + return err } } } @@ -506,24 +504,31 @@ func (c *Compiler) specCount(srcSpec, dstSpec, origDest, fwZone string, action c } return n * len(splitAddrs(origDest)) } - for _, src := range withFirewall(c.zoneSpecs(srcSpec), fwZone) { - for _, dst := range withFirewall(c.zoneSpecs(dstSpec), fwZone) { - if !skipPair(srcSpec, dstSpec, src.Zone, dst.Zone, fwZone) { - n += len(splitAddrs(src.Addr)) * len(splitAddrs(dst.Addr)) - } - } + for _, p := range c.zonePairs(srcSpec, dstSpec, fwZone) { + n += len(splitAddrs(p[0].Addr)) * len(splitAddrs(p[1].Addr)) } return n * len(splitAddrs(origDest)) } -// skipPair reports whether compileOneRule drops a src/dst zone pair from the expansion. -func skipPair(srcSpec, dstSpec, srcZone, dstZone, fwZone string) bool { - if srcZone == fwZone && dstZone == fwZone && (isGlobalZone(srcSpec) || isGlobalZone(dstSpec)) { - return true +// zonePairs is the src/dst zone expansion of a non-DNAT rule, with fw added beside all/any. +func (c *Compiler) zonePairs(srcSpec, dstSpec, fwZone string) [][2]config.ZoneSpec { + srcs, srcGlobal := withFirewall(c.zoneSpecs(srcSpec), fwZone) + dsts, dstGlobal := withFirewall(c.zoneSpecs(dstSpec), fwZone) + var out [][2]config.ZoneSpec + for _, src := range srcs { + for _, dst := range dsts { + if src.Zone == fwZone && dst.Zone == fwZone && (srcGlobal || dstGlobal) { + continue + } + // Exclusion expansion never pairs fw with itself, and pairs a zone with itself only for "all+". + if src.Zone == dst.Zone && (isZoneExclusion(srcSpec) || isZoneExclusion(dstSpec)) && + (src.Zone == fwZone || !strings.Contains(srcSpec, "+!") && !strings.Contains(dstSpec, "+!")) { + continue + } + out = append(out, [2]config.ZoneSpec{src, dst}) + } } - // Exclusion expansion never pairs fw with itself, and pairs a zone with itself only for "all+". - return srcZone == dstZone && (isZoneExclusion(srcSpec) || isZoneExclusion(dstSpec)) && - (srcZone == fwZone || !strings.Contains(srcSpec, "+!") && !strings.Contains(dstSpec, "+!")) + return out } // zoneSpecs expands a comma zone list; "all"/"any" stay global and "all!x,y" becomes every zone but x and y. @@ -543,14 +548,17 @@ func (c *Compiler) zoneSpecs(spec string) []config.ZoneSpec { } // withFirewall adds the firewall zone beside a global all/any spec, which otherwise only reaches forward. -func withFirewall(specs []config.ZoneSpec, fwZone string) []config.ZoneSpec { - out := specs +func withFirewall(specs []config.ZoneSpec, fwZone string) ([]config.ZoneSpec, bool) { + out, global := specs, false for _, s := range specs { if fwZone != "" && isGlobalZone(s.Zone) { - out = append(out, config.ZoneSpec{Zone: fwZone, Addr: s.Addr}) + global = true + if fw := (config.ZoneSpec{Zone: fwZone, Addr: s.Addr}); !slices.Contains(out, fw) { + out = append(out, fw) + } } } - return out + return out, global } func isGlobalZone(spec string) bool { diff --git a/internal/nftables/compiler_test.go b/internal/nftables/compiler_test.go index 776e1c5..a83cd22 100644 --- a/internal/nftables/compiler_test.go +++ b/internal/nftables/compiler_test.go @@ -2225,6 +2225,7 @@ func TestCompile_CommaZoneListLimitErrors(t *testing.T) { {Action: config.RuleAccept, Source: "all", Dest: "all", RateLimit: "10/sec"}, {Action: config.RuleAccept, Source: "net", Dest: "all", ConnLimit: "10"}, {Action: config.RuleAccept, Source: "all", Dest: "net", RateLimit: "10/sec"}, + {Action: config.RuleAccept, Source: "net,all", Dest: "fw", RateLimit: "10/sec"}, } { t.Run(r.Source+">"+r.Dest, func(t *testing.T) { cfg := &config.Config{ @@ -2855,6 +2856,8 @@ func TestCompile_AllIncludesFirewall(t *testing.T) { {"all:192.0.2.0/24", "fw", map[string]int{"input": 1, "output": 0, "forward": 0}}, {"all!fw", "all!fw", map[string]int{"input": 0, "output": 0, "forward": 2}}, {"all+", "all", map[string]int{"input": 1, "output": 1, "forward": 1}}, + {"net,all", "fw", map[string]int{"input": 2, "output": 0, "forward": 0}}, + {"fw,all", "net", map[string]int{"input": 0, "output": 1, "forward": 1}}, } for _, tc := range cases { t.Run(tc.src+"->"+tc.dst, func(t *testing.T) { @@ -2889,3 +2892,20 @@ func TestCompile_AllIncludesFirewall(t *testing.T) { }) } } + +func TestSpecCount_CommaAllMatchesExpansion(t *testing.T) { + c := NewCompiler(&config.Config{ + Zones: map[string]config.Zone{"fw": {Type: config.ZoneFirewall}, "net": {Type: config.ZoneIP}}, + }) + for _, tc := range []struct { + src, dst string + want int + }{ + {"net,all", "fw", 2}, + {"fw,all", "net", 2}, + } { + if got := c.specCount(tc.src, tc.dst, "", "fw", config.RuleAccept); got != tc.want { + t.Errorf("specCount(%s, %s) = %d, want %d", tc.src, tc.dst, got, tc.want) + } + } +}