Skip to content

[client] Guard against container DNAT bypass of ACL rules in iptables#5697

Merged
lixmal merged 2 commits intomainfrom
iptables-mangle-dnat-guard
Apr 16, 2026
Merged

[client] Guard against container DNAT bypass of ACL rules in iptables#5697
lixmal merged 2 commits intomainfrom
iptables-mangle-dnat-guard

Conversation

@lixmal
Copy link
Copy Markdown
Collaborator

@lixmal lixmal commented Mar 25, 2026

Describe your changes

When container runtimes (Docker, Podman) publish ports, they DNAT traffic in nat PREROUTING, causing it to traverse the FORWARD chain instead of INPUT. If the container runtime starts after NetBird, its ACCEPT rules in filter FORWARD are inserted above NetBird's rules, bypassing ACL port restrictions.

  • Add two guard rules in mangle FORWARD that enforce the ACL mark check for DNAT'd traffic from the WireGuard interface
  • Mangle table runs before filter, so container runtimes cannot override the verdict regardless of rule insertion order
  • Uses --ctstate DNAT to scope the guard to only DNAT'd traffic, leaving regular route-forwarded traffic unaffected
  • Rules are persisted in shutdown state for proper cleanup

Native nftables is not affected since it uses separate tables with independent chain priorities.

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

No user-facing behavior change. The fix is internal to the iptables firewall manager.

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved firewall forwarding chain management for better connection tracking and traffic handling.
    • Enhanced security enforcement to prevent external traffic from circumventing ACL checks.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1f93662d-07ac-48eb-97a2-b52d074d8923

📥 Commits

Reviewing files that changed from the base of the PR and between 9aaa05e and 3eac55a.

📒 Files selected for processing (1)
  • client/firewall/iptables/acl_linux.go

📝 Walkthrough

Walkthrough

A new mangleFwdKey was introduced to manage FORWARD chain rules in the mangle table. The cleanChains() function was updated to remove previously stored mangle rules, createDefaultChains() now appends these rules separately to the mangle table's FORWARD chain, and seedInitialEntries() now populates two guard rules for RELATED/ESTABLISHED traffic and DNAT-associated filtering.

Changes

Cohort / File(s) Summary
Mangle Table FORWARD Chain Rules
client/firewall/iptables/acl_linux.go
Introduced mangleFwdKey to store and manage FORWARD chain rules in the mangle table. Updated cleanChains() to delete previously installed rules, createDefaultChains() to append mangle rules separately, and seedInitialEntries() to populate guard rules for RELATED/ESTABLISHED traffic acceptance and DNAT-associated traffic dropping with prerouting redirect mark validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • pappz

Poem

🐰 Through the mangle, hops so quick,
FORWARD chains with a firewall trick,
RELATED and ESTABLISHED leap on through,
While DNAT drops get blocked on cue!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding guard rules in iptables mangle table to prevent container DNAT from bypassing ACL rules.
Description check ✅ Passed The description is comprehensive and covers required sections: detailed explanation of the problem, solution approach, checklist completed appropriately, and documentation decision justified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch iptables-mangle-dnat-guard

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 8, 2026

@lixmal lixmal merged commit d4c61ed into main Apr 16, 2026
44 of 45 checks passed
@lixmal lixmal deleted the iptables-mangle-dnat-guard branch April 16, 2026 12:02
@SISheogorath
Copy link
Copy Markdown

Thanks... This allowed me to spend my whole evening debugging why I can't access an internal service.

@lixmal
Copy link
Copy Markdown
Collaborator Author

lixmal commented Apr 22, 2026

Please open a GitHub issue. There's no point doing that in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants