From 8732d3cd139b04c4d0e101d45e9482397186b7b1 Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 6 Jul 2026 12:50:08 +0200 Subject: [PATCH] [client] Reuse parsed AllowedIPs from peerStore in lazy exclusion Instead of re-parsing the network map AllowedIPs strings, look up the already-parsed []netip.Prefix from peerStore.AllowedIPs (the same typed value the lazy manager itself consumes). A down/lazy peer still has its conn in the store, so exclusion is unaffected by connection state. Extract a pure prefixesContain helper and unit-test it. Co-Authored-By: Claude Opus 4.8 --- client/internal/engine.go | 26 +++++--- client/internal/engine_lazy_exclude_test.go | 66 +++++++++++++++++---- 2 files changed, 71 insertions(+), 21 deletions(-) diff --git a/client/internal/engine.go b/client/internal/engine.go index 6efbc7b73..0c407b039 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -2564,10 +2564,11 @@ func (e *Engine) toExcludedLazyPeers(rules []firewallManager.ForwardRule, peers // Ingress forward targets: inbound forwarded traffic is initiated remotely and // cannot wake a lazy connection, so the peer routing the target must stay - // permanently connected. + // permanently connected. AllowedIPs are already parsed on the peer conn, so + // reuse those typed prefixes instead of re-parsing the network map strings. for _, r := range rules { for _, p := range peers { - if peerRoutesAddr(p, r.TranslatedAddress) { + if e.peerRoutesAddr(p, r.TranslatedAddress) { log.Infof("exclude forwarder peer from lazy connection: %s", p.GetWgPubKey()) excludedPeers[p.GetWgPubKey()] = true } @@ -2577,13 +2578,20 @@ func (e *Engine) toExcludedLazyPeers(rules []firewallManager.ForwardRule, peers return excludedPeers } -// peerRoutesAddr verifies if the peer is a router for a given address. -func peerRoutesAddr(p *mgmProto.RemotePeerConfig, addr netip.Addr) bool { - for _, allowedIP := range p.GetAllowedIps() { - prefix, err := netip.ParsePrefix(allowedIP) - if err != nil { - continue - } +// peerRoutesAddr reports whether the peer is a router for addr, matched against +// the peer's already-parsed AllowedIPs from the store (the same typed value the +// lazy manager consumes) rather than re-parsing the network map strings. +func (e *Engine) peerRoutesAddr(p *mgmProto.RemotePeerConfig, addr netip.Addr) bool { + prefixes, ok := e.peerStore.AllowedIPs(p.GetWgPubKey()) + if !ok { + return false + } + return prefixesContain(prefixes, addr) +} + +// prefixesContain reports whether addr falls within any of the prefixes. +func prefixesContain(prefixes []netip.Prefix, addr netip.Addr) bool { + for _, prefix := range prefixes { if prefix.Contains(addr) { return true } diff --git a/client/internal/engine_lazy_exclude_test.go b/client/internal/engine_lazy_exclude_test.go index cfe405602..b5ef16c3b 100644 --- a/client/internal/engine_lazy_exclude_test.go +++ b/client/internal/engine_lazy_exclude_test.go @@ -7,20 +7,53 @@ import ( "github.com/stretchr/testify/require" firewallManager "github.com/netbirdio/netbird/client/firewall/manager" + "github.com/netbirdio/netbird/client/internal/peer" + "github.com/netbirdio/netbird/client/internal/peerstore" mgmProto "github.com/netbirdio/netbird/shared/management/proto" ) -// TestToExcludedLazyPeers_ForwardTarget guards a regression: AllowedIPs arrive as -// CIDR (a peer's overlay IP is a /32), so comparing them for equality against -// ForwardRule.TranslatedAddress.String() (unmasked) never matched and the -// forward-target peer was never excluded from lazy connections. -func TestToExcludedLazyPeers_ForwardTarget(t *testing.T) { - e := &Engine{} +func TestPrefixesContain(t *testing.T) { + tests := []struct { + name string + prefixes []string + addr string + want bool + }{ + {name: "own overlay /32 matches", prefixes: []string{"100.110.8.145/32"}, addr: "100.110.8.145", want: true}, + {name: "addr inside routed subnet", prefixes: []string{"10.121.0.0/16"}, addr: "10.121.208.4", want: true}, + {name: "addr outside subnet", prefixes: []string{"10.121.0.0/16"}, addr: "10.122.0.1", want: false}, + {name: "different /32", prefixes: []string{"100.110.8.145/32"}, addr: "100.110.8.146", want: false}, + {name: "ipv6 /128 matches", prefixes: []string{"fd00::1/128"}, addr: "fd00::1", want: true}, + {name: "no prefixes", prefixes: nil, addr: "10.121.208.4", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prefixes := make([]netip.Prefix, 0, len(tt.prefixes)) + for _, p := range tt.prefixes { + prefixes = append(prefixes, netip.MustParsePrefix(p)) + } + require.Equal(t, tt.want, prefixesContain(prefixes, netip.MustParseAddr(tt.addr))) + }) + } +} + +// TestToExcludedLazyPeers_ForwardTarget guards a regression: the forward-target +// peer (the peer routing a ForwardRule.TranslatedAddress) must be excluded from +// lazy connections, matched via the peer's already-parsed AllowedIPs. +func TestToExcludedLazyPeers_ForwardTarget(t *testing.T) { + const targetPeerKey = "cccccccccccccccccccccccccccccccccccccccccc0=" + const otherPeerKey = "dddddddddddddddddddddddddddddddddddddddddd0=" + + store := peerstore.NewConnStore() + store.AddPeerConn(targetPeerKey, newTestConn(t, targetPeerKey, "100.110.8.145/32")) + store.AddPeerConn(otherPeerKey, newTestConn(t, otherPeerKey, "100.110.9.10/32")) + + e := &Engine{peerStore: store} - const targetPeerKey = "target-peer" peers := []*mgmProto.RemotePeerConfig{ {WgPubKey: targetPeerKey, AllowedIps: []string{"100.110.8.145/32"}}, - {WgPubKey: "other-peer", AllowedIps: []string{"100.110.9.10/32"}}, + {WgPubKey: otherPeerKey, AllowedIps: []string{"100.110.9.10/32"}}, } rules := []firewallManager.ForwardRule{ {TranslatedAddress: netip.MustParseAddr("100.110.8.145")}, @@ -29,17 +62,26 @@ func TestToExcludedLazyPeers_ForwardTarget(t *testing.T) { excluded := e.toExcludedLazyPeers(rules, peers) require.True(t, excluded[targetPeerKey], "forward-target peer must be excluded from lazy connections") - require.False(t, excluded["other-peer"], "non-target peer must not be excluded") + require.False(t, excluded[otherPeerKey], "non-target peer must not be excluded") require.Len(t, excluded, 1) } func TestToExcludedLazyPeers_NoRules(t *testing.T) { - e := &Engine{} + e := &Engine{peerStore: peerstore.NewConnStore()} peers := []*mgmProto.RemotePeerConfig{ {WgPubKey: "peer-a", AllowedIps: []string{"100.110.8.145/32"}}, } - excluded := e.toExcludedLazyPeers(nil, peers) - require.Empty(t, excluded) + require.Empty(t, e.toExcludedLazyPeers(nil, peers)) +} + +func newTestConn(t *testing.T, key, allowedIP string) *peer.Conn { + t.Helper() + conn, err := peer.NewConn(peer.ConnConfig{ + Key: key, + WgConfig: peer.WgConfig{AllowedIps: []netip.Prefix{netip.MustParsePrefix(allowedIP)}}, + }, peer.ServiceDependencies{}) + require.NoError(t, err) + return conn }