Match dest zone oif on output-chain rules and policies #20

Merged

benvin merged 4 commits from benvin/output-oif into main 2026-10-03 22:51:29 +10:00

Conversation 3 Commits 4 Files Changed 2

+124 -34

unkin-agent commented 2026-10-03 22:15:25 +10:00

Member

Copy Link

Copy Source

Output-chain rules and policies ignored the dest zone interface, so fw->zone traffic was accepted or dropped on every interface. Zones with no interfaces had the same unscoped-rule problem in every chain.

• Matches the dest zone oif on output-chain rules and policies, as forward already does
• Skips rule/policy expansion for a zone with no interfaces (ipsec, hosts-only, any non-firewall type) and warns once per zone, unless a non-negated address match narrows the rule
• Applies the same fail-closed resolution to input, forward, output and DNAT
• Tests cover fw->ipsec, hosts-only zones, the fw->all expansion, negated addresses and per-zone warnings

Output-chain rules and policies ignored the dest zone interface, so fw->zone traffic was accepted or dropped on every interface. Zones with no interfaces had the same unscoped-rule problem in every chain. - Matches the dest zone oif on output-chain rules and policies, as forward already does - Skips rule/policy expansion for a zone with no interfaces (ipsec, hosts-only, any non-firewall type) and warns once per zone, unless a non-negated address match narrows the rule - Applies the same fail-closed resolution to input, forward, output and DNAT - Tests cover fw->ipsec, hosts-only zones, the fw->all expansion, negated addresses and per-zone warnings

unkin-agent added 1 commit 2026-10-03 22:15:25 +10:00

Match dest zone oif on output-chain rules and policies b5be665902

unkin-agent commented 2026-10-03 22:16:23 +10:00

Author

Member

Copy Link

Copy Source

• internal/nftables/compiler.go:602,1000 — fw-sourced rule/policy whose dest zone resolves to no interface (ipsec zone without an interface, hosts-only zone, fw all expanding over such a zone) still compiles to an oif-less output rule, i.e. an unconditional accept/drop to every interface; the leak this PR fixes remains for those zones (reproduced: fw ips ACCEPT with ips=ipsec, no iface → policy:0 with no oif) → for chain=="output" with a zone-typed dest and no interface, either skip with the existing warning or match on the zone members (hosts daddr/ipsec policy), and add a test for it.
• internal/nftables/compiler_test.go:1760 — nit: missing blank line before TestCompile_OutputPolicyMatchesOif.

- internal/nftables/compiler.go:602,1000 — fw-sourced rule/policy whose dest zone resolves to no interface (ipsec zone without an interface, hosts-only zone, `fw all` expanding over such a zone) still compiles to an oif-less output rule, i.e. an unconditional accept/drop to every interface; the leak this PR fixes remains for those zones (reproduced: `fw ips ACCEPT` with ips=ipsec, no iface → `policy:0` with no oif) → for chain=="output" with a zone-typed dest and no interface, either skip with the existing warning or match on the zone members (hosts daddr/ipsec policy), and add a test for it. - internal/nftables/compiler_test.go:1760 — nit: missing blank line before `TestCompile_OutputPolicyMatchesOif`.

unkin-agent added 1 commit 2026-10-03 22:18:12 +10:00

Skip rules and policies for zones without interfaces 44e1ba852e

unkin-agent commented 2026-10-03 22:19:19 +10:00

Author

Member

Copy Link

Copy Source

• internal/nftables/compiler.go:420 (also :421, :477) — srcAddr != ""/dstAddr != "" treats a negated match (vpn:!192.0.2.1) as scoping, so an interface-less zone keeps a rule with no iif/oif that matches nearly all traffic (fail-open, same bug the PR fixes) → only keep when the address is non-negated (!strings.HasPrefix(addr, "!")); add a test for vpn:!192.0.2.1 being skipped.
• nit: internal/nftables/compiler_test.go:1786 (TestCompile_InterfacelessZonesFailClosed) — only loops over rules that exist, so it passes when nothing is emitted and never asserts the once-per-zone warning → assert the interface-less zone yields zero rules for the pair and that warned is set once.

- internal/nftables/compiler.go:420 (also :421, :477) — `srcAddr != ""`/`dstAddr != ""` treats a negated match (`vpn:!192.0.2.1`) as scoping, so an interface-less zone keeps a rule with no iif/oif that matches nearly all traffic (fail-open, same bug the PR fixes) → only keep when the address is non-negated (`!strings.HasPrefix(addr, "!")`); add a test for `vpn:!192.0.2.1` being skipped. - nit: internal/nftables/compiler_test.go:1786 (TestCompile_InterfacelessZonesFailClosed) — only loops over rules that exist, so it passes when nothing is emitted and never asserts the once-per-zone warning → assert the interface-less zone yields zero rules for the pair and that `warned` is set once.

unkin-agent added 1 commit 2026-10-03 22:20:42 +10:00

Treat only non-negated addresses as scoping interfaceless zones 036021d726

unkin-agent commented 2026-10-03 22:21:30 +10:00

Author

Member

Copy Link

Copy Source

No findings.

No findings.

unkin-agent added 1 commit 2026-10-03 22:46:10 +10:00

Merge remote-tracking branch 'origin/main' into benvin/output-oif ... 5238886c6e

# Conflicts:
#	internal/nftables/compiler_test.go

benvin merged commit 63c46fec81 into main 2026-10-03 22:51:29 +10:00

benvin deleted branch benvin/output-oif 2026-10-03 22:51:29 +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

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unkin/tomswall#20