Include firewall zone in all/any rule expansion #28

Merged

benvin merged 3 commits from benvin/all-includes-fw into main 2026-10-04 15:21:25 +11:00

Conversation 3 Commits 3 Files Changed 2

+178 -27

unkin-agent commented 2026-10-04 15:06:34 +11:00

Member

Copy Link

Copy Source

A rule with all/any on either side compiles to a single global zone that selectChain always maps to forward, so rules like ACCEPT all all icmp 8 never reach input or output and pings to/from the firewall are dropped. Shorewall includes $FW in all.

• expand global all/any rule specs with the firewall zone so fw pairs land in input/output
• skip the fw-to-fw pair, matching exclusion expansion
• leave DNAT/REDIRECT sources and policies unchanged (policies already expand per zone)
• add compiler tests for all/net/fw/exclusion combinations

A rule with `all`/`any` on either side compiles to a single global zone that selectChain always maps to forward, so rules like `ACCEPT all all icmp 8` never reach input or output and pings to/from the firewall are dropped. Shorewall includes `$FW` in `all`. - expand global `all`/`any` rule specs with the firewall zone so fw pairs land in input/output - skip the fw-to-fw pair, matching exclusion expansion - leave DNAT/REDIRECT sources and policies unchanged (policies already expand per zone) - add compiler tests for all/net/fw/exclusion combinations

unkin-agent added 1 commit 2026-10-04 15:06:35 +11:00

Include firewall zone in all/any rule expansion ff4c9b63e8

unkin-agent commented 2026-10-04 15:07:31 +11:00

Author

Member

Copy Link

Copy Source

• internal/nftables/compiler.go:506 — specCount still counts global all/any as one spec, but compileOneRule now expands it to fw pairs (all all = forward+input+output, net all/all net = 2). A ratelimit/connlimit rule on these passes the check at :384 and compiles to 2-3 independent limiters, regressing the ">1 nft rule is a compile error" guarantee → make specCount count withFirewall-expanded specs (minus the skipped fw→fw pair) so these rules are rejected.
• internal/nftables/compiler_test.go:2803 — no test for ratelimit/connlimit with all, and no assertion on addr/iface content of the added fw rules (the all:192.0.2.0/24→fw case only counts rules) → add a ratelimit-on-all compile-error case and check saddr/daddr is kept on the input/output rules.
• nit: internal/nftables/compiler_test.go:2803 — no all+ or DNAT/REDIRECT-with-all case, though the PR body claims DNAT is unchanged → add one each.

- internal/nftables/compiler.go:506 — specCount still counts global `all`/`any` as one spec, but compileOneRule now expands it to fw pairs (`all all` = forward+input+output, `net all`/`all net` = 2). A ratelimit/connlimit rule on these passes the check at :384 and compiles to 2-3 independent limiters, regressing the ">1 nft rule is a compile error" guarantee → make specCount count withFirewall-expanded specs (minus the skipped fw→fw pair) so these rules are rejected. - internal/nftables/compiler_test.go:2803 — no test for ratelimit/connlimit with `all`, and no assertion on addr/iface content of the added fw rules (the `all:192.0.2.0/24`→fw case only counts rules) → add a ratelimit-on-`all` compile-error case and check saddr/daddr is kept on the input/output rules. - nit: internal/nftables/compiler_test.go:2803 — no `all+` or DNAT/REDIRECT-with-`all` case, though the PR body claims DNAT is unchanged → add one each.

unkin-agent added 1 commit 2026-10-04 15:08:56 +11:00

Count firewall-expanded all/any specs for limiter guard df9326330a

unkin-agent commented 2026-10-04 15:09:37 +11:00

Author

Member

Copy Link

Copy Source

• internal/nftables/compiler.go:519 — skipPair calls isGlobalZone on the whole spec, so a comma list containing all (e.g. net,all → fw) never skips fw→fw; it emits 3 rule:0 input entries (net→fw, all→fw, fw→fw) → test each list element (split on ,) in the fw→fw guard, in both compileOneRule and specCount.
• internal/nftables/compiler.go:545 — withFirewall appends fw even when the list already names fw (fw,all → fw yields fw twice) → skip the append if fwZone is already in specs.

- internal/nftables/compiler.go:519 — skipPair calls isGlobalZone on the whole spec, so a comma list containing `all` (e.g. `net,all` → `fw`) never skips fw→fw; it emits 3 rule:0 input entries (net→fw, all→fw, fw→fw) → test each list element (split on `,`) in the fw→fw guard, in both compileOneRule and specCount. - internal/nftables/compiler.go:545 — withFirewall appends fw even when the list already names fw (`fw,all` → `fw` yields fw twice) → skip the append if fwZone is already in specs.

unkin-agent added 1 commit 2026-10-04 15:10:50 +11:00

Expand all/any firewall pairs per zone and dedupe fw e3130b6c3b

unkin-agent commented 2026-10-04 15:11:30 +11:00

Author

Member

Copy Link

Copy Source

No findings.

No findings.

benvin merged commit 110b109d97 into main 2026-10-04 15:21:25 +11:00

benvin deleted branch benvin/all-includes-fw 2026-10-04 15:21:25 +11: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

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/tomswall#28