[PR #6781] [MERGED] [client] Evaluate IP fragments against firewall ACLs #29887

Closed
opened 2026-08-05 08:09:18 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6781
Author: @lixmal
Created: 7/15/2026
Status: Merged
Merged: 7/17/2026
Merged by: @lixmal

Base: 0.74.7-branchHead: fix-fragment-acl-bypass


📝 Commits (1)

  • 345566d Evaluate IP fragments against ACLs instead of allowing them

📊 Changes

4 files changed (+1144 additions, -53 deletions)

View changed files

📝 client/firewall/uspfilter/filter.go (+271 -53)
client/firewall/uspfilter/fragment.go (+204 -0)
client/firewall/uspfilter/fragment_bench_test.go (+115 -0)
client/firewall/uspfilter/fragment_test.go (+554 -0)

📄 Description

Describe your changes

The userspace firewall passed every IP fragment through without ACL evaluation, because gopacket does not decode the transport header of a fragment (not even the first one). This makes fragments follow the same ACL path as whole packets.

  • Decode and ACL-evaluate the first fragment of a datagram (IPv4 and IPv6) and record its verdict, keyed by the reassembly tuple (source, destination, protocol, identification).
  • Trailing fragments inherit the recorded verdict; a fragment with no allowed first fragment on record is dropped (fail closed). Downstream reassembly by the kernel/netstack is unchanged.
  • Drop first fragments too small to hold the full transport header, and drop fragments that overlap the already-inspected header (RFC 1858 / RFC 3128; RFC 5722 for IPv6).
  • Bound the verdict table with a TTL and a cap (NB_FRAGMENT_MAX_ENTRIES, default 16384), failing closed at capacity.
  • Emit netflow drop events for denied first fragments, matching non-fragmented packets.

Added unit tests and benchmarks for the fragment paths (v4/v6, tiny-fragment, overlap, conntrack, routed, TTL, capacity); the attacker-amplifiable paths stay zero-allocation and at or below the normal-packet baseline.

Stack

  • 0.74.7-branch - ⚠️ No PR associated with branch

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)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

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)

Internal firewall packet-handling behavior; no user-facing configuration beyond the optional NB_FRAGMENT_MAX_ENTRIES tuning env var.

Summary by CodeRabbit

  • New Features
    • Added secure handling for fragmented IPv4 and IPv6 traffic.
    • Fragmented packets are now evaluated consistently against inbound and routed firewall rules.
    • Allowed first fragments permit matching trailing fragments, while unknown, expired, overlapping, or unsupported fragments are blocked.
    • Added fail-closed safeguards when fragment tracking capacity is reached.
  • Bug Fixes
    • Improved firewall drop logging and flow reporting for denied fragmented traffic.
  • Tests
    • Added comprehensive coverage and performance benchmarks for fragmented packet scenarios.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6781 **Author:** [@lixmal](https://github.com/lixmal) **Created:** 7/15/2026 **Status:** ✅ Merged **Merged:** 7/17/2026 **Merged by:** [@lixmal](https://github.com/lixmal) **Base:** `0.74.7-branch` ← **Head:** `fix-fragment-acl-bypass` --- ### 📝 Commits (1) - [`345566d`](https://github.com/netbirdio/netbird/commit/345566d0b0d9be0708ab1ddf73af960d6dc33a99) Evaluate IP fragments against ACLs instead of allowing them ### 📊 Changes **4 files changed** (+1144 additions, -53 deletions) <details> <summary>View changed files</summary> 📝 `client/firewall/uspfilter/filter.go` (+271 -53) ➕ `client/firewall/uspfilter/fragment.go` (+204 -0) ➕ `client/firewall/uspfilter/fragment_bench_test.go` (+115 -0) ➕ `client/firewall/uspfilter/fragment_test.go` (+554 -0) </details> ### 📄 Description ## Describe your changes The userspace firewall passed every IP fragment through without ACL evaluation, because gopacket does not decode the transport header of a fragment (not even the first one). This makes fragments follow the same ACL path as whole packets. - Decode and ACL-evaluate the first fragment of a datagram (IPv4 and IPv6) and record its verdict, keyed by the reassembly tuple (source, destination, protocol, identification). - Trailing fragments inherit the recorded verdict; a fragment with no allowed first fragment on record is dropped (fail closed). Downstream reassembly by the kernel/netstack is unchanged. - Drop first fragments too small to hold the full transport header, and drop fragments that overlap the already-inspected header (RFC 1858 / RFC 3128; RFC 5722 for IPv6). - Bound the verdict table with a TTL and a cap (`NB_FRAGMENT_MAX_ENTRIES`, default 16384), failing closed at capacity. - Emit netflow drop events for denied first fragments, matching non-fragmented packets. Added unit tests and benchmarks for the fragment paths (v4/v6, tiny-fragment, overlap, conntrack, routed, TTL, capacity); the attacker-amplifiable paths stay zero-allocation and at or below the normal-packet baseline. ## Issue ticket number and link ## Stack - `0.74.7-branch` - :warning: No PR associated with branch <!-- branch-stack --> - \#6781 :point\_left: ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) Internal firewall packet-handling behavior; no user-facing configuration beyond the optional `NB_FRAGMENT_MAX_ENTRIES` tuning env var. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added secure handling for fragmented IPv4 and IPv6 traffic. - Fragmented packets are now evaluated consistently against inbound and routed firewall rules. - Allowed first fragments permit matching trailing fragments, while unknown, expired, overlapping, or unsupported fragments are blocked. - Added fail-closed safeguards when fragment tracking capacity is reached. - **Bug Fixes** - Improved firewall drop logging and flow reporting for denied fragmented traffic. - **Tests** - Added comprehensive coverage and performance benchmarks for fragmented packet scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:09:18 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#29887