From f7e9df6ffa84f7472b5dbb6228502d98b4c299a2 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 3 Jun 2026 11:40:41 +0200 Subject: [PATCH] Remove partial userspace firewall mode and open foreign chains via a table-less allower --- client/firewall/allower_other.go | 11 + client/firewall/allower_windows.go | 10 + client/firewall/create.go | 18 +- client/firewall/create_linux.go | 78 +++-- .../iptables/interface_allower_linux.go | 93 ++++++ client/firewall/iptables/manager_linux.go | 21 -- client/firewall/iptables/routing_linux.go | 18 +- client/firewall/manager/firewall.go | 3 - client/firewall/nftables/chains_linux.go | 88 +++--- ...nterface_allower_integration_linux_test.go | 90 ++++++ .../nftables/interface_allower_linux.go | 114 ++++++++ client/firewall/nftables/manager_linux.go | 41 +-- client/firewall/uspfilter/allow_netbird.go | 37 --- client/firewall/uspfilter/common/iface.go | 17 -- client/firewall/uspfilter/filter.go | 267 +++++++++++------- .../firewall/uspfilter/filter_bench_test.go | 96 ++++--- .../firewall/uspfilter/filter_filter_test.go | 8 +- .../uspfilter/filter_routeacl_test.go | 8 +- client/firewall/uspfilter/filter_test.go | 65 +++-- .../firewall/uspfilter/forwarder/forwarder.go | 11 +- ...indows.go => interface_allower_windows.go} | 65 +++-- client/firewall/uspfilter/localip.go | 4 +- client/firewall/uspfilter/nat.go | 35 +-- client/firewall/uspfilter/nat_bench_test.go | 40 ++- .../firewall/uspfilter/nat_stateful_test.go | 16 +- client/firewall/uspfilter/nat_test.go | 32 ++- .../firewall/uspfilter/peer_acl_bench_test.go | 20 +- .../firewall/uspfilter/peer_acl_dedup_test.go | 3 +- .../firewall/uspfilter/peer_acl_ipv6_test.go | 2 +- client/firewall/uspfilter/tracer_test.go | 2 +- client/internal/dns/server_test.go | 2 +- 31 files changed, 840 insertions(+), 475 deletions(-) create mode 100644 client/firewall/allower_other.go create mode 100644 client/firewall/allower_windows.go create mode 100644 client/firewall/iptables/interface_allower_linux.go create mode 100644 client/firewall/nftables/interface_allower_integration_linux_test.go create mode 100644 client/firewall/nftables/interface_allower_linux.go delete mode 100644 client/firewall/uspfilter/allow_netbird.go delete mode 100644 client/firewall/uspfilter/common/iface.go rename client/firewall/uspfilter/{allow_netbird_windows.go => interface_allower_windows.go} (79%) diff --git a/client/firewall/allower_other.go b/client/firewall/allower_other.go new file mode 100644 index 000000000..4d2ec9094 --- /dev/null +++ b/client/firewall/allower_other.go @@ -0,0 +1,11 @@ +//go:build android || (!linux && !windows) + +package firewall + +import "github.com/netbirdio/netbird/client/firewall/uspfilter" + +// interfaceAllower returns no allower: these platforms have no host firewall to +// open for the interface. +func interfaceAllower(IFaceMapper, uint16) uspfilter.InterfaceAllower { + return nil +} diff --git a/client/firewall/allower_windows.go b/client/firewall/allower_windows.go new file mode 100644 index 000000000..b9efa18a4 --- /dev/null +++ b/client/firewall/allower_windows.go @@ -0,0 +1,10 @@ +//go:build windows + +package firewall + +import "github.com/netbirdio/netbird/client/firewall/uspfilter" + +// interfaceAllower returns the Windows netsh-based interface allower. +func interfaceAllower(iface IFaceMapper, _ uint16) uspfilter.InterfaceAllower { + return uspfilter.NewWindowsInterfaceAllower(iface) +} diff --git a/client/firewall/create.go b/client/firewall/create.go index 0963af17f..cb68a0d04 100644 --- a/client/firewall/create.go +++ b/client/firewall/create.go @@ -6,8 +6,6 @@ import ( "fmt" "runtime" - log "github.com/sirupsen/logrus" - firewall "github.com/netbirdio/netbird/client/firewall/manager" "github.com/netbirdio/netbird/client/firewall/uspfilter" nftypes "github.com/netbirdio/netbird/client/internal/netflow/types" @@ -21,13 +19,11 @@ func NewFirewall(iface IFaceMapper, _ *statemanager.Manager, flowLogger nftypes. } // use userspace packet filtering firewall - fm, err := uspfilter.Create(iface, nil, disableServerRoutes, flowLogger, mtu) - if err != nil { - return nil, err - } - err = fm.AllowNetbird() - if err != nil { - log.Warnf("failed to allow netbird interface traffic: %v", err) - } - return fm, nil + return uspfilter.Create(uspfilter.Config{ + IFace: iface, + DisableServerRoutes: disableServerRoutes, + FlowLogger: flowLogger, + MTU: mtu, + InterfaceAllower: interfaceAllower(iface, mtu), + }) } diff --git a/client/firewall/create_linux.go b/client/firewall/create_linux.go index 09820e8e7..d585e85d7 100644 --- a/client/firewall/create_linux.go +++ b/client/firewall/create_linux.go @@ -16,6 +16,7 @@ import ( firewall "github.com/netbirdio/netbird/client/firewall/manager" nbnftables "github.com/netbirdio/netbird/client/firewall/nftables" "github.com/netbirdio/netbird/client/firewall/uspfilter" + "github.com/netbirdio/netbird/client/iface/netstack" nftypes "github.com/netbirdio/netbird/client/internal/netflow/types" "github.com/netbirdio/netbird/client/internal/statemanager" ) @@ -40,19 +41,29 @@ var errNoFirewallManager = errors.New("no firewall manager found") type FWType int func NewFirewall(iface IFaceMapper, stateManager *statemanager.Manager, flowLogger nftypes.FlowLogger, disableServerRoutes bool, mtu uint16) (firewall.Manager, error) { - // We run in userspace mode and force userspace firewall was requested. - if iface.IsUserspaceBind() && forceUserspaceFirewall() { - nativeFw, err := createNativeFirewall(iface, stateManager, disableServerRoutes, mtu) - if err != nil { - log.Warnf("failed to create native firewall: %v. Proceeding without it", err) + // Userspace firewall without a native counterpart: routing is handled + // entirely in userspace. The interface is opened in the kernel's foreign + // filter chains via a table-less allower, except in netstack mode where no + // kernel interface exists. + if netstack.IsEnabled() || (iface.IsUserspaceBind() && forceUserspaceFirewall()) { + if netstack.IsEnabled() { + log.Info("netstack mode, using userspace firewall") + } else { + log.Info("forcing userspace firewall") + } + cfg := uspfilter.Config{ + IFace: iface, + DisableServerRoutes: disableServerRoutes, + FlowLogger: flowLogger, + MTU: mtu, + InterfaceAllower: interfaceAllower(iface, mtu), } - log.Info("forcing userspace firewall") - return createUserspaceFirewall(iface, nativeFw, disableServerRoutes, flowLogger, mtu) + return uspfilter.Create(cfg) } // Use native firewall for either kernel or userspace, the interface appears identical to netfilter - fm, err := createNativeFirewall(iface, stateManager, disableServerRoutes, mtu) + fm, err := createNativeFirewall(iface, stateManager, mtu) switch { case err == nil && !iface.IsUserspaceBind(): // Nothing to do, fall through @@ -69,24 +80,41 @@ func NewFirewall(iface IFaceMapper, stateManager *statemanager.Manager, flowLogg case err != nil && iface.IsUserspaceBind(): // Fall back to the userspace packet filter if native is unavailable logNativeFirewallUnavailable(err) - return createUserspaceFirewall(iface, nil, disableServerRoutes, flowLogger, mtu) + return uspfilter.Create(uspfilter.Config{ + IFace: iface, + DisableServerRoutes: disableServerRoutes, + FlowLogger: flowLogger, + MTU: mtu, + InterfaceAllower: interfaceAllower(iface, mtu), + }) } return fm, nil } -// createUserspaceFirewall builds the userspace packet filter, optionally -// backed by a native firewall, and allows netbird interface traffic. -func createUserspaceFirewall(iface IFaceMapper, nativeFw firewall.Manager, disableServerRoutes bool, flowLogger nftypes.FlowLogger, mtu uint16) (firewall.Manager, error) { - fm, err := uspfilter.Create(iface, nativeFw, disableServerRoutes, flowLogger, mtu) - if err != nil { - return nil, err +// interfaceAllower selects how the userspace firewall opens the interface in +// foreign kernel chains: nftables when available (which also opens foreign nft +// tables), else iptables (the legacy fallback, filter INPUT only), else nil. +// firewalld trust is applied separately by the manager. Netstack has no kernel +// interface to open. +func interfaceAllower(iface IFaceMapper, mtu uint16) uspfilter.InterfaceAllower { + if netstack.IsEnabled() { + return nil } - if err := fm.AllowNetbird(); err != nil { - log.Errorf("failed to allow netbird interface traffic: %v", err) + nftAllower, err := nbnftables.NewInterfaceAllower(iface, mtu) + if err == nil { + return nftAllower } - return fm, nil + log.Infof("no nftables interface allower: %v", err) + + iptAllower, err := nbiptables.NewInterfaceAllower(iface) + if err == nil { + return iptAllower + } + log.Infof("no iptables interface allower: %v", err) + + return nil } // logNativeFirewallUnavailable logs the fallback to userspace at info level @@ -99,7 +127,7 @@ func logNativeFirewallUnavailable(err error) { } } -func createNativeFirewall(iface IFaceMapper, stateManager *statemanager.Manager, routes bool, mtu uint16) (firewall.Manager, error) { +func createNativeFirewall(iface IFaceMapper, stateManager *statemanager.Manager, mtu uint16) (firewall.Manager, error) { fm, err := createFW(iface, mtu) if err != nil { return nil, fmt.Errorf("create firewall: %w", err) @@ -193,15 +221,21 @@ func isIptablesClientAvailable(client *iptables.IPTables) bool { return err == nil } +// forceUserspaceFirewall reports whether the userspace firewall is forced. +// NB_FORCE_USERSPACE_ROUTER is an alias: forcing userspace routing implies the +// userspace firewall, since the two are no longer separable. func forceUserspaceFirewall() bool { - val := os.Getenv(EnvForceUserspaceFirewall) + return envForceBool(EnvForceUserspaceFirewall) || envForceBool(uspfilter.EnvForceUserspaceRouter) +} + +func envForceBool(name string) bool { + val := os.Getenv(name) if val == "" { return false } - force, err := strconv.ParseBool(val) if err != nil { - log.Warnf("failed to parse %s: %v", EnvForceUserspaceFirewall, err) + log.Warnf("failed to parse %s: %v", name, err) return false } return force diff --git a/client/firewall/iptables/interface_allower_linux.go b/client/firewall/iptables/interface_allower_linux.go new file mode 100644 index 000000000..40e9728e2 --- /dev/null +++ b/client/firewall/iptables/interface_allower_linux.go @@ -0,0 +1,93 @@ +package iptables + +import ( + "fmt" + + "github.com/coreos/go-iptables/iptables" + "github.com/hashicorp/go-multierror" + log "github.com/sirupsen/logrus" + + nberrors "github.com/netbirdio/netbird/client/errors" +) + +// InterfaceAllower opens the NetBird interface on the iptables filter INPUT +// chain so the host firewall doesn't drop traffic the userspace firewall +// handles. It is the fallback used when nftables is unavailable (an +// iptables-legacy host). +// +// It opens INPUT only: the userspace router never forwards in the kernel. +// firewalld trust is handled by the uspfilter manager, not here. +type InterfaceAllower struct { + ifaceName string + ipt4 *iptables.IPTables + // ipt6 is nil when the interface has no IPv6 overlay address. + ipt6 *iptables.IPTables +} + +// NewInterfaceAllower builds an iptables allower for the interface. It returns +// an error when iptables is unavailable, so the caller can fall back to +// firewalld trust. +func NewInterfaceAllower(wgIface iFaceMapper) (*InterfaceAllower, error) { + ipt4, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) + if err != nil { + return nil, fmt.Errorf("iptables not available: %w", err) + } + if _, err := ipt4.ListChains(tableFilter); err != nil { + return nil, fmt.Errorf("iptables filter table not available: %w", err) + } + + a := &InterfaceAllower{ifaceName: wgIface.Name(), ipt4: ipt4} + + // Missing v6 must not break the v4 path: open v4 only and continue. + if wgIface.Address().HasIPv6() { + ipt6, err := iptables.NewWithProtocol(iptables.ProtocolIPv6) + if err != nil { + log.Warnf("ip6tables not available, opening interface on v4 only: %v", err) + } else if _, err := ipt6.ListChains(tableFilter); err != nil { + log.Warnf("ip6tables filter table not available, opening interface on v4 only: %v", err) + } else { + a.ipt6 = ipt6 + } + } + + return a, nil +} + +// Apply inserts the interface accept rule on the filter INPUT chain. It removes +// any stale rule first so an unclean exit (e.g. SIGKILL, where Close never ran) +// is recovered deterministically rather than accumulating duplicates. +func (a *InterfaceAllower) Apply() error { + var merr *multierror.Error + for _, ipt := range a.clients() { + if err := ipt.DeleteIfExists(tableFilter, chainInput, a.inputRule()...); err != nil { + merr = multierror.Append(merr, fmt.Errorf("clean stale interface accept rule: %w", err)) + } + if err := ipt.Insert(tableFilter, chainInput, 1, a.inputRule()...); err != nil { + merr = multierror.Append(merr, fmt.Errorf("add interface accept rule: %w", err)) + } + } + return nberrors.FormatErrorOrNil(merr) +} + +// Close removes the interface accept rule. +func (a *InterfaceAllower) Close() error { + var merr *multierror.Error + for _, ipt := range a.clients() { + if err := ipt.DeleteIfExists(tableFilter, chainInput, a.inputRule()...); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove interface accept rule: %w", err)) + } + } + return nberrors.FormatErrorOrNil(merr) +} + +func (a *InterfaceAllower) inputRule() []string { + return []string{"-i", a.ifaceName, "-j", "ACCEPT"} +} + +func (a *InterfaceAllower) clients() []*iptables.IPTables { + clients := []*iptables.IPTables{a.ipt4} + if a.ipt6 != nil { + clients = append(clients, a.ipt6) + } + return clients +} diff --git a/client/firewall/iptables/manager_linux.go b/client/firewall/iptables/manager_linux.go index 7fbf22c8a..43e0b7635 100644 --- a/client/firewall/iptables/manager_linux.go +++ b/client/firewall/iptables/manager_linux.go @@ -312,27 +312,6 @@ func (m *Manager) Close(stateManager *statemanager.Manager) error { return nberrors.FormatErrorOrNil(merr) } -// AllowNetbird allows netbird interface traffic. -// This is called when USPFilter wraps the native firewall, adding blanket accept -// rules so that packet filtering is handled in userspace instead of by netfilter. -func (m *Manager) AllowNetbird() error { - var merr *multierror.Error - if _, err := m.AddFilterRule(nil, []netip.Prefix{netip.PrefixFrom(netip.IPv4Unspecified(), 0)}, firewall.Network{}, firewall.ProtocolALL, nil, nil, firewall.ActionAccept); err != nil { - merr = multierror.Append(merr, fmt.Errorf("allow netbird v4 interface traffic: %w", err)) - } - if m.hasIPv6() { - if _, err := m.AddFilterRule(nil, []netip.Prefix{netip.PrefixFrom(netip.IPv6Unspecified(), 0)}, firewall.Network{}, firewall.ProtocolALL, nil, nil, firewall.ActionAccept); err != nil { - merr = multierror.Append(merr, fmt.Errorf("allow netbird v6 interface traffic: %w", err)) - } - } - - if err := firewalld.TrustInterface(m.wgIface.Name()); err != nil { - log.Warnf("failed to trust interface in firewalld: %v", err) - } - - return nberrors.FormatErrorOrNil(merr) -} - // Flush doesn't need to be implemented for this manager func (m *Manager) Flush() error { return nil } diff --git a/client/firewall/iptables/routing_linux.go b/client/firewall/iptables/routing_linux.go index 76d587d0d..49fd03bbf 100644 --- a/client/firewall/iptables/routing_linux.go +++ b/client/firewall/iptables/routing_linux.go @@ -22,16 +22,14 @@ func (r *family) AddNatRule(pair firewall.RouterPair) error { } } - if !pair.Masquerade { - return nil - } + if pair.Masquerade { + if err := r.addNatRule(pair); err != nil { + return fmt.Errorf("add nat rule: %w", err) + } - if err := r.addNatRule(pair); err != nil { - return fmt.Errorf("add nat rule: %w", err) - } - - if err := r.addNatRule(firewall.GetInversePair(pair)); err != nil { - return fmt.Errorf("add inverse nat rule: %w", err) + if err := r.addNatRule(firewall.GetInversePair(pair)); err != nil { + return fmt.Errorf("add inverse nat rule: %w", err) + } } r.updateState() @@ -244,7 +242,6 @@ func (r *family) addNatRule(pair firewall.RouterPair) error { r.rules[ruleID] = rule - r.updateState() return nil } @@ -264,6 +261,5 @@ func (r *family) removeNatRule(pair firewall.RouterPair) error { log.Debugf("marking rule %s not found", ruleID) } - r.updateState() return nil } diff --git a/client/firewall/manager/firewall.go b/client/firewall/manager/firewall.go index bff1b8e3e..97a94d0f5 100644 --- a/client/firewall/manager/firewall.go +++ b/client/firewall/manager/firewall.go @@ -115,9 +115,6 @@ func (d Network) IsZero() bool { type Manager interface { Init(stateManager *statemanager.Manager) error - // AllowNetbird allows netbird interface traffic - AllowNetbird() error - // AddFilterRule adds a packet-filtering rule to the firewall. // // If destination is the zero Network, the rule applies to traffic diff --git a/client/firewall/nftables/chains_linux.go b/client/firewall/nftables/chains_linux.go index c39eac38c..745f2da54 100644 --- a/client/firewall/nftables/chains_linux.go +++ b/client/firewall/nftables/chains_linux.go @@ -83,8 +83,9 @@ func (r *family) createContainers() error { log.Errorf("failed to add MSS clamping rules: %s", err) } - if err := r.acceptForwardRules(); err != nil { - log.Errorf("failed to add accept rules for the forward chain: %s", err) + // Kernel routing opens both INPUT and FORWARD. + if err := r.openInterface(true); err != nil { + log.Errorf("failed to open interface in foreign chains: %s", err) } if err := firewalld.TrustInterface(r.wgIface.Name()); err != nil { @@ -174,21 +175,26 @@ func (r *family) setupDataPlaneMark() error { return nil } -func (r *family) acceptForwardRules() error { +// openInterface adds passthrough accept rules for the NetBird interface to the +// kernel's filter table and external chains so they don't drop our traffic. +// includeForward also opens the FORWARD chains (kernel routing); when false only +// INPUT is opened, which is all the userspace router needs since it never +// forwards in the kernel. +func (r *family) openInterface(includeForward bool) error { var merr *multierror.Error - if err := r.acceptFilterTableRules(); err != nil { + if err := r.acceptFilterTableRules(includeForward); err != nil { merr = multierror.Append(merr, err) } - if err := r.acceptExternalChainsRules(); err != nil { + if err := r.acceptExternalChainsRules(includeForward); err != nil { merr = multierror.Append(merr, fmt.Errorf("add accept rules to external chains: %w", err)) } return nberrors.FormatErrorOrNil(merr) } -func (r *family) acceptFilterTableRules() error { +func (r *family) acceptFilterTableRules(includeForward bool) error { if r.filterTable == nil { return nil } @@ -196,7 +202,7 @@ func (r *family) acceptFilterTableRules() error { fw := "iptables" defer func() { - log.Debugf("Used %s to add accept forward and input rules", fw) + log.Debugf("Used %s to add accept input/forward rules", fw) }() // Try iptables first and fallback to nftables if iptables is not available. @@ -206,25 +212,27 @@ func (r *family) acceptFilterTableRules() error { log.Warnf("Will use nftables to manipulate the filter table because iptables is not available: %v", err) fw = "nftables" - return r.acceptFilterRulesNftables(r.filterTable) + return r.acceptFilterRulesNftables(r.filterTable, includeForward) } - if err := r.acceptFilterRulesIptables(ipt); err != nil { + if err := r.acceptFilterRulesIptables(ipt, includeForward); err != nil { log.Warnf("iptables failed (table may be incompatible), falling back to nftables: %v", err) fw = "nftables" - return r.acceptFilterRulesNftables(r.filterTable) + return r.acceptFilterRulesNftables(r.filterTable, includeForward) } return nil } -func (r *family) acceptFilterRulesIptables(ipt *iptables.IPTables) error { +func (r *family) acceptFilterRulesIptables(ipt *iptables.IPTables, includeForward bool) error { var merr *multierror.Error - for _, rule := range r.getAcceptForwardRules() { - if err := ipt.Insert("filter", chainNameForward, 1, rule...); err != nil { - merr = multierror.Append(merr, fmt.Errorf("add iptables forward rule: %v", err)) - } else { - log.Debugf("added iptables forward rule: %v", rule) + if includeForward { + for _, rule := range r.getAcceptForwardRules() { + if err := ipt.Insert("filter", chainNameForward, 1, rule...); err != nil { + merr = multierror.Append(merr, fmt.Errorf("add iptables forward rule: %v", err)) + } else { + log.Debugf("added iptables forward rule: %v", rule) + } } } @@ -252,17 +260,19 @@ func (r *family) getAcceptInputRule() []string { // acceptFilterRulesNftables adds accept rules to the ip filter table using nftables. // This is used when iptables is not available. -func (r *family) acceptFilterRulesNftables(table *nftables.Table) error { +func (r *family) acceptFilterRulesNftables(table *nftables.Table, includeForward bool) error { intf := ifname(r.wgIface.Name()) - forwardChain := &nftables.Chain{ - Name: chainNameForward, - Table: table, - Type: nftables.ChainTypeFilter, - Hooknum: nftables.ChainHookForward, - Priority: nftables.ChainPriorityFilter, + if includeForward { + forwardChain := &nftables.Chain{ + Name: chainNameForward, + Table: table, + Type: nftables.ChainTypeFilter, + Hooknum: nftables.ChainHookForward, + Priority: nftables.ChainPriorityFilter, + } + r.insertForwardAcceptRules(forwardChain, intf) } - r.insertForwardAcceptRules(forwardChain, intf) inputChain := &nftables.Chain{ Name: chainNameInput, @@ -278,7 +288,7 @@ func (r *family) acceptFilterRulesNftables(table *nftables.Table) error { // acceptExternalChainsRules adds accept rules to external chains (non-netbird, non-iptables tables). // It dynamically finds chains at call time to handle chains that may have been created after startup. -func (r *family) acceptExternalChainsRules() error { +func (r *family) acceptExternalChainsRules(includeForward bool) error { chains := r.findExternalChains() if len(chains) == 0 { return nil @@ -286,7 +296,7 @@ func (r *family) acceptExternalChainsRules() error { intf := ifname(r.wgIface.Name()) for _, chain := range chains { - r.applyExternalChainAccept(chain, intf) + r.applyExternalChainAccept(chain, intf, includeForward) } if err := r.conn.Flush(); err != nil { @@ -295,7 +305,7 @@ func (r *family) acceptExternalChainsRules() error { return nil } -func (r *family) applyExternalChainAccept(chain *nftables.Chain, intf []byte) { +func (r *family) applyExternalChainAccept(chain *nftables.Chain, intf []byte, includeForward bool) { if chain.Hooknum == nil { log.Debugf("skipping external chain %s/%s: hooknum is nil", chain.Table.Name, chain.Name) return @@ -306,7 +316,9 @@ func (r *family) applyExternalChainAccept(chain *nftables.Chain, intf []byte) { switch *chain.Hooknum { case *nftables.ChainHookForward: - r.insertForwardAcceptRules(chain, intf) + if includeForward { + r.insertForwardAcceptRules(chain, intf) + } case *nftables.ChainHookInput: r.insertInputAcceptRule(chain, intf) } @@ -576,26 +588,6 @@ func (r *family) removeAcceptFilterRulesIptables(ipt *iptables.IPTables) error { return nberrors.FormatErrorOrNil(merr) } -func (r *family) createDefaultAllowRules() error { - expIn := []expr.Any{ - &expr.Verdict{ - Kind: expr.VerdictAccept, - }, - } - - _ = r.conn.InsertRule(&nftables.Rule{ - Table: r.workTable, - Chain: r.chainInputRules, - Position: 0, - Exprs: expIn, - }) - - if err := r.conn.Flush(); err != nil { - return fmt.Errorf(flushError, err) - } - return nil -} - // Flush rule/chain/set operations from the buffer // // Method also get all rules after flush and refreshes handle values in the rulesets diff --git a/client/firewall/nftables/interface_allower_integration_linux_test.go b/client/firewall/nftables/interface_allower_integration_linux_test.go new file mode 100644 index 000000000..a654727d3 --- /dev/null +++ b/client/firewall/nftables/interface_allower_integration_linux_test.go @@ -0,0 +1,90 @@ +//go:build integration && !android + +package nftables + +import ( + "bytes" + "os" + "testing" + + "github.com/google/nftables" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/iface" +) + +// TestInterfaceAllowerInputOnly verifies the userspace-mode allower opens the +// interface on the INPUT hook of foreign chains only (not FORWARD, since the +// userspace router never forwards in the kernel), creates no netbird work +// table, and removes its rules on Close. +func TestInterfaceAllowerInputOnly(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("root required") + } + + require.False(t, ipTableExists(t, getTableName()), "precondition: no stale netbird table") + + conn := &nftables.Conn{} + extTable := conn.AddTable(&nftables.Table{Name: "nbtest_extchains", Family: nftables.TableFamilyINet}) + inputChain := conn.AddChain(&nftables.Chain{ + Name: "ext_input", Table: extTable, + Hooknum: nftables.ChainHookInput, Priority: nftables.ChainPriorityFilter, Type: nftables.ChainTypeFilter, + }) + forwardChain := conn.AddChain(&nftables.Chain{ + Name: "ext_forward", Table: extTable, + Hooknum: nftables.ChainHookForward, Priority: nftables.ChainPriorityFilter, Type: nftables.ChainTypeFilter, + }) + require.NoError(t, conn.Flush(), "create external table and chains") + t.Cleanup(func() { + c := &nftables.Conn{} + c.DelTable(extTable) + _ = c.Flush() + }) + + allower, err := NewInterfaceAllower(ifaceMock, iface.DefaultMTU) + require.NoError(t, err, "create allower") + require.NoError(t, allower.Apply(), "apply") + + require.True(t, chainHasUserData(t, extTable, inputChain, userDataAcceptInputRule), + "external INPUT chain should get the accept rule") + require.Len(t, listRules(t, extTable, forwardChain), 0, + "external FORWARD chain must not be opened in userspace mode") + require.False(t, ipTableExists(t, getTableName()), + "allower must not create a netbird work table") + + require.NoError(t, allower.Close(), "close") + require.False(t, chainHasUserData(t, extTable, inputChain, userDataAcceptInputRule), + "accept rule should be removed on close") +} + +func listRules(t *testing.T, table *nftables.Table, chain *nftables.Chain) []*nftables.Rule { + t.Helper() + c := &nftables.Conn{} + rules, err := c.GetRules(table, chain) + require.NoError(t, err) + return rules +} + +func chainHasUserData(t *testing.T, table *nftables.Table, chain *nftables.Chain, ud string) bool { + for _, r := range listRules(t, table, chain) { + if bytes.Equal(r.UserData, []byte(ud)) { + return true + } + } + return false +} + +func ipTableExists(t *testing.T, name string) bool { + t.Helper() + c := &nftables.Conn{} + for _, fam := range []nftables.TableFamily{nftables.TableFamilyIPv4, nftables.TableFamilyIPv6} { + tbls, err := c.ListTablesOfFamily(fam) + require.NoError(t, err) + for _, tb := range tbls { + if tb.Name == name { + return true + } + } + } + return false +} diff --git a/client/firewall/nftables/interface_allower_linux.go b/client/firewall/nftables/interface_allower_linux.go new file mode 100644 index 000000000..d208d874d --- /dev/null +++ b/client/firewall/nftables/interface_allower_linux.go @@ -0,0 +1,114 @@ +package nftables + +import ( + "fmt" + + "github.com/google/nftables" + "github.com/hashicorp/go-multierror" + + nberrors "github.com/netbirdio/netbird/client/errors" +) + +// InterfaceAllower opens the NetBird interface in the kernel's filter table and +// external chains and keeps them reconciled via a netlink monitor, so the host +// firewall doesn't drop traffic the NetBird firewall handles. It is used by the +// userspace firewall, where routing happens in the forwarder, so only INPUT is +// opened (the userspace router never forwards in the kernel). +// +// It owns its own families/connection and never creates a netbird work table. +// firewalld trust is handled by the caller, not here. Its operations are serial +// (Apply before the monitor starts; reconciles run on the single monitor +// goroutine; Close stops the monitor before removing), so it needs no locking. +// +// TODO: this opens nftables and the iptables-nft filter table (detected via +// nft), but not a legacy-iptables ruleset running in parallel with nftables. +// Such a host would keep its legacy filter chains closed for the interface. +type InterfaceAllower struct { + family4 *family + family6 *family + extMonitor *externalChainMonitor +} + +// NewInterfaceAllower builds an allower for the given interface. It returns an +// error when nftables is unavailable (e.g. an iptables-legacy host), so the +// caller can fall back to firewalld trust. +func NewInterfaceAllower(wgIface iFaceMapper, mtu uint16) (*InterfaceAllower, error) { + tableName := getTableName() + + family4, err := newFamily(&nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv4}, wgIface, mtu) + if err != nil { + return nil, fmt.Errorf("create family: %w", err) + } + + // Probe nftables availability before committing to this backend. + if _, err := family4.conn.ListChainsOfTableFamily(nftables.TableFamilyINet); err != nil { + return nil, fmt.Errorf("nftables not available: %w", err) + } + + a := &InterfaceAllower{family4: family4} + + if wgIface.Address().HasIPv6() { + family6, err := newFamily(&nftables.Table{Name: tableName, Family: nftables.TableFamilyIPv6}, wgIface, mtu) + if err != nil { + return nil, fmt.Errorf("create v6 family: %w", err) + } + a.family6 = family6 + } + + a.extMonitor = newExternalChainMonitor(a) + return a, nil +} + +// Apply opens the interface (INPUT only) in the foreign filter chains and starts +// reconciling them on nftables changes. +func (a *InterfaceAllower) Apply() error { + var merr *multierror.Error + for _, f := range a.families() { + // Remove any stale accepts first so a prior unclean exit (e.g. SIGKILL, + // where Close never ran) is recovered deterministically rather than + // accumulating duplicate rules on the iptables filter table. + if err := f.removeAcceptFilterRules(); err != nil { + merr = multierror.Append(merr, fmt.Errorf("clean stale accept rules: %w", err)) + } + if err := f.openInterface(false); err != nil { + merr = multierror.Append(merr, err) + } + } + + a.extMonitor.start() + return nberrors.FormatErrorOrNil(merr) +} + +// families returns the configured address families (v4, and v6 when present). +func (a *InterfaceAllower) families() []*family { + families := []*family{a.family4} + if a.family6 != nil { + families = append(families, a.family6) + } + return families +} + +// reconcileExternalChains re-applies the INPUT accepts to external chains. It +// implements externalChainReconciler for the monitor. +func (a *InterfaceAllower) reconcileExternalChains() error { + var merr *multierror.Error + for _, f := range a.families() { + if err := f.acceptExternalChainsRules(false); err != nil { + merr = multierror.Append(merr, err) + } + } + return nberrors.FormatErrorOrNil(merr) +} + +// Close stops the monitor and removes the accept rules. +func (a *InterfaceAllower) Close() error { + a.extMonitor.stop() + + var merr *multierror.Error + for _, f := range a.families() { + if err := f.removeAcceptFilterRules(); err != nil { + merr = multierror.Append(merr, err) + } + } + return nberrors.FormatErrorOrNil(merr) +} diff --git a/client/firewall/nftables/manager_linux.go b/client/firewall/nftables/manager_linux.go index 49023978f..e00b0fd1a 100644 --- a/client/firewall/nftables/manager_linux.go +++ b/client/firewall/nftables/manager_linux.go @@ -15,7 +15,6 @@ import ( "golang.org/x/sys/unix" nberrors "github.com/netbirdio/netbird/client/errors" - "github.com/netbirdio/netbird/client/firewall/firewalld" firewall "github.com/netbirdio/netbird/client/firewall/manager" "github.com/netbirdio/netbird/client/iface/wgaddr" "github.com/netbirdio/netbird/client/internal/statemanager" @@ -140,19 +139,20 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error { // reconcileExternalChains re-applies passthrough accept rules to external // filter chains for both IPv4 and IPv6 routers. Called by the monitor when -// tables or chains appear (e.g. after firewalld reloads). +// tables or chains appear (e.g. after firewalld reloads). Kernel routing opens +// both INPUT and FORWARD. func (m *Manager) reconcileExternalChains() error { m.mutex.Lock() defer m.mutex.Unlock() var merr *multierror.Error if m.family4 != nil { - if err := m.family4.acceptExternalChainsRules(); err != nil { + if err := m.family4.acceptExternalChainsRules(true); err != nil { merr = multierror.Append(merr, fmt.Errorf("v4: %w", err)) } } if m.hasIPv6() { - if err := m.family6.acceptExternalChainsRules(); err != nil { + if err := m.family6.acceptExternalChainsRules(true); err != nil { merr = multierror.Append(merr, fmt.Errorf("v6: %w", err)) } } @@ -370,39 +370,6 @@ func (m *Manager) RemoveNatRule(pair firewall.RouterPair) error { return nberrors.FormatErrorOrNil(merr) } -// AllowNetbird allows netbird interface traffic. -// This is called when USPFilter wraps the native firewall, adding blanket accept -// rules so that packet filtering is handled in userspace instead of by netfilter. -// -// TODO: In USP mode this only adds ACCEPT to the netbird table's own chains, -// which doesn't override DROP rules in external tables (e.g. firewalld). -// Should add passthrough rules to external chains (like the native mode router's -// addExternalChainsRules does) for both the netbird table family and inet tables. -// The netbird table itself is fine (routing chains already exist there), but -// non-netbird tables with INPUT/FORWARD hooks can still DROP our WG traffic. -func (m *Manager) AllowNetbird() error { - m.mutex.Lock() - defer m.mutex.Unlock() - - if err := m.family4.createDefaultAllowRules(); err != nil { - return fmt.Errorf("create default allow rules: %w", err) - } - if m.hasIPv6() { - if err := m.family6.createDefaultAllowRules(); err != nil { - return fmt.Errorf("create v6 default allow rules: %w", err) - } - } - if err := m.rConn.Flush(); err != nil { - return fmt.Errorf("flush allow input netbird rules: %w", err) - } - - if err := firewalld.TrustInterface(m.wgIface.Name()); err != nil { - log.Warnf("failed to trust interface in firewalld: %v", err) - } - - return nil -} - // SetLegacyManagement sets the route manager to use legacy management func (m *Manager) SetLegacyManagement(isLegacy bool) error { if err := firewall.SetLegacyManagement(m.family4, isLegacy); err != nil { diff --git a/client/firewall/uspfilter/allow_netbird.go b/client/firewall/uspfilter/allow_netbird.go deleted file mode 100644 index b120cdf12..000000000 --- a/client/firewall/uspfilter/allow_netbird.go +++ /dev/null @@ -1,37 +0,0 @@ -//go:build !windows - -package uspfilter - -import ( - log "github.com/sirupsen/logrus" - - "github.com/netbirdio/netbird/client/firewall/firewalld" - "github.com/netbirdio/netbird/client/internal/statemanager" -) - -// Close cleans up the firewall manager by removing all rules and closing trackers -func (m *Manager) Close(stateManager *statemanager.Manager) error { - m.mutex.Lock() - defer m.mutex.Unlock() - - m.resetState() - - if m.nativeFirewall != nil { - return m.nativeFirewall.Close(stateManager) - } - if err := firewalld.UntrustInterface(m.wgIface.Name()); err != nil { - log.Warnf("failed to untrust interface in firewalld: %v", err) - } - return nil -} - -// AllowNetbird allows netbird interface traffic -func (m *Manager) AllowNetbird() error { - if m.nativeFirewall != nil { - return m.nativeFirewall.AllowNetbird() - } - if err := firewalld.TrustInterface(m.wgIface.Name()); err != nil { - log.Warnf("failed to trust interface in firewalld: %v", err) - } - return nil -} diff --git a/client/firewall/uspfilter/common/iface.go b/client/firewall/uspfilter/common/iface.go deleted file mode 100644 index 9c06eb3f7..000000000 --- a/client/firewall/uspfilter/common/iface.go +++ /dev/null @@ -1,17 +0,0 @@ -package common - -import ( - wgdevice "golang.zx2c4.com/wireguard/device" - - "github.com/netbirdio/netbird/client/iface/device" - "github.com/netbirdio/netbird/client/iface/wgaddr" -) - -// IFaceMapper defines subset methods of interface required for manager -type IFaceMapper interface { - Name() string - SetFilter(device.PacketFilter) error - Address() wgaddr.Address - GetWGDevice() *wgdevice.Device - GetDevice() *device.FilteredDevice -} diff --git a/client/firewall/uspfilter/filter.go b/client/firewall/uspfilter/filter.go index 22573a1d2..ca297e1be 100644 --- a/client/firewall/uspfilter/filter.go +++ b/client/firewall/uspfilter/filter.go @@ -19,14 +19,18 @@ import ( "github.com/google/uuid" "github.com/hashicorp/go-multierror" log "github.com/sirupsen/logrus" + wgdevice "golang.zx2c4.com/wireguard/device" nberrors "github.com/netbirdio/netbird/client/errors" + "github.com/netbirdio/netbird/client/firewall/firewalld" firewall "github.com/netbirdio/netbird/client/firewall/manager" "github.com/netbirdio/netbird/client/firewall/uspfilter/common" "github.com/netbirdio/netbird/client/firewall/uspfilter/conntrack" "github.com/netbirdio/netbird/client/firewall/uspfilter/forwarder" nblog "github.com/netbirdio/netbird/client/firewall/uspfilter/log" + "github.com/netbirdio/netbird/client/iface/device" "github.com/netbirdio/netbird/client/iface/netstack" + "github.com/netbirdio/netbird/client/iface/wgaddr" nbid "github.com/netbirdio/netbird/client/internal/acl/id" nftypes "github.com/netbirdio/netbird/client/internal/netflow/types" "github.com/netbirdio/netbird/client/internal/statemanager" @@ -57,7 +61,10 @@ const ( // EnvDisableMSSClamping disables TCP MSS clamping for forwarded traffic. EnvDisableMSSClamping = "NB_DISABLE_MSS_CLAMPING" - // EnvForceUserspaceRouter forces userspace routing even if native routing is available. + // EnvForceUserspaceRouter is a deprecated alias for + // NB_FORCE_USERSPACE_FIREWALL: the userspace firewall always routes in + // userspace, so forcing one forces the other. Kept for backward + // compatibility. EnvForceUserspaceRouter = "NB_FORCE_USERSPACE_ROUTER" // EnvEnableLocalForwarding enables forwarding of local traffic to the native stack for internal (non-NetBird) interfaces. @@ -69,7 +76,10 @@ const ( EnvEnableNetstackLocalForwarding = "NB_ENABLE_NETSTACK_LOCAL_FORWARDING" ) -var errNatNotSupported = errors.New("nat not supported with userspace firewall") +// errNotSupported is returned by firewall operations that only make sense with +// a kernel firewall (kernel NAT/DNAT, eBPF) and are not implemented in +// userspace mode, where they should not be called. +var errNotSupported = errors.New("not supported with userspace firewall") // peerRules is the canonical list-based storage for peer ACL rules. // Match order is significant: drop rules come before accept rules so @@ -105,17 +115,48 @@ type peerRuleSpec struct { action firewall.Action } +// Iface is the network interface the userspace firewall attaches to: the +// methods of the WireGuard device it actually uses. +type Iface interface { + Name() string + Address() wgaddr.Address + SetFilter(device.PacketFilter) error + GetWGDevice() *wgdevice.Device +} + +// InterfaceAllower opens the NetBird interface in the host firewall so it +// doesn't drop traffic the userspace firewall handles, without taking over +// packet filtering. Implementations (nftables, iptables, firewalld, the windows +// netsh rule) are selected per platform and injected into Create; Apply runs at +// creation and Close on teardown. +type InterfaceAllower interface { + Apply() error + Close() error +} + +// Config holds the dependencies and options for the userspace firewall. +type Config struct { + // IFace is the overlay interface the filter attaches to. + IFace Iface + // InterfaceAllower opens the NetBird interface in foreign kernel filter + // chains so the kernel doesn't drop traffic the userspace firewall handles. + // Nil in netstack mode, on non-Linux platforms without a backend, or when + // neither nftables nor iptables is available. firewalld trust is applied by + // the manager regardless, since firewalld owns its own chains and we cannot + // insert into them. + InterfaceAllower InterfaceAllower + // DisableServerRoutes indicates whether server routes are disabled. + DisableServerRoutes bool + FlowLogger nftypes.FlowLogger + MTU uint16 +} + // Manager userspace firewall manager type Manager struct { - decoders sync.Pool - wgIface common.IFaceMapper - // nativeFirewall is the kernel firewall (nftables/iptables) used for - // the split case where peer ACLs run in userspace here but routing - // stays in the kernel: when the userspace firewall is forced yet the - // router keeps using the kernel, route NAT/ACLs and DNAT are - // delegated to it. It is nil when no native backend is available. - nativeFirewall firewall.Manager - mutex sync.RWMutex + decoders sync.Pool + wgIface Iface + ifaceAllower InterfaceAllower + mutex sync.RWMutex incomingDenyRules peerRules incomingAcceptRules peerRules @@ -236,7 +277,7 @@ func parseCreateEnv() (bool, bool, bool) { return disableConntrack, enableLocalForwarding, disableMSSClamping } -func Create(iface common.IFaceMapper, nativeFirewall firewall.Manager, disableServerRoutes bool, flowLogger nftypes.FlowLogger, mtu uint16) (*Manager, error) { +func Create(cfg Config) (_ *Manager, err error) { disableConntrack, enableLocalForwarding, disableMSSClamping := parseCreateEnv() m := &Manager{ @@ -259,13 +300,13 @@ func Create(iface common.IFaceMapper, nativeFirewall firewall.Manager, disableSe return d }, }, - wgIface: iface, - nativeFirewall: nativeFirewall, + wgIface: cfg.IFace, + ifaceAllower: cfg.InterfaceAllower, localipmanager: newLocalIPManager(), - disableServerRoutes: disableServerRoutes, + disableServerRoutes: cfg.DisableServerRoutes, stateful: !disableConntrack, logger: nblog.NewFromLogrus(log.StandardLogger()), - flowLogger: flowLogger, + flowLogger: cfg.FlowLogger, netstack: netstack.IsEnabled(), localForwarding: enableLocalForwarding, peerRulesMap: make(map[nbid.RuleID]*PeerRule), @@ -273,46 +314,117 @@ func Create(iface common.IFaceMapper, nativeFirewall firewall.Manager, disableSe dnatMappings: make(map[netip.Addr]netip.Addr), portDNATRules: []portDNATRule{}, netstackServices: make(map[serviceKey]struct{}), - mtu: mtu, + mtu: cfg.MTU, } m.routingEnabled.Store(false) + // Release the allower (and its monitor) if setup fails after it was wired in. + defer func() { + if err != nil { + m.closeAllowerOnError() + } + }() + if !disableMSSClamping { - m.mssClampEnabled = true - if mtu > ipv4TCPHeaderMinSize { - m.mssClampValueIPv4 = mtu - ipv4TCPHeaderMinSize - } - if mtu > ipv6TCPHeaderMinSize { - m.mssClampValueIPv6 = mtu - ipv6TCPHeaderMinSize - } + m.enableMSSClamping(cfg.MTU) } - if err := m.localipmanager.UpdateLocalIPs(iface); err != nil { + if err := m.localipmanager.UpdateLocalIPs(cfg.IFace); err != nil { return nil, fmt.Errorf("update local IPs: %w", err) } - if disableConntrack { - log.Info("conntrack is disabled") - } else { - m.udpTracker = conntrack.NewUDPTracker(conntrack.DefaultUDPTimeout, m.logger, flowLogger) - m.icmpTracker = conntrack.NewICMPTracker(conntrack.DefaultICMPTimeout, m.logger, flowLogger) - m.tcpTracker = conntrack.NewTCPTracker(conntrack.DefaultTCPTimeout, m.logger, flowLogger) - } + m.setupConntrack(disableConntrack) if m.netstack && m.localForwarding { if err := m.initForwarder(); err != nil { log.Errorf("failed to initialize forwarder: %v", err) } } - if err := iface.SetFilter(m); err != nil { + if err := cfg.IFace.SetFilter(m); err != nil { return nil, fmt.Errorf("set filter: %w", err) } + + m.openHostFirewall(cfg.IFace.Name()) + return m, nil } +// closeAllowerOnError releases the allower (and its monitor) when Create fails +// after the allower was wired in. +func (m *Manager) closeAllowerOnError() { + if m.ifaceAllower == nil { + return + } + if err := m.ifaceAllower.Close(); err != nil { + log.Warnf("close interface allower after failed firewall setup: %v", err) + } +} + +// enableMSSClamping enables MSS clamping and computes the per-family clamp values. +func (m *Manager) enableMSSClamping(mtu uint16) { + m.mssClampEnabled = true + if mtu > ipv4TCPHeaderMinSize { + m.mssClampValueIPv4 = mtu - ipv4TCPHeaderMinSize + } + if mtu > ipv6TCPHeaderMinSize { + m.mssClampValueIPv6 = mtu - ipv6TCPHeaderMinSize + } +} + +// setupConntrack initializes the stateful trackers unless conntrack is disabled. +func (m *Manager) setupConntrack(disabled bool) { + if disabled { + log.Info("conntrack is disabled") + return + } + m.udpTracker = conntrack.NewUDPTracker(conntrack.DefaultUDPTimeout, m.logger, m.flowLogger) + m.icmpTracker = conntrack.NewICMPTracker(conntrack.DefaultICMPTimeout, m.logger, m.flowLogger) + m.tcpTracker = conntrack.NewTCPTracker(conntrack.DefaultTCPTimeout, m.logger, m.flowLogger) +} + +// openHostFirewall opens the NetBird interface in the kernel firewall so it +// doesn't drop traffic the userspace firewall handles. Best-effort: failures +// here shouldn't prevent the firewall from coming up. +func (m *Manager) openHostFirewall(ifaceName string) { + if m.ifaceAllower != nil { + if err := m.ifaceAllower.Apply(); err != nil { + log.Errorf("failed to allow netbird interface traffic: %v", err) + } + } + // firewalld owns its own chains we can't insert into, so trust the interface + // there in addition to the allower. Netstack has no kernel interface. + if !m.netstack { + if err := firewalld.TrustInterface(ifaceName); err != nil { + log.Warnf("failed to trust interface in firewalld: %v", err) + } + } +} + +// Close cleans up the firewall manager: removes rules, closes trackers, and +// closes the interface allower. +func (m *Manager) Close(*statemanager.Manager) error { + m.mutex.Lock() + defer m.mutex.Unlock() + + m.resetState() + + var merr *multierror.Error + if m.ifaceAllower != nil { + if err := m.ifaceAllower.Close(); err != nil { + merr = multierror.Append(merr, fmt.Errorf("close interface allower: %w", err)) + } + } + if !m.netstack { + if err := firewalld.UntrustInterface(m.wgIface.Name()); err != nil { + merr = multierror.Append(merr, fmt.Errorf("untrust interface in firewalld: %w", err)) + } + } + return nberrors.FormatErrorOrNil(merr) +} + // blockInvalidRouted installs drop rules for traffic to the wg overlay that // arrives via the routing path. v4 and v6 are independent: a v6 install // failure leaves v4 protection in place (and vice versa) so the returned // slice always contains whatever was successfully installed, even on error. // Callers must persist the slice so DisableRouting can clean partial state. -func (m *Manager) blockInvalidRouted(iface common.IFaceMapper) ([]firewall.Rule, error) { +func (m *Manager) blockInvalidRouted(iface Iface) ([]firewall.Rule, error) { wgPrefix := iface.Address().Network log.Debugf("blocking invalid routed traffic for %s", wgPrefix) @@ -360,20 +472,14 @@ func (m *Manager) blockInvalidRouted(iface common.IFaceMapper) ([]firewall.Rule, } func (m *Manager) determineRouting() error { - var disableUspRouting, forceUserspaceRouter bool - var err error + var disableUspRouting bool if val := os.Getenv(EnvDisableUserspaceRouting); val != "" { + var err error disableUspRouting, err = strconv.ParseBool(val) if err != nil { log.Warnf("failed to parse %s: %v", EnvDisableUserspaceRouting, err) } } - if val := os.Getenv(EnvForceUserspaceRouter); val != "" { - forceUserspaceRouter, err = strconv.ParseBool(val) - if err != nil { - log.Warnf("failed to parse %s: %v", EnvForceUserspaceRouter, err) - } - } switch { case disableUspRouting: @@ -388,26 +494,11 @@ func (m *Manager) determineRouting() error { log.Info("server routes are disabled") - case forceUserspaceRouter: - m.routingEnabled.Store(true) - m.nativeRouter.Store(false) - - log.Info("userspace routing is forced") - - case !m.netstack && m.nativeFirewall != nil: - // if the OS supports routing natively, then we don't need to filter/route ourselves - // netstack mode won't support native routing as there is no interface - - m.routingEnabled.Store(true) - m.nativeRouter.Store(true) - - log.Info("native routing is enabled") - default: m.routingEnabled.Store(true) m.nativeRouter.Store(false) - log.Info("userspace routing enabled by default") + log.Info("userspace routing enabled") } if m.routingEnabled.Load() && !m.nativeRouter.Load() { @@ -473,21 +564,14 @@ func (m *Manager) IsStateful() bool { return m.stateful } -func (m *Manager) AddNatRule(pair firewall.RouterPair) error { - if m.nativeRouter.Load() && m.nativeFirewall != nil { - return m.nativeFirewall.AddNatRule(pair) - } - +func (m *Manager) AddNatRule(firewall.RouterPair) error { // userspace routed packets are always SNATed to the inbound direction // TODO: implement outbound SNAT return nil } // RemoveNatRule removes a routing firewall rule -func (m *Manager) RemoveNatRule(pair firewall.RouterPair) error { - if m.nativeRouter.Load() && m.nativeFirewall != nil { - return m.nativeFirewall.RemoveNatRule(pair) - } +func (m *Manager) RemoveNatRule(firewall.RouterPair) error { return nil } @@ -651,10 +735,7 @@ func (m *Manager) DeleteFilterRule(rule firewall.Rule) error { return m.deletePeerRuleLocked(r) } - // Either our *RouteRule or, under native delegation, a native - // route-rule object that implements firewall.Rule but isn't one of - // our concrete types. The route path forwards the latter to the - // native firewall. + // Anything else is a route rule (matched on the forward path). return m.deleteRouteRule(rule) } @@ -666,10 +747,6 @@ func (m *Manager) addRouteRule( sPort, dPort *firewall.Port, action firewall.Action, ) (firewall.Rule, error) { - if m.nativeRouter.Load() && m.nativeFirewall != nil { - return m.nativeFirewall.AddFilterRule(id, sources, destination, proto, sPort, dPort, action) - } - ruleID := nbid.GenerateRuleID(sources, destination, proto, sPort, dPort, action) if existingRule, ok := m.routeRulesMap[ruleID]; ok { @@ -698,10 +775,6 @@ func (m *Manager) addRouteRule( } func (m *Manager) deleteRouteRule(rule firewall.Rule) error { - if m.nativeRouter.Load() && m.nativeFirewall != nil { - return m.nativeFirewall.DeleteFilterRule(rule) - } - ruleID := rule.ID() trimmed, _, ok := removeRuleByID(m.routeRules, ruleID) if !ok { @@ -743,12 +816,11 @@ func removeRuleByID[S ~[]T, T firewall.Rule](rules S, ruleID firewall.RuleID) (S return slices.Delete(rules, idx, idx+1), removed, true } -// SetLegacyManagement doesn't need to be implemented for this manager -func (m *Manager) SetLegacyManagement(isLegacy bool) error { - if m.nativeFirewall == nil { - return nil - } - return m.nativeFirewall.SetLegacyManagement(isLegacy) +// SetLegacyManagement is a no-op for the userspace firewall: it only matters +// when an old management server can't send route firewall rules, which the +// userspace router doesn't rely on. +func (m *Manager) SetLegacyManagement(bool) error { + return nil } // Flush doesn't need to be implemented for this manager @@ -793,21 +865,15 @@ func (m *Manager) resetState() { } } -// SetupEBPFProxyNoTrack creates notrack rules for eBPF proxy loopback traffic. -func (m *Manager) SetupEBPFProxyNoTrack(proxyPort, wgPort uint16) error { - if m.nativeFirewall == nil { - return nil - } - return m.nativeFirewall.SetupEBPFProxyNoTrack(proxyPort, wgPort) +// SetupEBPFProxyNoTrack is not supported by the userspace firewall: eBPF isn't +// used in userspace mode, so this should never be called. +func (m *Manager) SetupEBPFProxyNoTrack(uint16, uint16) error { + return errNotSupported } // UpdateSet updates the rule destinations associated with the given set // by merging the existing prefixes with the new ones, then deduplicating. func (m *Manager) UpdateSet(set firewall.Set, prefixes []netip.Prefix) error { - if m.nativeRouter.Load() && m.nativeFirewall != nil { - return m.nativeFirewall.UpdateSet(set, prefixes) - } - m.mutex.Lock() defer m.mutex.Unlock() @@ -1591,10 +1657,13 @@ func (m *Manager) EnableRouting() error { } rules, err := m.blockInvalidRouted(m.wgIface) - // Persist whatever was installed even on partial failure, so DisableRouting - // can clean it up later. m.blockRules = rules if err != nil { + // Roll back so forwarding can't stay active without the full set of + // block rules. + if derr := m.disableRouting(); derr != nil { + log.Warnf("roll back routing after block rule failure: %v", derr) + } return fmt.Errorf("block invalid routed: %w", err) } @@ -1605,6 +1674,10 @@ func (m *Manager) DisableRouting() error { m.mutex.Lock() defer m.mutex.Unlock() + return m.disableRouting() +} + +func (m *Manager) disableRouting() error { fwder := m.forwarder.Load() if fwder == nil { return nil diff --git a/client/firewall/uspfilter/filter_bench_test.go b/client/firewall/uspfilter/filter_bench_test.go index 8e99263a9..72f3417f2 100644 --- a/client/firewall/uspfilter/filter_bench_test.go +++ b/client/firewall/uspfilter/filter_bench_test.go @@ -164,9 +164,12 @@ func BenchmarkCoreFiltering(b *testing.B) { } // Create manager and basic setup - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) defer b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) @@ -204,9 +207,12 @@ func BenchmarkStateScaling(b *testing.B) { for _, count := range connCounts { b.Run(fmt.Sprintf("conns_%d", count), func(b *testing.B) { - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) @@ -247,9 +253,12 @@ func BenchmarkEstablishmentOverhead(b *testing.B) { for _, sc := range scenarios { b.Run(sc.name, func(b *testing.B) { - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) @@ -405,9 +414,12 @@ func BenchmarkRoutedNetworkReturn(b *testing.B) { for _, sc := range scenarios { b.Run(sc.name, func(b *testing.B) { - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) @@ -532,9 +544,12 @@ func BenchmarkLongLivedConnections(b *testing.B) { require.NoError(b, os.Unsetenv("NB_DISABLE_CONNTRACK")) } - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) defer b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) @@ -615,9 +630,12 @@ func BenchmarkShortLivedConnections(b *testing.B) { require.NoError(b, os.Unsetenv("NB_DISABLE_CONNTRACK")) } - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) defer b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) @@ -726,9 +744,12 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) { require.NoError(b, os.Unsetenv("NB_DISABLE_CONNTRACK")) } - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) defer b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) @@ -806,9 +827,12 @@ func BenchmarkParallelShortLivedConnections(b *testing.B) { require.NoError(b, os.Unsetenv("NB_DISABLE_CONNTRACK")) } - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) defer b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) @@ -1010,9 +1034,11 @@ func BenchmarkMSSClamping(b *testing.B) { for _, sc := range scenarios { b.Run(sc.name, func(b *testing.B) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(b, err) defer func() { require.NoError(b, manager.Close(nil)) @@ -1075,9 +1101,11 @@ func BenchmarkMSSClampingOverhead(b *testing.B) { for _, sc := range scenarios { b.Run(sc.name, func(b *testing.B) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(b, err) defer func() { require.NoError(b, manager.Close(nil)) @@ -1130,9 +1158,11 @@ func BenchmarkMSSClampingMemory(b *testing.B) { for _, sc := range scenarios { b.Run(sc.name, func(b *testing.B) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(b, err) defer func() { require.NoError(b, manager.Close(nil)) diff --git a/client/firewall/uspfilter/filter_filter_test.go b/client/firewall/uspfilter/filter_filter_test.go index d673ee90d..55ae04d96 100644 --- a/client/firewall/uspfilter/filter_filter_test.go +++ b/client/firewall/uspfilter/filter_filter_test.go @@ -32,7 +32,7 @@ func TestPeerACLFiltering(t *testing.T) { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) require.NotNil(t, manager) @@ -549,7 +549,7 @@ func TestPeerACLFilteringIPv6(t *testing.T) { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) }) @@ -788,7 +788,7 @@ func setupRoutedManager(tb testing.TB, network string) *Manager { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(tb, err) require.NoError(tb, manager.EnableRouting()) require.NotNil(tb, manager) @@ -1634,7 +1634,7 @@ func TestRouteACLSet(t *testing.T) { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) diff --git a/client/firewall/uspfilter/filter_routeacl_test.go b/client/firewall/uspfilter/filter_routeacl_test.go index 75461be9f..7f89b7704 100644 --- a/client/firewall/uspfilter/filter_routeacl_test.go +++ b/client/firewall/uspfilter/filter_routeacl_test.go @@ -187,7 +187,7 @@ func TestBlockInvalidRoutedDualStack(t *testing.T) { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) @@ -235,7 +235,7 @@ func TestBlockInvalidRoutedIdempotent(t *testing.T) { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) @@ -298,7 +298,7 @@ func TestBlockRuleNotAccumulatedOnRepeatedEnableRouting(t *testing.T) { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) @@ -417,7 +417,7 @@ func setupTestManager(t *testing.T) *Manager { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) require.NoError(t, manager.EnableRouting()) diff --git a/client/firewall/uspfilter/filter_test.go b/client/firewall/uspfilter/filter_test.go index e0b66df4d..8d6610272 100644 --- a/client/firewall/uspfilter/filter_test.go +++ b/client/firewall/uspfilter/filter_test.go @@ -78,11 +78,12 @@ func TestManagerCreate(t *testing.T) { SetFilterFunc: func(device.PacketFilter) error { return nil }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) if err != nil { t.Errorf("failed to create Manager: %v", err) return } + t.Cleanup(func() { require.NoError(t, m.Close(nil)) }) if m == nil { t.Error("Manager is nil") @@ -98,11 +99,12 @@ func TestManagerAddFilterRule(t *testing.T) { }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) if err != nil { t.Errorf("failed to create Manager: %v", err) return } + t.Cleanup(func() { require.NoError(t, m.Close(nil)) }) ip := net.ParseIP("192.168.1.1") proto := fw.ProtocolTCP @@ -131,11 +133,12 @@ func TestManagerDeleteRule(t *testing.T) { SetFilterFunc: func(device.PacketFilter) error { return nil }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) if err != nil { t.Errorf("failed to create Manager: %v", err) return } + t.Cleanup(func() { require.NoError(t, m.Close(nil)) }) ip := netip.MustParseAddr("192.168.1.1") proto := fw.ProtocolTCP @@ -166,9 +169,11 @@ func TestManagerDeleteRule(t *testing.T) { } func TestSetUDPPacketHook(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) }) @@ -190,9 +195,11 @@ func TestSetUDPPacketHook(t *testing.T) { } func TestSetTCPPacketHook(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) }) @@ -220,7 +227,7 @@ func TestPeerRuleLifecycleDenyRules(t *testing.T) { SetFilterFunc: func(device.PacketFilter) error { return nil }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, m.Close(nil)) @@ -267,7 +274,7 @@ func TestPeerRuleAddAndDeleteDontLeak(t *testing.T) { SetFilterFunc: func(device.PacketFilter) error { return nil }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, m.Close(nil)) @@ -307,7 +314,7 @@ func TestMixedAllowDenyRulesSameIP(t *testing.T) { SetFilterFunc: func(device.PacketFilter) error { return nil }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, m.Close(nil)) @@ -360,7 +367,7 @@ func TestManagerReset(t *testing.T) { SetFilterFunc: func(device.PacketFilter) error { return nil }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) if err != nil { t.Errorf("failed to create Manager: %v", err) return @@ -399,7 +406,7 @@ func TestNotMatchByIP(t *testing.T) { }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) if err != nil { t.Errorf("failed to create Manager: %v", err) return @@ -462,7 +469,7 @@ func TestRemovePacketHook(t *testing.T) { } // creating manager instance - manager, err := Create(iface, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{IFace: iface, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) if err != nil { t.Fatalf("Failed to create Manager: %s", err) } @@ -479,9 +486,11 @@ func TestRemovePacketHook(t *testing.T) { } func TestProcessOutgoingHooks(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) manager.udpTracker.Close() @@ -566,7 +575,7 @@ func TestUSPFilterCreatePerformance(t *testing.T) { ifaceMock := &IFaceMock{ SetFilterFunc: func(device.PacketFilter) error { return nil }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) time.Sleep(time.Second) @@ -591,9 +600,11 @@ func TestUSPFilterCreatePerformance(t *testing.T) { } func TestStatefulFirewall_UDPTracking(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) manager.udpTracker.Close() // Close the existing tracker @@ -805,7 +816,7 @@ func TestUpdateSetMerge(t *testing.T) { SetFilterFunc: func(device.PacketFilter) error { return nil }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) @@ -891,7 +902,7 @@ func TestUpdateSetDeduplication(t *testing.T) { SetFilterFunc: func(device.PacketFilter) error { return nil }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, manager.Close(nil)) @@ -1011,7 +1022,7 @@ func TestMSSClamping(t *testing.T) { }, } - manager, err := Create(ifaceMock, nil, false, flowLogger, 1280) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: 1280}) require.NoError(t, err) defer func() { require.NoError(t, manager.Close(nil)) @@ -1203,7 +1214,7 @@ func TestShouldForward(t *testing.T) { return wgaddr.Address{IP: wgIP, Network: netip.PrefixFrom(wgIP, 24)} } - manager, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, manager.Close(nil)) @@ -1318,7 +1329,7 @@ func TestShouldForward(t *testing.T) { // Re-create manager to pick up the new address with IPv6 require.NoError(t, manager.Close(nil)) - manager, err = Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + manager, err = Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err) v6Cases := []struct { diff --git a/client/firewall/uspfilter/forwarder/forwarder.go b/client/firewall/uspfilter/forwarder/forwarder.go index 6291eb285..15499872c 100644 --- a/client/firewall/uspfilter/forwarder/forwarder.go +++ b/client/firewall/uspfilter/forwarder/forwarder.go @@ -10,6 +10,7 @@ import ( "time" log "github.com/sirupsen/logrus" + wgdevice "golang.zx2c4.com/wireguard/device" "gvisor.dev/gvisor/pkg/buffer" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/header" @@ -20,9 +21,9 @@ import ( "gvisor.dev/gvisor/pkg/tcpip/transport/tcp" "gvisor.dev/gvisor/pkg/tcpip/transport/udp" - "github.com/netbirdio/netbird/client/firewall/uspfilter/common" "github.com/netbirdio/netbird/client/firewall/uspfilter/conntrack" nblog "github.com/netbirdio/netbird/client/firewall/uspfilter/log" + "github.com/netbirdio/netbird/client/iface/wgaddr" nftypes "github.com/netbirdio/netbird/client/internal/netflow/types" ) @@ -33,6 +34,12 @@ const ( iosMaxInFlight = 256 ) +// IFace provides the WireGuard device and overlay addresses the forwarder needs. +type IFace interface { + GetWGDevice() *wgdevice.Device + Address() wgaddr.Address +} + type Forwarder struct { logger *nblog.Logger flowLogger nftypes.FlowLogger @@ -51,7 +58,7 @@ type Forwarder struct { pingSemaphore chan struct{} } -func New(iface common.IFaceMapper, logger *nblog.Logger, flowLogger nftypes.FlowLogger, netstack bool, mtu uint16) (*Forwarder, error) { +func New(iface IFace, logger *nblog.Logger, flowLogger nftypes.FlowLogger, netstack bool, mtu uint16) (*Forwarder, error) { s := stack.New(stack.Options{ NetworkProtocols: []stack.NetworkProtocolFactory{ ipv4.NewProtocol, diff --git a/client/firewall/uspfilter/allow_netbird_windows.go b/client/firewall/uspfilter/interface_allower_windows.go similarity index 79% rename from client/firewall/uspfilter/allow_netbird_windows.go rename to client/firewall/uspfilter/interface_allower_windows.go index 10a2b9116..7f525e28c 100644 --- a/client/firewall/uspfilter/allow_netbird_windows.go +++ b/client/firewall/uspfilter/interface_allower_windows.go @@ -9,7 +9,6 @@ import ( log "github.com/sirupsen/logrus" nberrors "github.com/netbirdio/netbird/client/errors" - "github.com/netbirdio/netbird/client/internal/statemanager" ) type action string @@ -20,35 +19,20 @@ const ( firewallRuleName = "Netbird" ) -// Close cleans up the firewall manager by removing all rules and closing trackers -func (m *Manager) Close(*statemanager.Manager) error { - m.mutex.Lock() - defer m.mutex.Unlock() - - m.resetState() - - if !isWindowsFirewallReachable() { - return nil - } - - var merr *multierror.Error - if isFirewallRuleActive(firewallRuleName) { - if err := manageFirewallRule(firewallRuleName, deleteRule); err != nil { - merr = multierror.Append(merr, fmt.Errorf("remove windows firewall rule: %w", err)) - } - } - - if isFirewallRuleActive(firewallRuleName + "-v6") { - if err := manageFirewallRule(firewallRuleName+"-v6", deleteRule); err != nil { - merr = multierror.Append(merr, fmt.Errorf("remove windows v6 firewall rule: %w", err)) - } - } - - return nberrors.FormatErrorOrNil(merr) +// WindowsInterfaceAllower opens the NetBird interface in the Windows firewall +// via netsh advfirewall rules. It implements InterfaceAllower for the userspace +// firewall on Windows. +type WindowsInterfaceAllower struct { + iface Iface } -// AllowNetbird allows netbird interface traffic -func (m *Manager) AllowNetbird() error { +// NewWindowsInterfaceAllower builds the Windows netsh-based interface allower. +func NewWindowsInterfaceAllower(iface Iface) *WindowsInterfaceAllower { + return &WindowsInterfaceAllower{iface: iface} +} + +// Apply adds inbound-allow netsh rules for the interface's addresses. +func (a *WindowsInterfaceAllower) Apply() error { if !isWindowsFirewallReachable() { return nil } @@ -60,13 +44,13 @@ func (m *Manager) AllowNetbird() error { "enable=yes", "action=allow", "profile=any", - "localip="+m.wgIface.Address().IP.String(), + "localip="+a.iface.Address().IP.String(), ); err != nil { return err } } - if v6 := m.wgIface.Address().IPv6; v6.IsValid() && !isFirewallRuleActive(firewallRuleName+"-v6") { + if v6 := a.iface.Address().IPv6; v6.IsValid() && !isFirewallRuleActive(firewallRuleName+"-v6") { if err := manageFirewallRule(firewallRuleName+"-v6", addRule, "dir=in", @@ -82,8 +66,27 @@ func (m *Manager) AllowNetbird() error { return nil } -func manageFirewallRule(ruleName string, action action, extraArgs ...string) error { +// Close removes the netsh rules added by Apply. +func (a *WindowsInterfaceAllower) Close() error { + if !isWindowsFirewallReachable() { + return nil + } + var merr *multierror.Error + if isFirewallRuleActive(firewallRuleName) { + if err := manageFirewallRule(firewallRuleName, deleteRule); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove windows firewall rule: %w", err)) + } + } + if isFirewallRuleActive(firewallRuleName + "-v6") { + if err := manageFirewallRule(firewallRuleName+"-v6", deleteRule); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove windows v6 firewall rule: %w", err)) + } + } + return nberrors.FormatErrorOrNil(merr) +} + +func manageFirewallRule(ruleName string, action action, extraArgs ...string) error { args := []string{"advfirewall", "firewall", string(action), "rule", "name=" + ruleName} if action == addRule { args = append(args, extraArgs...) diff --git a/client/firewall/uspfilter/localip.go b/client/firewall/uspfilter/localip.go index b35be56c6..869832732 100644 --- a/client/firewall/uspfilter/localip.go +++ b/client/firewall/uspfilter/localip.go @@ -7,8 +7,6 @@ import ( "sync/atomic" log "github.com/sirupsen/logrus" - - "github.com/netbirdio/netbird/client/firewall/uspfilter/common" ) // localIPSnapshot is an immutable snapshot of local IP addresses, swapped @@ -60,7 +58,7 @@ func processInterface(iface net.Interface, ips map[netip.Addr]struct{}, addresse } // UpdateLocalIPs rebuilds the local IP snapshot and swaps it in atomically. -func (m *localIPManager) UpdateLocalIPs(iface common.IFaceMapper) (err error) { +func (m *localIPManager) UpdateLocalIPs(iface Iface) (err error) { defer func() { if r := recover(); r != nil { err = fmt.Errorf("panic: %v", r) diff --git a/client/firewall/uspfilter/nat.go b/client/firewall/uspfilter/nat.go index 5d51c1538..06312aabf 100644 --- a/client/firewall/uspfilter/nat.go +++ b/client/firewall/uspfilter/nat.go @@ -487,19 +487,13 @@ func incrementalUpdate(oldChecksum uint16, oldBytes, newBytes []byte) uint16 { } // AddDNATRule adds outbound DNAT rule for forwarding external traffic to NetBird network. -func (m *Manager) AddDNATRule(rule firewall.ForwardRule) (firewall.Rule, error) { - if m.nativeFirewall == nil { - return nil, errNatNotSupported - } - return m.nativeFirewall.AddDNATRule(rule) +func (m *Manager) AddDNATRule(firewall.ForwardRule) (firewall.Rule, error) { + return nil, errNotSupported } // DeleteDNATRule deletes outbound DNAT rule. -func (m *Manager) DeleteDNATRule(rule firewall.Rule) error { - if m.nativeFirewall == nil { - return errNatNotSupported - } - return m.nativeFirewall.DeleteDNATRule(rule) +func (m *Manager) DeleteDNATRule(firewall.Rule) error { + return errNotSupported } // addPortRedirection adds a port redirection rule. @@ -521,7 +515,6 @@ func (m *Manager) addPortRedirection(targetIP netip.Addr, protocol gopacket.Laye } // AddInboundDNAT adds an inbound DNAT rule redirecting traffic from NetBird peers to local services. -// TODO: also delegate to nativeFirewall when available for kernel WG mode func (m *Manager) AddInboundDNAT(localAddr netip.Addr, protocol firewall.Protocol, originalPort, translatedPort uint16) error { var layerType gopacket.LayerType switch protocol { @@ -567,20 +560,16 @@ func (m *Manager) RemoveInboundDNAT(localAddr netip.Addr, protocol firewall.Prot return m.removePortRedirection(localAddr, layerType, originalPort, translatedPort) } -// AddOutputDNAT delegates to the native firewall if available. -func (m *Manager) AddOutputDNAT(localAddr netip.Addr, protocol firewall.Protocol, originalPort, translatedPort uint16) error { - if m.nativeFirewall == nil { - return fmt.Errorf("output DNAT not supported without native firewall") - } - return m.nativeFirewall.AddOutputDNAT(localAddr, protocol, originalPort, translatedPort) +// AddOutputDNAT is not supported by the userspace firewall: it backs kernel DNS +// redirection, but userspace DNS is served in-process on the gVisor netstack, so +// this should never be called. +func (m *Manager) AddOutputDNAT(netip.Addr, firewall.Protocol, uint16, uint16) error { + return errNotSupported } -// RemoveOutputDNAT delegates to the native firewall if available. -func (m *Manager) RemoveOutputDNAT(localAddr netip.Addr, protocol firewall.Protocol, originalPort, translatedPort uint16) error { - if m.nativeFirewall == nil { - return nil - } - return m.nativeFirewall.RemoveOutputDNAT(localAddr, protocol, originalPort, translatedPort) +// RemoveOutputDNAT is a no-op for the userspace firewall (see AddOutputDNAT). +func (m *Manager) RemoveOutputDNAT(netip.Addr, firewall.Protocol, uint16, uint16) error { + return nil } // translateInboundPortDNAT applies port-specific DNAT translation to inbound packets. diff --git a/client/firewall/uspfilter/nat_bench_test.go b/client/firewall/uspfilter/nat_bench_test.go index f309f83c0..422c6b849 100644 --- a/client/firewall/uspfilter/nat_bench_test.go +++ b/client/firewall/uspfilter/nat_bench_test.go @@ -64,9 +64,11 @@ func BenchmarkDNATTranslation(b *testing.B) { for _, sc := range scenarios { b.Run(sc.name, func(b *testing.B) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(b, err) defer func() { require.NoError(b, manager.Close(nil)) @@ -124,9 +126,11 @@ func BenchmarkDNATTranslation(b *testing.B) { // BenchmarkDNATConcurrency tests DNAT performance under concurrent load func BenchmarkDNATConcurrency(b *testing.B) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(b, err) defer func() { require.NoError(b, manager.Close(nil)) @@ -196,9 +200,11 @@ func BenchmarkDNATScaling(b *testing.B) { for _, count := range mappingCounts { b.Run(fmt.Sprintf("mappings_%d", count), func(b *testing.B) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(b, err) defer func() { require.NoError(b, manager.Close(nil)) @@ -308,9 +314,11 @@ func BenchmarkChecksumUpdate(b *testing.B) { // BenchmarkDNATMemoryAllocations checks for memory allocations in DNAT operations func BenchmarkDNATMemoryAllocations(b *testing.B) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(b, err) defer func() { require.NoError(b, manager.Close(nil)) @@ -481,9 +489,11 @@ func BenchmarkPortDNAT(b *testing.B) { for _, sc := range scenarios { b.Run(sc.name, func(b *testing.B) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(b, err) defer func() { require.NoError(b, manager.Close(nil)) diff --git a/client/firewall/uspfilter/nat_stateful_test.go b/client/firewall/uspfilter/nat_stateful_test.go index 62eab6b30..5fa5da027 100644 --- a/client/firewall/uspfilter/nat_stateful_test.go +++ b/client/firewall/uspfilter/nat_stateful_test.go @@ -13,9 +13,11 @@ import ( // TestPortDNATBasic tests basic port DNAT functionality func TestPortDNATBasic(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, manager.Close(nil)) @@ -49,9 +51,11 @@ func TestPortDNATBasic(t *testing.T) { // TestPortDNATMultipleRules tests multiple port DNAT rules func TestPortDNATMultipleRules(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, manager.Close(nil)) diff --git a/client/firewall/uspfilter/nat_test.go b/client/firewall/uspfilter/nat_test.go index ee70789d1..5b5840383 100644 --- a/client/firewall/uspfilter/nat_test.go +++ b/client/firewall/uspfilter/nat_test.go @@ -15,9 +15,11 @@ import ( // TestDNATTranslationCorrectness verifies DNAT translation works correctly func TestDNATTranslationCorrectness(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, manager.Close(nil)) @@ -104,9 +106,11 @@ func parsePacket(t testing.TB, packetData []byte) *decoder { // TestDNATMappingManagement tests adding/removing DNAT mappings func TestDNATMappingManagement(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, manager.Close(nil)) @@ -152,9 +156,11 @@ func TestDNATMappingManagement(t *testing.T) { } func TestInboundPortDNAT(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, manager.Close(nil)) @@ -202,9 +208,11 @@ func TestInboundPortDNAT(t *testing.T) { } func TestInboundPortDNATNegative(t *testing.T) { - manager, err := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) defer func() { require.NoError(t, manager.Close(nil)) diff --git a/client/firewall/uspfilter/peer_acl_bench_test.go b/client/firewall/uspfilter/peer_acl_bench_test.go index 4abed3dd6..bcb0ca5c2 100644 --- a/client/firewall/uspfilter/peer_acl_bench_test.go +++ b/client/firewall/uspfilter/peer_acl_bench_test.go @@ -88,10 +88,13 @@ func runPeerACLBench(b *testing.B, m, k int, hit, v6 bool) { IPv6Net: netip.PrefixFrom(dstIP, bits), } } - manager, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - AddressFunc: func() wgaddr.Address { return mockAddr }, - }, nil, false, flowLogger, iface.DefaultMTU) + manager, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + AddressFunc: func() wgaddr.Address { return mockAddr }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) b.Cleanup(func() { require.NoError(b, manager.Close(nil)) }) // Generate M policies × K source peers, all distinct. @@ -169,9 +172,12 @@ func BenchmarkPeerACLIndexMemory(b *testing.B) { b.Run(c.name, func(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { - mgr, _ := Create(&IFaceMock{ - SetFilterFunc: func(device.PacketFilter) error { return nil }, - }, nil, false, flowLogger, iface.DefaultMTU) + mgr, err := Create(Config{ + IFace: &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + }, + FlowLogger: flowLogger, MTU: iface.DefaultMTU}) + require.NoError(b, err) populateIndexedRules(b, mgr, c.M, c.K, c.overlapFrac) diff --git a/client/firewall/uspfilter/peer_acl_dedup_test.go b/client/firewall/uspfilter/peer_acl_dedup_test.go index 2078932c2..5c5038208 100644 --- a/client/firewall/uspfilter/peer_acl_dedup_test.go +++ b/client/firewall/uspfilter/peer_acl_dedup_test.go @@ -17,8 +17,9 @@ func newTestManager(t *testing.T) *Manager { ifaceMock := &IFaceMock{ SetFilterFunc: func(device.PacketFilter) error { return nil }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err, "create manager") + t.Cleanup(func() { require.NoError(t, m.Close(nil)) }) return m } diff --git a/client/firewall/uspfilter/peer_acl_ipv6_test.go b/client/firewall/uspfilter/peer_acl_ipv6_test.go index 6b7761293..282b44ebb 100644 --- a/client/firewall/uspfilter/peer_acl_ipv6_test.go +++ b/client/firewall/uspfilter/peer_acl_ipv6_test.go @@ -29,7 +29,7 @@ func newV6TestManager(t *testing.T, localV6 string) *Manager { } }, } - m, err := Create(ifaceMock, nil, false, flowLogger, nbiface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: nbiface.DefaultMTU}) require.NoError(t, err, "create manager") t.Cleanup(func() { require.NoError(t, m.Close(nil)) }) return m diff --git a/client/firewall/uspfilter/tracer_test.go b/client/firewall/uspfilter/tracer_test.go index 73fa36e90..27b5e3f9e 100644 --- a/client/firewall/uspfilter/tracer_test.go +++ b/client/firewall/uspfilter/tracer_test.go @@ -45,7 +45,7 @@ func TestTracePacket(t *testing.T) { }, } - m, err := Create(ifaceMock, nil, false, flowLogger, iface.DefaultMTU) + m, err := Create(Config{IFace: ifaceMock, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) require.NoError(t, err) if !statefulMode { diff --git a/client/internal/dns/server_test.go b/client/internal/dns/server_test.go index 403628aa4..36c7c8b8d 100644 --- a/client/internal/dns/server_test.go +++ b/client/internal/dns/server_test.go @@ -900,7 +900,7 @@ func createWgInterfaceWithBind(t *testing.T) (*iface.WGIface, error) { return nil, err } - pf, err := uspfilter.Create(wgIface, nil, false, flowLogger, iface.DefaultMTU) + pf, err := uspfilter.Create(uspfilter.Config{IFace: wgIface, FlowLogger: flowLogger, MTU: iface.DefaultMTU}) if err != nil { t.Fatalf("failed to create uspfilter: %v", err) return nil, err