From b5be665902ea6b6e497c704c1e0f9ab3fded5cfd Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 3 Oct 2026 22:15:24 +1000 Subject: [PATCH 1/3] Match dest zone oif on output-chain rules and policies --- internal/nftables/compiler.go | 4 ++-- internal/nftables/compiler_test.go | 23 ++++++++++++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go index 416ca06..f492694 100644 --- a/internal/nftables/compiler.go +++ b/internal/nftables/compiler.go @@ -599,7 +599,7 @@ func (c *Compiler) compilePolicies(state *FirewallState) error { if si != "" { exprs = append(exprs, matchIfaceName(true, si)...) } - if di != "" && chain == "forward" { + if di != "" && chain != "input" { exprs = append(exprs, matchIfaceName(false, di)...) } @@ -997,7 +997,7 @@ func (c *Compiler) buildMatchExprs(srcIface, dstIface, chain, proto string, dpor if srcIface != "" { exprs = append(exprs, matchIfaceName(true, srcIface)...) } - if dstIface != "" && chain == "forward" { + if dstIface != "" && chain != "input" { exprs = append(exprs, matchIfaceName(false, dstIface)...) } diff --git a/internal/nftables/compiler_test.go b/internal/nftables/compiler_test.go index 623b4c3..46203af 100644 --- a/internal/nftables/compiler_test.go +++ b/internal/nftables/compiler_test.go @@ -1757,6 +1757,17 @@ func TestCompile_PortAndProtoLists(t *testing.T) { }) } } +func TestCompile_OutputPolicyMatchesOif(t *testing.T) { + cfg := listCfg(func(c *config.Config) { + c.Zones["lan"] = config.Zone{Type: config.ZoneIP} + c.Interfaces = append(c.Interfaces, config.Interface{Zone: "lan", Interface: "eth1"}) + c.Policy = []config.Policy{{Source: "fw", Dest: "lan", Action: config.PolicyAccept}} + }) + got := taggedRules(mustCompile(t, cfg), "output", "policy:0") + if len(got) != 1 || describeRule(got[0]) != "oif=eth1" { + t.Fatalf("fw->lan policy = %v, want one rule oif=eth1", got) + } +} // describeRule renders a rule's iif/oif/saddr/daddr matches, e.g. "iif=eth1 oif=eth2 daddr=192.0.2.1". func describeRule(r ManagedRule) string { @@ -1803,7 +1814,7 @@ func TestCompile_CommaZoneLists(t *testing.T) { { name: "fw in source list goes to output", rule: config.Rule{Action: config.RuleAccept, Source: "fw,lan", Dest: "svr", Proto: "tcp", DPort: config.PortSpec{"22"}}, - want: map[string][]string{"output": {""}, "forward": {"iif=eth1 oif=eth2"}}, + want: map[string][]string{"output": {"oif=eth2"}, "forward": {"iif=eth1 oif=eth2"}}, }, { name: "dest list with fw splits input and forward", @@ -1850,6 +1861,16 @@ func TestCompile_CommaZoneLists(t *testing.T) { rule: config.Rule{Action: config.RuleAccept, Source: "lan", Dest: "vpn,dmz"}, want: map[string][]string{"forward": {"iif=eth1"}}, }, + { + name: "fw source matches dest zone oif", + rule: config.Rule{Action: config.RuleAccept, Source: "fw", Dest: "lan,vpn,dmz", Proto: "tcp", DPort: config.PortSpec{"22"}}, + want: map[string][]string{"output": {"oif=eth1", ""}}, + }, + { + name: "fw to all has no oif", + rule: config.Rule{Action: config.RuleAccept, Source: "fw", Dest: "all:192.0.2.1"}, + want: map[string][]string{"output": {"daddr=192.0.2.1"}}, + }, { name: "blrule zone list", blrule: &config.BlruleRule{Action: config.BlruleDrop, Source: "net,anycast", Dest: "fw"}, From 44e1ba852ec450f3805488326194cd60a6142482 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 3 Oct 2026 22:18:10 +1000 Subject: [PATCH 2/3] Skip rules and policies for zones without interfaces --- internal/nftables/compiler.go | 43 ++++++++++-------------- internal/nftables/compiler_test.go | 53 ++++++++++++++++++++++++++---- 2 files changed, 64 insertions(+), 32 deletions(-) diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go index f492694..3a77c7a 100644 --- a/internal/nftables/compiler.go +++ b/internal/nftables/compiler.go @@ -417,8 +417,8 @@ func splitAddrs(addr string) []string { func (c *Compiler) compileZonePair(state *FirewallState, tag, srcZone, srcAddr, dstZone, dstAddr, proto string, dports, sports config.PortSpec, action config.RuleAction, logLevel string, fwZone string, section config.RuleSection) error { - srcIfaces := c.resolveZoneInterfaces(srcZone) - dstIfaces := c.resolveZoneInterfaces(dstZone) + srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr != "") + dstIfaces := c.resolveZoneInterfaces(dstZone, dstAddr != "") chain := c.selectChain(srcZone, dstZone, fwZone) for _, srcIface := range srcIfaces { @@ -474,7 +474,7 @@ func (c *Compiler) compileDNATRule(state *FirewallState, tag, srcZone, srcAddr, dnatPort = uint16(p) } - srcIfaces := c.resolveZoneInterfaces(srcZone) + srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr != "") matches, err := l4Matches(proto, dports, nil) if err != nil { @@ -589,8 +589,8 @@ func (c *Compiler) compilePolicies(state *FirewallState) error { } chain := c.selectChain(sz, dz, fwZone) - srcIfaces := c.resolveZoneInterfaces(sz) - dstIfaces := c.resolveZoneInterfaces(dz) + srcIfaces := c.resolveZoneInterfaces(sz, false) + dstIfaces := c.resolveZoneInterfaces(dz, false) for _, si := range srcIfaces { for _, di := range dstIfaces { @@ -932,34 +932,25 @@ func (c *Compiler) selectChain(srcZone, dstZone, fwZone string) string { return "forward" } -func (c *Compiler) resolveZoneInterfaces(zone string) []string { - if zone == "all" || zone == "" { +// resolveZoneInterfaces returns nil (fail closed) for a zone with no interfaces unless an address match narrows the rule. +func (c *Compiler) resolveZoneInterfaces(zone string, addrNarrowed bool) []string { + if z, ok := c.cfg.Zones[zone]; !ok || z.Type == config.ZoneFirewall { return []string{""} } - ifaces := c.cfg.ZoneInterfaces(zone) - if len(ifaces) > 0 { + if ifaces := c.cfg.ZoneInterfaces(zone); len(ifaces) > 0 { return ifaces } - if z, ok := c.cfg.Zones[zone]; ok && z.Type == config.ZoneIP && !c.zoneHasHosts(zone) { - if !c.warned[zone] { - if c.warned == nil { - c.warned = map[string]bool{} - } - c.warned[zone] = true - slog.Warn("compiler: zone has no interfaces, skipping its rules", "zone", zone) - } - return nil + if addrNarrowed { + return []string{""} } - return []string{""} -} - -func (c *Compiler) zoneHasHosts(zone string) bool { - for _, h := range c.cfg.Hosts { - if h.Zone == zone { - return true + if !c.warned[zone] { + if c.warned == nil { + c.warned = map[string]bool{} } + c.warned[zone] = true + slog.Warn("compiler: zone has no interfaces, skipping its rules", "zone", zone) } - return false + return nil } func (c *Compiler) expandZoneRef(ref string) []string { diff --git a/internal/nftables/compiler_test.go b/internal/nftables/compiler_test.go index 46203af..e40219f 100644 --- a/internal/nftables/compiler_test.go +++ b/internal/nftables/compiler_test.go @@ -141,17 +141,17 @@ func TestCompiler_ResolveZoneInterfaces(t *testing.T) { } c := NewCompiler(cfg) - ifaces := c.resolveZoneInterfaces("net") + ifaces := c.resolveZoneInterfaces("net", false) if len(ifaces) != 1 || ifaces[0] != "eth0" { t.Errorf("resolveZoneInterfaces(net) = %v, want [eth0]", ifaces) } - ifaces = c.resolveZoneInterfaces("all") + ifaces = c.resolveZoneInterfaces("all", false) if len(ifaces) != 1 || ifaces[0] != "" { t.Errorf("resolveZoneInterfaces(all) = %v, want [\"\"]", ifaces) } - ifaces = c.resolveZoneInterfaces("fw") + ifaces = c.resolveZoneInterfaces("fw", false) if len(ifaces) != 1 || ifaces[0] != "" { t.Errorf("resolveZoneInterfaces(fw) = %v, want [\"\"]", ifaces) } @@ -1757,6 +1757,7 @@ func TestCompile_PortAndProtoLists(t *testing.T) { }) } } + func TestCompile_OutputPolicyMatchesOif(t *testing.T) { cfg := listCfg(func(c *config.Config) { c.Zones["lan"] = config.Zone{Type: config.ZoneIP} @@ -1769,6 +1770,41 @@ func TestCompile_OutputPolicyMatchesOif(t *testing.T) { } } +func TestCompile_InterfacelessZonesFailClosed(t *testing.T) { + tests := []struct { + name string + mod func(*config.Config) + }{ + {"ipsec zone without interface", func(c *config.Config) { + c.Zones["ips"] = config.Zone{Type: config.ZoneIPSec} + c.Policy = []config.Policy{{Source: "fw", Dest: "ips", Action: config.PolicyAccept}} + }}, + {"hosts-only zone", func(c *config.Config) { + c.Zones["hst"] = config.Zone{Type: config.ZoneIP} + c.Hosts = []config.Host{{Zone: "hst", Interface: "eth0", Addresses: []string{"192.0.2.0/24"}}} + c.Policy = []config.Policy{{Source: "fw", Dest: "hst", Action: config.PolicyAccept}} + }}, + {"fw all expansion", func(c *config.Config) { + c.Zones["ips"] = config.Zone{Type: config.ZoneIPSec} + c.Zones["hst"] = config.Zone{Type: config.ZoneIP} + c.Hosts = []config.Host{{Zone: "hst", Interface: "eth0", Addresses: []string{"192.0.2.0/24"}}} + c.Policy = []config.Policy{{Source: "fw", Dest: "all", Action: config.PolicyDrop}} + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + state := mustCompile(t, listCfg(tt.mod)) + for chain := range state.Rules { + for _, r := range taggedRules(state, chain, "policy:0") { + if describeRule(r) == "" { + t.Errorf("%s: un-scoped policy rule", chain) + } + } + } + }) + } +} + // describeRule renders a rule's iif/oif/saddr/daddr matches, e.g. "iif=eth1 oif=eth2 daddr=192.0.2.1". func describeRule(r ManagedRule) string { var parts []string @@ -1857,14 +1893,19 @@ func TestCompile_CommaZoneLists(t *testing.T) { want: map[string][]string{"forward": {"iif=eth1 oif=eth3", "iif=eth1 oif=eth0"}}, }, { - name: "interface-less ipsec zone keeps zone-agnostic rule, ip zone skipped", + name: "interface-less zones are skipped", rule: config.Rule{Action: config.RuleAccept, Source: "lan", Dest: "vpn,dmz"}, - want: map[string][]string{"forward": {"iif=eth1"}}, + want: map[string][]string{}, + }, + { + name: "interface-less zone kept when address narrows it", + rule: config.Rule{Action: config.RuleAccept, Source: "lan", Dest: "vpn:192.0.2.1"}, + want: map[string][]string{"forward": {"iif=eth1 daddr=192.0.2.1"}}, }, { name: "fw source matches dest zone oif", rule: config.Rule{Action: config.RuleAccept, Source: "fw", Dest: "lan,vpn,dmz", Proto: "tcp", DPort: config.PortSpec{"22"}}, - want: map[string][]string{"output": {"oif=eth1", ""}}, + want: map[string][]string{"output": {"oif=eth1"}}, }, { name: "fw to all has no oif", From 036021d7269b812e1256d2fbed1e4a9c8d75129e Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 3 Oct 2026 22:20:41 +1000 Subject: [PATCH 3/3] Treat only non-negated addresses as scoping interfaceless zones --- internal/nftables/compiler.go | 16 ++++----- internal/nftables/compiler_test.go | 58 +++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/internal/nftables/compiler.go b/internal/nftables/compiler.go index 3a77c7a..bb84c78 100644 --- a/internal/nftables/compiler.go +++ b/internal/nftables/compiler.go @@ -417,8 +417,8 @@ func splitAddrs(addr string) []string { func (c *Compiler) compileZonePair(state *FirewallState, tag, srcZone, srcAddr, dstZone, dstAddr, proto string, dports, sports config.PortSpec, action config.RuleAction, logLevel string, fwZone string, section config.RuleSection) error { - srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr != "") - dstIfaces := c.resolveZoneInterfaces(dstZone, dstAddr != "") + srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr) + dstIfaces := c.resolveZoneInterfaces(dstZone, dstAddr) chain := c.selectChain(srcZone, dstZone, fwZone) for _, srcIface := range srcIfaces { @@ -474,7 +474,7 @@ func (c *Compiler) compileDNATRule(state *FirewallState, tag, srcZone, srcAddr, dnatPort = uint16(p) } - srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr != "") + srcIfaces := c.resolveZoneInterfaces(srcZone, srcAddr) matches, err := l4Matches(proto, dports, nil) if err != nil { @@ -589,8 +589,8 @@ func (c *Compiler) compilePolicies(state *FirewallState) error { } chain := c.selectChain(sz, dz, fwZone) - srcIfaces := c.resolveZoneInterfaces(sz, false) - dstIfaces := c.resolveZoneInterfaces(dz, false) + srcIfaces := c.resolveZoneInterfaces(sz, "") + dstIfaces := c.resolveZoneInterfaces(dz, "") for _, si := range srcIfaces { for _, di := range dstIfaces { @@ -932,15 +932,15 @@ func (c *Compiler) selectChain(srcZone, dstZone, fwZone string) string { return "forward" } -// resolveZoneInterfaces returns nil (fail closed) for a zone with no interfaces unless an address match narrows the rule. -func (c *Compiler) resolveZoneInterfaces(zone string, addrNarrowed bool) []string { +// resolveZoneInterfaces returns nil (fail closed) for a zone with no interfaces unless a non-negated address match narrows the rule. +func (c *Compiler) resolveZoneInterfaces(zone, addr string) []string { if z, ok := c.cfg.Zones[zone]; !ok || z.Type == config.ZoneFirewall { return []string{""} } if ifaces := c.cfg.ZoneInterfaces(zone); len(ifaces) > 0 { return ifaces } - if addrNarrowed { + if addr != "" && !strings.HasPrefix(addr, "!") { return []string{""} } if !c.warned[zone] { diff --git a/internal/nftables/compiler_test.go b/internal/nftables/compiler_test.go index e40219f..7dc7aaa 100644 --- a/internal/nftables/compiler_test.go +++ b/internal/nftables/compiler_test.go @@ -1,8 +1,10 @@ package nftables import ( + "bytes" "encoding/binary" "fmt" + "log/slog" "reflect" "strings" "testing" @@ -141,17 +143,17 @@ func TestCompiler_ResolveZoneInterfaces(t *testing.T) { } c := NewCompiler(cfg) - ifaces := c.resolveZoneInterfaces("net", false) + ifaces := c.resolveZoneInterfaces("net", "") if len(ifaces) != 1 || ifaces[0] != "eth0" { t.Errorf("resolveZoneInterfaces(net) = %v, want [eth0]", ifaces) } - ifaces = c.resolveZoneInterfaces("all", false) + ifaces = c.resolveZoneInterfaces("all", "") if len(ifaces) != 1 || ifaces[0] != "" { t.Errorf("resolveZoneInterfaces(all) = %v, want [\"\"]", ifaces) } - ifaces = c.resolveZoneInterfaces("fw", false) + ifaces = c.resolveZoneInterfaces("fw", "") if len(ifaces) != 1 || ifaces[0] != "" { t.Errorf("resolveZoneInterfaces(fw) = %v, want [\"\"]", ifaces) } @@ -1771,36 +1773,66 @@ func TestCompile_OutputPolicyMatchesOif(t *testing.T) { } func TestCompile_InterfacelessZonesFailClosed(t *testing.T) { + ipsec := func(c *config.Config) { c.Zones["ips"] = config.Zone{Type: config.ZoneIPSec} } + rule := func(dest string) func(*config.Config) { + return func(c *config.Config) { + ipsec(c) + c.Rules = []config.Rule{{Action: config.RuleAccept, Source: "fw", Dest: dest}} + } + } tests := []struct { - name string - mod func(*config.Config) + name string + mod func(*config.Config) + tag string + want int + warns []string }{ {"ipsec zone without interface", func(c *config.Config) { - c.Zones["ips"] = config.Zone{Type: config.ZoneIPSec} + ipsec(c) c.Policy = []config.Policy{{Source: "fw", Dest: "ips", Action: config.PolicyAccept}} - }}, + }, "policy:0", 0, []string{"ips"}}, {"hosts-only zone", func(c *config.Config) { c.Zones["hst"] = config.Zone{Type: config.ZoneIP} c.Hosts = []config.Host{{Zone: "hst", Interface: "eth0", Addresses: []string{"192.0.2.0/24"}}} c.Policy = []config.Policy{{Source: "fw", Dest: "hst", Action: config.PolicyAccept}} - }}, - {"fw all expansion", func(c *config.Config) { - c.Zones["ips"] = config.Zone{Type: config.ZoneIPSec} + }, "policy:0", 0, []string{"hst"}}, + {"fw all expansion keeps zones with interfaces", func(c *config.Config) { + ipsec(c) c.Zones["hst"] = config.Zone{Type: config.ZoneIP} c.Hosts = []config.Host{{Zone: "hst", Interface: "eth0", Addresses: []string{"192.0.2.0/24"}}} c.Policy = []config.Policy{{Source: "fw", Dest: "all", Action: config.PolicyDrop}} - }}, + }, "policy:0", 1, []string{"hst", "ips"}}, + {"negated address does not scope", rule("ips:!192.0.2.1"), "rule:0", 0, []string{"ips"}}, + {"address scopes", rule("ips:192.0.2.1"), "rule:0", 1, []string{"ips"}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + var logs bytes.Buffer + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(&logs, nil))) + defer slog.SetDefault(prev) + state := mustCompile(t, listCfg(tt.mod)) + got := 0 for chain := range state.Rules { - for _, r := range taggedRules(state, chain, "policy:0") { + for _, r := range taggedRules(state, chain, tt.tag) { + got++ if describeRule(r) == "" { - t.Errorf("%s: un-scoped policy rule", chain) + t.Errorf("%s: un-scoped rule", chain) } } } + if got != tt.want { + t.Errorf("got %d %s rules, want %d", got, tt.tag, tt.want) + } + if n := strings.Count(logs.String(), "zone has no interfaces"); n != len(tt.warns) { + t.Errorf("got %d warnings, want %d:\n%s", n, len(tt.warns), logs.String()) + } + for _, z := range tt.warns { + if n := strings.Count(logs.String(), "zone="+z+"\n"); n != 1 { + t.Errorf("zone %s warned %d times, want 1", z, n) + } + } }) } }