Count firewall-expanded all/any specs for limiter guard

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.

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) {