From 7006113897f3a5b496c494bb764c4d3c9e1d9fb8 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:14:19 +0200 Subject: [PATCH] [management] update reserved IP exclusion when generating peer IP (#8144) --- management/server/account.go | 8 ++ management/server/account_test.go | 21 +++++ .../handlers/accounts/accounts_handler.go | 2 +- .../accounts/accounts_handler_test.go | 8 +- management/server/types/network.go | 53 ++++++++---- management/server/types/network_test.go | 86 ++++++++++++++++--- 6 files changed, 144 insertions(+), 34 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 6de1642e5..946b9ad1c 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -2683,6 +2683,14 @@ func (am *DefaultAccountManager) validateIPForUpdate(account *types.Account, pee return status.Errorf(status.InvalidArgument, "IP %s is not within the account network range %s", newIP.String(), account.Network.Net.String()) } + prefix, err := netip.ParsePrefix(account.Network.Net.String()) + if err != nil { + return fmt.Errorf("parse account network %s: %w", account.Network.Net.String(), err) + } + if types.IsReservedPeerIP(prefix, newIP) { + return status.Errorf(status.InvalidArgument, "IP %s is reserved in the account network range %s", newIP.String(), account.Network.Net.String()) + } + for _, peer := range peers { if peer.ID != peerID && peer.IP == newIP { return status.Errorf(status.InvalidArgument, "IP %s is already assigned to peer %s", newIP.String(), peer.ID) diff --git a/management/server/account_test.go b/management/server/account_test.go index d3ad2dfc9..7aa88ee64 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -4454,6 +4454,27 @@ func TestDefaultAccountManager_UpdatePeerIP(t *testing.T) { assert.Contains(t, err.Error(), "not within the account network range", "error should mention network range") }) + t.Run("update peer IP to reserved address should fail", func(t *testing.T) { + account, err := manager.Store.GetAccount(context.Background(), accountID) + require.NoError(t, err, "unable to get account") + + network := account.Network.Net + broadcast := make(net.IP, len(network.IP.To4())) + for i := range broadcast { + broadcast[i] = network.IP.To4()[i] | ^network.Mask[i] + } + broadcastIP, ok := netip.AddrFromSlice(broadcast) + require.True(t, ok, "unable to parse broadcast IP") + networkIP, ok := netip.AddrFromSlice(network.IP.To4()) + require.True(t, ok, "unable to parse network IP") + + for _, reserved := range []netip.Addr{networkIP, broadcastIP.Prev(), broadcastIP} { + err = manager.UpdatePeerIP(context.Background(), accountID, userID, peer1.ID, reserved) + require.Error(t, err, "should fail for reserved IP %s", reserved) + assert.Contains(t, err.Error(), "is reserved", "error should mention reserved IP") + } + }) + t.Run("update peer IP with invalid peer ID should fail", func(t *testing.T) { newAddr := netip.MustParseAddr("100.64.0.101") err := manager.UpdatePeerIP(context.Background(), accountID, userID, "invalid-peer-id", newAddr) diff --git a/management/server/http/handlers/accounts/accounts_handler.go b/management/server/http/handlers/accounts/accounts_handler.go index 795214c31..e5d75007f 100644 --- a/management/server/http/handlers/accounts/accounts_handler.go +++ b/management/server/http/handlers/accounts/accounts_handler.go @@ -152,7 +152,7 @@ func calculateMaxHosts(prefix netip.Prefix) int64 { maxHosts := int64(1) << hostBits if prefix.Addr().Is4() { - maxHosts -= 2 // network and broadcast addresses + maxHosts -= 3 // network, client DNS resolver and broadcast addresses } return maxHosts diff --git a/management/server/http/handlers/accounts/accounts_handler_test.go b/management/server/http/handlers/accounts/accounts_handler_test.go index 06419019e..1ccac2b04 100644 --- a/management/server/http/handlers/accounts/accounts_handler_test.go +++ b/management/server/http/handlers/accounts/accounts_handler_test.go @@ -12,9 +12,9 @@ import ( "testing" "time" - "go.uber.org/mock/gomock" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" nbcontext "github.com/netbirdio/netbird/management/server/context" "github.com/netbirdio/netbird/management/server/mock_server" @@ -469,9 +469,9 @@ func TestCalculateMaxHosts(t *testing.T) { prefix string min int64 }{ - {"v4 /24", "100.64.0.0/24", 254}, - {"v4 /16", "100.64.0.0/16", 65534}, - {"v4 /28", "100.64.0.0/28", 14}, + {"v4 /24", "100.64.0.0/24", 253}, + {"v4 /16", "100.64.0.0/16", 65533}, + {"v4 /28", "100.64.0.0/28", 13}, {"v6 /64", "fd00::/64", math.MaxInt64}, {"v6 /120", "fd00::/120", 256}, {"v6 /112", "fd00::/112", 65536}, diff --git a/management/server/types/network.go b/management/server/types/network.go index 1ce6465b5..fdb8cb521 100644 --- a/management/server/types/network.go +++ b/management/server/types/network.go @@ -118,14 +118,9 @@ func AllocatePeerIP(prefix netip.Prefix, takenIps []netip.Addr) (netip.Addr, err return netip.Addr{}, err } - b := prefix.Masked().Addr().As4() - baseIP := binary.BigEndian.Uint32(b[:]) - hostBits := 32 - prefix.Bits() - totalIPs := uint32(1 << hostBits) + baseIP, firstOffset, lastOffset := ipv4PeerRange(prefix) - taken := make(map[uint32]struct{}, len(takenIps)+1) - taken[baseIP] = struct{}{} // reserve network IP - taken[baseIP+totalIPs-1] = struct{}{} // reserve broadcast IP + taken := make(map[uint32]struct{}, len(takenIps)) for _, ip := range takenIps { if !ip.Is4() { @@ -135,17 +130,16 @@ func AllocatePeerIP(prefix netip.Prefix, takenIps []netip.Addr) (netip.Addr, err taken[binary.BigEndian.Uint32(ab[:])] = struct{}{} } - maxAttempts := (int(totalIPs) - len(taken)) / 100 + maxAttempts := (int(lastOffset-firstOffset+1) - len(taken)) / 100 for i := 0; i < maxAttempts; i++ { - offset := uint32(util.RandIntn(int(totalIPs-2))) + 1 - candidate := baseIP + offset + candidate := baseIP + randomOffset(firstOffset, lastOffset) if _, exists := taken[candidate]; !exists { return uint32ToIP(candidate), nil } } - for offset := uint32(1); offset < totalIPs-1; offset++ { + for offset := firstOffset; offset <= lastOffset; offset++ { candidate := baseIP + offset if _, exists := taken[candidate]; !exists { return uint32ToIP(candidate), nil @@ -161,15 +155,38 @@ func AllocateRandomPeerIP(prefix netip.Prefix) (netip.Addr, error) { return netip.Addr{}, err } + baseIP, firstOffset, lastOffset := ipv4PeerRange(prefix) + return uint32ToIP(baseIP + randomOffset(firstOffset, lastOffset)), nil +} + +// IsReservedPeerIP reports whether ip is an address of the IPv4 prefix that is never assigned to a peer: +// the network address, the broadcast address or the address before it, which clients use for their DNS resolver. +// It returns false if prefix is not a valid IPv4 peer network or ip is not within it. +func IsReservedPeerIP(prefix netip.Prefix, ip netip.Addr) bool { + ip = ip.Unmap() + if validateIPv4Prefix(prefix) != nil || !ip.Is4() || !prefix.Contains(ip) { + return false + } + + baseIP, firstOffset, lastOffset := ipv4PeerRange(prefix) + b := ip.As4() + offset := binary.BigEndian.Uint32(b[:]) - baseIP + return offset < firstOffset || offset > lastOffset +} + +// ipv4PeerRange returns the network address of the IPv4 prefix as uint32 and the first and last offset +// from it that can be assigned to a peer. Reserved are the network address, the broadcast address and the +// address before it, which clients in userspace and netstack mode use for their DNS resolver. +func ipv4PeerRange(prefix netip.Prefix) (baseIP, firstOffset, lastOffset uint32) { b := prefix.Masked().Addr().As4() - baseIP := binary.BigEndian.Uint32(b[:]) - hostBits := 32 - prefix.Bits() - totalIPs := uint32(1 << hostBits) + broadcastOffset := uint32(1)<<(32-prefix.Bits()) - 1 + dnsResolverOffset := broadcastOffset - 1 + return binary.BigEndian.Uint32(b[:]), 1, dnsResolverOffset - 1 +} - offset := uint32(util.RandIntn(int(totalIPs-2))) + 1 - - candidate := baseIP + offset - return uint32ToIP(candidate), nil +// randomOffset returns a random offset in [firstOffset, lastOffset]. +func randomOffset(firstOffset, lastOffset uint32) uint32 { + return firstOffset + uint32(util.RandIntn(int(lastOffset-firstOffset+1))) } // AllocateRandomPeerIPv6 picks a random host address within the given IPv6 prefix. diff --git a/management/server/types/network_test.go b/management/server/types/network_test.go index 239f72426..aee59f7ed 100644 --- a/management/server/types/network_test.go +++ b/management/server/types/network_test.go @@ -42,12 +42,12 @@ func TestAllocatePeerIP(t *testing.T) { } func TestAllocatePeerIPSmallSubnet(t *testing.T) { - // Test /27 network (10.0.0.0/27) - should only have 30 usable IPs (10.0.0.1 to 10.0.0.30) + // Test /27 network (10.0.0.0/27) - should only have 29 usable IPs (10.0.0.1 to 10.0.0.29) prefix := netip.MustParsePrefix("10.0.0.0/27") var ips []netip.Addr // Allocate all available IPs in the /27 network - for i := 0; i < 30; i++ { + for i := 0; i < 29; i++ { ip, err := AllocatePeerIP(prefix, ips) if err != nil { t.Fatal(err) @@ -61,7 +61,7 @@ func TestAllocatePeerIPSmallSubnet(t *testing.T) { ips = append(ips, ip) } - assert.Len(t, ips, 30) + assert.Len(t, ips, 29) // Verify all IPs are unique uniq := make(map[string]struct{}) @@ -86,13 +86,13 @@ func TestAllocatePeerIPVariousCIDRs(t *testing.T) { cidr string expectedUsable int }{ - {"/30 network", "192.168.1.0/30", 2}, // 4 total - 2 reserved = 2 usable - {"/29 network", "192.168.1.0/29", 6}, // 8 total - 2 reserved = 6 usable - {"/28 network", "192.168.1.0/28", 14}, // 16 total - 2 reserved = 14 usable - {"/27 network", "192.168.1.0/27", 30}, // 32 total - 2 reserved = 30 usable - {"/26 network", "192.168.1.0/26", 62}, // 64 total - 2 reserved = 62 usable - {"/25 network", "192.168.1.0/25", 126}, // 128 total - 2 reserved = 126 usable - {"/16 network", "10.0.0.0/16", 65534}, // 65536 total - 2 reserved = 65534 usable + {"/30 network", "192.168.1.0/30", 1}, // 4 total - 3 reserved = 1 usable + {"/29 network", "192.168.1.0/29", 5}, // 8 total - 3 reserved = 5 usable + {"/28 network", "192.168.1.0/28", 13}, // 16 total - 3 reserved = 13 usable + {"/27 network", "192.168.1.0/27", 29}, // 32 total - 3 reserved = 29 usable + {"/26 network", "192.168.1.0/26", 61}, // 64 total - 3 reserved = 61 usable + {"/25 network", "192.168.1.0/25", 125}, // 128 total - 3 reserved = 125 usable + {"/16 network", "10.0.0.0/16", 65533}, // 65536 total - 3 reserved = 65533 usable } for _, tc := range testCases { @@ -117,14 +117,16 @@ func TestAllocatePeerIPVariousCIDRs(t *testing.T) { // Verify IP is within the correct range assert.True(t, prefix.Contains(ip), "allocated IP %s is not within network %s", ip.String(), prefix.String()) - // Verify IP is not network or broadcast address + // Verify IP is not network, DNS resolver or broadcast address networkAddr := prefix.Masked().Addr() hostBits := 32 - prefix.Bits() b := networkAddr.As4() baseIP := binary.BigEndian.Uint32(b[:]) + dnsIP := uint32ToIP(baseIP + (1 << hostBits) - 2) broadcastIP := uint32ToIP(baseIP + (1 << hostBits) - 1) assert.NotEqual(t, networkAddr, ip, "allocated network address %s", ip.String()) + assert.NotEqual(t, dnsIP, ip, "allocated DNS resolver address %s", ip.String()) assert.NotEqual(t, broadcastIP, ip, "allocated broadcast address %s", ip.String()) ips = append(ips, ip) @@ -143,6 +145,68 @@ func TestAllocatePeerIPVariousCIDRs(t *testing.T) { } } +func TestAllocatePeerIPSkipsDNSResolverIP(t *testing.T) { + prefix := netip.MustParsePrefix("192.168.1.0/29") + ips := []netip.Addr{ + netip.MustParseAddr("192.168.1.1"), + netip.MustParseAddr("192.168.1.2"), + netip.MustParseAddr("192.168.1.3"), + netip.MustParseAddr("192.168.1.4"), + } + + ip, err := AllocatePeerIP(prefix, ips) + require.NoError(t, err) + assert.Equal(t, netip.MustParseAddr("192.168.1.5"), ip) + + _, err = AllocatePeerIP(prefix, append(ips, ip)) + assert.Error(t, err, "192.168.1.6 is the DNS resolver address and must not be allocated") +} + +func TestAllocateRandomPeerIPSkipsReservedIPs(t *testing.T) { + for _, cidr := range []string{"192.168.1.0/30", "192.168.1.0/29"} { + t.Run(cidr, func(t *testing.T) { + prefix := netip.MustParsePrefix(cidr) + for i := 0; i < 1000; i++ { + ip, err := AllocateRandomPeerIP(prefix) + require.NoError(t, err) + require.True(t, prefix.Contains(ip), "allocated IP %s is not within network %s", ip, prefix) + require.False(t, IsReservedPeerIP(prefix, ip), "allocated reserved IP %s", ip) + } + }) + } + + ip, err := AllocateRandomPeerIP(netip.MustParsePrefix("192.168.1.0/30")) + require.NoError(t, err) + assert.Equal(t, netip.MustParseAddr("192.168.1.1"), ip) +} + +func TestIsReservedPeerIP(t *testing.T) { + prefix := netip.MustParsePrefix("100.64.0.0/24") + testCases := []struct { + ip string + reserved bool + }{ + {"100.64.0.0", true}, + {"100.64.0.1", false}, + {"100.64.0.253", false}, + {"100.64.0.254", true}, + {"100.64.0.255", true}, + {"::ffff:100.64.0.254", true}, + {"100.64.1.254", false}, + {"fd12:3456:7890:abcd::1", false}, + } + + for _, tc := range testCases { + t.Run(tc.ip, func(t *testing.T) { + assert.Equal(t, tc.reserved, IsReservedPeerIP(prefix, netip.MustParseAddr(tc.ip))) + }) + } + + smallest := netip.MustParsePrefix("192.168.1.0/30") + assert.False(t, IsReservedPeerIP(smallest, netip.MustParseAddr("192.168.1.1"))) + assert.True(t, IsReservedPeerIP(smallest, netip.MustParseAddr("192.168.1.2"))) +} + func TestAllocateIPv4InvalidPrefixes(t *testing.T) { prefixes := []netip.Prefix{ {},