Replace IP allocation logic (netbirdio#342)

The peer IP allocation logic was allocating sequential peer IP from the 100.64.0.0/10 
address block.
Each account is created with a random subnet from 100.64.0.0/10.
The total amount of potential subnets is 64.
The new logic allocates random peer IP
from the account subnet.
This gives us flexibility to add support for
multi subnet accounts without overlapping IPs.

client/testdata/store.json

@@ -18,7 +18,7 @@
                 "Id": "af1c8024-ha40-4ce2-9418-34653101fc3c",
                 "Net": {
                     "IP": "100.64.0.0",
-                    "Mask": "/8AAAA=="
+                    "Mask": "//8AAA=="
                 },
                 "Dns": null
             },

go.mod

@@ -29,6 +29,7 @@ require (
 
 require (
 	fyne.io/fyne/v2 v2.1.4
+	github.com/c-robinson/iplib v1.0.3
 	github.com/getlantern/systray v1.2.1
 	github.com/magiconair/properties v1.8.5
 	github.com/rs/xid v1.3.0

go.sum

@@ -70,6 +70,8 @@ github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
 github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
+github.com/c-robinson/iplib v1.0.3 h1:NG0UF0GoEsrC1/vyfX1Lx2Ss7CySWl3KqqXh3q4DdPU=
+github.com/c-robinson/iplib v1.0.3/go.mod h1:i3LuuFL1hRT5gFpBRnEydzw8R6yhGkF4szNDIbF8pgo=
 github.com/cenkalti/backoff/v4 v4.1.2 h1:6Yo7N8UP2K6LWZnW94DLVSSrbobcWdVzAYOisuDPIFo=
 github.com/cenkalti/backoff/v4 v4.1.2/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=

management/server/account_test.go

@@ -265,10 +265,6 @@ func TestAccountManager_AddAccount(t *testing.T) {
 	userId := "account_creator"
 	expectedPeersSize := 0
 	expectedSetupKeysSize := 2
-	expectedNetwork := net.IPNet{
-		IP:   net.IP{100, 64, 0, 0},
-		Mask: net.IPMask{255, 255, 0, 0},
-	}
 
 	account, err := manager.AddAccount(expectedId, userId, "")
 	if err != nil {
@@ -287,8 +283,9 @@ func TestAccountManager_AddAccount(t *testing.T) {
 		t.Errorf("expected account to have len(SetupKeys) = %v, got %v", expectedSetupKeysSize, len(account.SetupKeys))
 	}
 
-	if account.Network.Net.String() != expectedNetwork.String() {
-		t.Errorf("expected account to have Network = %v, got %v", expectedNetwork.String(), account.Network.Net.String())
+	ipNet := net.IPNet{IP: net.ParseIP("100.64.0.0"), Mask: net.IPMask{255, 192, 0, 0}}
+	if !ipNet.Contains(account.Network.Net.IP) {
+		t.Errorf("expected account's Network to be a subnet of %v, got %v", ipNet.String(), account.Network.Net.String())
 	}
 }
 
@@ -419,7 +416,6 @@ func TestAccountManager_AddPeer(t *testing.T) {
 		return
 	}
 	expectedPeerKey := key.PublicKey().String()
-	expectedPeerIP := "100.64.0.1"
 	expectedSetupKey := setupKey.Key
 
 	peer, err := manager.AddPeer(setupKey.Key, "", &Peer{
@@ -442,8 +438,8 @@ func TestAccountManager_AddPeer(t *testing.T) {
 		t.Errorf("expecting just added peer to have key = %s, got %s", expectedPeerKey, peer.Key)
 	}
 
-	if peer.IP.String() != expectedPeerIP {
-		t.Errorf("expecting just added peer to have IP = %s, got %s", expectedPeerIP, peer.IP.String())
+	if !account.Network.Net.Contains(peer.IP) {
+		t.Errorf("expecting just added peer's IP %s to be in a network range %s", peer.IP.String(), account.Network.Net.String())
 	}
 
 	if peer.SetupKey != expectedSetupKey {
@@ -482,7 +478,6 @@ func TestAccountManager_AddPeerWithUserID(t *testing.T) {
 		return
 	}
 	expectedPeerKey := key.PublicKey().String()
-	expectedPeerIP := "100.64.0.1"
 	expectedUserId := userId
 
 	peer, err := manager.AddPeer("", userId, &Peer{
@@ -505,8 +500,8 @@ func TestAccountManager_AddPeerWithUserID(t *testing.T) {
 		t.Errorf("expecting just added peer to have key = %s, got %s", expectedPeerKey, peer.Key)
 	}
 
-	if peer.IP.String() != expectedPeerIP {
-		t.Errorf("expecting just added peer to have IP = %s, got %s", expectedPeerIP, peer.IP.String())
+	if !account.Network.Net.Contains(peer.IP) {
+		t.Errorf("expecting just added peer's IP %s to be in a network range %s", peer.IP.String(), account.Network.Net.String())
 	}
 
 	if peer.UserID != expectedUserId {
@@ -596,7 +591,7 @@ func TestGetUsersFromAccount(t *testing.T) {
 	for _, userInfo := range userInfos {
 		id := userInfo.ID
 		assert.Equal(t, userInfo.ID, users[id].Id)
-		assert.Equal(t, string(userInfo.Role), string(users[id].Role))
+		assert.Equal(t, userInfo.Role, string(users[id].Role))
 		assert.Equal(t, userInfo.Name, "")
 		assert.Equal(t, userInfo.Email, "")
 	}

management/server/management_proto_test.go

@@ -251,8 +251,10 @@ func Test_SyncProtocol(t *testing.T) {
 		t.Fatal("expecting SyncResponse to have NetworkMap with a non-nil PeerConfig")
 	}
 
-	if networkMap.GetPeerConfig().GetAddress() != "100.64.0.1/16" {
-		t.Fatal("expecting SyncResponse to have NetworkMap with a PeerConfig having valid Address")
+	expectedIPNet := net.IPNet{IP: net.ParseIP("100.64.0.0"), Mask: net.IPMask{255, 192, 0, 0}}
+	ip, _, _ := net.ParseCIDR(networkMap.GetPeerConfig().GetAddress())
+	if !expectedIPNet.Contains(ip) {
+		t.Fatalf("expecting SyncResponse to have NetworkMap with a PeerConfig having valid IP address %s", networkMap.GetPeerConfig().GetAddress())
 	}
 
 	if networkMap.GetSerial() <= 0 {

management/server/management_test.go

@@ -107,8 +107,6 @@ var _ = Describe("Management service", func() {
 				err = encryption.DecryptMessage(serverPubKey, key, encryptedResponse.Body, resp)
 				Expect(err).NotTo(HaveOccurred())
 
-				Expect(resp.PeerConfig.Address).To(BeEquivalentTo("100.64.0.1/16"))
-
 				expectedSignalConfig := &mgmtProto.HostConfig{
 					Uri:      "signal.wiretrustee.com:10000",
 					Protocol: mgmtProto.HostConfig_HTTP,
@@ -308,10 +306,10 @@ var _ = Describe("Management service", func() {
 		})
 	})
 
-	Context("when there are 50 peers registered under one account", func() {
+	Context("when there are 10 peers registered under one account", func() {
 		Context("when there are 10 more peers registered under the same account", func() {
-			Specify("all of the 50 peers will get updates of 10 newly registered peers", func() {
-				initialPeers := 20
+			Specify("all of the 10 peers will get updates of 10 newly registered peers", func() {
+				initialPeers := 10
 				additionalPeers := 10
 
 				var peers []wgtypes.Key
@@ -367,7 +365,7 @@ var _ = Describe("Management service", func() {
 					key, _ := wgtypes.GenerateKey()
 					loginPeerWithValidSetupKey(serverPubKey, key, client)
 					rand.Seed(time.Now().UnixNano())
-					n := rand.Intn(500)
+					n := rand.Intn(200)
 					time.Sleep(time.Duration(n) * time.Millisecond)
 				}
 

management/server/network.go

@@ -1,16 +1,13 @@
 package server
 
 import (
-	"encoding/binary"
 	"fmt"
+	"github.com/c-robinson/iplib"
 	"github.com/rs/xid"
+	"math/rand"
 	"net"
 	"sync"
-)
-
-var (
-	upperIPv4 = []byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xff, 0xff, 255, 255, 255, 255}
-	upperIPv6 = []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}
+	"time"
 )
 
 type NetworkMap struct {
@@ -30,10 +27,19 @@ type Network struct {
 }
 
 // NewNetwork creates a new Network initializing it with a Serial=0
+// It takes a random /16 subnet from 100.64.0.0/10 (64 different subnets)
 func NewNetwork() *Network {
+
+	n := iplib.NewNet4(net.ParseIP("100.64.0.0"), 10)
+	sub, _ := n.Subnet(16)
+
+	s := rand.NewSource(time.Now().Unix())
+	r := rand.New(s)
+	intn := r.Intn(len(sub))
+
 	return &Network{
 		Id:     xid.New().String(),
-		Net:    net.IPNet{IP: net.ParseIP("100.64.0.0"), Mask: net.IPMask{255, 255, 0, 0}},
+		Net:    sub[intn].IPNet,
 		Dns:    "",
 		Serial: 0}
 }
@@ -63,51 +69,63 @@ func (n *Network) Copy() *Network {
 
 // AllocatePeerIP pics an available IP from an net.IPNet.
 // This method considers already taken IPs and reuses IPs if there are gaps in takenIps
-// E.g. if ipNet=100.30.0.0/16 and takenIps=[100.30.0.1, 100.30.0.5] then the result would be 100.30.0.2
+// E.g. if ipNet=100.30.0.0/16 and takenIps=[100.30.0.1, 100.30.0.4] then the result would be 100.30.0.2 or 100.30.0.3
 func AllocatePeerIP(ipNet net.IPNet, takenIps []net.IP) (net.IP, error) {
-	takenIpMap := make(map[string]net.IP)
-	takenIpMap[ipNet.IP.String()] = ipNet.IP
+	takenIPMap := make(map[string]struct{})
+	takenIPMap[ipNet.IP.String()] = struct{}{}
 	for _, ip := range takenIps {
-		takenIpMap[ip.String()] = ip
+		takenIPMap[ip.String()] = struct{}{}
 	}
-	for ip := ipNet.IP.Mask(ipNet.Mask); ipNet.Contains(ip); ip = GetNextIP(ip) {
-		if _, ok := takenIpMap[ip.String()]; !ok {
-			return ip, nil
-		}
+
+	ips, _, err := generateIPs(&ipNet, takenIPMap)
+	if err != nil {
+		return nil, fmt.Errorf("failed allocating new IP for the ipNet %s and takenIps %s", ipNet.String(), takenIps)
 	}
 
-	return nil, fmt.Errorf("failed allocating new IP for the ipNet %s and takenIps %s", ipNet.String(), takenIps)
+	if len(ips) == 0 {
+		return nil, fmt.Errorf("failed allocating new IP for the ipNet %s - network is out of IPs", ipNet.String())
+	}
+
+	// pick a random IP
+	s := rand.NewSource(time.Now().Unix())
+	r := rand.New(s)
+	intn := r.Intn(len(ips))
+
+	return ips[intn], nil
 }
 
-// GetNextIP returns the next IP from the given IP address. If the given IP is
-// the last IP of a v4 or v6 range, the same IP is returned.
-// Credits to Cilium team.
-// Copyright 2017-2020 Authors of Cilium
-func GetNextIP(ip net.IP) net.IP {
-	if ip.Equal(upperIPv4) || ip.Equal(upperIPv6) {
-		return ip
-	}
+// generateIPs generates a list of all possible IPs of the given network excluding IPs specified in the exclusion list
+func generateIPs(ipNet *net.IPNet, exclusions map[string]struct{}) ([]net.IP, int, error) {
 
-	nextIP := make(net.IP, len(ip))
-	switch len(ip) {
-	case net.IPv4len:
-		ipU32 := binary.BigEndian.Uint32(ip)
-		ipU32++
-		binary.BigEndian.PutUint32(nextIP, ipU32)
-		return nextIP
-	case net.IPv6len:
-		ipU64 := binary.BigEndian.Uint64(ip[net.IPv6len/2:])
-		ipU64++
-		binary.BigEndian.PutUint64(nextIP[net.IPv6len/2:], ipU64)
-		if ipU64 == 0 {
-			ipU64 = binary.BigEndian.Uint64(ip[:net.IPv6len/2])
-			ipU64++
-			binary.BigEndian.PutUint64(nextIP[:net.IPv6len/2], ipU64)
-		} else {
-			copy(nextIP[:net.IPv6len/2], ip[:net.IPv6len/2])
+	var ips []net.IP
+	for ip := ipNet.IP.Mask(ipNet.Mask); ipNet.Contains(ip); incIP(ip) {
+		if _, ok := exclusions[ip.String()]; !ok && ip[3] != 0 {
+			ips = append(ips, copyIP(ip))
 		}
-		return nextIP
+	}
+
+	// remove network address and broadcast address
+	lenIPs := len(ips)
+	switch {
+	case lenIPs < 2:
+		return ips, lenIPs, nil
+
 	default:
-		return ip
+		return ips[1 : len(ips)-1], lenIPs - 2, nil
+	}
+}
+
+func copyIP(ip net.IP) net.IP {
+	dup := make(net.IP, len(ip))
+	copy(dup, ip)
+	return dup
+}
+
+func incIP(ip net.IP) {
+	for j := len(ip) - 1; j >= 0; j-- {
+		ip[j]++
+		if ip[j] > 0 {
+			break
+		}
 	}
 }

management/server/network_test.go

@@ -0,0 +1,39 @@
+package server
+
+import (
+	"github.com/stretchr/testify/assert"
+	"net"
+	"testing"
+)
+
+func TestNewNetwork(t *testing.T) {
+	network := NewNetwork()
+
+	// generated net should be a subnet of a larger 100.64.0.0/10 net
+	ipNet := net.IPNet{IP: net.ParseIP("100.64.0.0"), Mask: net.IPMask{255, 192, 0, 0}}
+	assert.Equal(t, ipNet.Contains(network.Net.IP), true)
+}
+
+func TestAllocatePeerIP(t *testing.T) {
+
+	ipNet := net.IPNet{IP: net.ParseIP("100.64.0.0"), Mask: net.IPMask{255, 255, 255, 0}}
+	var ips []net.IP
+	for i := 0; i < 253; i++ {
+		ip, err := AllocatePeerIP(ipNet, ips)
+		if err != nil {
+			t.Fatal(err)
+		}
+		ips = append(ips, ip)
+	}
+
+	assert.Len(t, ips, 253)
+
+	uniq := make(map[string]struct{})
+	for _, ip := range ips {
+		if _, ok := uniq[ip.String()]; !ok {
+			uniq[ip.String()] = struct{}{}
+		} else {
+			t.Errorf("found duplicate IP %s", ip.String())
+		}
+	}
+}

management/server/testdata/store.json

@@ -21,7 +21,7 @@
                 "Id": "af1c8024-ha40-4ce2-9418-34653101fc3c",
                 "Net": {
                     "IP": "100.64.0.0",
-                    "Mask": "/8AAAA=="
+                    "Mask": "//8AAA=="
                 },
                 "Dns": null
             },