From df9326330a0a4466e1bac9dcb6ac3bdfc36130b3 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 4 Oct 2026 15:08:48 +1100 Subject: [PATCH] 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) {