unkin/tomswall
4
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases 3 Wiki Activity

Merge pull request 'Include firewall zone in all/any rule expansion' (#28) from benvin/all-includes-fw into main

Browse Source 
Reviewed-on: #28
This commit was merged in pull request #28.
This commit is contained in:
BenVincent
2026-10-04 15:21:24 +11:00
parent 2dd408c54e e3130b6c3b
commit 110b109d97
2 changed files with 178 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/nftables/compiler.go
+68 -27
View File
@@ -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)
}
}
@@ -467,26 +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 {
for _, src := range c.zoneSpecs(srcSpec) {
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 c.zoneSpecs(dstSpec) {
// 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
}
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
}
}
}
@@ -496,18 +496,39 @@ 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 _, p := range c.zonePairs(srcSpec, dstSpec, fwZone) {
n += len(splitAddrs(p[0].Addr)) * len(splitAddrs(p[1].Addr))
}
return n * len(splitAddrs(origDest))
}
// 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})
}
}
return out
}
// zoneSpecs expands a comma zone list; "all"/"any" stay global and "all!x,y" becomes every zone but x and y.
@@ -526,6 +547,26 @@ 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, bool) {
out, global := specs, false
for _, s := range specs {
if fwZone != "" && isGlobalZone(s.Zone) {
global = true
if fw := (config.ZoneSpec{Zone: fwZone, Addr: s.Addr}); !slices.Contains(out, fw) {
out = append(out, fw)
}
}
}
return out, global
}
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, "+")
internal/nftables/compiler_test.go
+110
View File
@@ -2222,6 +2222,10 @@ 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"},
{Action: config.RuleAccept, Source: "net,all", Dest: "fw", RateLimit: "10/sec"},
} {
t.Run(r.Source+">"+r.Dest, func(t *testing.T) {
cfg := &config.Config{
@@ -2799,3 +2803,109 @@ 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
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}},
{"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) {
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)
}
}
})
}
}
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)
}
}
}