From 5b79cbc8248923c652737e108731e1e9a752c945 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 25 Aug 2026 18:03:10 +0200 Subject: [PATCH] Drop the connection tracking exemptions for the WireGuard proxy --- client/firewall/iptables/manager_linux.go | 118 +------------ client/firewall/manager/firewall.go | 4 - client/firewall/nftables/manager_linux.go | 195 ++-------------------- client/firewall/uspfilter/filter.go | 8 - client/iface/iface.go | 7 - client/iface/iface_close_test.go | 1 - client/iface/wgproxy/bench_linux_test.go | 187 --------------------- client/iface/wgproxy/factory_kernel.go | 8 - client/iface/wgproxy/factory_usp.go | 5 - client/iface/wgproxy/loopback/proxy.go | 4 - client/internal/engine.go | 21 --- client/internal/engine_test.go | 7 - client/internal/iface_common.go | 1 - 13 files changed, 27 insertions(+), 539 deletions(-) delete mode 100644 client/iface/wgproxy/bench_linux_test.go diff --git a/client/firewall/iptables/manager_linux.go b/client/firewall/iptables/manager_linux.go index 090aa9f42..976f0c958 100644 --- a/client/firewall/iptables/manager_linux.go +++ b/client/firewall/iptables/manager_linux.go @@ -28,10 +28,9 @@ type Manager struct { wgIface iFaceMapper - ipv4Client *iptables.IPTables - aclMgr *aclManager - router *router - rawSupported bool + ipv4Client *iptables.IPTables + aclMgr *aclManager + router *router // IPv6 counterparts, nil when no v6 overlay ipv6Client *iptables.IPTables @@ -121,8 +120,8 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error { return err } - if err := m.initNoTrackChain(); err != nil { - log.Warnf("raw table not available, notrack rules will be disabled: %v", err) + if err := m.cleanupNoTrackChain(); err != nil { + log.Debugf("cleanup notrack chain: %v", err) } // Trust after all fatal init steps so a later failure doesn't leave the @@ -524,110 +523,12 @@ const ( tableRaw = "raw" ) -// SetupWGProxyNoTrack creates notrack rules for WireGuard proxy loopback traffic. -// This prevents conntrack from tracking WireGuard proxy traffic on loopback, which -// can interfere with MASQUERADE rules (e.g., from container runtimes like Podman/netavark). -// -// Every relayed peer has its own loopback endpoint address, so the rules match the -// whole 127.0.0.0/8 range. -// -// Traffic flows that need NOTRACK: -// -// 1. Egress: WireGuard -> peer endpoint -// src=127.0.0.1:wgPort -> dst=127.x.x.x:proxyPort -// Matched by: sport=wgPort -// -// 2. Egress: Proxy -> WireGuard (via raw socket) -// src=127.x.x.x:proxyPort -> dst=127.0.0.1:wgPort -// Matched by: dport=wgPort -// -// 3. Ingress: Packets to WireGuard -// dst=127.0.0.1:wgPort -// Matched by: dport=wgPort -// -// 4. Ingress: Packets to the proxy -// dst=127.x.x.x:proxyPort -// Matched by: dport=proxyPort -// -// Rules are cleaned up when the firewall manager is closed. -func (m *Manager) SetupWGProxyNoTrack(proxyPort, wgPort uint16) error { - m.mutex.Lock() - defer m.mutex.Unlock() - - if !m.rawSupported { - return fmt.Errorf("raw table not available") - } - - wgPortStr := fmt.Sprintf("%d", wgPort) - proxyPortStr := fmt.Sprintf("%d", proxyPort) - - // Egress rules: match outgoing loopback UDP packets - outputRuleSport := []string{"-o", "lo", "-s", "127.0.0.0/8", "-d", "127.0.0.0/8", "-p", "udp", "--sport", wgPortStr, "-j", "NOTRACK"} - if err := m.ipv4Client.AppendUnique(tableRaw, chainNameRaw, outputRuleSport...); err != nil { - return fmt.Errorf("add output sport notrack rule: %w", err) - } - - outputRuleDport := []string{"-o", "lo", "-s", "127.0.0.0/8", "-d", "127.0.0.0/8", "-p", "udp", "--dport", wgPortStr, "-j", "NOTRACK"} - if err := m.ipv4Client.AppendUnique(tableRaw, chainNameRaw, outputRuleDport...); err != nil { - return fmt.Errorf("add output dport notrack rule: %w", err) - } - - // Ingress rules: match incoming loopback UDP packets - preroutingRuleWg := []string{"-i", "lo", "-s", "127.0.0.0/8", "-d", "127.0.0.0/8", "-p", "udp", "--dport", wgPortStr, "-j", "NOTRACK"} - if err := m.ipv4Client.AppendUnique(tableRaw, chainNameRaw, preroutingRuleWg...); err != nil { - return fmt.Errorf("add prerouting wg notrack rule: %w", err) - } - - preroutingRuleProxy := []string{"-i", "lo", "-s", "127.0.0.0/8", "-d", "127.0.0.0/8", "-p", "udp", "--dport", proxyPortStr, "-j", "NOTRACK"} - if err := m.ipv4Client.AppendUnique(tableRaw, chainNameRaw, preroutingRuleProxy...); err != nil { - return fmt.Errorf("add prerouting proxy notrack rule: %w", err) - } - - log.Debugf("set up wg proxy notrack rules for ports %d,%d", proxyPort, wgPort) - return nil -} - -func (m *Manager) initNoTrackChain() error { - if err := m.cleanupNoTrackChain(); err != nil { - log.Debugf("cleanup notrack chain: %v", err) - } - - if err := m.ipv4Client.NewChain(tableRaw, chainNameRaw); err != nil { - return fmt.Errorf("create chain: %w", err) - } - - jumpRule := []string{"-j", chainNameRaw} - - if err := m.ipv4Client.InsertUnique(tableRaw, chainOUTPUT, 1, jumpRule...); err != nil { - if delErr := m.ipv4Client.DeleteChain(tableRaw, chainNameRaw); delErr != nil { - log.Debugf("delete orphan chain: %v", delErr) - } - return fmt.Errorf("add output jump rule: %w", err) - } - - if err := m.ipv4Client.InsertUnique(tableRaw, chainPREROUTING, 1, jumpRule...); err != nil { - if delErr := m.ipv4Client.DeleteIfExists(tableRaw, chainOUTPUT, jumpRule...); delErr != nil { - log.Debugf("delete output jump rule: %v", delErr) - } - if delErr := m.ipv4Client.DeleteChain(tableRaw, chainNameRaw); delErr != nil { - log.Debugf("delete orphan chain: %v", delErr) - } - return fmt.Errorf("add prerouting jump rule: %w", err) - } - - m.rawSupported = true - return nil -} - +// cleanupNoTrackChain removes the chain that earlier versions used to exempt the +// WireGuard proxy's loopback traffic from connection tracking. The raw table is +// not always available, so a lookup failure is not an error here. func (m *Manager) cleanupNoTrackChain() error { exists, err := m.ipv4Client.ChainExists(tableRaw, chainNameRaw) - if err != nil { - if !m.rawSupported { - return nil - } - return fmt.Errorf("check chain exists: %w", err) - } - if !exists { + if err != nil || !exists { return nil } @@ -645,7 +546,6 @@ func (m *Manager) cleanupNoTrackChain() error { return fmt.Errorf("clear and delete chain: %w", err) } - m.rawSupported = false return nil } diff --git a/client/firewall/manager/firewall.go b/client/firewall/manager/firewall.go index 221abde66..ef5caca51 100644 --- a/client/firewall/manager/firewall.go +++ b/client/firewall/manager/firewall.go @@ -179,10 +179,6 @@ type Manager interface { // RemoveOutputDNAT removes an OUTPUT chain DNAT rule. RemoveOutputDNAT(localAddr netip.Addr, protocol Protocol, originalPort, translatedPort uint16) error - - // SetupWGProxyNoTrack creates static notrack rules for WireGuard proxy loopback traffic. - // This prevents conntrack from interfering with WireGuard proxy communication. - SetupWGProxyNoTrack(proxyPort, wgPort uint16) error } func GenKey(format string, pair RouterPair) string { diff --git a/client/firewall/nftables/manager_linux.go b/client/firewall/nftables/manager_linux.go index ffa1fc80b..99edbfeee 100644 --- a/client/firewall/nftables/manager_linux.go +++ b/client/firewall/nftables/manager_linux.go @@ -13,7 +13,6 @@ import ( "github.com/google/nftables/expr" "github.com/hashicorp/go-multierror" log "github.com/sirupsen/logrus" - "golang.org/x/sys/unix" nberrors "github.com/netbirdio/netbird/client/errors" "github.com/netbirdio/netbird/client/firewall/firewalld" @@ -58,9 +57,6 @@ type Manager struct { router6 *router aclManager6 *AclManager - notrackOutputChain *nftables.Chain - notrackPreroutingChain *nftables.Chain - extMonitor *externalChainMonitor } @@ -202,8 +198,8 @@ func (m *Manager) initFirewall() (err error) { } } - if err := m.initNoTrackChains(workTable); err != nil { - log.Warnf("raw priority chains not available, notrack rules will be disabled: %v", err) + if err := m.cleanupNoTrackChains(); err != nil { + log.Debugf("cleanup notrack chains: %v", err) } return nil @@ -556,10 +552,6 @@ func (m *Manager) Flush() error { } } - if err := m.refreshNoTrackChains(); err != nil { - log.Errorf("failed to refresh notrack chains: %v", err) - } - return nil } @@ -672,189 +664,38 @@ func (m *Manager) RemoveOutputDNAT(localAddr netip.Addr, protocol firewall.Proto return m.router.RemoveOutputDNAT(localAddr, protocol, originalPort, translatedPort) } -// The proxy hands every relayed peer its own address out of 127.0.0.0/8, so the -// notrack rules match the whole loopback range. -var ( - loopbackNet = []byte{127, 0, 0, 0} - loopbackMask = []byte{255, 0, 0, 0} - loopbackXor = []byte{0, 0, 0, 0} -) - const ( chainNameRawOutput = "netbird-raw-out" chainNameRawPrerouting = "netbird-raw-pre" ) -// SetupWGProxyNoTrack creates notrack rules for WireGuard proxy loopback traffic. -// This prevents conntrack from tracking WireGuard proxy traffic on loopback, which -// can interfere with MASQUERADE rules (e.g., from container runtimes like Podman/netavark). -// -// Every relayed peer has its own loopback endpoint address, so the rules match the -// whole 127.0.0.0/8 range. -// -// Traffic flows that need NOTRACK: -// -// 1. Egress: WireGuard -> peer endpoint -// src=127.0.0.1:wgPort -> dst=127.x.x.x:proxyPort -// Matched by: sport=wgPort -// -// 2. Egress: Proxy -> WireGuard (via raw socket) -// src=127.x.x.x:proxyPort -> dst=127.0.0.1:wgPort -// Matched by: dport=wgPort -// -// 3. Ingress: Packets to WireGuard -// dst=127.0.0.1:wgPort -// Matched by: dport=wgPort -// -// 4. Ingress: Packets to the proxy -// dst=127.x.x.x:proxyPort -// Matched by: dport=proxyPort -// -// Rules are cleaned up when the firewall manager is closed. -func (m *Manager) SetupWGProxyNoTrack(proxyPort, wgPort uint16) error { - m.mutex.Lock() - defer m.mutex.Unlock() - - if m.notrackOutputChain == nil || m.notrackPreroutingChain == nil { - return fmt.Errorf("notrack chains not initialized") - } - - proxyPortBytes := binaryutil.BigEndian.PutUint16(proxyPort) - wgPortBytes := binaryutil.BigEndian.PutUint16(wgPort) - - // Egress rules: match outgoing loopback UDP packets - m.rConn.AddRule(&nftables.Rule{ - Table: m.notrackOutputChain.Table, - Chain: m.notrackOutputChain, - Exprs: []expr.Any{ - &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: ifname("lo")}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, // saddr - &expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: loopbackMask, Xor: loopbackXor}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: loopbackNet}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 16, Len: 4}, // daddr - &expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: loopbackMask, Xor: loopbackXor}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: loopbackNet}, - &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_UDP}}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 0, Len: 2}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: wgPortBytes}, // sport=wgPort - &expr.Counter{}, - &expr.Notrack{}, - }, - }) - m.rConn.AddRule(&nftables.Rule{ - Table: m.notrackOutputChain.Table, - Chain: m.notrackOutputChain, - Exprs: []expr.Any{ - &expr.Meta{Key: expr.MetaKeyOIFNAME, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: ifname("lo")}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, // saddr - &expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: loopbackMask, Xor: loopbackXor}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: loopbackNet}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 16, Len: 4}, // daddr - &expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: loopbackMask, Xor: loopbackXor}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: loopbackNet}, - &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_UDP}}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: wgPortBytes}, // dport=wgPort - &expr.Counter{}, - &expr.Notrack{}, - }, - }) - - // Ingress rules: match incoming loopback UDP packets - m.rConn.AddRule(&nftables.Rule{ - Table: m.notrackPreroutingChain.Table, - Chain: m.notrackPreroutingChain, - Exprs: []expr.Any{ - &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: ifname("lo")}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, // saddr - &expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: loopbackMask, Xor: loopbackXor}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: loopbackNet}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 16, Len: 4}, // daddr - &expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: loopbackMask, Xor: loopbackXor}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: loopbackNet}, - &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_UDP}}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: wgPortBytes}, // dport=wgPort - &expr.Counter{}, - &expr.Notrack{}, - }, - }) - m.rConn.AddRule(&nftables.Rule{ - Table: m.notrackPreroutingChain.Table, - Chain: m.notrackPreroutingChain, - Exprs: []expr.Any{ - &expr.Meta{Key: expr.MetaKeyIIFNAME, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: ifname("lo")}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, // saddr - &expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: loopbackMask, Xor: loopbackXor}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: loopbackNet}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 16, Len: 4}, // daddr - &expr.Bitwise{SourceRegister: 1, DestRegister: 1, Len: 4, Mask: loopbackMask, Xor: loopbackXor}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: loopbackNet}, - &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_UDP}}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: proxyPortBytes}, // dport=proxyPort - &expr.Counter{}, - &expr.Notrack{}, - }, - }) - - if err := m.rConn.Flush(); err != nil { - return fmt.Errorf("flush notrack rules: %w", err) - } - - log.Debugf("set up wg proxy notrack rules for ports %d,%d", proxyPort, wgPort) - return nil -} - -func (m *Manager) initNoTrackChains(table *nftables.Table) error { - m.notrackOutputChain = m.rConn.AddChain(&nftables.Chain{ - Name: chainNameRawOutput, - Table: table, - Type: nftables.ChainTypeFilter, - Hooknum: nftables.ChainHookOutput, - Priority: nftables.ChainPriorityRaw, - }) - - m.notrackPreroutingChain = m.rConn.AddChain(&nftables.Chain{ - Name: chainNameRawPrerouting, - Table: table, - Type: nftables.ChainTypeFilter, - Hooknum: nftables.ChainHookPrerouting, - Priority: nftables.ChainPriorityRaw, - }) - - if err := m.rConn.Flush(); err != nil { - return fmt.Errorf("flush chain creation: %w", err) - } - - return nil -} - -func (m *Manager) refreshNoTrackChains() error { +// cleanupNoTrackChains removes the chains that earlier versions used to exempt +// the WireGuard proxy's loopback traffic from connection tracking. +func (m *Manager) cleanupNoTrackChains() error { chains, err := m.rConn.ListChainsOfTableFamily(nftables.TableFamilyIPv4) if err != nil { return fmt.Errorf("list chains: %w", err) } tableName := getTableName() + var found bool for _, c := range chains { if c.Table.Name != tableName { continue } - switch c.Name { - case chainNameRawOutput: - m.notrackOutputChain = c - case chainNameRawPrerouting: - m.notrackPreroutingChain = c + if c.Name != chainNameRawOutput && c.Name != chainNameRawPrerouting { + continue } + m.rConn.DelChain(c) + found = true + } + + if !found { + return nil + } + + if err := m.rConn.Flush(); err != nil { + return fmt.Errorf("flush chain removal: %w", err) } return nil diff --git a/client/firewall/uspfilter/filter.go b/client/firewall/uspfilter/filter.go index f12cbe762..f00a31f58 100644 --- a/client/firewall/uspfilter/filter.go +++ b/client/firewall/uspfilter/filter.go @@ -751,14 +751,6 @@ func (m *Manager) resetState() { } } -// SetupWGProxyNoTrack creates notrack rules for WireGuard proxy loopback traffic. -func (m *Manager) SetupWGProxyNoTrack(proxyPort, wgPort uint16) error { - if m.nativeFirewall == nil { - return nil - } - return m.nativeFirewall.SetupWGProxyNoTrack(proxyPort, wgPort) -} - // 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 { diff --git a/client/iface/iface.go b/client/iface/iface.go index 247f421a2..f6006fa87 100644 --- a/client/iface/iface.go +++ b/client/iface/iface.go @@ -51,7 +51,6 @@ func ValidateMTU(mtu uint16) error { type wgProxyFactory interface { GetProxy() wgproxy.Proxy - GetProxyPort() uint16 Free() error } @@ -81,12 +80,6 @@ func (w *WGIface) GetProxy() wgproxy.Proxy { return w.wgProxyFactory.GetProxy() } -// GetProxyPort returns the proxy port used by the WireGuard proxy. -// Returns 0 if no proxy port is used (e.g., for userspace WireGuard). -func (w *WGIface) GetProxyPort() uint16 { - return w.wgProxyFactory.GetProxyPort() -} - // GetBind returns the EndpointManager userspace bind mode. func (w *WGIface) GetBind() device.EndpointManager { w.mu.Lock() diff --git a/client/iface/iface_close_test.go b/client/iface/iface_close_test.go index 171e15d0a..ea3115ec0 100644 --- a/client/iface/iface_close_test.go +++ b/client/iface/iface_close_test.go @@ -52,7 +52,6 @@ func (f *fakeTunDevice) Close() error { type fakeProxyFactory struct{} func (fakeProxyFactory) GetProxy() wgproxy.Proxy { return nil } -func (fakeProxyFactory) GetProxyPort() uint16 { return 0 } func (fakeProxyFactory) Free() error { return nil } // TestWGIface_CloseReleasesMutexBeforeTunClose guards against a deadlock diff --git a/client/iface/wgproxy/bench_linux_test.go b/client/iface/wgproxy/bench_linux_test.go deleted file mode 100644 index 11353018b..000000000 --- a/client/iface/wgproxy/bench_linux_test.go +++ /dev/null @@ -1,187 +0,0 @@ -//go:build linux && !android && privileged - -package wgproxy - -import ( - "context" - "fmt" - "net" - "sync" - "sync/atomic" - "testing" - "time" - - "github.com/netbirdio/netbird/client/iface/wgproxy/loopback" - "github.com/netbirdio/netbird/client/iface/wgproxy/udp" -) - -// Benchmarks the WireGuard -> relay direction: a stand-in WireGuard socket sends -// to each peer's endpoint, the proxy picks the packet up and forwards it to the -// relayed connection. Peer count matters because the kernel proxy shares one -// socket between all peers while the userspace proxy has one per peer. - -const ( - benchLoopbackWgPort = 51841 - benchUDPWgPort = 51842 - benchPktSize = 1200 -) - -type benchVariant struct { - name string - wgPort int - setup func(peers int) (proxies []Proxy, cleanup func(), err error) -} - -func benchVariants() []benchVariant { - return []benchVariant{ - { - name: "loopback", - wgPort: benchLoopbackWgPort, - setup: func(peers int) ([]Proxy, func(), error) { - loopbackProxy := loopback.NewProxy(benchLoopbackWgPort, 1280) - if err := loopbackProxy.Listen(); err != nil { - return nil, nil, fmt.Errorf("listen loopback proxy: %w", err) - } - proxies := make([]Proxy, 0, peers) - for i := 0; i < peers; i++ { - proxies = append(proxies, loopback.NewProxyWrapper(loopbackProxy)) - } - return proxies, func() { _ = loopbackProxy.Free() }, nil - }, - }, - { - name: "udp", - wgPort: benchUDPWgPort, - setup: func(peers int) ([]Proxy, func(), error) { - proxies := make([]Proxy, 0, peers) - for i := 0; i < peers; i++ { - proxies = append(proxies, udp.NewWGUDPProxy(benchUDPWgPort, 1280)) - } - return proxies, func() {}, nil - }, - }, - } -} - -func BenchmarkProxyForwarding(b *testing.B) { - for _, peers := range []int{1, 8, 32} { - for _, v := range benchVariants() { - b.Run(fmt.Sprintf("%s/peers=%d", v.name, peers), func(b *testing.B) { - runForwardingBench(b, v, peers) - }) - } - } -} - -func runForwardingBench(b *testing.B, v benchVariant, peers int) { - b.Helper() - - relayServer, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}) - if err != nil { - b.Fatalf("relay server: %v", err) - } - defer relayServer.Close() - if err := relayServer.SetReadBuffer(8 << 20); err != nil { - b.Logf("set relay read buffer: %v", err) - } - - proxies, cleanup, err := v.setup(peers) - if err != nil { - b.Skipf("setup %s: %v", v.name, err) - } - defer cleanup() - - ctx := context.Background() - endpoints := make([]*net.UDPAddr, 0, peers) - for i, p := range proxies { - relayConn, err := net.Dial("udp", relayServer.LocalAddr().String()) - if err != nil { - b.Fatalf("relay conn %d: %v", i, err) - } - defer relayConn.Close() - - nbAddr := &net.UDPAddr{IP: net.IPv4(10, 0, 0, byte(i+1)), Port: 51820} - if err := p.AddRelayedConn(ctx, nbAddr, relayConn); err != nil { - b.Fatalf("add relayed conn %d: %v", i, err) - } - p.Work() - - addr := p.EndpointAddr() - if addr == nil { - b.Fatalf("proxy %d has no endpoint address", i) - } - endpoints = append(endpoints, addr) - } - defer func() { - for _, p := range proxies { - _ = p.CloseConn() - } - }() - - // stand-in for the WireGuard socket: bound to the wg port the proxy expects - wgSock, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: v.wgPort}) - if err != nil { - b.Fatalf("wg socket: %v", err) - } - defer wgSock.Close() - if err := wgSock.SetWriteBuffer(8 << 20); err != nil { - b.Logf("set wg write buffer: %v", err) - } - - var received atomic.Int64 - done := make(chan struct{}) - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - buf := make([]byte, 65536) - for { - select { - case <-done: - return - default: - } - if err := relayServer.SetReadDeadline(time.Now().Add(200 * time.Millisecond)); err != nil { - return - } - n, _, err := relayServer.ReadFrom(buf) - if err != nil { - continue - } - if n > 0 { - received.Add(1) - } - } - }() - - pkt := make([]byte, benchPktSize) - // warm up the datapath and the per-peer lookups - for i := 0; i < 200; i++ { - if _, err := wgSock.WriteTo(pkt, endpoints[i%len(endpoints)]); err != nil { - b.Fatalf("warmup write: %v", err) - } - } - time.Sleep(200 * time.Millisecond) - received.Store(0) - - b.SetBytes(int64(benchPktSize)) - b.ResetTimer() - start := time.Now() - for i := 0; i < b.N; i++ { - if _, err := wgSock.WriteTo(pkt, endpoints[i%len(endpoints)]); err != nil { - b.Fatalf("write: %v", err) - } - } - sent := b.N - // let the tail drain - time.Sleep(300 * time.Millisecond) - elapsed := time.Since(start) - b.StopTimer() - - close(done) - wg.Wait() - - got := received.Load() - b.ReportMetric(float64(got)/elapsed.Seconds(), "fwd_pps") - b.ReportMetric(float64(got)/float64(sent)*100, "delivered_%") -} diff --git a/client/iface/wgproxy/factory_kernel.go b/client/iface/wgproxy/factory_kernel.go index 2fd545b29..0b2329b96 100644 --- a/client/iface/wgproxy/factory_kernel.go +++ b/client/iface/wgproxy/factory_kernel.go @@ -55,14 +55,6 @@ func (w *KernelFactory) GetProxy() Proxy { return loopback.NewProxyWrapper(w.loopbackProxy) } -// GetProxyPort returns the loopback proxy port, or 0 if the kernel proxy is not active. -func (w *KernelFactory) GetProxyPort() uint16 { - if w.loopbackProxy == nil { - return 0 - } - return w.loopbackProxy.GetProxyPort() -} - func (w *KernelFactory) Free() error { if w.loopbackProxy == nil { return nil diff --git a/client/iface/wgproxy/factory_usp.go b/client/iface/wgproxy/factory_usp.go index bbd67e076..a1b1c34d7 100644 --- a/client/iface/wgproxy/factory_usp.go +++ b/client/iface/wgproxy/factory_usp.go @@ -24,11 +24,6 @@ func (w *USPFactory) GetProxy() Proxy { return proxyBind.NewProxyBind(w.bind, w.mtu) } -// GetProxyPort returns 0 as userspace WireGuard doesn't use a separate proxy port. -func (w *USPFactory) GetProxyPort() uint16 { - return 0 -} - func (w *USPFactory) Free() error { return nil } diff --git a/client/iface/wgproxy/loopback/proxy.go b/client/iface/wgproxy/loopback/proxy.go index 9007b6af1..975bb3d58 100644 --- a/client/iface/wgproxy/loopback/proxy.go +++ b/client/iface/wgproxy/loopback/proxy.go @@ -197,10 +197,6 @@ func (p *Proxy) Free() error { return nberrors.FormatErrorOrNil(result) } -// GetProxyPort returns the port every peer endpoint address is reached on. -func (p *Proxy) GetProxyPort() uint16 { - return uint16(p.proxyPort) -} // proxyToRemote reads packets from the local WireGuard instance and forwards // them to the relayed connection the destination address belongs to. diff --git a/client/internal/engine.go b/client/internal/engine.go index 036de3e59..f1f098783 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -638,10 +638,6 @@ func (e *Engine) Start(netbirdConfig *mgmProto.NetbirdConfig, mgmtURL *url.URL) return fmt.Errorf("up wg interface: %w", err) } - // Set up notrack rules immediately after proxy is listening to prevent - // conntrack entries from being created before the rules are in place - e.setupWGProxyNoTrack() - // Start after interface is up since port may have been resolved from 0 or changed if occupied e.shutdownWg.Add(1) go func() { @@ -774,23 +770,6 @@ func (e *Engine) initFirewall() error { return nil } -// setupWGProxyNoTrack configures connection tracking exclusion for WireGuard proxy traffic. -// This prevents conntrack/MASQUERADE from affecting loopback traffic between WireGuard and the proxy. -func (e *Engine) setupWGProxyNoTrack() { - if e.firewall == nil { - return - } - - proxyPort := e.wgInterface.GetProxyPort() - if proxyPort == 0 { - return - } - - if err := e.firewall.SetupWGProxyNoTrack(proxyPort, uint16(e.config.WgPort)); err != nil { - log.Warnf("failed to setup ebpf proxy notrack: %v", err) - } -} - func (e *Engine) blockLanAccess() { if e.config.BlockInbound { // no need to set up extra deny rules if inbound is already blocked in general diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index fbd47ed74..2251a43d6 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -63,7 +63,6 @@ type MockWGIface struct { GetStatsFunc func() (map[string]configurer.WGStats, error) GetInterfaceGUIDStringFunc func() (string, error) GetProxyFunc func() wgproxy.Proxy - GetProxyPortFunc func() uint16 GetNetFunc func() *netstack.Net LastActivitiesFunc func() map[string]monotime.Time } @@ -160,12 +159,6 @@ func (m *MockWGIface) GetProxy() wgproxy.Proxy { return m.GetProxyFunc() } -func (m *MockWGIface) GetProxyPort() uint16 { - if m.GetProxyPortFunc != nil { - return m.GetProxyPortFunc() - } - return 0 -} func (m *MockWGIface) GetNet() *netstack.Net { return m.GetNetFunc() diff --git a/client/internal/iface_common.go b/client/internal/iface_common.go index 8ffa0b102..d772a3a03 100644 --- a/client/internal/iface_common.go +++ b/client/internal/iface_common.go @@ -28,7 +28,6 @@ type wgIfaceBase interface { Up() (*udpmux.UniversalUDPMuxDefault, error) UpdateAddr(newAddr wgaddr.Address) error GetProxy() wgproxy.Proxy - GetProxyPort() uint16 UpdatePeer(peerKey string, allowedIps []netip.Prefix, keepAlive time.Duration, endpoint *net.UDPAddr, preSharedKey *wgtypes.Key) error RemoveEndpointAddress(key string) error RemovePeer(peerKey string) error