Lock by PeerID when CreateChannel

.gitattributes

@@ -0,0 +1 @@
+*.go text eol=lf

.github/workflows/golangci-lint.yml

@@ -1,12 +1,23 @@
 name: golangci-lint
 on: [pull_request]
+
+permissions: 
+  contents: read
+  pull-requests: read
+
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}-${{ github.head_ref || github.actor_id }}
   cancel-in-progress: true
+
 jobs:
   golangci:
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [macos-latest, windows-latest, ubuntu-latest]
     name: lint
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
+    timeout-minutes: 15
     steps:
       - name: Checkout code
         uses: actions/checkout@v3
@@ -14,7 +25,12 @@ jobs:
         uses: actions/setup-go@v4
         with:
           go-version: "1.20.x"
+          cache: false
       - name: Install dependencies
+        if: matrix.os == 'ubuntu-latest'
         run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: latest
+          args: --timeout=12m

client/ui/build-ui-linux.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+sudo apt update
+sudo apt remove gir1.2-appindicator3-0.1
+sudo apt install -y libayatana-appindicator3-dev
+go build

client/ui/client_ui.go

@@ -202,9 +202,10 @@ func (s *serviceClient) getSettingsForm() *widget.Form {
 				}
 
 				_, err = client.Login(s.ctx, &proto.LoginRequest{
-					ManagementUrl: s.iMngURL.Text,
-					AdminURL:      s.iAdminURL.Text,
-					PreSharedKey:  s.iPreSharedKey.Text,
+					ManagementUrl:        s.iMngURL.Text,
+					AdminURL:             s.iAdminURL.Text,
+					PreSharedKey:         s.iPreSharedKey.Text,
+					IsLinuxDesktopClient: runtime.GOOS == "linux",
 				})
 				if err != nil {
 					log.Errorf("login to management URL: %v", err)
@@ -233,7 +234,9 @@ func (s *serviceClient) login() error {
 		return err
 	}
 
-	loginResp, err := conn.Login(s.ctx, &proto.LoginRequest{})
+	loginResp, err := conn.Login(s.ctx, &proto.LoginRequest{
+		IsLinuxDesktopClient: runtime.GOOS == "linux",
+	})
 	if err != nil {
 		log.Errorf("login to management URL with: %v", err)
 		return err

infrastructure_files/configure.sh

@@ -170,10 +170,6 @@ if [ "$NETBIRD_DASH_AUTH_USE_AUDIENCE" = "false" ]; then
     export NETBIRD_AUTH_PKCE_AUDIENCE=
 fi
 
-# Read the encryption key
-encKey=$(grep DataStoreEncryptionKey management.json | awk -F'"' '{$0=$4}1')
-export NETBIRD_DATASTORE_ENC_KEY=$encKey
-
 env | grep NETBIRD
 
 envsubst <docker-compose.yml.tmpl >docker-compose.yml

infrastructure_files/management.json.tmpl

@@ -27,7 +27,6 @@
         "Password": null
     },
     "Datadir": "",
-    "DataStoreEncryptionKey": "$NETBIRD_DATASTORE_ENC_KEY",
     "HttpConfig": {
         "Address": "0.0.0.0:$NETBIRD_MGMT_API_PORT",
         "AuthIssuer": "$NETBIRD_AUTH_AUTHORITY",

management/cmd/management.go

@@ -301,23 +301,15 @@ var (
 func initEventStore(dataDir string, key string) (activity.Store, string, error) {
 	var err error
 	if key == "" {
-		log.Debugf("restore or generate new activity store encryption key")
-		key, err = sqlite.RestoreKey(dataDir)
-		if err == nil {
-			goto CreateStore
-		} else {
-			log.Debugf("failed to restore encryption key for activity store: %s", err)
-		}
-
-		log.Infof("generate new encryption key for activity store")
-		key, err = sqlite.GenerateKey(dataDir)
+		log.Debugf("generate new activity store encryption key")
+		key, err = sqlite.GenerateKey()
 		if err != nil {
 			return nil, "", err
 		}
 	}
-CreateStore:
 	store, err := sqlite.NewSQLiteStore(dataDir, key)
 	return store, key, err
+
 }
 
 func notifyStop(msg string) {

management/server/activity/sqlite/crypt.go

@@ -7,12 +7,6 @@ import (
 	"crypto/rand"
 	"encoding/base64"
 	"fmt"
-	"os"
-	"path/filepath"
-)
-
-const (
-	backupFile = ".datastore.key"
 )
 
 var iv = []byte{10, 22, 13, 79, 05, 8, 52, 91, 87, 98, 88, 98, 35, 25, 13, 05}
@@ -21,40 +15,16 @@ type FieldEncrypt struct {
 	block cipher.Block
 }
 
-func RestoreKey(dataDir string) (string, error) {
-	fName := filepath.Join(dataDir, backupFile)
-	data, err := os.ReadFile(fName)
-	return string(data), err
-}
-
-func GenerateKey(dataDir string) (string, error) {
+func GenerateKey() (string, error) {
 	key := make([]byte, 32)
 	_, err := rand.Read(key)
 	if err != nil {
 		return "", err
 	}
 	readableKey := base64.StdEncoding.EncodeToString(key)
-
-	err = saveKey(dataDir, readableKey)
-	if err != nil {
-		return "", err
-	}
 	return readableKey, nil
 }
 
-func saveKey(dataDir, key string) error {
-	f, err := os.Create(filepath.Join(dataDir, backupFile))
-	if err != nil {
-		return err
-	}
-	defer f.Close()
-	_, err = f.WriteString(key)
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 func NewFieldEncrypt(key string) (*FieldEncrypt, error) {
 	binKey, err := base64.StdEncoding.DecodeString(key)
 	if err != nil {

management/server/activity/sqlite/crypt_test.go

@@ -2,20 +2,11 @@ package sqlite
 
 import (
 	"testing"
-
-	log "github.com/sirupsen/logrus"
 )
 
-func TestRestoreKey(t *testing.T) {
-	_, err := RestoreKey(t.TempDir())
-	if err != nil {
-		log.Infof("err: %s", err)
-	}
-}
-
 func TestGenerateKey(t *testing.T) {
 	testData := "exampl@netbird.io"
-	key, err := GenerateKey(t.TempDir())
+	key, err := GenerateKey()
 	if err != nil {
 		t.Fatalf("failed to generate key: %s", err)
 	}
@@ -41,7 +32,7 @@ func TestGenerateKey(t *testing.T) {
 
 func TestCorruptKey(t *testing.T) {
 	testData := "exampl@netbird.io"
-	key, err := GenerateKey(t.TempDir())
+	key, err := GenerateKey()
 	if err != nil {
 		t.Fatalf("failed to generate key: %s", err)
 	}
@@ -55,7 +46,7 @@ func TestCorruptKey(t *testing.T) {
 		t.Fatalf("invalid encrypted text")
 	}
 
-	newKey, err := GenerateKey(t.TempDir())
+	newKey, err := GenerateKey()
 	if err != nil {
 		t.Fatalf("failed to generate key: %s", err)
 	}

management/server/activity/sqlite/sqlite.go

@@ -45,9 +45,6 @@ const (
 		"VALUES(?, ?, ?, ?, ?, ?)"
 
 	insertDeleteUserQuery = `INSERT INTO deleted_users(id, email, name) VALUES(?, ?, ?)`
-
-	fallbackName  = "unknown"
-	fallbackEmail = "unknown@unknown.com"
 )
 
 // Store is the implementation of the activity.Store interface backed by SQLite
@@ -131,7 +128,6 @@ func NewSQLiteStore(dataDir string, encryptionKey string) (*Store, error) {
 
 func (store *Store) processResult(result *sql.Rows) ([]*activity.Event, error) {
 	events := make([]*activity.Event, 0)
-	var cryptErr error
 	for result.Next() {
 		var id int64
 		var operation activity.Activity
@@ -160,8 +156,8 @@ func (store *Store) processResult(result *sql.Rows) ([]*activity.Event, error) {
 		if targetUserName != nil {
 			name, err := store.fieldEncrypt.Decrypt(*targetUserName)
 			if err != nil {
-				cryptErr = fmt.Errorf("failed to decrypt username for target id: %s", target)
-				meta["username"] = fallbackName
+				log.Errorf("failed to decrypt username for target id: %s", target)
+				meta["username"] = ""
 			} else {
 				meta["username"] = name
 			}
@@ -170,8 +166,8 @@ func (store *Store) processResult(result *sql.Rows) ([]*activity.Event, error) {
 		if targetEmail != nil {
 			email, err := store.fieldEncrypt.Decrypt(*targetEmail)
 			if err != nil {
-				cryptErr = fmt.Errorf("failed to decrypt email address for target id: %s", target)
-				meta["email"] = fallbackEmail
+				log.Errorf("failed to decrypt email address for target id: %s", target)
+				meta["email"] = ""
 			} else {
 				meta["email"] = email
 			}
@@ -190,8 +186,7 @@ func (store *Store) processResult(result *sql.Rows) ([]*activity.Event, error) {
 		if initiatorName != nil {
 			name, err := store.fieldEncrypt.Decrypt(*initiatorName)
 			if err != nil {
-				cryptErr = fmt.Errorf("failed to decrypt username of initiator: %s", initiator)
-				event.InitiatorName = fallbackName
+				log.Errorf("failed to decrypt username of initiator: %s", initiator)
 			} else {
 				event.InitiatorName = name
 			}
@@ -200,8 +195,7 @@ func (store *Store) processResult(result *sql.Rows) ([]*activity.Event, error) {
 		if initiatorEmail != nil {
 			email, err := store.fieldEncrypt.Decrypt(*initiatorEmail)
 			if err != nil {
-				cryptErr = fmt.Errorf("failed to decrypt email address of initiator: %s", initiator)
-				event.InitiatorEmail = fallbackEmail
+				log.Errorf("failed to decrypt email address of initiator: %s", initiator)
 			} else {
 				event.InitiatorEmail = email
 			}
@@ -210,10 +204,6 @@ func (store *Store) processResult(result *sql.Rows) ([]*activity.Event, error) {
 		events = append(events, event)
 	}
 
-	if cryptErr != nil {
-		log.Warnf("%s", cryptErr)
-	}
-
 	return events, nil
 }
 

management/server/activity/sqlite/sqlite_test.go

@@ -12,7 +12,7 @@ import (
 
 func TestNewSQLiteStore(t *testing.T) {
 	dataDir := t.TempDir()
-	key, _ := GenerateKey(dataDir)
+	key, _ := GenerateKey()
 	store, err := NewSQLiteStore(dataDir, key)
 	if err != nil {
 		t.Fatal(err)

management/server/grpcserver.go

@@ -61,7 +61,7 @@ func NewServer(config *Config, accountManager AccountManager, peersUpdateManager
 	if appMetrics != nil {
 		// update gauge based on number of connected peers which is equal to open gRPC streams
 		err = appMetrics.GRPCMetrics().RegisterConnectedStreams(func() int64 {
-			return int64(len(peersUpdateManager.peerChannels))
+			return peersUpdateManager.Len()
 		})
 		if err != nil {
 			return nil, err

management/server/updatechannel.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"sync"
+	"sync/atomic"
 
 	log "github.com/sirupsen/logrus"
 
@@ -14,25 +15,29 @@ type UpdateMessage struct {
 	Update *proto.SyncResponse
 }
 
+type UpdateChannel chan *UpdateMessage
+
 type PeersUpdateManager struct {
 	// peerChannels is an update channel indexed by Peer.ID
-	peerChannels map[string]chan *UpdateMessage
-	channelsMux  *sync.Mutex
+	peerChannels sync.Map
+	// peerChannelLocks keeps the peer locks to organize channel creations
+	peerChannelLocks sync.Map
+	// len is the length of peerChannels
+	len atomic.Int64
 }
 
 // NewPeersUpdateManager returns a new instance of PeersUpdateManager
 func NewPeersUpdateManager() *PeersUpdateManager {
-	return &PeersUpdateManager{
-		peerChannels: make(map[string]chan *UpdateMessage),
-		channelsMux:  &sync.Mutex{},
-	}
+	return &PeersUpdateManager{}
 }
 
 // SendUpdate sends update message to the peer's channel
 func (p *PeersUpdateManager) SendUpdate(peerID string, update *UpdateMessage) {
-	p.channelsMux.Lock()
-	defer p.channelsMux.Unlock()
-	if channel, ok := p.peerChannels[peerID]; ok {
+	if ch, ok := p.peerChannels.Load(peerID); ok {
+		channel, ok := ch.(UpdateChannel)
+		if !ok {
+			log.Warnf("could not cast to UpdateChannel")
+		}
 		select {
 		case channel <- update:
 			log.Debugf("update was sent to channel for peer %s", peerID)
@@ -45,35 +50,48 @@ func (p *PeersUpdateManager) SendUpdate(peerID string, update *UpdateMessage) {
 }
 
 // CreateChannel creates a go channel for a given peer used to deliver updates relevant to the peer.
-func (p *PeersUpdateManager) CreateChannel(peerID string) chan *UpdateMessage {
-	p.channelsMux.Lock()
-	defer p.channelsMux.Unlock()
+func (p *PeersUpdateManager) CreateChannel(peerID string) UpdateChannel {
+	// we have to lock the whole operation by peerID as we do two non atomic operations:
+	// - closeChannel()
+	// - Store
+	value, _ := p.peerChannelLocks.LoadOrStore(peerID, &sync.Mutex{})
+	mtx := value.(*sync.Mutex)
+	mtx.Lock()
+	defer mtx.Unlock()
+
+	p.closeChannel(peerID)
 
-	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[peerID] = channel
+	channel := make(UpdateChannel, channelBufferSize)
+	p.peerChannels.Store(peerID, channel)
+	p.len.Add(1)
 
 	log.Debugf("opened updates channel for a peer %s", peerID)
 	return channel
 }
 
-func (p *PeersUpdateManager) closeChannel(peerID string) {
-	if channel, ok := p.peerChannels[peerID]; ok {
-		delete(p.peerChannels, peerID)
-		close(channel)
+func (p *PeersUpdateManager) GetChannel(peerID string) UpdateChannel {
+	if ch, ok := p.peerChannels.Load(peerID); ok {
+		channel := ch.(UpdateChannel)
+		return channel
 	}
+	return nil
+}
 
-	log.Debugf("closed updates channel of a peer %s", peerID)
+func (p *PeersUpdateManager) closeChannel(peerID string) {
+	if ch, ok := p.peerChannels.LoadAndDelete(peerID); ok {
+		channel, ok := ch.(UpdateChannel)
+		if !ok {
+			log.Errorf("could not cast to UpdateChannel")
+		}
+		p.len.Add(-1)
+		close(channel)
+		log.Debugf("closed updates channel of a peer %s", peerID)
+	}
 }
 
 // CloseChannels closes updates channel for each given peer
 func (p *PeersUpdateManager) CloseChannels(peerIDs []string) {
-	p.channelsMux.Lock()
-	defer p.channelsMux.Unlock()
 	for _, id := range peerIDs {
 		p.closeChannel(id)
 	}
@@ -81,18 +99,22 @@ func (p *PeersUpdateManager) CloseChannels(peerIDs []string) {
 
 // CloseChannel closes updates channel of a given peer
 func (p *PeersUpdateManager) CloseChannel(peerID string) {
-	p.channelsMux.Lock()
-	defer p.channelsMux.Unlock()
 	p.closeChannel(peerID)
 }
 
 // GetAllConnectedPeers returns a copy of the connected peers map
 func (p *PeersUpdateManager) GetAllConnectedPeers() map[string]struct{} {
-	p.channelsMux.Lock()
-	defer p.channelsMux.Unlock()
 	m := make(map[string]struct{})
-	for ID := range p.peerChannels {
-		m[ID] = struct{}{}
-	}
+	p.peerChannels.Range(func(key any, value any) bool {
+		if ID, ok := key.(string); ok {
+			m[ID] = struct{}{}
+		}
+		return true
+	})
 	return m
 }
+
+// Len returns the length of the peer channels
+func (p *PeersUpdateManager) Len() (len int64) {
+	return p.len.Load()
+}

management/server/updatechannel_test.go

@@ -1,9 +1,10 @@
 package server
 
 import (
-	"github.com/netbirdio/netbird/management/proto"
 	"testing"
 	"time"
+
+	"github.com/netbirdio/netbird/management/proto"
 )
 
 //var peersUpdater *PeersUpdateManager
@@ -13,10 +14,22 @@ func TestCreateChannel(t *testing.T) {
 	peersUpdater := NewPeersUpdateManager()
 	defer peersUpdater.CloseChannel(peer)
 
+	if peersUpdater.Len() != 0 {
+		t.Error("peersUpdated should not have any channels yet")
+	}
+
+	if ch := peersUpdater.GetChannel(peer); ch != nil {
+		t.Errorf("We should not have channel for %s yet", peer)
+	}
+
 	_ = peersUpdater.CreateChannel(peer)
-	if _, ok := peersUpdater.peerChannels[peer]; !ok {
+	if ch := peersUpdater.GetChannel(peer); ch == nil {
 		t.Error("Error creating the channel")
 	}
+
+	if peersUpdater.Len() != 1 {
+		t.Error("peersUpdated should have 1 channel")
+	}
 }
 
 func TestSendUpdate(t *testing.T) {
@@ -28,12 +41,12 @@ func TestSendUpdate(t *testing.T) {
 		},
 	}}
 	_ = peersUpdater.CreateChannel(peer)
-	if _, ok := peersUpdater.peerChannels[peer]; !ok {
+	if ch := peersUpdater.GetChannel(peer); ch == nil {
 		t.Error("Error creating the channel")
 	}
 	peersUpdater.SendUpdate(peer, update1)
 	select {
-	case <-peersUpdater.peerChannels[peer]:
+	case <-peersUpdater.GetChannel(peer):
 	default:
 		t.Error("Update wasn't send")
 	}
@@ -54,7 +67,7 @@ func TestSendUpdate(t *testing.T) {
 		select {
 		case <-timeout:
 			t.Error("timed out reading previously sent updates")
-		case updateReader := <-peersUpdater.peerChannels[peer]:
+		case updateReader := <-peersUpdater.GetChannel(peer):
 			if updateReader.Update.NetworkMap.Serial == update2.Update.NetworkMap.Serial {
 				t.Error("got the update that shouldn't have been sent")
 			}
@@ -67,11 +80,11 @@ func TestCloseChannel(t *testing.T) {
 	peer := "test-close"
 	peersUpdater := NewPeersUpdateManager()
 	_ = peersUpdater.CreateChannel(peer)
-	if _, ok := peersUpdater.peerChannels[peer]; !ok {
+	if ch := peersUpdater.GetChannel(peer); ch == nil {
 		t.Error("Error creating the channel")
 	}
 	peersUpdater.CloseChannel(peer)
-	if _, ok := peersUpdater.peerChannels[peer]; ok {
+	if ch := peersUpdater.GetChannel(peer); ch != nil {
 		t.Error("Error closing the channel")
 	}
 }