From 9a5bdb508e0731043ea81cb5f74428f4f2d42de0 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Tue, 20 Oct 2026 22:08:55 -0400 Subject: [PATCH] Apply remote_allow_list before capping addrs at MaxRemotes (#1985) unlockedSetV4/V6 truncated the reported list to MaxRemotes and only then ran the allow list check. A host reporting more than MaxRemotes addrs (docker bridges, temporary v6 addrs, etc) could have every allowed addr discarded if the denied ones sorted first. Filter the full list and stop once MaxRemotes allowed addrs are collected. This affects lighthouses processing HostUpdateNotification and clients processing HostQueryReply. --- remote_list.go | 16 ++++++++++++---- remote_list_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/remote_list.go b/remote_list.go index 9d1b387e..90406b47 100644 --- a/remote_list.go +++ b/remote_list.go @@ -460,10 +460,14 @@ func (r *RemoteList) unlockedSetV4(ownerVpnIp, vpnIp netip.Addr, to []*V4AddrPor // Reset the slice c.reported = c.reported[:0] - // We can't take their array but we can take their pointers - for _, v := range to[:minInt(len(to), MaxRemotes)] { + // We can't take their array but we can take their pointers. + // Filter before capping so denied addrs do not crowd out allowed ones reported later in the list. + for _, v := range to { if check(vpnIp, v) { c.reported = append(c.reported, v) + if len(c.reported) == MaxRemotes { + break + } } } } @@ -508,10 +512,14 @@ func (r *RemoteList) unlockedSetV6(ownerVpnIp, vpnIp netip.Addr, to []*V6AddrPor // Reset the slice c.reported = c.reported[:0] - // We can't take their array but we can take their pointers - for _, v := range to[:minInt(len(to), MaxRemotes)] { + // We can't take their array but we can take their pointers. + // Filter before capping so denied addrs do not crowd out allowed ones reported later in the list. + for _, v := range to { if check(vpnIp, v) { c.reported = append(c.reported, v) + if len(c.reported) == MaxRemotes { + break + } } } } diff --git a/remote_list_test.go b/remote_list_test.go index 0b5b7d5d..38e98b6b 100644 --- a/remote_list_test.go +++ b/remote_list_test.go @@ -2,6 +2,7 @@ package nebula import ( "encoding/binary" + "fmt" "net/netip" "testing" @@ -346,3 +347,38 @@ func newIp6AndPortFromString(s string) *V6AddrPort { Port: uint32(a.Port()), } } + +func TestRemoteList_SetFiltersBeforeCap(t *testing.T) { + owner := netip.MustParseAddr("0.0.0.0") + rl := NewRemoteList([]netip.Addr{owner}, nil) + + // MaxRemotes denied addrs first, then the only allowed one, then enough allowed to exceed the cap + var v4 []*V4AddrPort + for i := 0; i < MaxRemotes; i++ { + v4 = append(v4, newIp4AndPortFromString(fmt.Sprintf("172.17.0.%d:4242", i+1))) + } + for i := 0; i < MaxRemotes+1; i++ { + v4 = append(v4, newIp4AndPortFromString(fmt.Sprintf("70.199.182.%d:4242", i+1))) + } + rl.unlockedSetV4(owner, owner, v4, func(_ netip.Addr, a *V4AddrPort) bool { + return protoV4AddrPortToNetAddrPort(a).Addr().As4()[0] != 172 + }) + + var v6 []*V6AddrPort + for i := 0; i < MaxRemotes; i++ { + v6 = append(v6, newIp6AndPortFromString(fmt.Sprintf("[fd00::%d]:4242", i+1))) + } + for i := 0; i < MaxRemotes+1; i++ { + v6 = append(v6, newIp6AndPortFromString(fmt.Sprintf("[2001:db8::%d]:4242", i+1))) + } + rl.unlockedSetV6(owner, owner, v6, func(_ netip.Addr, a *V6AddrPort) bool { + return protoV6AddrPortToNetAddrPort(a).Addr().As16()[0] != 0xfd + }) + + rl.Rebuild([]netip.Prefix{}) + assert.Len(t, rl.addrs, MaxRemotes*2) + for _, a := range rl.addrs { + assert.NotEqual(t, byte(172), a.Addr().As16()[12], "denied v4 addr %v was kept", a) + assert.NotEqual(t, byte(0xfd), a.Addr().As16()[0], "denied v6 addr %v was kept", a) + } +}