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

Emit the implied ACCEPT for DNAT and REDIRECT rules #29

Merged
benvin merged 3 commits from benvin/dnat-implied-accept into main 2026-10-04 15:28:05 +11:00
Conversation 3 Commits 3 Files Changed 2
+229 -15
unkin-agent commented 2026-10-04 15:08:19 +11:00
Member
Copy Link
Copy Source

Shorewall DNAT/REDIRECT imply a filter ACCEPT for the translated flow; tomswall emitted only the prerouting NAT, so DNATed services stayed dropped.

  • Emit the implied ACCEPT (forward, or input for fw/REDIRECT) gated on ct status dnat, tagged rule:N:accept.
  • Keep rule extras (ratelimit, connlimit, mark, user, time) off it, as with the NAT rule.
  • Expand all/any and zone-list DNAT sources per zone (fw skipped), never pairing a zone with itself unless +.
  • Match SPORT in both the prerouting DNAT and the accept.
Shorewall DNAT/REDIRECT imply a filter ACCEPT for the translated flow; tomswall emitted only the prerouting NAT, so DNATed services stayed dropped. - Emit the implied ACCEPT (forward, or input for fw/REDIRECT) gated on `ct status dnat`, tagged `rule:N:accept`. - Keep rule extras (ratelimit, connlimit, mark, user, time) off it, as with the NAT rule. - Expand all/any and zone-list DNAT sources per zone (fw skipped), never pairing a zone with itself unless `+`. - Match SPORT in both the prerouting DNAT and the accept.
unkin-agent added 1 commit 2026-10-04 15:08:20 +11:00
Emit the implied ACCEPT for DNAT and REDIRECT rules
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
b29ed2446e
unkin-agent commented 2026-10-04 15:10:16 +11:00
Author
Member
Copy Link
Copy Source
  • internal/nftables/compiler.go:384 — limiter guard still passes when the DNAT target zone has several interfaces, yet compileDNATAccept (compileZonePair loops dstIfaces) emits one forward accept per oif, each with its own limit (verified: ratelimit 10/sec:5 + zone with eth2,eth3 → 2 limited forward rules, no error) → count dst interfaces/addrs for DNAT/REDIRECT in the guard (or reject).
  • internal/nftables/compiler.go:384 — rate-limit/mark/user extras now apply to the forward/input accept (applyRuleExtras only skips prerouting); DNAT+user yields meta skuid in forward, which can never match, so the published service stays dropped → don't apply the user extra to the implied accept (or reject DNAT+user), and add a test for extras on the accept.
  • internal/nftables/compiler.go:507 — plain all/any source is not skipped for the target zone (zoneSpecs keeps it global, srcZone "all" != dstZone), so the accept is iif-less and also covers target→target hairpin; shorewall skips source==dest for all unless all+ → treat all/any as wild in dnatSkipsIntrazone and expand per zone.
  • internal/nftables/compiler.go:515 — accept matches sports but compileDNATRule ignores SPORT, so a rule with SPORT NATs every source port but only accepts the listed one → drop sports from the accept, or apply it to the nat rule too.
  • internal/nftables/compiler_test.go:2272 — expected exprs are built from the same matchCtBits/ctStatusDNAT as the code, so a wrong mask constant or Cmp op still passes → assert literal mask 32 (native-endian) and CmpOpNeq.
  • nit: internal/nftables/compiler_test.go:1630 — !ok || r.Chain != "prerouting" && r.Tag == "rule:3" is precedence-obscure and skips the input accept silently → parenthesise or key want by chain+tag.
- internal/nftables/compiler.go:384 — limiter guard still passes when the DNAT target zone has several interfaces, yet compileDNATAccept (compileZonePair loops dstIfaces) emits one forward accept per oif, each with its own `limit` (verified: `ratelimit 10/sec:5` + zone with eth2,eth3 → 2 limited forward rules, no error) → count dst interfaces/addrs for DNAT/REDIRECT in the guard (or reject). - internal/nftables/compiler.go:384 — rate-limit/mark/user extras now apply to the forward/input accept (applyRuleExtras only skips prerouting); DNAT+`user` yields `meta skuid` in forward, which can never match, so the published service stays dropped → don't apply the user extra to the implied accept (or reject DNAT+user), and add a test for extras on the accept. - internal/nftables/compiler.go:507 — plain `all`/`any` source is not skipped for the target zone (zoneSpecs keeps it global, srcZone "all" != dstZone), so the accept is iif-less and also covers target→target hairpin; shorewall skips source==dest for `all` unless `all+` → treat `all`/`any` as wild in dnatSkipsIntrazone and expand per zone. - internal/nftables/compiler.go:515 — accept matches `sports` but compileDNATRule ignores SPORT, so a rule with SPORT NATs every source port but only accepts the listed one → drop sports from the accept, or apply it to the nat rule too. - internal/nftables/compiler_test.go:2272 — expected exprs are built from the same matchCtBits/ctStatusDNAT as the code, so a wrong mask constant or Cmp op still passes → assert literal mask 32 (native-endian) and CmpOpNeq. - nit: internal/nftables/compiler_test.go:1630 — `!ok || r.Chain != "prerouting" && r.Tag == "rule:3"` is precedence-obscure and skips the input accept silently → parenthesise or key `want` by chain+tag.
unkin-agent added 1 commit 2026-10-04 15:12:30 +11:00
Keep rule extras off the DNAT implied accept, expand all/any DNAT sources and match SPORT
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
ac63f65f2f
unkin-agent commented 2026-10-04 15:13:49 +11:00
Author
Member
Copy Link
Copy Source

No findings.

No findings.
unkin-agent added 1 commit 2026-10-04 15:22:45 +11:00
Merge remote-tracking branch 'origin/main' into benvin/dnat-implied-accept
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
87be6bc6af 
# Conflicts:
#	internal/nftables/compiler.go
#	internal/nftables/compiler_test.go
unkin-agent commented 2026-10-04 15:23:42 +11:00
Author
Member
Copy Link
Copy Source

No findings.

No findings.
benvin merged commit 1e32bde555 into main 2026-10-04 15:28:05 +11:00
benvin deleted branch benvin/dnat-implied-accept 2026-10-04 15:28:05 +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
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unkin/tomswall#29
Delete Branch "benvin/dnat-implied-accept"

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?