From c87089d09fa649a3cae902ca27982c6ef68f1fff Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 6 Jul 2026 10:58:24 +0200 Subject: [PATCH 1/4] [client] Extract peerRoutesAddr helper in toExcludedLazyPeers Pure stylistic refactor: pull the AllowedIPs match into a named peerRoutesAddr helper and document why forward-target peers are excluded from lazy connections. No behavior change; the existing address match is preserved as-is. Co-Authored-By: Claude Opus 4.8 --- client/internal/engine.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/client/internal/engine.go b/client/internal/engine.go index a08bea31b46..5f10166a2a8 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -2561,13 +2561,13 @@ func (e *Engine) updateForwardRules(rules []*mgmProto.ForwardingRule) ([]firewal func (e *Engine) toExcludedLazyPeers(rules []firewallManager.ForwardRule, peers []*mgmProto.RemotePeerConfig) map[string]bool { excludedPeers := make(map[string]bool) + + // 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. for _, r := range rules { - ip := r.TranslatedAddress for _, p := range peers { - for _, allowedIP := range p.GetAllowedIps() { - if allowedIP != ip.String() { - continue - } + if peerRoutesAddr(p, r.TranslatedAddress) { log.Infof("exclude forwarder peer from lazy connection: %s", p.GetWgPubKey()) excludedPeers[p.GetWgPubKey()] = true } @@ -2577,6 +2577,16 @@ 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() { + if allowedIP == addr.String() { + return true + } + } + return false +} + // isChecksEqual checks if two slices of checks are equal. func isChecksEqual(checks1, checks2 []*mgmProto.Checks) bool { normalize := func(checks []*mgmProto.Checks) []string { From 460967d2186a6cd3d6e489fc81fb8f8ce03bd5b3 Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 6 Jul 2026 10:58:24 +0200 Subject: [PATCH 2/4] [client] Add failing test for lazy-conn forward-target exclusion toExcludedLazyPeers compares AllowedIPs (CIDR) against the unmasked TranslatedAddress, so forward-target peers are never excluded. This test asserts the peer is excluded and fails on the current behavior; the fix follows. Co-Authored-By: Claude Opus 4.8 --- client/internal/engine_lazy_exclude_test.go | 45 +++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 client/internal/engine_lazy_exclude_test.go diff --git a/client/internal/engine_lazy_exclude_test.go b/client/internal/engine_lazy_exclude_test.go new file mode 100644 index 00000000000..cfe40560238 --- /dev/null +++ b/client/internal/engine_lazy_exclude_test.go @@ -0,0 +1,45 @@ +package internal + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/require" + + firewallManager "github.com/netbirdio/netbird/client/firewall/manager" + 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{} + + 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"}}, + } + rules := []firewallManager.ForwardRule{ + {TranslatedAddress: netip.MustParseAddr("100.110.8.145")}, + } + + 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.Len(t, excluded, 1) +} + +func TestToExcludedLazyPeers_NoRules(t *testing.T) { + e := &Engine{} + + peers := []*mgmProto.RemotePeerConfig{ + {WgPubKey: "peer-a", AllowedIps: []string{"100.110.8.145/32"}}, + } + + excluded := e.toExcludedLazyPeers(nil, peers) + require.Empty(t, excluded) +} From 22434897b6d23b76e6f77d752e35e784b7bd946c Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 6 Jul 2026 11:03:17 +0200 Subject: [PATCH 3/4] [client] Fix lazy-conn exclusion for ingress forward peers peerRoutesAddr compared AllowedIPs (CIDR, e.g. a peer's overlay IP as /32) against the unmasked TranslatedAddress string, so the match never fired and forward-target peers were never excluded from lazy connections. Use prefix containment so a routed address matches the peer's AllowedIP. Co-Authored-By: Claude Opus 4.8 --- client/internal/engine.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/client/internal/engine.go b/client/internal/engine.go index 5f10166a2a8..6efbc7b73b0 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -2580,7 +2580,11 @@ func (e *Engine) toExcludedLazyPeers(rules []firewallManager.ForwardRule, peers // 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() { - if allowedIP == addr.String() { + prefix, err := netip.ParsePrefix(allowedIP) + if err != nil { + continue + } + if prefix.Contains(addr) { return true } } From 8732d3cd139b04c4d0e101d45e9482397186b7b1 Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 6 Jul 2026 12:50:08 +0200 Subject: [PATCH 4/4] [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 | 64 +++++++++++++++++---- 2 files changed, 70 insertions(+), 20 deletions(-) diff --git a/client/internal/engine.go b/client/internal/engine.go index 6efbc7b73b0..0c407b03913 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 cfe40560238..b5ef16c3be9 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 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) { - e := &Engine{} + 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 }