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

Expand comma zone lists in rule source and dest #16

Merged
benvin merged 4 commits from benvin/comma-zones into benvin/multiport-multiproto 2026-10-03 21:12:53 +10:00
Conversation 2 Commits 4 Files Changed 5
+390 -34
unkin-agent commented 2026-10-03 20:45:38 +10:00
Member
Copy Link
Copy Source

Rules with a comma zone list in SOURCE/DEST (e.g. fw,lan,svr) compile to one rule with no interface match, so they apply to every interface and fw never reaches input/output. The validator also misreads net:192.0.2.1,198.51.100.1 as zones.

  • add config.SplitZoneList: commas before the first colon are zones, after it are addresses
  • expand rules, blrules and DNAT sources into one rule per zone pair and address
  • skip (warning once per zone) ip zones with no interfaces or hosts; other zone types keep the zone-agnostic match
  • AND negated address lists into one rule; reject ! anywhere but the list start
  • reject ratelimit/connlimit on rules that expand to several zone pairs or addresses (no shared limiter)
Rules with a comma zone list in SOURCE/DEST (e.g. `fw,lan,svr`) compile to one rule with no interface match, so they apply to every interface and `fw` never reaches input/output. The validator also misreads `net:192.0.2.1,198.51.100.1` as zones. - add `config.SplitZoneList`: commas before the first colon are zones, after it are addresses - expand rules, blrules and DNAT sources into one rule per zone pair and address - skip (warning once per zone) `ip` zones with no interfaces or hosts; other zone types keep the zone-agnostic match - AND negated address lists into one rule; reject `!` anywhere but the list start - reject ratelimit/connlimit on rules that expand to several zone pairs or addresses (no shared limiter)
unkin-agent added 1 commit 2026-10-03 20:45:38 +10:00
Expand comma zone lists in rule source and dest
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
211bacd507
unkin-agent added 1 commit 2026-10-03 20:46:41 +10:00
Merge benvin/multiport-multiproto; reject limits on zone and address lists
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ad2dd9e3e7
unkin-agent commented 2026-10-03 20:48:00 +10:00
Author
Member
Copy Link
Copy Source
  • internal/nftables/compiler.go:396 (zoneSpecs) — strings.HasPrefix(spec, "all"/"any") also matches real zone names like anycast/allied; their comma lists skip SplitZoneList, the zone becomes anycast,lan, resolveZoneInterfaces returns [""] and the rule matches every interface (validator passes it via SplitZoneList) → match exactly all/any or all+/all!/any! prefixes as validateRules does; add a test with such a zone name.
  • internal/nftables/compiler.go:930 (resolveZoneInterfaces) — skip applies to every caller (compilePolicies:587, DNAT:472), not just rules, and to ipsec/bport/local/loopback zones with no interface rows, so all all policies silently stop covering them → limit the skip to rule expansion (or ZoneIP only) and add a policy test, or split into its own PR.
  • internal/nftables/compiler.go:326 (applyRuleExtras) — now applies mark/user/time/ratelimit to prerouting DNAT rules, which previously got none (wrong chain was selected); behaviour change not in the PR body, no test → add a DNAT extras test or state it in the body.
  • Scope — split: (1) SplitZoneList + validator + rule/blrule/DNAT expansion; (2) interface-less zone skip; (3) negated address list AND-ing in matchAddrCIDR.
  • internal/nftables/compiler.go:407 (splitAddrs) — net:a,!b (negation mid-list) becomes alternatives a OR !b, which matches almost everything; shorewall only allows ! leading the list → reject ! after the first element in validation.
  • internal/nftables/compiler_test.go — no test for blrules list expansion, negated address list inside a compiled rule (net:!a,b ANDs to one rule), or a DNAT source zone:a,b → add them; TestNegatedAddressList only covers matchDestCIDR.
  • nit: compiler.go:938 — slog.Warn fires per zone pair per compile (policies all all repeat it N times) → warn once per zone.
- internal/nftables/compiler.go:396 (zoneSpecs) — `strings.HasPrefix(spec, "all"/"any")` also matches real zone names like `anycast`/`allied`; their comma lists skip SplitZoneList, the zone becomes `anycast,lan`, resolveZoneInterfaces returns `[""]` and the rule matches every interface (validator passes it via SplitZoneList) → match exactly `all`/`any` or `all+`/`all!`/`any!` prefixes as validateRules does; add a test with such a zone name. - internal/nftables/compiler.go:930 (resolveZoneInterfaces) — skip applies to every caller (compilePolicies:587, DNAT:472), not just rules, and to ipsec/bport/local/loopback zones with no interface rows, so `all all` policies silently stop covering them → limit the skip to rule expansion (or ZoneIP only) and add a policy test, or split into its own PR. - internal/nftables/compiler.go:326 (applyRuleExtras) — now applies mark/user/time/ratelimit to prerouting DNAT rules, which previously got none (wrong chain was selected); behaviour change not in the PR body, no test → add a DNAT extras test or state it in the body. - Scope — split: (1) SplitZoneList + validator + rule/blrule/DNAT expansion; (2) interface-less zone skip; (3) negated address list AND-ing in matchAddrCIDR. - internal/nftables/compiler.go:407 (splitAddrs) — `net:a,!b` (negation mid-list) becomes alternatives `a` OR `!b`, which matches almost everything; shorewall only allows `!` leading the list → reject `!` after the first element in validation. - internal/nftables/compiler_test.go — no test for blrules list expansion, negated address list inside a compiled rule (`net:!a,b` ANDs to one rule), or a DNAT source `zone:a,b` → add them; TestNegatedAddressList only covers matchDestCIDR. - nit: compiler.go:938 — slog.Warn fires per zone pair per compile (policies `all all` repeat it N times) → warn once per zone.
unkin-agent added 2 commits 2026-10-03 20:50:38 +10:00
Merge remote-tracking branch 'origin/benvin/multiport-multiproto' into benvin/comma-zones f522589a83
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
8efed72c96
unkin-agent commented 2026-10-03 20:51:59 +10:00
Author
Member
Copy Link
Copy Source

No findings.

No findings.
benvin merged commit e6244d6bf0 into benvin/multiport-multiproto 2026-10-03 21:12:53 +10:00
benvin deleted branch benvin/comma-zones 2026-10-03 21:12:53 +10:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unkin/tomswall#16
Delete Branch "benvin/comma-zones"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?