From 9adadfade405785654e7dca4313cc9e6950e8550 Mon Sep 17 00:00:00 2001 From: Misha Bragin Date: Fri, 3 Feb 2023 10:33:28 +0100 Subject: [PATCH] Use Peer.ID instead of Peer.Key as peer identifier (#664) Replace Peer.Key as internal identifier with a randomly generated Peer.ID in the Management service. Every group now references peers by ID instead of a public key. Every route now references peers by ID instead of a public key. FileStore does store.json file migration on startup by generating Peer.ID and replacing all Peer.Key identifier references . --- management/server/account.go | 68 ++++--- management/server/account_test.go | 20 +-- management/server/dns_test.go | 53 ++++-- management/server/file_store.go | 71 +++++++- management/server/file_store_test.go | 1 + management/server/group.go | 19 +- management/server/grpcserver.go | 26 +-- management/server/http/groups.go | 21 ++- management/server/http/groups_test.go | 8 +- management/server/http/peers.go | 34 ++-- management/server/http/routes.go | 48 ++--- management/server/http/routes_test.go | 37 ++-- management/server/metrics/selfhosted.go | 2 +- management/server/mock_server/account_mock.go | 24 +-- management/server/peer.go | 113 ++++++------ management/server/peer_test.go | 24 +-- management/server/route.go | 41 +++-- management/server/route_test.go | 170 ++++++++++-------- management/server/store.go | 3 +- management/server/turncredentials.go | 26 +-- management/server/updatechannel.go | 33 ++-- route/route.go | 2 +- 22 files changed, 485 insertions(+), 359 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 5c49c582c..9c7ad0709 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -50,24 +50,24 @@ type AccountManager interface { GetAccountFromToken(claims jwtclaims.AuthorizationClaims) (*Account, *User, error) IsUserAdmin(claims jwtclaims.AuthorizationClaims) (bool, error) AccountExists(accountId string) (*bool, error) - GetPeer(peerKey string) (*Peer, error) + GetPeerByKey(peerKey string) (*Peer, error) GetPeers(accountID, userID string) ([]*Peer, error) MarkPeerConnected(peerKey string, connected bool) error - DeletePeer(accountID, peerKey, userID string) (*Peer, error) + DeletePeer(accountID, peerID, userID string) (*Peer, error) GetPeerByIP(accountId string, peerIP string) (*Peer, error) UpdatePeer(accountID, userID string, peer *Peer) (*Peer, error) - GetNetworkMap(peerKey string) (*NetworkMap, error) - GetPeerNetwork(peerKey string) (*Network, error) + GetNetworkMap(peerID string) (*NetworkMap, error) + GetPeerNetwork(peerID string) (*Network, error) AddPeer(setupKey, userID string, peer *Peer) (*Peer, error) - UpdatePeerMeta(peerKey string, meta PeerSystemMeta) error - UpdatePeerSSHKey(peerKey string, sshKey string) error + UpdatePeerMeta(peerID string, meta PeerSystemMeta) error + UpdatePeerSSHKey(peerID string, sshKey string) error GetUsersFromAccount(accountID, userID string) ([]*UserInfo, error) GetGroup(accountId, groupID string) (*Group, error) SaveGroup(accountID, userID string, group *Group) error UpdateGroup(accountID string, groupID string, operations []GroupUpdateOperation) (*Group, error) DeleteGroup(accountId, groupID string) error ListGroups(accountId string) ([]*Group, error) - GroupAddPeer(accountId, groupID, peerKey string) error + GroupAddPeer(accountId, groupID, peerID string) error GroupDeletePeer(accountId, groupID, peerKey string) error GroupListPeers(accountId, groupID string) ([]*Peer, error) GetRule(accountID, ruleID, userID string) (*Rule, error) @@ -76,7 +76,7 @@ type AccountManager interface { DeleteRule(accountID, ruleID, userID string) error ListRules(accountID, userID string) ([]*Rule, error) GetRoute(accountID, routeID, userID string) (*route.Route, error) - CreateRoute(accountID string, prefix, peerIP, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) + CreateRoute(accountID string, prefix, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) SaveRoute(accountID, userID string, route *route.Route) error UpdateRoute(accountID, routeID string, operations []RouteUpdateOperation) (*route.Route, error) DeleteRoute(accountID, routeID, userID string) error @@ -144,7 +144,8 @@ type UserInfo struct { } // getRoutesToSync returns the enabled routes for the peer ID and the routes -// from the ACL peers that have distribution groups associated with the peer ID +// from the ACL peers that have distribution groups associated with the peer ID. +// Please mind, that the returned route.Route objects will contain Peer.Key instead of Peer.ID. func (a *Account) getRoutesToSync(peerID string, aclPeers []*Peer) []*route.Route { routes, peerDisabledRoutes := a.getEnabledAndDisabledRoutesByPeer(peerID) peerRoutesMembership := make(lookupMap) @@ -154,7 +155,7 @@ func (a *Account) getRoutesToSync(peerID string, aclPeers []*Peer) []*route.Rout groupListMap := a.getPeerGroups(peerID) for _, peer := range aclPeers { - activeRoutes, _ := a.getEnabledAndDisabledRoutesByPeer(peer.Key) + activeRoutes, _ := a.getEnabledAndDisabledRoutesByPeer(peer.ID) groupFilteredRoutes := a.filterRoutesByGroups(activeRoutes, groupListMap) filteredRoutes := a.filterRoutesFromPeersOfSameHAGroup(groupFilteredRoutes, peerRoutesMembership) routes = append(routes, filteredRoutes...) @@ -190,18 +191,27 @@ func (a *Account) filterRoutesByGroups(routes []*route.Route, groupListMap looku return filteredRoutes } -// getEnabledAndDisabledRoutesByPeer returns the enabled and disabled lists of routes that belong to a peer -func (a *Account) getEnabledAndDisabledRoutesByPeer(peerPubKey string) ([]*route.Route, []*route.Route) { - //TODO Peer.ID migration: we will need to replace search by Peer.ID here +// getEnabledAndDisabledRoutesByPeer returns the enabled and disabled lists of routes that belong to a peer. +// Please mind, that the returned route.Route objects will contain Peer.Key instead of Peer.ID. +func (a *Account) getEnabledAndDisabledRoutesByPeer(peerID string) ([]*route.Route, []*route.Route) { var enabledRoutes []*route.Route var disabledRoutes []*route.Route for _, r := range a.Routes { - if r.Peer == peerPubKey { - if r.Enabled { - enabledRoutes = append(enabledRoutes, r) + if r.Peer == peerID { + // We need to set Peer.Key instead of Peer.ID because this object will be sent to agents as part of a network map. + // Ideally we should have a separate field for that, but fine for now. + peer := a.GetPeer(peerID) + if peer == nil { + log.Errorf("route %s has peer %s that doesn't exist under account %s", r.ID, peerID, a.Id) continue } - disabledRoutes = append(disabledRoutes, r) + raut := r.Copy() + raut.Peer = peer.Key + if r.Enabled { + enabledRoutes = append(enabledRoutes, raut) + continue + } + disabledRoutes = append(disabledRoutes, raut) } } return enabledRoutes, disabledRoutes @@ -232,7 +242,7 @@ func (a *Account) GetPeerByIP(peerIP string) *Peer { } // GetPeerRules returns a list of source or destination rules of a given peer. -func (a *Account) GetPeerRules(peerPubKey string) (srcRules []*Rule, dstRules []*Rule) { +func (a *Account) GetPeerRules(peerID string) (srcRules []*Rule, dstRules []*Rule) { // Rules are group based so there is no direct access to peers. // First, find all groups that the given peer belongs to @@ -240,7 +250,7 @@ func (a *Account) GetPeerRules(peerPubKey string) (srcRules []*Rule, dstRules [] for s, group := range a.Groups { for _, peer := range group.Peers { - if peerPubKey == peer { + if peerID == peer { peerGroups[s] = struct{}{} break } @@ -284,18 +294,15 @@ func (a *Account) GetPeers() []*Peer { // UpdatePeer saves new or replaces existing peer func (a *Account) UpdatePeer(update *Peer) { - //TODO Peer.ID migration: we will need to replace search by Peer.ID here - a.Peers[update.Key] = update + a.Peers[update.ID] = update } // DeletePeer deletes peer from the account cleaning up all the references -func (a *Account) DeletePeer(peerPubKey string) { - // TODO Peer.ID migration: we will need to replace search by Peer.ID here - +func (a *Account) DeletePeer(peerID string) { // delete peer from groups for _, g := range a.Groups { for i, pk := range g.Peers { - if pk == peerPubKey { + if pk == peerID { g.Peers = append(g.Peers[:i], g.Peers[i+1:]...) break } @@ -303,13 +310,13 @@ func (a *Account) DeletePeer(peerPubKey string) { } for _, r := range a.Routes { - if r.Peer == peerPubKey { + if r.Peer == peerID { r.Enabled = false r.Peer = "" } } - delete(a.Peers, peerPubKey) + delete(a.Peers, peerID) a.Network.IncSerial() } @@ -476,6 +483,11 @@ func (a *Account) GetGroupAll() (*Group, error) { return nil, fmt.Errorf("no group ALL found") } +// GetPeer looks up a Peer by ID +func (a *Account) GetPeer(peerID string) *Peer { + return a.Peers[peerID] +} + // BuildManager creates a new DefaultAccountManager with a provided Store func BuildManager(store Store, peersUpdateManager *PeersUpdateManager, idpManager idp.Manager, singleAccountModeDomain string, dnsDomain string, eventStore activity.Store) (*DefaultAccountManager, error) { @@ -1015,7 +1027,7 @@ func addAllGroup(account *Account) { Name: "All", } for _, peer := range account.Peers { - allGroup.Peers = append(allGroup.Peers, peer.Key) + allGroup.Peers = append(allGroup.Peers, peer.ID) } account.Groups = map[string]*Group{allGroup.ID: allGroup} diff --git a/management/server/account_test.go b/management/server/account_test.go index b99c2e182..58ea86181 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -493,8 +493,8 @@ func TestAccountManager_GetAccount(t *testing.T) { } for _, peer := range account.Peers { - if _, ok := getAccount.Peers[peer.Key]; !ok { - t.Errorf("expected account to have peer %s, not found", peer.Key) + if _, ok := getAccount.Peers[peer.ID]; !ok { + t.Errorf("expected account to have peer %s, not found", peer.ID) } } @@ -580,7 +580,7 @@ func TestAccountManager_AddPeer(t *testing.T) { assert.Equal(t, peer.Name, ev.Meta["name"]) assert.Equal(t, peer.FQDN(account.Domain), ev.Meta["fqdn"]) assert.Equal(t, setupKey.Id, ev.InitiatorID) - assert.Equal(t, peer.IP.String(), ev.TargetID) + assert.Equal(t, peer.ID, ev.TargetID) assert.Equal(t, peer.IP.String(), fmt.Sprint(ev.Meta["ip"])) } @@ -650,7 +650,7 @@ func TestAccountManager_AddPeerWithUserID(t *testing.T) { assert.Equal(t, peer.Name, ev.Meta["name"]) assert.Equal(t, peer.FQDN(account.Domain), ev.Meta["fqdn"]) assert.Equal(t, userID, ev.InitiatorID) - assert.Equal(t, peer.IP.String(), ev.TargetID) + assert.Equal(t, peer.ID, ev.TargetID) assert.Equal(t, peer.IP.String(), fmt.Sprint(ev.Meta["ip"])) } @@ -717,13 +717,13 @@ func TestAccountManager_NetworkUpdates(t *testing.T) { return } - updMsg := manager.peersUpdateManager.CreateChannel(peer1.Key) - defer manager.peersUpdateManager.CloseChannel(peer1.Key) + updMsg := manager.peersUpdateManager.CreateChannel(peer1.ID) + defer manager.peersUpdateManager.CloseChannel(peer1.ID) group := Group{ ID: "group-id", Name: "GroupA", - Peers: []string{peer1.Key, peer2.Key, peer3.Key}, + Peers: []string{peer1.ID, peer2.ID, peer3.ID}, } rule := Rule{ @@ -810,7 +810,7 @@ func TestAccountManager_NetworkUpdates(t *testing.T) { } }() - if _, err := manager.DeletePeer(account.Id, peer3.Key, userID); err != nil { + if _, err := manager.DeletePeer(account.Id, peer3.ID, userID); err != nil { t.Errorf("delete peer: %v", err) return } @@ -1001,13 +1001,13 @@ func TestAccountManager_UpdatePeerMeta(t *testing.T) { OS: "new-OS", WtVersion: "new-WtVersion", } - err = manager.UpdatePeerMeta(peer.Key, newMeta) + err = manager.UpdatePeerMeta(peer.ID, newMeta) if err != nil { t.Error(err) return } - p, err := manager.GetPeer(peer.Key) + p, err := manager.GetPeerByKey(peer.Key) if err != nil { return } diff --git a/management/server/dns_test.go b/management/server/dns_test.go index d1d83269a..155520acb 100644 --- a/management/server/dns_test.go +++ b/management/server/dns_test.go @@ -147,7 +147,17 @@ func TestGetNetworkMap_DNSConfigSync(t *testing.T) { t.Error("failed to init testing account") } - newAccountDNSConfig, err := am.GetNetworkMap(dnsPeer1Key) + peer1, err := account.FindPeerByPubKey(dnsPeer1Key) + if err != nil { + t.Error("failed to init testing account") + } + + peer2, err := account.FindPeerByPubKey(dnsPeer2Key) + if err != nil { + t.Error("failed to init testing account") + } + + newAccountDNSConfig, err := am.GetNetworkMap(peer1.ID) require.NoError(t, err) require.Len(t, newAccountDNSConfig.DNSConfig.CustomZones, 1, "default DNS config should have one custom zone for peers") require.True(t, newAccountDNSConfig.DNSConfig.ServiceEnable, "default DNS config should have local DNS service enabled") @@ -158,12 +168,12 @@ func TestGetNetworkMap_DNSConfigSync(t *testing.T) { err = am.Store.SaveAccount(account) require.NoError(t, err) - updatedAccountDNSConfig, err := am.GetNetworkMap(dnsPeer1Key) + updatedAccountDNSConfig, err := am.GetNetworkMap(peer1.ID) require.NoError(t, err) require.Len(t, updatedAccountDNSConfig.DNSConfig.CustomZones, 0, "updated DNS config should have no custom zone when peer belongs to a disabled group") require.False(t, updatedAccountDNSConfig.DNSConfig.ServiceEnable, "updated DNS config should have local DNS service disabled when peer belongs to a disabled group") - peer2AccountDNSConfig, err := am.GetNetworkMap(dnsPeer2Key) + peer2AccountDNSConfig, err := am.GetNetworkMap(peer2.ID) require.NoError(t, err) require.Len(t, peer2AccountDNSConfig.DNSConfig.CustomZones, 1, "DNS config should have one custom zone for peers not in the disabled group") require.True(t, peer2AccountDNSConfig.DNSConfig.ServiceEnable, "DNS config should have DNS service enabled for peers not in the disabled group") @@ -234,9 +244,33 @@ func initTestDNSAccount(t *testing.T, am *DefaultAccountManager) (*Account, erro return nil, err } + _, err = am.AddPeer("", dnsAdminUserID, peer1) + if err != nil { + return nil, err + } + _, err = am.AddPeer("", dnsAdminUserID, peer2) + if err != nil { + return nil, err + } + + account, err = am.Store.GetAccount(account.Id) + if err != nil { + return nil, err + } + + peer1, err = account.FindPeerByPubKey(peer1.Key) + if err != nil { + return nil, err + } + + _, err = account.FindPeerByPubKey(peer2.Key) + if err != nil { + return nil, err + } + newGroup1 := &Group{ ID: dnsGroup1ID, - Peers: []string{peer1.Key}, + Peers: []string{peer1.ID}, Name: dnsGroup1ID, } @@ -253,14 +287,5 @@ func initTestDNSAccount(t *testing.T, am *DefaultAccountManager) (*Account, erro return nil, err } - _, err = am.AddPeer("", dnsAdminUserID, peer1) - if err != nil { - return nil, err - } - _, err = am.AddPeer("", dnsAdminUserID, peer2) - if err != nil { - return nil, err - } - - return account, nil + return am.Store.GetAccount(account.Id) } diff --git a/management/server/file_store.go b/management/server/file_store.go index cd66d4e5b..fe71b7578 100644 --- a/management/server/file_store.go +++ b/management/server/file_store.go @@ -2,6 +2,7 @@ package server import ( "github.com/netbirdio/netbird/management/server/status" + "github.com/rs/xid" log "github.com/sirupsen/logrus" "os" "path/filepath" @@ -20,6 +21,7 @@ type FileStore struct { Accounts map[string]*Account SetupKeyID2AccountID map[string]string `json:"-"` PeerKeyID2AccountID map[string]string `json:"-"` + PeerID2AccountID map[string]string `json:"-"` UserID2AccountID map[string]string `json:"-"` PrivateDomain2AccountID map[string]string `json:"-"` InstallationID string @@ -53,6 +55,7 @@ func restore(file string) (*FileStore, error) { PeerKeyID2AccountID: make(map[string]string), UserID2AccountID: make(map[string]string), PrivateDomain2AccountID: make(map[string]string), + PeerID2AccountID: make(map[string]string), storeFile: file, } @@ -75,6 +78,7 @@ func restore(file string) (*FileStore, error) { store.PeerKeyID2AccountID = make(map[string]string) store.UserID2AccountID = make(map[string]string) store.PrivateDomain2AccountID = make(map[string]string) + store.PeerID2AccountID = make(map[string]string) for accountID, account := range store.Accounts { for setupKeyId := range account.SetupKeys { @@ -83,6 +87,7 @@ func restore(file string) (*FileStore, error) { for _, peer := range account.Peers { store.PeerKeyID2AccountID[peer.Key] = accountID + store.PeerID2AccountID[peer.ID] = accountID // reset all peers to status = Disconnected if peer.Status != nil && peer.Status.Connected { peer.Status.Connected = false @@ -118,6 +123,47 @@ func restore(file string) (*FileStore, error) { route.Groups = []string{allGroup.ID} } } + + // migration to Peer.ID from Peer.Key. + // Old peers that require migration have an empty Peer.ID in the store.json. + // Generate new ID with xid for these peers. + // Set the Peer.ID to the newly generated value. + // Replace all the mentions of Peer.Key as ID (groups and routes). + // Swap Peer.Key with Peer.ID in the Account.Peers map. + migrationPeers := make(map[string]*Peer) // key to Peer + for key, peer := range account.Peers { + if peer.ID != "" { + continue + } + id := xid.New().String() + peer.ID = id + migrationPeers[key] = peer + } + + if len(migrationPeers) > 0 { + + // swap Peer.Key with Peer.ID in the Account.Peers map. + for key, peer := range migrationPeers { + delete(account.Peers, key) + account.Peers[peer.ID] = peer + store.PeerID2AccountID[peer.ID] = accountID + } + + // detect groups that have Peer.Key as a reference and replace it with ID. + for _, group := range account.Groups { + for i, peer := range group.Peers { + if p, ok := migrationPeers[peer]; ok { + group.Peers[i] = p.ID + } + } + } + // detect routes that have Peer.Key as a reference and replace it with ID. + for _, route := range account.Routes { + if peer, ok := migrationPeers[route.Peer]; ok { + route.Peer = peer.ID + } + } + } } // we need this persist to apply changes we made to account.Peers (we set them to Disconnected) @@ -183,6 +229,7 @@ func (s *FileStore) SaveAccount(account *Account) error { // enforce peer to account index and delete peer to route indexes for rebuild for _, peer := range accountCopy.Peers { s.PeerKeyID2AccountID[peer.Key] = accountCopy.Id + s.PeerID2AccountID[peer.ID] = accountCopy.Id } for _, user := range accountCopy.Users { @@ -284,6 +331,24 @@ func (s *FileStore) GetAccountByUser(userID string) (*Account, error) { return account.Copy(), nil } +// GetAccountByPeerID returns an account for a given peer ID +func (s *FileStore) GetAccountByPeerID(peerID string) (*Account, error) { + s.mux.Lock() + defer s.mux.Unlock() + + accountID, accountIDFound := s.PeerID2AccountID[peerID] + if !accountIDFound { + return nil, status.Errorf(status.NotFound, "provided peer ID doesn't exists %s", peerID) + } + + account, err := s.getAccount(accountID) + if err != nil { + return nil, err + } + + return account.Copy(), nil +} + // GetAccountByPeerPubKey returns an account for a given peer WireGuard public key func (s *FileStore) GetAccountByPeerPubKey(peerKey string) (*Account, error) { s.mux.Lock() @@ -319,7 +384,7 @@ func (s *FileStore) SaveInstallationID(ID string) error { // SavePeerStatus stores the PeerStatus in memory. It doesn't attempt to persist data to speed up things. // PeerStatus will be saved eventually when some other changes occur. -func (s *FileStore) SavePeerStatus(accountID, peerKey string, peerStatus PeerStatus) error { +func (s *FileStore) SavePeerStatus(accountID, peerID string, peerStatus PeerStatus) error { s.mux.Lock() defer s.mux.Unlock() @@ -328,9 +393,9 @@ func (s *FileStore) SavePeerStatus(accountID, peerKey string, peerStatus PeerSta return err } - peer := account.Peers[peerKey] + peer := account.Peers[peerID] if peer == nil { - return status.Errorf(status.NotFound, "peer %s not found", peerKey) + return status.Errorf(status.NotFound, "peer %s not found", peerID) } peer.Status = &peerStatus diff --git a/management/server/file_store_test.go b/management/server/file_store_test.go index 69574bdec..6a4823457 100644 --- a/management/server/file_store_test.go +++ b/management/server/file_store_test.go @@ -244,6 +244,7 @@ func TestFileStore_SavePeerStatus(t *testing.T) { // save new status of existing peer account.Peers["testpeer"] = &Peer{ Key: "peerkey", + ID: "testpeer", SetupKey: "peerkeysetupkey", IP: net.IP{127, 0, 0, 1}, Meta: PeerSystemMeta{}, diff --git a/management/server/group.go b/management/server/group.go index 8ee1222de..2959f3ad5 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -88,6 +88,13 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G return err } + err = am.updateAccountPeers(account) + if err != nil { + return err + } + + // the following snippet tracks the activity and stores the group events in the event store. + // It has to happen after all the operations have been successfully performed. addedPeers := make([]string, 0) removedPeers := make([]string, 0) if exists { @@ -104,7 +111,7 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G log.Errorf("peer %s not found under account %s while saving group", p, accountID) continue } - am.storeEvent(userID, peer.IP.String(), accountID, activity.GroupAddedToPeer, + am.storeEvent(userID, peer.ID, accountID, activity.GroupAddedToPeer, map[string]any{"group": newGroup.Name, "group_id": newGroup.ID, "peer_ip": peer.IP.String(), "peer_fqdn": peer.FQDN(am.GetDNSDomain())}) } @@ -115,12 +122,12 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G log.Errorf("peer %s not found under account %s while saving group", p, accountID) continue } - am.storeEvent(userID, peer.IP.String(), accountID, activity.GroupRemovedFromPeer, + am.storeEvent(userID, peer.ID, accountID, activity.GroupRemovedFromPeer, map[string]any{"group": newGroup.Name, "group_id": newGroup.ID, "peer_ip": peer.IP.String(), "peer_fqdn": peer.FQDN(am.GetDNSDomain())}) } - return am.updateAccountPeers(account) + return nil } // difference returns the elements in `a` that aren't in `b`. @@ -230,7 +237,7 @@ func (am *DefaultAccountManager) ListGroups(accountID string) ([]*Group, error) } // GroupAddPeer appends peer to the group -func (am *DefaultAccountManager) GroupAddPeer(accountID, groupID, peerKey string) error { +func (am *DefaultAccountManager) GroupAddPeer(accountID, groupID, peerID string) error { unlock := am.Store.AcquireAccountLock(accountID) defer unlock() @@ -247,13 +254,13 @@ func (am *DefaultAccountManager) GroupAddPeer(accountID, groupID, peerKey string add := true for _, itemID := range group.Peers { - if itemID == peerKey { + if itemID == peerID { add = false break } } if add { - group.Peers = append(group.Peers, peerKey) + group.Peers = append(group.Peers, peerID) } account.Network.IncSerial() diff --git a/management/server/grpcserver.go b/management/server/grpcserver.go index f0d66f641..9a7ade111 100644 --- a/management/server/grpcserver.go +++ b/management/server/grpcserver.go @@ -111,7 +111,7 @@ func (s *GRPCServer) Sync(req *proto.EncryptedMessage, srv proto.ManagementServi return status.Errorf(codes.InvalidArgument, "provided wgPubKey %s is invalid", peerKey.String()) } - peer, err := s.accountManager.GetPeer(peerKey.String()) + peer, err := s.accountManager.GetPeerByKey(peerKey.String()) if err != nil { p, _ := gPeer.FromContext(srv.Context()) msg := status.Errorf(codes.PermissionDenied, "provided peer with the key wgPubKey %s is not registered, remote addr is %s", peerKey.String(), p.Addr.String()) @@ -134,14 +134,14 @@ func (s *GRPCServer) Sync(req *proto.EncryptedMessage, srv proto.ManagementServi return err } - updates := s.peersUpdateManager.CreateChannel(peerKey.String()) + updates := s.peersUpdateManager.CreateChannel(peer.ID) err = s.accountManager.MarkPeerConnected(peerKey.String(), true) if err != nil { log.Warnf("failed marking peer as connected %s %v", peerKey, err) } if s.config.TURNConfig.TimeBasedCredentials { - s.turnCredentialsManager.SetupRefresh(peerKey.String()) + s.turnCredentialsManager.SetupRefresh(peer.ID) } // keep a connection to the peer and send updates when available for { @@ -171,7 +171,7 @@ func (s *GRPCServer) Sync(req *proto.EncryptedMessage, srv proto.ManagementServi case <-srv.Context().Done(): // happens when connection drops, e.g. client disconnects log.Debugf("stream of peer %s has been closed", peerKey.String()) - s.peersUpdateManager.CloseChannel(peerKey.String()) + s.peersUpdateManager.CloseChannel(peer.ID) s.turnCredentialsManager.CancelRefresh(peerKey.String()) err = s.accountManager.MarkPeerConnected(peerKey.String(), false) if err != nil { @@ -252,19 +252,19 @@ func (s *GRPCServer) registerPeer(peerKey wgtypes.Key, req *proto.LoginRequest) } // todo move to DefaultAccountManager the code below - networkMap, err := s.accountManager.GetNetworkMap(peer.Key) + networkMap, err := s.accountManager.GetNetworkMap(peer.ID) if err != nil { return nil, status.Errorf(codes.Internal, "unable to fetch network map after registering peer, error: %v", err) } // notify other peers of our registration for _, remotePeer := range networkMap.Peers { - remotePeerNetworkMap, err := s.accountManager.GetNetworkMap(remotePeer.Key) + remotePeerNetworkMap, err := s.accountManager.GetNetworkMap(remotePeer.ID) if err != nil { return nil, status.Errorf(codes.Internal, "unable to fetch network map after registering peer, error: %v", err) } update := toSyncResponse(s.config, remotePeer, nil, remotePeerNetworkMap, s.accountManager.GetDNSDomain()) - err = s.peersUpdateManager.SendUpdate(remotePeer.Key, &UpdateMessage{Update: update}) + err = s.peersUpdateManager.SendUpdate(remotePeer.ID, &UpdateMessage{Update: update}) if err != nil { // todo rethink if we should keep this return return nil, status.Errorf(codes.Internal, "unable to send update after registering peer, error: %v", err) @@ -299,7 +299,7 @@ func (s *GRPCServer) Login(ctx context.Context, req *proto.EncryptedMessage) (*p return nil, status.Errorf(codes.InvalidArgument, "invalid request message") } - peer, err := s.accountManager.GetPeer(peerKey.String()) + peer, err := s.accountManager.GetPeerByKey(peerKey.String()) if err != nil { if errStatus, ok := internalStatus.FromError(err); ok && errStatus.Type() == internalStatus.NotFound { // peer doesn't exist -> check if setup key was provided @@ -324,7 +324,7 @@ func (s *GRPCServer) Login(ctx context.Context, req *proto.EncryptedMessage) (*p } } else if loginReq.GetMeta() != nil { // update peer's system meta data on Login - err = s.accountManager.UpdatePeerMeta(peerKey.String(), PeerSystemMeta{ + err = s.accountManager.UpdatePeerMeta(peer.ID, PeerSystemMeta{ Hostname: loginReq.GetMeta().GetHostname(), GoOS: loginReq.GetMeta().GetGoOS(), Kernel: loginReq.GetMeta().GetKernel(), @@ -347,13 +347,13 @@ func (s *GRPCServer) Login(ctx context.Context, req *proto.EncryptedMessage) (*p } if len(sshKey) > 0 { - err = s.accountManager.UpdatePeerSSHKey(peerKey.String(), string(sshKey)) + err = s.accountManager.UpdatePeerSSHKey(peer.ID, string(sshKey)) if err != nil { return nil, err } } - network, err := s.accountManager.GetPeerNetwork(peer.Key) + network, err := s.accountManager.GetPeerNetwork(peer.ID) if err != nil { return nil, status.Errorf(codes.Internal, "failed getting peer network on login") } @@ -491,9 +491,9 @@ func (s *GRPCServer) IsHealthy(ctx context.Context, req *proto.Empty) (*proto.Em // sendInitialSync sends initial proto.SyncResponse to the peer requesting synchronization func (s *GRPCServer) sendInitialSync(peerKey wgtypes.Key, peer *Peer, srv proto.ManagementService_SyncServer) error { - networkMap, err := s.accountManager.GetNetworkMap(peer.Key) + networkMap, err := s.accountManager.GetNetworkMap(peer.ID) if err != nil { - log.Warnf("error getting a list of peers for a peer %s", peer.Key) + log.Warnf("error getting a list of peers for a peer %s", peer.ID) return err } diff --git a/management/server/http/groups.go b/management/server/http/groups.go index 8ee4f6bae..c0bd21ded 100644 --- a/management/server/http/groups.go +++ b/management/server/http/groups.go @@ -96,10 +96,16 @@ func (h *Groups) UpdateGroupHandler(w http.ResponseWriter, r *http.Request) { return } + var peers []string + if req.Peers == nil { + peers = make([]string, 0) + } else { + peers = *req.Peers + } group := server.Group{ ID: groupID, Name: *req.Name, - Peers: peerIPsToKeys(account, req.Peers), + Peers: peers, } if err := h.accountManager.SaveGroup(account.Id, user.Id, &group); err != nil { @@ -191,10 +197,9 @@ func (h *Groups) PatchGroupHandler(w http.ResponseWriter, r *http.Request) { Values: peerKeys, }) case api.GroupPatchOperationOpAdd: - peerKeys := peerIPsToKeys(account, &patch.Value) operations = append(operations, server.GroupUpdateOperation{ Type: server.InsertPeersToGroup, - Values: peerKeys, + Values: patch.Value, }) default: util.WriteError(status.Errorf(status.InvalidArgument, @@ -237,10 +242,16 @@ func (h *Groups) CreateGroupHandler(w http.ResponseWriter, r *http.Request) { return } + var peers []string + if req.Peers == nil { + peers = make([]string, 0) + } else { + peers = *req.Peers + } group := server.Group{ ID: xid.New().String(), Name: req.Name, - Peers: peerIPsToKeys(account, req.Peers), + Peers: peers, } err = h.accountManager.SaveGroup(account.Id, user.Id, &group) @@ -359,7 +370,7 @@ func toGroupResponse(account *server.Account, group *server.Group) *api.Group { continue } peerResp := api.PeerMinimum{ - Id: peer.IP.String(), + Id: peer.ID, Name: peer.Name, } cache[pid] = peerResp diff --git a/management/server/http/groups_test.go b/management/server/http/groups_test.go index 25a66e471..380b52546 100644 --- a/management/server/http/groups_test.go +++ b/management/server/http/groups_test.go @@ -22,8 +22,8 @@ import ( ) var TestPeers = map[string]*server.Peer{ - "A": {Key: "A", IP: net.ParseIP("100.100.100.100")}, - "B": {Key: "B", IP: net.ParseIP("200.200.200.200")}, + "A": {Key: "A", ID: "peer-A-ID", IP: net.ParseIP("100.100.100.100")}, + "B": {Key: "B", ID: "peer-B-ID", IP: net.ParseIP("200.200.200.200")}, } func initGroupTestData(user *server.User, groups ...*server.Group) *Groups { @@ -269,8 +269,8 @@ func TestWriteGroup(t *testing.T) { Id: "id-existed", PeersCount: 2, Peers: []api.PeerMinimum{ - {Id: "100.100.100.100"}, - {Id: "200.200.200.200"}}, + {Id: "peer-A-ID"}, + {Id: "peer-B-ID"}}, }, }, } diff --git a/management/server/http/peers.go b/management/server/http/peers.go index d6c36944f..910802d4b 100644 --- a/management/server/http/peers.go +++ b/management/server/http/peers.go @@ -27,7 +27,7 @@ func NewPeers(accountManager server.AccountManager, authAudience string) *Peers } } -func (h *Peers) updatePeer(account *server.Account, user *server.User, peer *server.Peer, w http.ResponseWriter, r *http.Request) { +func (h *Peers) updatePeer(account *server.Account, user *server.User, peerID string, w http.ResponseWriter, r *http.Request) { req := &api.PutApiPeersIdJSONBody{} err := json.NewDecoder(r.Body).Decode(&req) if err != nil { @@ -35,8 +35,8 @@ func (h *Peers) updatePeer(account *server.Account, user *server.User, peer *ser return } - update := &server.Peer{Key: peer.Key, SSHEnabled: req.SshEnabled, Name: req.Name} - peer, err = h.accountManager.UpdatePeer(account.Id, user.Id, update) + update := &server.Peer{ID: peerID, SSHEnabled: req.SshEnabled, Name: req.Name} + peer, err := h.accountManager.UpdatePeer(account.Id, user.Id, update) if err != nil { util.WriteError(err, w) return @@ -45,8 +45,8 @@ func (h *Peers) updatePeer(account *server.Account, user *server.User, peer *ser util.WriteJSONObject(w, toPeerResponse(peer, account, dnsDomain)) } -func (h *Peers) deletePeer(accountID, userID string, peer *server.Peer, w http.ResponseWriter, r *http.Request) { - _, err := h.accountManager.DeletePeer(accountID, peer.Key, userID) +func (h *Peers) deletePeer(accountID, userID string, peerID string, w http.ResponseWriter) { + _, err := h.accountManager.DeletePeer(accountID, peerID, userID) if err != nil { util.WriteError(err, w) return @@ -62,31 +62,19 @@ func (h *Peers) HandlePeer(w http.ResponseWriter, r *http.Request) { return } vars := mux.Vars(r) - peerId := vars["id"] //effectively peer IP address - if len(peerId) == 0 { + peerID := vars["id"] + if len(peerID) == 0 { util.WriteError(status.Errorf(status.InvalidArgument, "invalid peer ID"), w) return } - peer, err := h.accountManager.GetPeerByIP(account.Id, peerId) - if err != nil { - util.WriteError(err, w) - return - } - - dnsDomain := h.accountManager.GetDNSDomain() - switch r.Method { case http.MethodDelete: - h.deletePeer(account.Id, user.Id, peer, w, r) + h.deletePeer(account.Id, user.Id, peerID, w) return case http.MethodPut: - h.updatePeer(account, user, peer, w, r) + h.updatePeer(account, user, peerID, w, r) return - case http.MethodGet: - util.WriteJSONObject(w, toPeerResponse(peer, account, dnsDomain)) - return - default: util.WriteError(status.Errorf(status.NotFound, "unknown METHOD"), w) } @@ -132,7 +120,7 @@ func toPeerResponse(peer *server.Peer, account *server.Account, dnsDomain string } groupsChecked[group.ID] = struct{}{} for _, pk := range group.Peers { - if pk == peer.Key { + if pk == peer.ID { info := api.GroupMinimum{ Id: group.ID, Name: group.Name, @@ -149,7 +137,7 @@ func toPeerResponse(peer *server.Peer, account *server.Account, dnsDomain string fqdn = peer.DNSLabel } return &api.Peer{ - Id: peer.IP.String(), + Id: peer.ID, Name: peer.Name, Ip: peer.IP.String(), Connected: peer.Status.Connected, diff --git a/management/server/http/routes.go b/management/server/http/routes.go index 5f6bd09e8..93cd5ff78 100644 --- a/management/server/http/routes.go +++ b/management/server/http/routes.go @@ -45,7 +45,7 @@ func (h *Routes) GetAllRoutesHandler(w http.ResponseWriter, r *http.Request) { } apiRoutes := make([]*api.Route, 0) for _, r := range routes { - apiRoutes = append(apiRoutes, toRouteResponse(account, r)) + apiRoutes = append(apiRoutes, toRouteResponse(r)) } util.WriteJSONObject(w, apiRoutes) @@ -85,7 +85,7 @@ func (h *Routes) CreateRouteHandler(w http.ResponseWriter, r *http.Request) { return } - resp := toRouteResponse(account, newRoute) + resp := toRouteResponse(newRoute) util.WriteJSONObject(w, &resp) } @@ -126,16 +126,6 @@ func (h *Routes) UpdateRouteHandler(w http.ResponseWriter, r *http.Request) { return } - peerKey := req.Peer - if req.Peer != "" { - peer := account.GetPeerByIP(req.Peer) - if peer == nil { - util.WriteError(status.Errorf(status.NotFound, "peer %s not found", req.Peer), w) - return - } - peerKey = peer.Key - } - if utf8.RuneCountInString(req.NetworkId) > route.MaxNetIDChar || req.NetworkId == "" { util.WriteError(status.Errorf(status.InvalidArgument, "identifier should be between 1 and %d", route.MaxNetIDChar), w) @@ -148,7 +138,7 @@ func (h *Routes) UpdateRouteHandler(w http.ResponseWriter, r *http.Request) { NetID: req.NetworkId, NetworkType: prefixType, Masquerade: req.Masquerade, - Peer: peerKey, + Peer: req.Peer, Metric: req.Metric, Description: req.Description, Enabled: req.Enabled, @@ -161,7 +151,7 @@ func (h *Routes) UpdateRouteHandler(w http.ResponseWriter, r *http.Request) { return } - resp := toRouteResponse(account, newRoute) + resp := toRouteResponse(newRoute) util.WriteJSONObject(w, &resp) } @@ -245,18 +235,9 @@ func (h *Routes) PatchRouteHandler(w http.ResponseWriter, r *http.Request) { "value field only accepts 1 value, got %d", len(patch.Value)), w) return } - peerValue := patch.Value - if patch.Value[0] != "" { - peer, err := h.accountManager.GetPeerByIP(account.Id, patch.Value[0]) - if err != nil { - util.WriteError(err, w) - return - } - peerValue = []string{peer.Key} - } operations = append(operations, server.RouteUpdateOperation{ Type: server.UpdateRoutePeer, - Values: peerValue, + Values: patch.Value, }) case api.RoutePatchOperationPathMetric: if patch.Op != api.RoutePatchOperationOpReplace { @@ -305,13 +286,13 @@ func (h *Routes) PatchRouteHandler(w http.ResponseWriter, r *http.Request) { } } - route, err := h.accountManager.UpdateRoute(account.Id, routeID, operations) + root, err := h.accountManager.UpdateRoute(account.Id, routeID, operations) if err != nil { util.WriteError(err, w) return } - resp := toRouteResponse(account, route) + resp := toRouteResponse(root) util.WriteJSONObject(w, &resp) } @@ -361,25 +342,16 @@ func (h *Routes) GetRouteHandler(w http.ResponseWriter, r *http.Request) { return } - util.WriteJSONObject(w, toRouteResponse(account, foundRoute)) + util.WriteJSONObject(w, toRouteResponse(foundRoute)) } -func toRouteResponse(account *server.Account, serverRoute *route.Route) *api.Route { - var peerIP string - if serverRoute.Peer != "" { - peer, found := account.Peers[serverRoute.Peer] - if !found { - panic("peer ID not found") - } - peerIP = peer.IP.String() - } - +func toRouteResponse(serverRoute *route.Route) *api.Route { return &api.Route{ Id: serverRoute.ID, Description: serverRoute.Description, NetworkId: serverRoute.NetID, Enabled: serverRoute.Enabled, - Peer: peerIP, + Peer: serverRoute.Peer, Network: serverRoute.Network.String(), NetworkType: serverRoute.NetworkType.String(), Masquerade: serverRoute.Masquerade, diff --git a/management/server/http/routes_test.go b/management/server/http/routes_test.go index ef4011fd1..f4dbec6cd 100644 --- a/management/server/http/routes_test.go +++ b/management/server/http/routes_test.go @@ -24,8 +24,9 @@ import ( const ( existingRouteID = "existingRouteID" notFoundRouteID = "notFoundRouteID" - existingPeerID = "100.64.0.100" - notFoundPeerID = "100.64.0.200" + existingPeerIP = "100.64.0.100" + existingPeerID = "peer-id" + notFoundPeerID = "nonExistingPeer" existingPeerKey = "existingPeerKey" testAccountID = "test_id" existingGroupID = "testGroup" @@ -47,9 +48,10 @@ var testingAccount = &server.Account{ Id: testAccountID, Domain: "hotmail.com", Peers: map[string]*server.Peer{ - existingPeerKey: { + existingPeerID: { Key: existingPeerKey, - IP: netip.MustParseAddr(existingPeerID).AsSlice(), + IP: netip.MustParseAddr(existingPeerIP).AsSlice(), + ID: existingPeerID, }, }, Users: map[string]*server.User{ @@ -66,18 +68,15 @@ func initRoutesTestData() *Routes { } return nil, status.Errorf(status.NotFound, "route with ID %s not found", routeID) }, - CreateRouteFunc: func(accountID string, network, peerIP, description, netID string, masquerade bool, metric int, groups []string, enabled bool, _ string) (*route.Route, error) { - - peer := testingAccount.GetPeerByIP(peerIP) - if peer == nil { - return nil, status.Errorf(status.NotFound, "peer %s not found", peerIP) + CreateRouteFunc: func(accountID string, network, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, _ string) (*route.Route, error) { + if peerID == notFoundPeerID { + return nil, status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID) } - networkType, p, _ := route.ParseNetwork(network) return &route.Route{ ID: existingRouteID, NetID: netID, - Peer: peer.Key, + Peer: peerID, Network: p, NetworkType: networkType, Description: description, @@ -86,7 +85,10 @@ func initRoutesTestData() *Routes { Groups: groups, }, nil }, - SaveRouteFunc: func(_, _ string, _ *route.Route) error { + SaveRouteFunc: func(_, _ string, r *route.Route) error { + if r.Peer == notFoundPeerID { + return status.Errorf(status.InvalidArgument, "peer with ID %s not found", r.Peer) + } return nil }, DeleteRouteFunc: func(_ string, routeID string, _ string) error { @@ -119,6 +121,9 @@ func initRoutesTestData() *Routes { routeToUpdate.NetID = operation.Values[0] case server.UpdateRoutePeer: routeToUpdate.Peer = operation.Values[0] + if routeToUpdate.Peer == notFoundPeerID { + return nil, status.Errorf(status.InvalidArgument, "peer with ID %s not found", routeToUpdate.Peer) + } case server.UpdateRouteMetric: routeToUpdate.Metric, _ = strconv.Atoi(operation.Values[0]) case server.UpdateRouteMasquerade: @@ -166,7 +171,7 @@ func TestRoutesHandlers(t *testing.T) { requestPath: "/api/routes/" + existingRouteID, expectedStatus: http.StatusOK, expectedBody: true, - expectedRoute: toRouteResponse(testingAccount, baseExistingRoute), + expectedRoute: toRouteResponse(baseExistingRoute), }, { name: "Get Not Existing Route", @@ -212,7 +217,7 @@ func TestRoutesHandlers(t *testing.T) { requestType: http.MethodPost, requestPath: "/api/routes", requestBody: bytes.NewBufferString(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"Peer\":\"%s\",\"groups\":[\"%s\"]}", notFoundPeerID, existingGroupID)), - expectedStatus: http.StatusNotFound, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -263,7 +268,7 @@ func TestRoutesHandlers(t *testing.T) { requestType: http.MethodPut, requestPath: "/api/routes/" + existingRouteID, requestBody: bytes.NewBufferString(fmt.Sprintf("{\"Description\":\"Post\",\"Network\":\"192.168.0.0/16\",\"network_id\":\"awesomeNet\",\"Peer\":\"%s\",\"groups\":[\"%s\"]}", notFoundPeerID, existingGroupID)), - expectedStatus: http.StatusNotFound, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { @@ -326,7 +331,7 @@ func TestRoutesHandlers(t *testing.T) { requestType: http.MethodPatch, requestPath: "/api/routes/" + existingRouteID, requestBody: bytes.NewBufferString(fmt.Sprintf("[{\"op\":\"replace\",\"path\":\"peer\",\"value\":[\"%s\"]}]", notFoundPeerID)), - expectedStatus: http.StatusNotFound, + expectedStatus: http.StatusUnprocessableEntity, expectedBody: false, }, { diff --git a/management/server/metrics/selfhosted.go b/management/server/metrics/selfhosted.go index c658a4215..bea9d05e9 100644 --- a/management/server/metrics/selfhosted.go +++ b/management/server/metrics/selfhosted.go @@ -207,7 +207,7 @@ func (w *Worker) generateProperties() properties { osUIClients[uiOSKey] = osUICount + 1 } - _, connected := connections[peer.Key] + _, connected := connections[peer.ID] if connected || peer.Status.LastSeen.After(w.lastRun) { activePeersLastDay++ osActiveKey := osKey + "_active" diff --git a/management/server/mock_server/account_mock.go b/management/server/mock_server/account_mock.go index 81186a40d..7b56ce1a2 100644 --- a/management/server/mock_server/account_mock.go +++ b/management/server/mock_server/account_mock.go @@ -42,8 +42,8 @@ type MockAccountManager struct { DeleteRuleFunc func(accountID, ruleID, userID string) error ListRulesFunc func(accountID, userID string) ([]*server.Rule, error) GetUsersFromAccountFunc func(accountID, userID string) ([]*server.UserInfo, error) - UpdatePeerMetaFunc func(peerKey string, meta server.PeerSystemMeta) error - UpdatePeerSSHKeyFunc func(peerKey string, sshKey string) error + UpdatePeerMetaFunc func(peerID string, meta server.PeerSystemMeta) error + UpdatePeerSSHKeyFunc func(peerID string, sshKey string) error UpdatePeerFunc func(accountID, userID string, peer *server.Peer) (*server.Peer, error) CreateRouteFunc func(accountID string, prefix, peer, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) GetRouteFunc func(accountID, routeID, userID string) (*route.Route, error) @@ -77,9 +77,9 @@ func (am *MockAccountManager) GetUsersFromAccount(accountID string, userID strin } // DeletePeer mock implementation of DeletePeer from server.AccountManager interface -func (am *MockAccountManager) DeletePeer(accountID, peerKey, userID string) (*server.Peer, error) { +func (am *MockAccountManager) DeletePeer(accountID, peerID, userID string) (*server.Peer, error) { if am.DeletePeerFunc != nil { - return am.DeletePeerFunc(accountID, peerKey, userID) + return am.DeletePeerFunc(accountID, peerID, userID) } return nil, status.Errorf(codes.Unimplemented, "method DeletePeer is not implemented") } @@ -143,11 +143,11 @@ func (am *MockAccountManager) AccountExists(accountId string) (*bool, error) { } // GetPeer mock implementation of GetPeer from server.AccountManager interface -func (am *MockAccountManager) GetPeer(peerKey string) (*server.Peer, error) { +func (am *MockAccountManager) GetPeerByKey(peerKey string) (*server.Peer, error) { if am.GetPeerFunc != nil { return am.GetPeerFunc(peerKey) } - return nil, status.Errorf(codes.Unimplemented, "method GetPeer is not implemented") + return nil, status.Errorf(codes.Unimplemented, "method GetPeerByKey is not implemented") } // MarkPeerConnected mock implementation of MarkPeerConnected from server.AccountManager interface @@ -299,9 +299,9 @@ func (am *MockAccountManager) ListRules(accountID, userID string) ([]*server.Rul } // UpdatePeerMeta mock implementation of UpdatePeerMeta from server.AccountManager interface -func (am *MockAccountManager) UpdatePeerMeta(peerKey string, meta server.PeerSystemMeta) error { +func (am *MockAccountManager) UpdatePeerMeta(peerID string, meta server.PeerSystemMeta) error { if am.UpdatePeerMetaFunc != nil { - return am.UpdatePeerMetaFunc(peerKey, meta) + return am.UpdatePeerMetaFunc(peerID, meta) } return status.Errorf(codes.Unimplemented, "method UpdatePeerMetaFunc is not implemented") } @@ -315,9 +315,9 @@ func (am *MockAccountManager) IsUserAdmin(claims jwtclaims.AuthorizationClaims) } // UpdatePeerSSHKey mocks UpdatePeerSSHKey function of the account manager -func (am *MockAccountManager) UpdatePeerSSHKey(peerKey string, sshKey string) error { +func (am *MockAccountManager) UpdatePeerSSHKey(peerID string, sshKey string) error { if am.UpdatePeerSSHKeyFunc != nil { - return am.UpdatePeerSSHKeyFunc(peerKey, sshKey) + return am.UpdatePeerSSHKeyFunc(peerID, sshKey) } return status.Errorf(codes.Unimplemented, "method UpdatePeerSSHKey is is not implemented") } @@ -331,9 +331,9 @@ func (am *MockAccountManager) UpdatePeer(accountID, userID string, peer *server. } // CreateRoute mock implementation of CreateRoute from server.AccountManager interface -func (am *MockAccountManager) CreateRoute(accountID string, network, peerIP, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) { +func (am *MockAccountManager) CreateRoute(accountID string, network, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) { if am.CreateRouteFunc != nil { - return am.CreateRouteFunc(accountID, network, peerIP, description, netID, masquerade, metric, groups, enabled, userID) + return am.CreateRouteFunc(accountID, network, peerID, description, netID, masquerade, metric, groups, enabled, userID) } return nil, status.Errorf(codes.Unimplemented, "method CreateRoute is not implemented") } diff --git a/management/server/peer.go b/management/server/peer.go index 021c98601..ceafce978 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -5,6 +5,7 @@ import ( nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/status" + "github.com/rs/xid" "net" "strings" "time" @@ -34,9 +35,11 @@ type PeerStatus struct { } // Peer represents a machine connected to the network. -// The Peer is a Wireguard peer identified by a public key +// The Peer is a WireGuard peer identified by a public key type Peer struct { - // Wireguard public key + // ID is an internal ID of the peer + ID string + // WireGuard public key Key string // A setup key this peer was registered with SetupKey string @@ -62,6 +65,7 @@ type Peer struct { // Copy copies Peer object func (p *Peer) Copy() *Peer { return &Peer{ + ID: p.ID, Key: p.Key, SetupKey: p.SetupKey, IP: p.IP, @@ -97,7 +101,7 @@ func (p *PeerStatus) Copy() *PeerStatus { } // GetPeer looks up peer by its public WireGuard key -func (am *DefaultAccountManager) GetPeer(peerPubKey string) (*Peer, error) { +func (am *DefaultAccountManager) GetPeerByKey(peerPubKey string) (*Peer, error) { account, err := am.Store.GetAccountByPeerPubKey(peerPubKey) if err != nil { @@ -130,14 +134,14 @@ func (am *DefaultAccountManager) GetPeers(accountID, userID string) ([]*Peer, er } p := peer.Copy() peers = append(peers, p) - peersMap[peer.Key] = p + peersMap[peer.ID] = p } // fetch all the peers that have access to the user's peers for _, peer := range peers { - aclPeers := account.getPeersByACL(peer.Key) + aclPeers := account.getPeersByACL(peer.ID) for _, p := range aclPeers { - peersMap[p.Key] = p + peersMap[p.ID] = p } } @@ -177,7 +181,7 @@ func (am *DefaultAccountManager) MarkPeerConnected(peerPubKey string, connected peer.Status = newStatus account.UpdatePeer(peer) - err = am.Store.SavePeerStatus(account.Id, peerPubKey, *newStatus) + err = am.Store.SavePeerStatus(account.Id, peer.ID, *newStatus) if err != nil { return err } @@ -195,10 +199,9 @@ func (am *DefaultAccountManager) UpdatePeer(accountID, userID string, update *Pe return nil, err } - //TODO Peer.ID migration: we will need to replace search by ID here - peer, err := account.FindPeerByPubKey(update.Key) - if err != nil { - return nil, err + peer := account.GetPeer(update.ID) + if peer == nil { + return nil, status.Errorf(status.NotFound, "peer %s not found", update.ID) } if peer.SSHEnabled != update.SSHEnabled { @@ -222,7 +225,7 @@ func (am *DefaultAccountManager) UpdatePeer(accountID, userID string, update *Pe peer.DNSLabel = newLabel - am.storeEvent(userID, peer.IP.String(), accountID, activity.PeerRenamed, peer.EventMeta(am.GetDNSDomain())) + am.storeEvent(userID, peer.ID, accountID, activity.PeerRenamed, peer.EventMeta(am.GetDNSDomain())) } account.UpdatePeer(peer) @@ -241,7 +244,7 @@ func (am *DefaultAccountManager) UpdatePeer(accountID, userID string, update *Pe } // DeletePeer removes peer from the account by its IP -func (am *DefaultAccountManager) DeletePeer(accountID, peerPubKey, userID string) (*Peer, error) { +func (am *DefaultAccountManager) DeletePeer(accountID, peerID, userID string) (*Peer, error) { unlock := am.Store.AcquireAccountLock(accountID) defer unlock() @@ -251,19 +254,19 @@ func (am *DefaultAccountManager) DeletePeer(accountID, peerPubKey, userID string return nil, err } - peer, err := account.FindPeerByPubKey(peerPubKey) - if err != nil { - return nil, err + peer := account.GetPeer(peerID) + if peer == nil { + return nil, status.Errorf(status.NotFound, "peer %s not found", peerID) } - account.DeletePeer(peerPubKey) + account.DeletePeer(peerID) err = am.Store.SaveAccount(account) if err != nil { return nil, err } - err = am.peersUpdateManager.SendUpdate(peerPubKey, + err = am.peersUpdateManager.SendUpdate(peer.ID, &UpdateMessage{ Update: &proto.SyncResponse{ // fill those field for backward compatibility @@ -281,13 +284,12 @@ func (am *DefaultAccountManager) DeletePeer(accountID, peerPubKey, userID string return nil, err } - // TODO Peer.ID migration: we will need to replace search by Peer.ID here if err := am.updateAccountPeers(account); err != nil { return nil, err } - am.peersUpdateManager.CloseChannel(peerPubKey) - am.storeEvent(userID, peer.IP.String(), account.Id, activity.PeerRemovedByUser, peer.EventMeta(am.GetDNSDomain())) + am.peersUpdateManager.CloseChannel(peerID) + am.storeEvent(userID, peer.ID, account.Id, activity.PeerRemovedByUser, peer.EventMeta(am.GetDNSDomain())) return peer, nil } @@ -312,17 +314,23 @@ func (am *DefaultAccountManager) GetPeerByIP(accountID string, peerIP string) (* } // GetNetworkMap returns Network map for a given peer (omits original peer from the Peers result) -func (am *DefaultAccountManager) GetNetworkMap(peerPubKey string) (*NetworkMap, error) { +func (am *DefaultAccountManager) GetNetworkMap(peerID string) (*NetworkMap, error) { - account, err := am.Store.GetAccountByPeerPubKey(peerPubKey) + account, err := am.Store.GetAccountByPeerID(peerID) if err != nil { return nil, err } - aclPeers := account.getPeersByACL(peerPubKey) - routesUpdate := account.getRoutesToSync(peerPubKey, aclPeers) + peer := account.GetPeer(peerID) + if peer == nil { + return nil, status.Errorf(status.NotFound, "peer with ID %s not found", peerID) + } - dnsManagementStatus := account.getPeerDNSManagementStatus(peerPubKey) + aclPeers := account.getPeersByACL(peerID) + // Please mind, that the returned route.Route objects will contain Peer.Key instead of Peer.ID. + routesUpdate := account.getRoutesToSync(peerID, aclPeers) + + dnsManagementStatus := account.getPeerDNSManagementStatus(peerID) dnsUpdate := nbdns.Config{ ServiceEnable: dnsManagementStatus, } @@ -334,7 +342,7 @@ func (am *DefaultAccountManager) GetNetworkMap(peerPubKey string) (*NetworkMap, zones = append(zones, peersCustomZone) } dnsUpdate.CustomZones = zones - dnsUpdate.NameServerGroups = getPeerNSGroups(account, peerPubKey) + dnsUpdate.NameServerGroups = getPeerNSGroups(account, peerID) } return &NetworkMap{ @@ -346,9 +354,9 @@ func (am *DefaultAccountManager) GetNetworkMap(peerPubKey string) (*NetworkMap, } // GetPeerNetwork returns the Network for a given peer -func (am *DefaultAccountManager) GetPeerNetwork(peerPubKey string) (*Network, error) { +func (am *DefaultAccountManager) GetPeerNetwork(peerID string) (*Network, error) { - account, err := am.Store.GetAccountByPeerPubKey(peerPubKey) + account, err := am.Store.GetAccountByPeerID(peerID) if err != nil { return nil, err } @@ -357,7 +365,7 @@ func (am *DefaultAccountManager) GetPeerNetwork(peerPubKey string) (*Network, er } // AddPeer adds a new peer to the Store. -// Each Account has a list of pre-authorised SetupKey and if no Account has a given key err wit ha code codes.Unauthenticated +// Each Account has a list of pre-authorised SetupKey and if no Account has a given key err with a code codes.Unauthenticated // will be returned, meaning the key is invalid // If a User ID is provided, it means that we passed the authentication using JWT, then we look for account by User ID and register the peer // to it. We also add the User ID to the peer metadata to identify registrant. @@ -428,6 +436,7 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (* } newPeer := &Peer{ + ID: xid.New().String(), Key: peer.Key, SetupKey: upperKey, IP: nextIp, @@ -445,7 +454,7 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (* if err != nil { return nil, err } - group.Peers = append(group.Peers, newPeer.Key) + group.Peers = append(group.Peers, newPeer.ID) var groupsToAdd []string if addedByUser { @@ -463,19 +472,19 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (* if len(groupsToAdd) > 0 { for _, s := range groupsToAdd { if g, ok := account.Groups[s]; ok && g.Name != "All" { - g.Peers = append(g.Peers, newPeer.Key) + g.Peers = append(g.Peers, newPeer.ID) } } } - account.Peers[newPeer.Key] = newPeer + account.Peers[newPeer.ID] = newPeer account.Network.IncSerial() err = am.Store.SaveAccount(account) if err != nil { return nil, err } - opEvent.TargetID = newPeer.IP.String() + opEvent.TargetID = newPeer.ID opEvent.Meta = newPeer.EventMeta(am.GetDNSDomain()) am.storeEvent(opEvent.InitiatorID, opEvent.TargetID, opEvent.AccountID, opEvent.Activity, opEvent.Meta) @@ -483,14 +492,14 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (* } // UpdatePeerSSHKey updates peer's public SSH key -func (am *DefaultAccountManager) UpdatePeerSSHKey(peerPubKey string, sshKey string) error { +func (am *DefaultAccountManager) UpdatePeerSSHKey(peerID string, sshKey string) error { if sshKey == "" { - log.Debugf("empty SSH key provided for peer %s, skipping update", peerPubKey) + log.Debugf("empty SSH key provided for peer %s, skipping update", peerID) return nil } - account, err := am.Store.GetAccountByPeerPubKey(peerPubKey) + account, err := am.Store.GetAccountByPeerID(peerID) if err != nil { return err } @@ -504,13 +513,13 @@ func (am *DefaultAccountManager) UpdatePeerSSHKey(peerPubKey string, sshKey stri return err } - peer, err := account.FindPeerByPubKey(peerPubKey) - if err != nil { - return err + peer := account.GetPeer(peerID) + if peer == nil { + return status.Errorf(status.NotFound, "peer with ID %s not found", peerID) } if peer.SSHKey == sshKey { - log.Debugf("same SSH key provided for peer %s, skipping update", peerPubKey) + log.Debugf("same SSH key provided for peer %s, skipping update", peerID) return nil } @@ -527,9 +536,9 @@ func (am *DefaultAccountManager) UpdatePeerSSHKey(peerPubKey string, sshKey stri } // UpdatePeerMeta updates peer's system metadata -func (am *DefaultAccountManager) UpdatePeerMeta(peerPubKey string, meta PeerSystemMeta) error { +func (am *DefaultAccountManager) UpdatePeerMeta(peerID string, meta PeerSystemMeta) error { - account, err := am.Store.GetAccountByPeerPubKey(peerPubKey) + account, err := am.Store.GetAccountByPeerID(peerID) if err != nil { return err } @@ -537,9 +546,9 @@ func (am *DefaultAccountManager) UpdatePeerMeta(peerPubKey string, meta PeerSyst unlock := am.Store.AcquireAccountLock(account.Id) defer unlock() - peer, err := account.FindPeerByPubKey(peerPubKey) - if err != nil { - return err + peer := account.GetPeer(peerID) + if peer == nil { + return status.Errorf(status.NotFound, "peer with ID %s not found", peerID) } // Avoid overwriting UIVersion if the update was triggered sole by the CLI client @@ -558,9 +567,9 @@ func (am *DefaultAccountManager) UpdatePeerMeta(peerPubKey string, meta PeerSyst } // getPeersByACL returns all peers that given peer has access to. -func (a *Account) getPeersByACL(peerPubKey string) []*Peer { +func (a *Account) getPeersByACL(peerID string) []*Peer { var peers []*Peer - srcRules, dstRules := a.GetPeerRules(peerPubKey) + srcRules, dstRules := a.GetPeerRules(peerID) groups := map[string]*Group{} for _, r := range srcRules { @@ -603,8 +612,8 @@ func (a *Account) getPeersByACL(peerPubKey string) []*Peer { continue } // exclude original peer - if _, ok := peersSet[peer.Key]; peer.Key != peerPubKey && !ok { - peersSet[peer.Key] = struct{}{} + if _, ok := peersSet[peer.ID]; peer.ID != peerID && !ok { + peersSet[peer.ID] = struct{}{} peers = append(peers, peer.Copy()) } } @@ -619,13 +628,13 @@ func (am *DefaultAccountManager) updateAccountPeers(account *Account) error { peers := account.GetPeers() for _, peer := range peers { - remotePeerNetworkMap, err := am.GetNetworkMap(peer.Key) + remotePeerNetworkMap, err := am.GetNetworkMap(peer.ID) if err != nil { return err } update := toSyncResponse(nil, peer, nil, remotePeerNetworkMap, am.GetDNSDomain()) - err = am.peersUpdateManager.SendUpdate(peer.Key, &UpdateMessage{Update: update}) + err = am.peersUpdateManager.SendUpdate(peer.ID, &UpdateMessage{Update: update}) if err != nil { return err } diff --git a/management/server/peer_test.go b/management/server/peer_test.go index 28242cddc..a0afdafe6 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -34,7 +34,7 @@ func TestAccountManager_GetNetworkMap(t *testing.T) { return } - _, err = manager.AddPeer(setupKey.Key, "", &Peer{ + peer1, err := manager.AddPeer(setupKey.Key, "", &Peer{ Key: peerKey1.PublicKey().String(), Meta: PeerSystemMeta{}, Name: "test-peer-2", @@ -61,7 +61,7 @@ func TestAccountManager_GetNetworkMap(t *testing.T) { return } - networkMap, err := manager.GetNetworkMap(peerKey1.PublicKey().String()) + networkMap, err := manager.GetNetworkMap(peer1.ID) if err != nil { t.Fatal(err) return @@ -107,7 +107,7 @@ func TestAccountManager_GetNetworkMapWithRule(t *testing.T) { return } - _, err = manager.AddPeer(setupKey.Key, "", &Peer{ + peer1, err := manager.AddPeer(setupKey.Key, "", &Peer{ Key: peerKey1.PublicKey().String(), Meta: PeerSystemMeta{}, Name: "test-peer-2", @@ -123,7 +123,7 @@ func TestAccountManager_GetNetworkMapWithRule(t *testing.T) { t.Fatal(err) return } - _, err = manager.AddPeer(setupKey.Key, "", &Peer{ + peer2, err := manager.AddPeer(setupKey.Key, "", &Peer{ Key: peerKey2.PublicKey().String(), Meta: PeerSystemMeta{}, Name: "test-peer-2", @@ -156,8 +156,8 @@ func TestAccountManager_GetNetworkMapWithRule(t *testing.T) { group1.Name = "src" group2.Name = "dst" rule.ID = xid.New().String() - group1.Peers = append(group1.Peers, peerKey1.PublicKey().String()) - group2.Peers = append(group2.Peers, peerKey2.PublicKey().String()) + group1.Peers = append(group1.Peers, peer1.ID) + group2.Peers = append(group2.Peers, peer2.ID) err = manager.SaveGroup(account.Id, userID, &group1) if err != nil { @@ -180,7 +180,7 @@ func TestAccountManager_GetNetworkMapWithRule(t *testing.T) { return } - networkMap1, err := manager.GetNetworkMap(peerKey1.PublicKey().String()) + networkMap1, err := manager.GetNetworkMap(peer1.ID) if err != nil { t.Fatal(err) return @@ -203,7 +203,7 @@ func TestAccountManager_GetNetworkMapWithRule(t *testing.T) { ) } - networkMap2, err := manager.GetNetworkMap(peerKey2.PublicKey().String()) + networkMap2, err := manager.GetNetworkMap(peer2.ID) if err != nil { t.Fatal(err) return @@ -228,7 +228,7 @@ func TestAccountManager_GetNetworkMapWithRule(t *testing.T) { return } - networkMap1, err = manager.GetNetworkMap(peerKey1.PublicKey().String()) + networkMap1, err = manager.GetNetworkMap(peer1.ID) if err != nil { t.Fatal(err) return @@ -243,7 +243,7 @@ func TestAccountManager_GetNetworkMapWithRule(t *testing.T) { return } - networkMap2, err = manager.GetNetworkMap(peerKey2.PublicKey().String()) + networkMap2, err = manager.GetNetworkMap(peer2.ID) if err != nil { t.Fatal(err) return @@ -281,7 +281,7 @@ func TestAccountManager_GetPeerNetwork(t *testing.T) { return } - _, err = manager.AddPeer(setupKey.Key, "", &Peer{ + peer1, err := manager.AddPeer(setupKey.Key, "", &Peer{ Key: peerKey1.PublicKey().String(), Meta: PeerSystemMeta{}, Name: "test-peer-2", @@ -308,7 +308,7 @@ func TestAccountManager_GetPeerNetwork(t *testing.T) { return } - network, err := manager.GetPeerNetwork(peerKey1.PublicKey().String()) + network, err := manager.GetPeerNetwork(peer1.ID) if err != nil { t.Fatal(err) return diff --git a/management/server/route.go b/management/server/route.go index d67cde83e..c02729a72 100644 --- a/management/server/route.go +++ b/management/server/route.go @@ -91,9 +91,9 @@ func (am *DefaultAccountManager) GetRoute(accountID, routeID, userID string) (*r } // checkPrefixPeerExists checks the combination of prefix and peer id, if it exists returns an error, otherwise returns nil -func (am *DefaultAccountManager) checkPrefixPeerExists(accountID, peer string, prefix netip.Prefix) error { +func (am *DefaultAccountManager) checkPrefixPeerExists(accountID, peerID string, prefix netip.Prefix) error { - if peer == "" { + if peerID == "" { return nil } @@ -111,15 +111,15 @@ func (am *DefaultAccountManager) checkPrefixPeerExists(accountID, peer string, p return status.Errorf(status.InvalidArgument, "failed to parse prefix %s", prefix.String()) } for _, prefixRoute := range routesWithPrefix { - if prefixRoute.Peer == peer { - return status.Errorf(status.AlreadyExists, "failed a route with prefix %s and peer already exist", prefix.String()) + if prefixRoute.Peer == peerID { + return status.Errorf(status.AlreadyExists, "failed to add route with prefix %s - peer already has this route", prefix.String()) } } return nil } // CreateRoute creates and saves a new route -func (am *DefaultAccountManager) CreateRoute(accountID string, network, peerIP, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) { +func (am *DefaultAccountManager) CreateRoute(accountID string, network, peerID, description, netID string, masquerade bool, metric int, groups []string, enabled bool, userID string) (*route.Route, error) { unlock := am.Store.AcquireAccountLock(accountID) defer unlock() @@ -128,13 +128,11 @@ func (am *DefaultAccountManager) CreateRoute(accountID string, network, peerIP, return nil, err } - peerKey := "" - if peerIP != "" { - peer := account.GetPeerByIP(peerIP) + if peerID != "" { + peer := account.GetPeer(peerID) if peer == nil { - return nil, status.Errorf(status.NotFound, "peer %s not found", peerIP) + return nil, status.Errorf(status.InvalidArgument, "peer with ID %s not found", peerID) } - peerKey = peer.Key } var newRoute route.Route @@ -142,7 +140,7 @@ func (am *DefaultAccountManager) CreateRoute(accountID string, network, peerIP, if err != nil { return nil, status.Errorf(status.InvalidArgument, "failed to parse IP %s", network) } - err = am.checkPrefixPeerExists(accountID, peerKey, newPrefix) + err = am.checkPrefixPeerExists(accountID, peerID, newPrefix) if err != nil { return nil, err } @@ -160,7 +158,7 @@ func (am *DefaultAccountManager) CreateRoute(accountID string, network, peerIP, return nil, err } - newRoute.Peer = peerKey + newRoute.Peer = peerID newRoute.ID = xid.New().String() newRoute.Network = newPrefix newRoute.NetworkType = prefixType @@ -220,9 +218,9 @@ func (am *DefaultAccountManager) SaveRoute(accountID, userID string, routeToSave } if routeToSave.Peer != "" { - _, peerExist := account.Peers[routeToSave.Peer] - if !peerExist { - return status.Errorf(status.InvalidArgument, "failed to find Peer %s", routeToSave.Peer) + peer := account.GetPeer(routeToSave.Peer) + if peer == nil { + return status.Errorf(status.InvalidArgument, "peer with ID %s not found", routeToSave.Peer) } } @@ -238,9 +236,14 @@ func (am *DefaultAccountManager) SaveRoute(accountID, userID string, routeToSave return err } + err = am.updateAccountPeers(account) + if err != nil { + return err + } + am.storeEvent(userID, routeToSave.ID, accountID, activity.RouteUpdated, routeToSave.EventMeta()) - return am.updateAccountPeers(account) + return nil } // UpdateRoute updates existing route with set of operations @@ -287,9 +290,9 @@ func (am *DefaultAccountManager) UpdateRoute(accountID, routeID string, operatio newRoute.NetworkType = prefixType case UpdateRoutePeer: if operation.Values[0] != "" { - _, peerExist := account.Peers[operation.Values[0]] - if !peerExist { - return nil, status.Errorf(status.InvalidArgument, "failed to find Peer %s", operation.Values[0]) + peer := account.GetPeer(operation.Values[0]) + if peer == nil { + return nil, status.Errorf(status.InvalidArgument, "peer with ID %s not found", operation.Values[0]) } } diff --git a/management/server/route_test.go b/management/server/route_test.go index 39866492c..b59e2f870 100644 --- a/management/server/route_test.go +++ b/management/server/route_test.go @@ -12,6 +12,8 @@ import ( const ( peer1Key = "BhRPtynAAYRDy08+q4HTMsos8fs4plTP4NOSh7C1ry8=" peer2Key = "/yF0+vCfv+mRR5k0dca0TrGdO/oiNeAI58gToZm5NyI=" + peer1ID = "peer-1-id" + peer2ID = "peer-2-id" routeGroup1 = "routeGroup1" routeGroup2 = "routeGroup2" routeInvalidGroup1 = "routeInvalidGroup1" @@ -23,7 +25,7 @@ func TestCreateRoute(t *testing.T) { type input struct { network string netID string - peer string + peerKey string description string masquerade bool metric int @@ -43,7 +45,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/16", netID: "happy", - peer: peer1Key, + peerKey: peer1ID, description: "super", masquerade: false, metric: 9999, @@ -56,7 +58,7 @@ func TestCreateRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetworkType: route.IPv4Network, NetID: "happy", - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -69,7 +71,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/34", netID: "happy", - peer: peer1Key, + peerKey: peer1ID, description: "super", masquerade: false, metric: 9999, @@ -84,7 +86,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/16", netID: "happy", - peer: "notExistingPeer", + peerKey: "notExistingPeer", description: "super", masquerade: false, metric: 9999, @@ -99,7 +101,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/16", netID: "happy", - peer: "", + peerKey: "", description: "super", masquerade: false, metric: 9999, @@ -124,7 +126,7 @@ func TestCreateRoute(t *testing.T) { name: "Large Metric Should Fail", inputArgs: input{ network: "192.168.0.0/16", - peer: peer1Key, + peerKey: peer1ID, netID: "happy", description: "super", masquerade: false, @@ -140,7 +142,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/16", netID: "happy", - peer: peer1Key, + peerKey: peer1ID, description: "super", masquerade: false, metric: 0, @@ -154,7 +156,7 @@ func TestCreateRoute(t *testing.T) { name: "Large NetID Should Fail", inputArgs: input{ network: "192.168.0.0/16", - peer: peer1Key, + peerKey: peer1ID, netID: "12345678901234567890qwertyuiopqwertyuiop1", description: "super", masquerade: false, @@ -170,7 +172,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/16", netID: "", - peer: peer1Key, + peerKey: peer1ID, description: "", masquerade: false, metric: 9999, @@ -185,7 +187,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/16", netID: "NewId", - peer: peer1Key, + peerKey: peer1ID, description: "", masquerade: false, metric: 9999, @@ -200,7 +202,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/16", netID: "NewId", - peer: peer1Key, + peerKey: peer1ID, description: "", masquerade: false, metric: 9999, @@ -215,7 +217,7 @@ func TestCreateRoute(t *testing.T) { inputArgs: input{ network: "192.168.0.0/16", netID: "NewId", - peer: peer1Key, + peerKey: peer1ID, description: "", masquerade: false, metric: 9999, @@ -238,18 +240,10 @@ func TestCreateRoute(t *testing.T) { t.Error("failed to init testing account") } - peerIP := "99.99.99.99" - peer := account.Peers[testCase.inputArgs.peer] - if testCase.inputArgs.peer == "" { - peerIP = "" - } else if peer != nil { - peerIP = peer.IP.String() - } - outRoute, err := am.CreateRoute( account.Id, testCase.inputArgs.network, - peerIP, + testCase.inputArgs.peerKey, testCase.inputArgs.description, testCase.inputArgs.netID, testCase.inputArgs.masquerade, @@ -278,7 +272,7 @@ func TestCreateRoute(t *testing.T) { func TestSaveRoute(t *testing.T) { - validPeer := peer2Key + validPeer := peer2ID invalidPeer := "nonExisting" validPrefix := netip.MustParsePrefix("192.168.0.0/24") invalidPrefix, _ := netip.ParsePrefix("192.168.0.0/34") @@ -306,7 +300,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: validNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -339,7 +333,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: validNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -356,7 +350,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: validNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -373,7 +367,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: validNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -390,7 +384,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: invalidNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -407,7 +401,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: validNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -424,7 +418,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: validNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -441,7 +435,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: validNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -458,7 +452,7 @@ func TestSaveRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: validNetID, NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -495,7 +489,6 @@ func TestSaveRoute(t *testing.T) { if testCase.newPeer != nil { routeToSave.Peer = *testCase.newPeer } - if testCase.newMetric != nil { routeToSave.Metric = *testCase.newMetric } @@ -541,7 +534,7 @@ func TestUpdateRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: "superRoute", NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -563,7 +556,7 @@ func TestUpdateRoute(t *testing.T) { operations: []RouteUpdateOperation{ { Type: UpdateRoutePeer, - Values: []string{peer2Key}, + Values: []string{peer2ID}, }, }, errFunc: require.NoError, @@ -573,7 +566,7 @@ func TestUpdateRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: "superRoute", NetworkType: route.IPv4Network, - Peer: peer2Key, + Peer: peer2ID, Description: "super", Masquerade: false, Metric: 9999, @@ -595,7 +588,7 @@ func TestUpdateRoute(t *testing.T) { }, { Type: UpdateRoutePeer, - Values: []string{peer2Key}, + Values: []string{peer2ID}, }, { Type: UpdateRouteMetric, @@ -625,7 +618,7 @@ func TestUpdateRoute(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/24"), NetID: "megaRoute", NetworkType: route.IPv4Network, - Peer: peer2Key, + Peer: peer2ID, Description: "great", Masquerade: true, Metric: 3030, @@ -649,7 +642,7 @@ func TestUpdateRoute(t *testing.T) { operations: []RouteUpdateOperation{ { Type: UpdateRoutePeer, - Values: []string{peer2Key, peer1Key}, + Values: []string{peer2ID, peer1ID}, }, }, errFunc: require.Error, @@ -847,7 +840,7 @@ func TestGetNetworkMap_RouteSync(t *testing.T) { Network: netip.MustParsePrefix("192.168.0.0/16"), NetID: "superNet", NetworkType: route.IPv4Network, - Peer: peer1Key, + Peer: peer1ID, Description: "super", Masquerade: false, Metric: 9999, @@ -865,39 +858,42 @@ func TestGetNetworkMap_RouteSync(t *testing.T) { t.Error("failed to init testing account") } - newAccountRoutes, err := am.GetNetworkMap(peer1Key) + newAccountRoutes, err := am.GetNetworkMap(peer1ID) require.NoError(t, err) require.Len(t, newAccountRoutes.Routes, 0, "new accounts should have no routes") - peer := account.Peers[baseRoute.Peer] - createdRoute, err := am.CreateRoute(account.Id, baseRoute.Network.String(), peer.IP.String(), + createdRoute, err := am.CreateRoute(account.Id, baseRoute.Network.String(), peer1ID, baseRoute.Description, baseRoute.NetID, baseRoute.Masquerade, baseRoute.Metric, baseRoute.Groups, false, userID) require.NoError(t, err) - noDisabledRoutes, err := am.GetNetworkMap(peer1Key) + noDisabledRoutes, err := am.GetNetworkMap(peer1ID) require.NoError(t, err) require.Len(t, noDisabledRoutes.Routes, 0, "no routes for disabled routes") enabledRoute := createdRoute.Copy() enabledRoute.Enabled = true + // network map contains route.Route objects that have Route.Peer field filled with Peer.Key instead of Peer.ID + expectedRoute := enabledRoute.Copy() + expectedRoute.Peer = peer1Key + err = am.SaveRoute(account.Id, userID, enabledRoute) require.NoError(t, err) - peer1Routes, err := am.GetNetworkMap(peer1Key) + peer1Routes, err := am.GetNetworkMap(peer1ID) require.NoError(t, err) require.Len(t, peer1Routes.Routes, 1, "we should receive one route for peer1") - require.True(t, enabledRoute.IsEqual(peer1Routes.Routes[0]), "received route should be equal") + require.True(t, expectedRoute.IsEqual(peer1Routes.Routes[0]), "received route should be equal") - peer2Routes, err := am.GetNetworkMap(peer2Key) + peer2Routes, err := am.GetNetworkMap(peer2ID) require.NoError(t, err) require.Len(t, peer2Routes.Routes, 0, "no routes for peers not in the distribution group") - err = am.GroupAddPeer(account.Id, routeGroup1, peer2Key) + err = am.GroupAddPeer(account.Id, routeGroup1, peer2ID) require.NoError(t, err) - peer2Routes, err = am.GetNetworkMap(peer2Key) + peer2Routes, err = am.GetNetworkMap(peer2ID) require.NoError(t, err) require.Len(t, peer2Routes.Routes, 1, "we should receive one route") require.True(t, peer1Routes.Routes[0].IsEqual(peer2Routes.Routes[0]), "routes should be the same for peers in the same group") @@ -905,7 +901,7 @@ func TestGetNetworkMap_RouteSync(t *testing.T) { newGroup := &Group{ ID: xid.New().String(), Name: "peer1 group", - Peers: []string{peer1Key}, + Peers: []string{peer1ID}, } err = am.SaveGroup(account.Id, userID, newGroup) require.NoError(t, err) @@ -926,18 +922,18 @@ func TestGetNetworkMap_RouteSync(t *testing.T) { err = am.DeleteRule(account.Id, defaultRule.ID, userID) require.NoError(t, err) - peer1GroupRoutes, err := am.GetNetworkMap(peer1Key) + peer1GroupRoutes, err := am.GetNetworkMap(peer1ID) require.NoError(t, err) require.Len(t, peer1GroupRoutes.Routes, 1, "we should receive one route for peer1") - peer2GroupRoutes, err := am.GetNetworkMap(peer2Key) + peer2GroupRoutes, err := am.GetNetworkMap(peer2ID) require.NoError(t, err) require.Len(t, peer2GroupRoutes.Routes, 0, "we should not receive routes for peer2") err = am.DeleteRoute(account.Id, enabledRoute.ID, userID) require.NoError(t, err) - peer1DeletedRoute, err := am.GetNetworkMap(peer1Key) + peer1DeletedRoute, err := am.GetNetworkMap(peer1ID) require.NoError(t, err) require.Len(t, peer1DeletedRoute.Routes, 0, "we should receive one route for peer1") @@ -964,9 +960,27 @@ func createRouterStore(t *testing.T) (Store, error) { func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, error) { + accountID := "testingAcc" + domain := "example.com" + + account := newAccountWithId(accountID, userID, domain) + err := am.Store.SaveAccount(account) + if err != nil { + return nil, err + } + + ips := account.getTakenIPs() + peer1IP, err := AllocatePeerIP(account.Network.Net, ips) + if err != nil { + return nil, err + } + peer1 := &Peer{ - Key: peer1Key, - Name: "test-host1@netbird.io", + IP: peer1IP, + ID: peer1ID, + Key: peer1Key, + Name: "test-host1@netbird.io", + UserID: userID, Meta: PeerSystemMeta{ Hostname: "test-host1@netbird.io", GoOS: "linux", @@ -978,9 +992,20 @@ func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, er UIVersion: "development", }, } + account.Peers[peer1.ID] = peer1 + + ips = account.getTakenIPs() + peer2IP, err := AllocatePeerIP(account.Network.Net, ips) + if err != nil { + return nil, err + } + peer2 := &Peer{ - Key: peer2Key, - Name: "test-host2@netbird.io", + IP: peer2IP, + ID: peer2ID, + Key: peer2Key, + Name: "test-host2@netbird.io", + UserID: userID, Meta: PeerSystemMeta{ Hostname: "test-host2@netbird.io", GoOS: "linux", @@ -992,28 +1017,29 @@ func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, er UIVersion: "development", }, } + account.Peers[peer2.ID] = peer2 - accountID := "testingAcc" - domain := "example.com" - - account := newAccountWithId(accountID, userID, domain) - err := am.Store.SaveAccount(account) + err = am.Store.SaveAccount(account) + if err != nil { + return nil, err + } + groupAll, err := account.GetGroupAll() + if err != nil { + return nil, err + } + err = am.GroupAddPeer(accountID, groupAll.ID, peer1ID) + if err != nil { + return nil, err + } + err = am.GroupAddPeer(accountID, groupAll.ID, peer2ID) if err != nil { return nil, err } - _, err = am.AddPeer("", userID, peer1) - if err != nil { - return nil, err - } - _, err = am.AddPeer("", userID, peer2) - if err != nil { - return nil, err - } newGroup := &Group{ ID: routeGroup1, Name: routeGroup1, - Peers: []string{peer1Key}, + Peers: []string{peer1.ID}, } err = am.SaveGroup(accountID, userID, newGroup) if err != nil { @@ -1023,7 +1049,7 @@ func initTestRouteAccount(t *testing.T, am *DefaultAccountManager) (*Account, er newGroup = &Group{ ID: routeGroup2, Name: routeGroup2, - Peers: []string{peer1Key}, + Peers: []string{peer2.ID}, } err = am.SaveGroup(accountID, userID, newGroup) diff --git a/management/server/store.go b/management/server/store.go index 3954f0e8c..02041dfda 100644 --- a/management/server/store.go +++ b/management/server/store.go @@ -5,6 +5,7 @@ type Store interface { GetAccount(accountID string) (*Account, error) GetAccountByUser(userID string) (*Account, error) GetAccountByPeerPubKey(peerKey string) (*Account, error) + GetAccountByPeerID(peerID string) (*Account, error) GetAccountBySetupKey(setupKey string) (*Account, error) //todo use key hash later GetAccountByPrivateDomain(domain string) (*Account, error) SaveAccount(account *Account) error @@ -14,7 +15,7 @@ type Store interface { AcquireAccountLock(accountID string) func() // AcquireGlobalLock should attempt to acquire a global lock and return a function that releases the lock AcquireGlobalLock() func() - SavePeerStatus(accountID, peerKey string, status PeerStatus) error + SavePeerStatus(accountID, peerID string, status PeerStatus) error // Close should close the store persisting all unsaved data. Close() error } diff --git a/management/server/turncredentials.go b/management/server/turncredentials.go index 82ce0a329..dcfab57dd 100644 --- a/management/server/turncredentials.go +++ b/management/server/turncredentials.go @@ -11,14 +11,14 @@ import ( "time" ) -//TURNCredentialsManager used to manage TURN credentials +// TURNCredentialsManager used to manage TURN credentials type TURNCredentialsManager interface { GenerateCredentials() TURNCredentials SetupRefresh(peerKey string) CancelRefresh(peerKey string) } -//TimeBasedAuthSecretsManager generates credentials with TTL and using pre-shared secret known to TURN server +// TimeBasedAuthSecretsManager generates credentials with TTL and using pre-shared secret known to TURN server type TimeBasedAuthSecretsManager struct { mux sync.Mutex config *TURNConfig @@ -40,7 +40,7 @@ func NewTimeBasedAuthSecretsManager(updateManager *PeersUpdateManager, config *T } } -//GenerateCredentials generates new time-based secret credentials - basically username is a unix timestamp and password is a HMAC hash of a timestamp with a preshared TURN secret +// GenerateCredentials generates new time-based secret credentials - basically username is a unix timestamp and password is a HMAC hash of a timestamp with a preshared TURN secret func (m *TimeBasedAuthSecretsManager) GenerateCredentials() TURNCredentials { mac := hmac.New(sha1.New, []byte(m.config.Secret)) @@ -70,22 +70,22 @@ func (m *TimeBasedAuthSecretsManager) cancel(peerKey string) { } } -//CancelRefresh cancels scheduled peer credentials refresh +// CancelRefresh cancels scheduled peer credentials refresh func (m *TimeBasedAuthSecretsManager) CancelRefresh(peerKey string) { m.mux.Lock() defer m.mux.Unlock() m.cancel(peerKey) } -//SetupRefresh starts peer credentials refresh. Since credentials are expiring (TTL) it is necessary to always generate them and send to the peer. -//A goroutine is created and put into TimeBasedAuthSecretsManager.cancelMap. This routine should be cancelled if peer is gone. -func (m *TimeBasedAuthSecretsManager) SetupRefresh(peerKey string) { +// SetupRefresh starts peer credentials refresh. Since credentials are expiring (TTL) it is necessary to always generate them and send to the peer. +// A goroutine is created and put into TimeBasedAuthSecretsManager.cancelMap. This routine should be cancelled if peer is gone. +func (m *TimeBasedAuthSecretsManager) SetupRefresh(peerID string) { m.mux.Lock() defer m.mux.Unlock() - m.cancel(peerKey) + m.cancel(peerID) cancel := make(chan struct{}, 1) - m.cancelMap[peerKey] = cancel - log.Debugf("starting turn refresh for %s", peerKey) + m.cancelMap[peerID] = cancel + log.Debugf("starting turn refresh for %s", peerID) go func() { //we don't want to regenerate credentials right on expiration, so we do it slightly before (at 3/4 of TTL) @@ -94,7 +94,7 @@ func (m *TimeBasedAuthSecretsManager) SetupRefresh(peerKey string) { for { select { case <-cancel: - log.Debugf("stopping turn refresh for %s", peerKey) + log.Debugf("stopping turn refresh for %s", peerID) return case <-ticker.C: c := m.GenerateCredentials() @@ -115,9 +115,9 @@ func (m *TimeBasedAuthSecretsManager) SetupRefresh(peerKey string) { Turns: turns, }, } - err := m.updateManager.SendUpdate(peerKey, &UpdateMessage{Update: update}) + err := m.updateManager.SendUpdate(peerID, &UpdateMessage{Update: update}) if err != nil { - log.Errorf("error while sending TURN update to peer %s %v", peerKey, err) + log.Errorf("error while sending TURN update to peer %s %v", peerID, err) // todo maybe continue trying? } } diff --git a/management/server/updatechannel.go b/management/server/updatechannel.go index 4943fe247..4b4d6e3d1 100644 --- a/management/server/updatechannel.go +++ b/management/server/updatechannel.go @@ -13,6 +13,7 @@ type UpdateMessage struct { } type PeersUpdateManager struct { + // peerChannels is an update channel indexed by Peer.ID peerChannels map[string]chan *UpdateMessage channelsMux *sync.Mutex } @@ -26,49 +27,49 @@ func NewPeersUpdateManager() *PeersUpdateManager { } // SendUpdate sends update message to the peer's channel -func (p *PeersUpdateManager) SendUpdate(peer string, update *UpdateMessage) error { +func (p *PeersUpdateManager) SendUpdate(peerID string, update *UpdateMessage) error { p.channelsMux.Lock() defer p.channelsMux.Unlock() - if channel, ok := p.peerChannels[peer]; ok { + if channel, ok := p.peerChannels[peerID]; ok { select { case channel <- update: - log.Infof("update was sent to channel for peer %s", peer) + log.Infof("update was sent to channel for peer %s", peerID) default: - log.Warnf("channel for peer %s is %d full", peer, len(channel)) + log.Warnf("channel for peer %s is %d full", peerID, len(channel)) } return nil } - log.Debugf("peer %s has no channel", peer) + log.Debugf("peer %s has no channel", peerID) return nil } // CreateChannel creates a go channel for a given peer used to deliver updates relevant to the peer. -func (p *PeersUpdateManager) CreateChannel(peerKey string) chan *UpdateMessage { +func (p *PeersUpdateManager) CreateChannel(peerID string) chan *UpdateMessage { p.channelsMux.Lock() defer p.channelsMux.Unlock() - if channel, ok := p.peerChannels[peerKey]; ok { - delete(p.peerChannels, peerKey) + if channel, ok := p.peerChannels[peerID]; ok { + delete(p.peerChannels, peerID) close(channel) } //mbragin: todo shouldn't it be more? or configurable? channel := make(chan *UpdateMessage, channelBufferSize) - p.peerChannels[peerKey] = channel + p.peerChannels[peerID] = channel - log.Debugf("opened updates channel for a peer %s", peerKey) + log.Debugf("opened updates channel for a peer %s", peerID) return channel } // CloseChannel closes updates channel of a given peer -func (p *PeersUpdateManager) CloseChannel(peerKey string) { +func (p *PeersUpdateManager) CloseChannel(peerID string) { p.channelsMux.Lock() defer p.channelsMux.Unlock() - if channel, ok := p.peerChannels[peerKey]; ok { - delete(p.peerChannels, peerKey) + if channel, ok := p.peerChannels[peerID]; ok { + delete(p.peerChannels, peerID) close(channel) } - log.Debugf("closed updates channel of a peer %s", peerKey) + log.Debugf("closed updates channel of a peer %s", peerID) } // GetAllConnectedPeers returns a copy of the connected peers map @@ -76,8 +77,8 @@ func (p *PeersUpdateManager) GetAllConnectedPeers() map[string]struct{} { p.channelsMux.Lock() defer p.channelsMux.Unlock() m := make(map[string]struct{}) - for key := range p.peerChannels { - m[key] = struct{}{} + for ID := range p.peerChannels { + m[ID] = struct{}{} } return m } diff --git a/route/route.go b/route/route.go index b4a68e03c..fbd077bc2 100644 --- a/route/route.go +++ b/route/route.go @@ -78,7 +78,7 @@ type Route struct { // EventMeta returns activity event meta related to the route func (r *Route) EventMeta() map[string]any { - return map[string]any{"name": r.NetID, "network_range": r.Network.String()} + return map[string]any{"name": r.NetID, "network_range": r.Network.String(), "peer_id": r.Peer} } // Copy copies a route object