From 9d1ecbbfb288c874046c68f22314db06c6584359 Mon Sep 17 00:00:00 2001 From: Mikhail Bragin Date: Fri, 14 Jan 2022 14:34:27 +0100 Subject: [PATCH] Management - add serial to Network reflecting network updates (#179) * chore: [management] - add account serial ID * Fix concurrency on the client (#183) * reworked peer connection establishment logic eliminating race conditions and deadlocks while running many peers * chore: move serial to Network from Account * feature: increment Network serial ID when adding/removing peers * chore: extract network struct init to network.go * chore: add serial test when adding peer to the account * test: add ModificationID test on AddPeer and DeletePeer --- management/server/account.go | 25 ++++------ management/server/account_test.go | 70 ++++++++++++++++++++++++++++ management/server/file_store_test.go | 4 +- management/server/network.go | 37 +++++++++++++-- management/server/peer.go | 13 ++++++ management/server/user.go | 2 +- 6 files changed, 130 insertions(+), 21 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 29a589478..6d1984082 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1,13 +1,11 @@ package server import ( - "github.com/google/uuid" "github.com/rs/xid" log "github.com/sirupsen/logrus" "github.com/wiretrustee/wiretrustee/util" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "net" "sync" ) @@ -29,6 +27,12 @@ type Account struct { Users map[string]*User } +// NewAccount creates a new Account with a generated ID and generated default setup keys +func NewAccount(userId string) *Account { + accountId := xid.New().String() + return newAccountWithId(accountId, userId) +} + func (a *Account) Copy() *Account { peers := map[string]*Peer{} for id, peer := range a.Peers { @@ -186,7 +190,7 @@ func (am *AccountManager) AddAccount(accountId string, userId string) (*Account, } func (am *AccountManager) createAccount(accountId string, userId string) (*Account, error) { - account, _ := newAccountWithId(accountId, userId) + account := newAccountWithId(accountId, userId) err := am.Store.SaveAccount(account) if err != nil { @@ -197,7 +201,7 @@ func (am *AccountManager) createAccount(accountId string, userId string) (*Accou } // newAccountWithId creates a new Account with a default SetupKey (doesn't store in a Store) and provided id -func newAccountWithId(accountId string, userId string) (*Account, *SetupKey) { +func newAccountWithId(accountId string, userId string) *Account { log.Debugf("creating new account") @@ -206,22 +210,13 @@ func newAccountWithId(accountId string, userId string) (*Account, *SetupKey) { oneOffKey := GenerateSetupKey("One-off key", SetupKeyOneOff, DefaultSetupKeyDuration) setupKeys[defaultKey.Key] = defaultKey setupKeys[oneOffKey.Key] = oneOffKey - network := &Network{ - Id: uuid.New().String(), - Net: net.IPNet{IP: net.ParseIP("100.64.0.0"), Mask: net.IPMask{255, 192, 0, 0}}, - Dns: ""} + network := NewNetwork() peers := make(map[string]*Peer) users := make(map[string]*User) log.Debugf("created new account %s with setup key %s", accountId, defaultKey.Key) - return &Account{Id: accountId, SetupKeys: setupKeys, Network: network, Peers: peers, Users: users, CreatedBy: userId}, defaultKey -} - -// newAccount creates a new Account with a default SetupKey and a provided User.Id of a user who issued account creation (doesn't store in a Store) -func newAccount(userId string) (*Account, *SetupKey) { - accountId := xid.New().String() - return newAccountWithId(accountId, userId) + return &Account{Id: accountId, SetupKeys: setupKeys, Network: network, Peers: peers, Users: users, CreatedBy: userId} } func getAccountSetupKeyById(acc *Account, keyId string) *SetupKey { diff --git a/management/server/account_test.go b/management/server/account_test.go index 832d2474f..4c6e6e2a6 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -146,6 +146,8 @@ func TestAccountManager_AddPeer(t *testing.T) { t.Fatal(err) } + serial := account.Network.Serial() //should be 0 + var setupKey *SetupKey for _, key := range account.SetupKeys { setupKey = key @@ -156,6 +158,11 @@ func TestAccountManager_AddPeer(t *testing.T) { return } + if account.Network.serial != 0 { + t.Errorf("expecting account network to have an initial serial=0") + return + } + key, err := wgtypes.GenerateKey() if err != nil { t.Fatal(err) @@ -174,6 +181,12 @@ func TestAccountManager_AddPeer(t *testing.T) { return } + account, err = manager.GetAccount(account.Id) + if err != nil { + t.Fatal(err) + return + } + if peer.Key != expectedPeerKey { t.Errorf("expecting just added peer to have key = %s, got %s", expectedPeerKey, peer.Key) } @@ -182,7 +195,64 @@ func TestAccountManager_AddPeer(t *testing.T) { t.Errorf("expecting just added peer to have IP = %s, got %s", expectedPeerIP, peer.IP.String()) } + if account.Network.Serial() != 1 { + t.Errorf("expecting Network serial=%d to be incremented by 1 and be equal to %d when adding new peer to account", serial, account.Network.Serial()) + } + } + +func TestAccountManager_DeletePeer(t *testing.T) { + manager, err := createManager(t) + if err != nil { + t.Fatal(err) + return + } + + account, err := manager.AddAccount("test_account", "account_creator") + if err != nil { + t.Fatal(err) + } + + var setupKey *SetupKey + for _, key := range account.SetupKeys { + setupKey = key + } + + key, err := wgtypes.GenerateKey() + if err != nil { + t.Fatal(err) + return + } + + peerKey := key.PublicKey().String() + + _, err = manager.AddPeer(setupKey.Key, Peer{ + Key: peerKey, + Meta: PeerSystemMeta{}, + Name: peerKey, + }) + if err != nil { + t.Errorf("expecting peer to be added, got failure %v", err) + return + } + + _, err = manager.DeletePeer(account.Id, peerKey) + if err != nil { + return + } + + account, err = manager.GetAccount(account.Id) + if err != nil { + t.Fatal(err) + return + } + + if account.Network.Serial() != 2 { + t.Errorf("expecting Network serial=%d to be incremented and be equal to 2 after adding and deleteing a peer", account.Network.Serial()) + } + +} + func createManager(t *testing.T) (*AccountManager, error) { store, err := createStore(t) if err != nil { diff --git a/management/server/file_store_test.go b/management/server/file_store_test.go index 5b23be40d..4d9b86cf3 100644 --- a/management/server/file_store_test.go +++ b/management/server/file_store_test.go @@ -32,7 +32,7 @@ func TestNewStore(t *testing.T) { func TestSaveAccount(t *testing.T) { store := newStore(t) - account, _ := newAccount("testuser") + account := NewAccount("testuser") account.Users["testuser"] = NewAdminUser("testuser") setupKey := GenerateDefaultSetupKey() account.SetupKeys[setupKey.Key] = setupKey @@ -72,7 +72,7 @@ func TestSaveAccount(t *testing.T) { func TestStore(t *testing.T) { store := newStore(t) - account, _ := newAccount("testuser") + account := NewAccount("testuser") account.Users["testuser"] = NewAdminUser("testuser") account.Peers["testpeer"] = &Peer{ Key: "peerkey", diff --git a/management/server/network.go b/management/server/network.go index d08fa94a6..8590212d8 100644 --- a/management/server/network.go +++ b/management/server/network.go @@ -3,7 +3,9 @@ package server import ( "encoding/binary" "fmt" + "github.com/rs/xid" "net" + "sync" ) var ( @@ -15,13 +17,42 @@ type Network struct { Id string Net net.IPNet Dns string + // serial is an ID that increments by 1 when any change to the network happened (e.g. new peer has been added). + // Used to synchronize state to the client apps. + serial uint64 + + mu sync.Mutex `json:"-"` +} + +// NewNetwork creates a new Network initializing it with a serial=0 +func NewNetwork() *Network { + return &Network{ + Id: xid.New().String(), + Net: net.IPNet{IP: net.ParseIP("100.64.0.0"), Mask: net.IPMask{255, 192, 0, 0}}, + Dns: "", + serial: 0} +} + +// IncSerial increments serial by 1 reflecting that the network state has been changed +func (n *Network) IncSerial() { + n.mu.Lock() + defer n.mu.Unlock() + n.serial = n.serial + 1 +} + +// Serial returns the Network.serial of the network (latest state id) +func (n *Network) Serial() uint64 { + n.mu.Lock() + defer n.mu.Unlock() + return n.serial } func (n *Network) Copy() *Network { return &Network{ - Id: n.Id, - Net: n.Net, - Dns: n.Dns, + Id: n.Id, + Net: n.Net, + Dns: n.Dns, + serial: n.serial, } } diff --git a/management/server/peer.go b/management/server/peer.go index b5007e7f6..7213786c9 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -118,11 +118,22 @@ func (am *AccountManager) DeletePeer(accountId string, peerKey string) (*Peer, e am.mux.Lock() defer am.mux.Unlock() + account, err := am.Store.GetAccount(accountId) + if err != nil { + return nil, status.Errorf(codes.NotFound, "account not found") + } + peer, err := am.Store.DeletePeer(accountId, peerKey) if err != nil { return nil, err } + account.Network.IncSerial() + err = am.Store.SaveAccount(account) + if err != nil { + return nil, err + } + err = am.peersUpdateManager.SendUpdate(peerKey, &UpdateMessage{ Update: &proto.SyncResponse{ @@ -255,6 +266,8 @@ func (am *AccountManager) AddPeer(setupKey string, peer Peer) (*Peer, error) { account.Peers[newPeer.Key] = newPeer account.SetupKeys[sk.Key] = sk.IncrementUsage() + account.Network.IncSerial() + err = am.Store.SaveAccount(account) if err != nil { return nil, status.Errorf(codes.Internal, "failed adding peer") diff --git a/management/server/user.go b/management/server/user.go index 26b2b5cb6..2134a944f 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -47,7 +47,7 @@ func (am *AccountManager) GetOrCreateAccountByUser(userId string) (*Account, err account, err := am.Store.GetUserAccount(userId) if err != nil { if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - account, _ = newAccount(userId) + account = NewAccount(userId) account.Users[userId] = NewAdminUser(userId) err = am.Store.SaveAccount(account) if err != nil {