[PR #5504] [client] scope macOS DNS state keys by WireGuard interface name #28068

Open
opened 2026-08-05 08:05:44 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/5504
Author: @siriobalmelli
Created: 3/4/2026
Status: 🔄 Open

Base: mainHead: fix/dns-iface-scoping-v2


📝 Commits (1)

  • f79ff3b client: isolate macOS DNS state per interface

📊 Changes

6 files changed (+1626 additions, -342 deletions)

View changed files

📝 client/internal/dns/host_darwin.go (+201 -35)
client/internal/dns/host_darwin_privileged_test.go (+985 -0)
📝 client/internal/dns/host_darwin_test.go (+348 -296)
📝 client/internal/dns/server_darwin.go (+1 -1)
📝 client/internal/dns/unclean_shutdown_darwin.go (+70 -3)
📝 docs/testing-privileged.md (+21 -7)

📄 Description

Summary

Fixes a bug where multiple netbird instances on the same macOS host clobber each other's DNS entries in scutil, because the key format State:/Network/Service/NetBird-%s/DNS has no per-instance disambiguation.

Alleviates #446: while one instance wouldn't connect to multiple networks, removes the barrier to having multiple instances running simultaneously.

Problem

When two netbird instances run on the same host with different WireGuard interfaces (e.g. utun0 and utun1), they both write to the same scutil keys (NetBird-Match/DNS, NetBird-Search/DNS). The second instance silently overwrites the first's DNS configuration, breaking DNS for the first instance's network.

Solution

Add the WireGuard interface name as a component in all NetBird scutil key formats:

Key type Before After
Named NetBird-Match/DNS NetBird-utun0-Match/DNS
Batched NetBird-Match-0/DNS NetBird-utun0-Match-0/DNS

This builds on top of the domain-batching work from #5368 (which prevents silent domain loss with 60+ domains) — both features are now combined.

Upgrade migration

discoverExistingKeys() now discovers and removes legacy-format keys (without interface scope) on startup, so stale scutil entries from older versions are cleaned up automatically rather than left orphaned.

Changes

  • host_darwin.go: Updated key format constants to include interface name; added interfaceName field to systemConfigurator; updated all key generation in addBatchedDomains, addLocalDNS, discoverExistingKeys; fixed primaryServiceStateKeyFormat bug; updated getKeyWithInput signature
  • server_darwin.go: Pass s.wgInterface.Name() to newHostManager()
  • unclean_shutdown_darwin.go: Added InterfaceName field to ShutdownState for proper cleanup scoping after unclean shutdown
  • host_darwin_test.go: Updated all existing tests for new key format; added TestGetKeyWithInput, TestNewHostManagerWithInterfaceName, TestMultipleInterfacesGenerateDifferentKeys, TestShutdownStateIncludesInterfaceName, TestPrimaryServiceKeyFormatNotAffected, TestMultipleInstancesBatchedIsolation

Testing

Unit tests (no scutil required, any platform):

go test -short ./client/internal/dns/ -run "TestGetKeyWithInput|TestNewHostManager|TestSplitDomainsIntoBatches|TestMultipleInterfaces|TestShutdownState|TestPrimaryService"

Integration tests (macOS with scutil, requires admin):

go test ./client/internal/dns/ -run "TestDarwinDNS|TestMatchDomainBatching|TestOriginalNameservers|TestMultipleInstancesBatchedIsolation" -v -timeout 120s

This supersedes PR #4633 (which was opened from main and became unmergeable after #5368 landed).

cc @mlsmaycon and @lixmal

Summary by CodeRabbit

  • Bug Fixes

    • DNS state is now scoped per network interface; shutdown/restoration use the interface context and preserve legacy keys for seamless upgrades.
    • Shutdown state now records interface info to prevent cross-interface cleanup and improve reliability.
  • Tests

    • Expanded test coverage for per-interface keying, batched domain isolation, legacy migration, and multi-interface isolation.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change

Internal implementation detail: scopes macOS scutil DNS state keys by WireGuard interface name to prevent multi-instance conflicts. No user-facing API, CLI, or configuration changes.

Reference

Related PR in nixpkgs-darwin: https://github.com/nix-darwin/nix-darwin/pull/1610


🔄 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/5504 **Author:** [@siriobalmelli](https://github.com/siriobalmelli) **Created:** 3/4/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/dns-iface-scoping-v2` --- ### 📝 Commits (1) - [`f79ff3b`](https://github.com/netbirdio/netbird/commit/f79ff3b30ddc8477e63e608a8c26ea06abd01731) client: isolate macOS DNS state per interface ### 📊 Changes **6 files changed** (+1626 additions, -342 deletions) <details> <summary>View changed files</summary> 📝 `client/internal/dns/host_darwin.go` (+201 -35) ➕ `client/internal/dns/host_darwin_privileged_test.go` (+985 -0) 📝 `client/internal/dns/host_darwin_test.go` (+348 -296) 📝 `client/internal/dns/server_darwin.go` (+1 -1) 📝 `client/internal/dns/unclean_shutdown_darwin.go` (+70 -3) 📝 `docs/testing-privileged.md` (+21 -7) </details> ### 📄 Description ## Summary Fixes a bug where multiple netbird instances on the same macOS host clobber each other's DNS entries in `scutil`, because the key format `State:/Network/Service/NetBird-%s/DNS` has no per-instance disambiguation. Alleviates #446: while one instance wouldn't connect to multiple networks, removes the barrier to having multiple instances running simultaneously. ## Problem When two netbird instances run on the same host with different WireGuard interfaces (e.g. `utun0` and `utun1`), they both write to the same scutil keys (`NetBird-Match/DNS`, `NetBird-Search/DNS`). The second instance silently overwrites the first's DNS configuration, breaking DNS for the first instance's network. ## Solution Add the WireGuard interface name as a component in all NetBird scutil key formats: | Key type | Before | After | |---|---|---| | Named | `NetBird-Match/DNS` | `NetBird-utun0-Match/DNS` | | Batched | `NetBird-Match-0/DNS` | `NetBird-utun0-Match-0/DNS` | This builds on top of the domain-batching work from #5368 (which prevents silent domain loss with 60+ domains) — both features are now combined. ## Upgrade migration `discoverExistingKeys()` now discovers and removes legacy-format keys (without interface scope) on startup, so stale scutil entries from older versions are cleaned up automatically rather than left orphaned. ## Changes - `host_darwin.go`: Updated key format constants to include interface name; added `interfaceName` field to `systemConfigurator`; updated all key generation in `addBatchedDomains`, `addLocalDNS`, `discoverExistingKeys`; fixed `primaryServiceStateKeyFormat` bug; updated `getKeyWithInput` signature - `server_darwin.go`: Pass `s.wgInterface.Name()` to `newHostManager()` - `unclean_shutdown_darwin.go`: Added `InterfaceName` field to `ShutdownState` for proper cleanup scoping after unclean shutdown - `host_darwin_test.go`: Updated all existing tests for new key format; added `TestGetKeyWithInput`, `TestNewHostManagerWithInterfaceName`, `TestMultipleInterfacesGenerateDifferentKeys`, `TestShutdownStateIncludesInterfaceName`, `TestPrimaryServiceKeyFormatNotAffected`, `TestMultipleInstancesBatchedIsolation` ## Testing Unit tests (no scutil required, any platform): ``` go test -short ./client/internal/dns/ -run "TestGetKeyWithInput|TestNewHostManager|TestSplitDomainsIntoBatches|TestMultipleInterfaces|TestShutdownState|TestPrimaryService" ``` Integration tests (macOS with scutil, requires admin): ``` go test ./client/internal/dns/ -run "TestDarwinDNS|TestMatchDomainBatching|TestOriginalNameservers|TestMultipleInstancesBatchedIsolation" -v -timeout 120s ``` This supersedes PR #4633 (which was opened from `main` and became unmergeable after #5368 landed). cc @mlsmaycon and @lixmal <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * DNS state is now scoped per network interface; shutdown/restoration use the interface context and preserve legacy keys for seamless upgrades. * Shutdown state now records interface info to prevent cross-interface cleanup and improve reliability. * **Tests** * Expanded test coverage for per-interface keying, batched domain isolation, legacy migration, and multi-interface isolation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change Internal implementation detail: scopes macOS scutil DNS state keys by WireGuard interface name to prevent multi-instance conflicts. No user-facing API, CLI, or configuration changes. ## Reference Related PR in nixpkgs-darwin: https://github.com/nix-darwin/nix-darwin/pull/1610 --- <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:05:44 -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#28068