From 87631cbc8b48ba225d6d494ceebebb39bb54c0ee Mon Sep 17 00:00:00 2001 From: Misha Bragin Date: Sun, 29 May 2022 22:43:39 +0200 Subject: [PATCH] Replace IP allocation logic (#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 | 2 +- go.mod | 1 + go.sum | 2 + management/server/account_test.go | 21 ++-- management/server/management_proto_test.go | 6 +- management/server/management_test.go | 10 +- management/server/network.go | 106 ++++++++++++--------- management/server/network_test.go | 39 ++++++++ management/server/testdata/store.json | 2 +- 9 files changed, 122 insertions(+), 67 deletions(-) create mode 100644 management/server/network_test.go diff --git a/client/testdata/store.json b/client/testdata/store.json index d2c4743b0..8236f2703 100644 --- a/client/testdata/store.json +++ b/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 }, diff --git a/go.mod b/go.mod index b53602215..3d362d70f 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 64876ddd8..8a52afa8d 100644 --- a/go.sum +++ b/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= diff --git a/management/server/account_test.go b/management/server/account_test.go index 7120f046e..432b17d3f 100644 --- a/management/server/account_test.go +++ b/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, "") } diff --git a/management/server/management_proto_test.go b/management/server/management_proto_test.go index 09e9e668c..877e7c50a 100644 --- a/management/server/management_proto_test.go +++ b/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 { diff --git a/management/server/management_test.go b/management/server/management_test.go index f6fcbbe81..75a258009 100644 --- a/management/server/management_test.go +++ b/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) } diff --git a/management/server/network.go b/management/server/network.go index f2fb0831d..8b9cd1d9b 100644 --- a/management/server/network.go +++ b/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 - } - for ip := ipNet.IP.Mask(ipNet.Mask); ipNet.Contains(ip); ip = GetNextIP(ip) { - if _, ok := takenIpMap[ip.String()]; !ok { - return ip, nil - } + takenIPMap[ip.String()] = struct{}{} } - return nil, fmt.Errorf("failed allocating new IP for the ipNet %s and takenIps %s", ipNet.String(), takenIps) + 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) + } + + 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) { + + 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)) + } } - 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]) - } - 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 + } } } diff --git a/management/server/network_test.go b/management/server/network_test.go new file mode 100644 index 000000000..892afa5e4 --- /dev/null +++ b/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()) + } + } +} diff --git a/management/server/testdata/store.json b/management/server/testdata/store.json index 1bd7311a1..8eddeca23 100644 --- a/management/server/testdata/store.json +++ b/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 },