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

Match exact all/any zone tokens, skip only interface-less ip zones, keep DNAT free of rule extras, reject '!' inside address lists
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details

Browse Source
This commit is contained in:
Unkin Agent
2026-10-03 20:50:36 +10:00
parent f522589a83
commit 8efed72c96
5 changed files with 100 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/config/blrules.go
+6
View File
@@ -61,6 +61,9 @@ func (c *Config) validateBlrules() error {
if _, ok := c.Zones[zs.Zone]; !ok {
return fmt.Errorf("blrules[%d]: source zone %q not defined", i, zs.Zone)
}
if !validAddrList(zs.Addr) {
return fmt.Errorf("blrules[%d]: source %q: '!' may only prefix the whole address list", i, zs.Addr)
}
}
}
@@ -70,6 +73,9 @@ func (c *Config) validateBlrules() error {
if _, ok := c.Zones[zs.Zone]; !ok {
return fmt.Errorf("blrules[%d]: dest zone %q not defined", i, zs.Zone)
}
if !validAddrList(zs.Addr) {
return fmt.Errorf("blrules[%d]: dest %q: '!' may only prefix the whole address list", i, zs.Addr)
}
}
}
}
internal/config/config_test.go
+13
View File
@@ -868,6 +868,19 @@ func TestValidateRules(t *testing.T) {
},
wantErr: "dest zone \"missing\" not defined",
},
{
name: "negation prefixing the whole address list is valid",
rules: []Rule{
{Action: RuleAccept, Source: "net:!192.0.2.1,198.51.100.1", Dest: "fw"},
},
},
{
name: "negation inside an address list",
rules: []Rule{
{Action: RuleAccept, Source: "net", Dest: "loc:192.0.2.1,!198.51.100.1"},
},
wantErr: "'!' may only prefix the whole address list",
},
{
name: "all keyword is valid source",
rules: []Rule{
internal/config/rules.go
+11
View File
@@ -179,6 +179,9 @@ func (c *Config) validateRules() error {
if _, ok := c.Zones[zs.Zone]; !ok {
return fmt.Errorf("rule[%d]: source zone %q not defined", i, zs.Zone)
}
if !validAddrList(zs.Addr) {
return fmt.Errorf("rule[%d]: source %q: '!' may only prefix the whole address list", i, zs.Addr)
}
}
}
@@ -189,6 +192,9 @@ func (c *Config) validateRules() error {
if _, ok := c.Zones[zs.Zone]; !ok {
return fmt.Errorf("rule[%d]: dest zone %q not defined", i, zs.Zone)
}
if !validAddrList(zs.Addr) {
return fmt.Errorf("rule[%d]: dest %q: '!' may only prefix the whole address list", i, zs.Addr)
}
}
}
}
@@ -243,6 +249,11 @@ func SplitZoneList(spec string) []ZoneSpec {
return out
}
// validAddrList reports whether '!' appears only at the start, negating the whole list.
func validAddrList(addr string) bool {
return !strings.Contains(strings.TrimPrefix(addr, "!"), "!")
}
// zoneFromSpec extracts the zone name from a zone spec like "net" or "net:192.168.1.0/24".
func zoneFromSpec(spec string) string {
for i, c := range spec {
internal/nftables/compiler.go
+16 -6
View File
@@ -15,7 +15,8 @@ import (
)
type Compiler struct {
cfg *config.Config
cfg *config.Config
warned map[string]bool
}
func NewCompiler(cfg *config.Config) *Compiler {
@@ -26,6 +27,7 @@ func (c *Compiler) Compile() (*FirewallState, error) {
state := &FirewallState{
Rules: make(map[string][]ManagedRule),
}
c.warned = nil
c.compileLoopback(state)
if err := c.compileConntrackFastPath(state); err != nil {
@@ -297,7 +299,9 @@ func (c *Compiler) compileRules(state *FirewallState) error {
func (c *Compiler) applyRuleExtras(state *FirewallState, tag string, rule config.Rule) {
for chain := range state.Rules {
c.applyChainExtras(state, chain, tag, rule)
if chain != "prerouting" {
c.applyChainExtras(state, chain, tag, rule)
}
}
}
@@ -394,8 +398,8 @@ func specCount(srcSpec, dstSpec string, action config.RuleAction) int {
// zoneSpecs expands a comma zone list; "all"/"any" forms keep their own comma (exclusion) syntax.
func zoneSpecs(spec string) []config.ZoneSpec {
if strings.HasPrefix(spec, "all") || strings.HasPrefix(spec, "any") {
zone, addr := splitZoneSpec(spec)
zone, addr := splitZoneSpec(spec)
if base, _, _ := strings.Cut(strings.TrimSuffix(zone, "+"), "!"); base == "all" || base == "any" {
return []config.ZoneSpec{{Zone: zone, Addr: addr}}
}
return config.SplitZoneList(spec)
@@ -935,8 +939,14 @@ func (c *Compiler) resolveZoneInterfaces(zone string) []string {
if len(ifaces) > 0 {
return ifaces
}
if z, ok := c.cfg.Zones[zone]; ok && z.Type != config.ZoneFirewall && !c.zoneHasHosts(zone) {
slog.Warn("compiler: zone has no interfaces, skipping its rules", "zone", zone)
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
}
return []string{""}
internal/nftables/compiler_test.go
+54 -4
View File
@@ -1655,6 +1655,9 @@ func describeRule(r ManagedRule) string {
case *expr.Payload:
if m.Base == expr.PayloadBaseNetworkHeader && m.Len == 4 {
name := map[uint32]string{12: "saddr", 16: "daddr"}[m.Offset]
if cmp.Op == expr.CmpOpNeq {
name = "!" + name
}
parts = append(parts, fmt.Sprintf("%s=%d.%d.%d.%d", name, cmp.Data[0], cmp.Data[1], cmp.Data[2], cmp.Data[3]))
}
}
@@ -1664,9 +1667,10 @@ func describeRule(r ManagedRule) string {
func TestCompile_CommaZoneLists(t *testing.T) {
tests := []struct {
name string
rule config.Rule
want map[string][]string
name string
rule config.Rule
blrule *config.BlruleRule
want map[string][]string
}{
{
name: "fw in source list goes to output",
@@ -1698,6 +1702,31 @@ func TestCompile_CommaZoneLists(t *testing.T) {
rule: config.Rule{Action: config.RuleDNAT, Source: "net,lan", Dest: "svr:192.0.2.10", Proto: "tcp", DPort: config.PortSpec{"80"}},
want: map[string][]string{"prerouting": {"iif=eth0", "iif=eth1"}},
},
{
name: "dnat source address list",
rule: config.Rule{Action: config.RuleDNAT, Source: "net:192.0.2.5,198.51.100.5", Dest: "svr:192.0.2.10", Proto: "tcp", DPort: config.PortSpec{"80"}},
want: map[string][]string{"prerouting": {"iif=eth0 saddr=192.0.2.5", "iif=eth0 saddr=198.51.100.5"}},
},
{
name: "negated address list stays one AND-ed rule",
rule: config.Rule{Action: config.RuleAccept, Source: "net:!192.0.2.5,198.51.100.5", Dest: "fw"},
want: map[string][]string{"input": {"iif=eth0 !saddr=192.0.2.5 !saddr=198.51.100.5"}},
},
{
name: "zone named like all/any keyword is a plain zone",
rule: config.Rule{Action: config.RuleAccept, Source: "lan", Dest: "anycast,net"},
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",
rule: config.Rule{Action: config.RuleAccept, Source: "lan", Dest: "vpn,dmz"},
want: map[string][]string{"forward": {"iif=eth1"}},
},
{
name: "blrule zone list",
blrule: &config.BlruleRule{Action: config.BlruleDrop, Source: "net,anycast", Dest: "fw"},
want: map[string][]string{"input": {"iif=eth0", "iif=eth3"}},
},
}
for _, tt := range tests {
@@ -1707,13 +1736,19 @@ func TestCompile_CommaZoneLists(t *testing.T) {
Zones: map[string]config.Zone{
"fw": {Type: config.ZoneFirewall}, "net": {Type: config.ZoneIP},
"lan": {Type: config.ZoneIP}, "svr": {Type: config.ZoneIP}, "dmz": {Type: config.ZoneIP},
"anycast": {Type: config.ZoneIP}, "vpn": {Type: config.ZoneIPSec},
},
Interfaces: []config.Interface{
{Zone: "net", Interface: "eth0"}, {Zone: "lan", Interface: "eth1"}, {Zone: "svr", Interface: "eth2"},
{Zone: "anycast", Interface: "eth3"},
},
Rules: []config.Rule{tt.rule},
PortGroups: make(map[string]config.PortGroup),
}
tag := "rule:0"
if tt.blrule != nil {
cfg.Rules, cfg.Blrules, tag = nil, []config.BlruleRule{*tt.blrule}, "blrule:0"
}
state, err := NewCompiler(cfg).Compile()
if err != nil {
t.Fatalf("Compile() error: %v", err)
@@ -1721,7 +1756,7 @@ func TestCompile_CommaZoneLists(t *testing.T) {
got := map[string][]string{}
for chain, rules := range state.Rules {
for _, r := range rules {
if r.Tag == "rule:0" {
if r.Tag == tag {
got[chain] = append(got[chain], describeRule(r))
}
}
@@ -1789,6 +1824,21 @@ func TestCompile_ListExpansionCounts(t *testing.T) {
}
}
func TestCompile_DNATGetsNoRuleExtras(t *testing.T) {
compile := func(mark string) []expr.Any {
state, err := NewCompiler(listCfg(func(c *config.Config) {
c.Rules = []config.Rule{{Action: config.RuleDNAT, Source: "net", Dest: "fw:192.0.2.10", Proto: "tcp", DPort: config.PortSpec{"80"}, Mark: mark}}
})).Compile()
if err != nil {
t.Fatalf("Compile() error: %v", err)
}
return state.Rules["prerouting"][len(state.Rules["prerouting"])-1].Exprs
}
if plain, marked := compile(""), compile("0x1"); !reflect.DeepEqual(plain, marked) {
t.Errorf("DNAT prerouting rule changed by mark extra: %d exprs vs %d", len(plain), len(marked))
}
}
func TestCompile_CommaZoneListLimitErrors(t *testing.T) {
for _, r := range []config.Rule{
{Action: config.RuleAccept, Source: "net", Dest: "fw,lan", RateLimit: "10/sec:5"},