[client] Fix DNS resolution with userspace WireGuard and kernel firewall#5873
Merged
[client] Fix DNS resolution with userspace WireGuard and kernel firewall#5873
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a hooks-only userspace packet filter ( Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant IF as Interface/Netstack
participant HF as HooksFilter
participant CM as common.HookMatches
participant HK as PacketHook
App->>IF: send outbound packet
IF->>HF: FilterOutbound(packetData)
HF->>HF: validate IPv4 header, compute IHL, drop fragments
HF->>HF: extract dst IP and dst port
HF->>CM: HookMatches(hookPtr, dstIP, dPort, packetData)
alt Hook present & matches
CM->>HK: invoke hook function(packetData)
HK-->>CM: return bool (drop?)
CM-->>HF: return decision
else No hook or mismatch
CM-->>HF: return false
end
HF-->>IF: drop or pass decision
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
lixmal
force-pushed
the
fix-dns-userspace-bind-kernel-fw
branch
from
922b1dc to
5440b0c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/firewall/create_linux.go`:
- Around line 62-64: The call to iface.SetFilter(&uspfilter.HooksFilter{})
currently only logs a warning and continues, leaving the device without DNS
interception; change this so SetFilter failures abort startup: in the function
that constructs/returns fm, replace the log.Warnf branch with error propagation
(returning the error or wrapping it) so that a non-nil error from
iface.SetFilter prevents returning fm; reference the SetFilter call,
uspfilter.HooksFilter, and the returned fm to locate and update the code path.
In `@client/firewall/uspfilter/hooks_filter.go`:
- Around line 46-57: Check and skip malformed or fragmented IPv4 packets before
reading L4 fields: after computing ihl (ihl := int(packetData[0]&0x0f) * 4)
ensure ihl >= 20 and that packetData has at least ihl+dstPortOffset+2 bytes, and
inspect the IPv4 flags/fragment offset (compute fragOff :=
(uint16(packetData[6]&0x1F) << 8) | uint16(packetData[7]) and MF flag via
packetData[6]&0x20) and return false if fragOff != 0 or MF is set; only then
proceed to read dstIP
(netip.AddrFromSlice(packetData[ipv4DstOffset:ipv4DstOffset+4])), proto
(packetData[ipv4ProtoOffset]) and dstPort
(binary.BigEndian.Uint16(packetData[ihl+dstPortOffset:ihl+dstPortOffset+2])) so
you never treat payload bytes as a UDP/TCP header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc2ea47f-166e-4d3f-b999-aca054832d4a
📒 Files selected for processing (4)
client/firewall/create_linux.goclient/firewall/uspfilter/common/hooks.goclient/firewall/uspfilter/filter.goclient/firewall/uspfilter/hooks_filter.go
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
client/firewall/create_linux.go (1)
59-64:⚠️ Potential issue | 🟠 MajorDo not continue when hooks filter installation fails.
If
iface.SetFilter(&uspfilter.HooksFilter{})fails, startup currently proceeds withfm, but the DNS interception path this PR adds is not active. This should fail fast (or fallback explicitly), not degrade silently.Proposed fix
// Native firewall handles packet filtering, but the userspace WireGuard bind // needs a device filter for DNS interception hooks. Install a minimal // hooks-only filter that passes all traffic through to the kernel firewall. if err := iface.SetFilter(&uspfilter.HooksFilter{}); err != nil { - log.Warnf("failed to set hooks filter: %v", err) + return nil, fmt.Errorf("set hooks filter: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/firewall/create_linux.go` around lines 59 - 64, The call to iface.SetFilter(&uspfilter.HooksFilter{}) currently only logs a warning and allows startup to continue, leaving the DNS interception path inactive; change this to fail-fast by returning or propagating the error instead of continuing with fm: replace the log.Warnf branch with code that logs an error (log.Errorf or log.WithError) and returns the error from the surrounding function so startup aborts (or triggers the existing failure path) when iface.SetFilter fails; ensure the change references iface.SetFilter and uspfilter.HooksFilter and prevents further initialization using fm when the filter installation fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@client/firewall/create_linux.go`:
- Around line 59-64: The call to iface.SetFilter(&uspfilter.HooksFilter{})
currently only logs a warning and allows startup to continue, leaving the DNS
interception path inactive; change this to fail-fast by returning or propagating
the error instead of continuing with fm: replace the log.Warnf branch with code
that logs an error (log.Errorf or log.WithError) and returns the error from the
surrounding function so startup aborts (or triggers the existing failure path)
when iface.SetFilter fails; ensure the change references iface.SetFilter and
uspfilter.HooksFilter and prevents further initialization using fm when the
filter installation fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ceddca2-7661-438c-8a97-1370c68711de
📒 Files selected for processing (5)
client/firewall/create_linux.goclient/firewall/uspfilter/common/hooks.goclient/firewall/uspfilter/filter.goclient/firewall/uspfilter/filter_test.goclient/firewall/uspfilter/hooks_filter.go
✅ Files skipped from review due to trivial changes (1)
- client/firewall/uspfilter/filter_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- client/firewall/uspfilter/hooks_filter.go
pappz
previously approved these changes
Apr 13, 2026
…l with userspace WG
lixmal
force-pushed
the
fix-dns-userspace-bind-kernel-fw
branch
from
5440b0c to
d124310
Compare
pappz
previously approved these changes
Apr 13, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/firewall/uspfilter/hooks_filter.go`:
- Around line 20-21: The fragment-offset mask ipv4FragOffMask is wrong (0x1f) so
checks like flagsAndOffset&ipv4FragOffMask != 0 miss higher offset bits; update
ipv4FragOffMask to the full 13-bit mask 0x1fff and ensure the existing guard
that inspects flagsAndOffset (the fragment-check around lines with
flagsAndOffset&ipv4FragOffMask != 0) uses that constant so any non-zero IPv4
fragment offset is correctly detected and parsed as non-first fragment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79e25a76-731f-4bf1-90aa-3f79fee953b3
📒 Files selected for processing (5)
client/firewall/create_linux.goclient/firewall/uspfilter/common/hooks.goclient/firewall/uspfilter/filter.goclient/firewall/uspfilter/filter_test.goclient/firewall/uspfilter/hooks_filter.go
✅ Files skipped from review due to trivial changes (3)
- client/firewall/create_linux.go
- client/firewall/uspfilter/filter_test.go
- client/firewall/uspfilter/common/hooks.go
🚧 Files skipped from review as they are similar to previous changes (1)
- client/firewall/uspfilter/filter.go
pappz
approved these changes
Apr 13, 2026
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes
When the client runs with userspace WireGuard but uses a kernel firewall (nftables/iptables), DNS resolution via the in-memory DNS service fails because no packet filter is installed on the WireGuard device. The
ServiceViaMemoryDNS service needs a device filter to intercept DNS packets, but onlyuspfiltercallsSetFilter. After #5668 stopped wrapping native firewalls withuspfilter, the filter is never set in this configuration.HooksFilter, a minimal packet filter that only handles outbound DNS hooks without conntrack, netflow, or MSS clamping (those are handled by the kernel firewall)HooksFilteron the WireGuard device when userspace bind is active and native firewall succeedsPacketHook,HookMatches,SetHook) touspfilter/commonso bothHooksFilterand the fullManageruse the same dispatch logicHooksFilterto avoid interfering with IPv6 trafficIssue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
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
New Features
Refactor
Tests