diff --git a/client/android/client.go b/client/android/client.go index 99ccdf393..3a0a02c0b 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -151,9 +151,9 @@ func (c *Client) Run(platformFiles PlatformFiles, urlOpener URLOpener, isAndroid // todo do not throw error in case of cancelled context ctx = internal.CtxInitState(ctx) - connectClient := internal.NewConnectClient(ctx, cfg, c.recorder) + connectClient := internal.NewConnectClient(ctx, c.recorder) c.setState(cfg, cacheDir, connectClient) - return connectClient.RunOnAndroid(c.tunAdapter, c.iFaceDiscover, c.networkChangeListener, slices.Clone(dns.items), dnsReadyListener, stateFile, cacheDir) + return connectClient.RunOnAndroid(cfg, c.tunAdapter, c.iFaceDiscover, c.networkChangeListener, slices.Clone(dns.items), dnsReadyListener, stateFile, cacheDir) } // RunWithoutLogin we apply this type of run function when the backed has been started without UI (i.e. after reboot). @@ -186,9 +186,9 @@ func (c *Client) RunWithoutLogin(platformFiles PlatformFiles, dns *DNSList, dnsR // todo do not throw error in case of cancelled context ctx = internal.CtxInitState(ctx) - connectClient := internal.NewConnectClient(ctx, cfg, c.recorder) + connectClient := internal.NewConnectClient(ctx, c.recorder) c.setState(cfg, cacheDir, connectClient) - return connectClient.RunOnAndroid(c.tunAdapter, c.iFaceDiscover, c.networkChangeListener, slices.Clone(dns.items), dnsReadyListener, stateFile, cacheDir) + return connectClient.RunOnAndroid(cfg, c.tunAdapter, c.iFaceDiscover, c.networkChangeListener, slices.Clone(dns.items), dnsReadyListener, stateFile, cacheDir) } // Stop the internal client and free the resources diff --git a/client/cmd/up.go b/client/cmd/up.go index cabd0aacf..79e47ad99 100644 --- a/client/cmd/up.go +++ b/client/cmd/up.go @@ -202,10 +202,10 @@ func runInForegroundMode(ctx context.Context, cmd *cobra.Command, activeProf *pr r := peer.NewRecorder(config.ManagementURL.String()) r.GetFullStatus() - connectClient := internal.NewConnectClient(ctx, config, r) + connectClient := internal.NewConnectClient(ctx, r) SetupDebugHandler(ctx, config, r, connectClient, "") - return connectClient.Run(nil, util.FindFirstLogPath(logFiles)) + return connectClient.Run(config, nil, util.FindFirstLogPath(logFiles)) } func runInDaemonMode(ctx context.Context, cmd *cobra.Command, pm *profilemanager.ProfileManager, activeProf *profilemanager.Profile, profileSwitched bool) error { diff --git a/client/embed/embed.go b/client/embed/embed.go index 0e8991be2..e0d123a4e 100644 --- a/client/embed/embed.go +++ b/client/embed/embed.go @@ -264,7 +264,7 @@ func (c *Client) Start(startCtx context.Context) error { if err, _ := authClient.Login(ctx, c.setupKey, c.jwtToken); err != nil { return fmt.Errorf("login: %w", err) } - client := internal.NewConnectClient(ctx, c.config, c.recorder) + client := internal.NewConnectClient(ctx, c.recorder) client.SetSyncResponsePersistence(true) // either startup error (permanent backoff err) or nil err (successful engine up) @@ -272,7 +272,7 @@ func (c *Client) Start(startCtx context.Context) error { run := make(chan struct{}) clientErr := make(chan error, 1) go func() { - if err := client.Run(run, ""); err != nil { + if err := client.Run(c.config, run, ""); err != nil { clientErr <- err } }() diff --git a/client/internal/connect.go b/client/internal/connect.go index b20bff289..8a99bb1f9 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -50,11 +50,10 @@ import ( // androidRunOverride is set on Android to inject mobile dependencies // when using embed.Client (which calls Run() with empty MobileDependency). -var androidRunOverride func(c *ConnectClient, runningChan chan struct{}, logPath string) error +var androidRunOverride func(c *ConnectClient, config *profilemanager.Config, runningChan chan struct{}, logPath string) error type ConnectClient struct { ctx context.Context - config *profilemanager.Config statusRecorder *peer.Status engine *Engine @@ -71,12 +70,10 @@ type ConnectClient struct { func NewConnectClient( ctx context.Context, - config *profilemanager.Config, statusRecorder *peer.Status, ) *ConnectClient { c := &ConnectClient{ ctx: ctx, - config: config, statusRecorder: statusRecorder, engineMutex: sync.Mutex{}, } @@ -89,15 +86,16 @@ func (c *ConnectClient) SetUpdateManager(um *updater.Manager) { } // Run with main logic. -func (c *ConnectClient) Run(runningChan chan struct{}, logPath string) error { +func (c *ConnectClient) Run(config *profilemanager.Config, runningChan chan struct{}, logPath string) error { if androidRunOverride != nil { - return androidRunOverride(c, runningChan, logPath) + return androidRunOverride(c, config, runningChan, logPath) } - return c.sup.start(MobileDependency{}, runningChan, logPath) + return c.sup.start(config, MobileDependency{}, runningChan, logPath) } // RunOnAndroid with main logic on mobile system func (c *ConnectClient) RunOnAndroid( + config *profilemanager.Config, tunAdapter device.TunAdapter, iFaceDiscover stdnet.ExternalIFaceDiscover, networkChangeListener listener.NetworkChangeListener, @@ -116,10 +114,11 @@ func (c *ConnectClient) RunOnAndroid( StateFilePath: stateFilePath, TempDir: cacheDir, } - return c.sup.start(mobileDependency, nil, "") + return c.sup.start(config, mobileDependency, nil, "") } func (c *ConnectClient) RunOniOS( + config *profilemanager.Config, fileDescriptor int32, networkChangeListener listener.NetworkChangeListener, dnsManager dns.IosDnsManager, @@ -134,12 +133,12 @@ func (c *ConnectClient) RunOniOS( DnsManager: dnsManager, StateFilePath: stateFilePath, } - return c.sup.start(mobileDependency, nil, "") + return c.sup.start(config, mobileDependency, nil, "") } // run executes a single client run. runCtx is owned by the supervisor: cancelling // it tears the run down (it is the parent of the per-attempt engine context). -func (c *ConnectClient) run(runCtx context.Context, mobileDependency MobileDependency, runningChan chan struct{}, logPath string) error { +func (c *ConnectClient) run(runCtx context.Context, config *profilemanager.Config, mobileDependency MobileDependency, runningChan chan struct{}, logPath string) error { defer func() { if r := recover(); r != nil { rec := c.statusRecorder @@ -203,18 +202,18 @@ func (c *ConnectClient) run(runCtx context.Context, mobileDependency MobileDepen }() wrapErr := state.Wrap - myPrivateKey, err := wgtypes.ParseKey(c.config.PrivateKey) + myPrivateKey, err := wgtypes.ParseKey(config.PrivateKey) if err != nil { - log.Errorf("failed parsing Wireguard key %s: [%s]", c.config.PrivateKey, err.Error()) + log.Errorf("failed parsing Wireguard key %s: [%s]", config.PrivateKey, err.Error()) return wrapErr(err) } var mgmTlsEnabled bool - if c.config.ManagementURL.Scheme == "https" { + if config.ManagementURL.Scheme == "https" { mgmTlsEnabled = true } - publicSSHKey, err := ssh.GeneratePublicKey([]byte(c.config.SSHKey)) + publicSSHKey, err := ssh.GeneratePublicKey([]byte(config.SSHKey)) if err != nil { return err } @@ -262,8 +261,8 @@ func (c *ConnectClient) run(runCtx context.Context, mobileDependency MobileDepen cancel() }() - log.Debugf("connecting to the Management service %s", c.config.ManagementURL.Host) - mgmClient, err := mgm.NewClient(engineCtx, c.config.ManagementURL.Host, myPrivateKey, mgmTlsEnabled) + log.Debugf("connecting to the Management service %s", config.ManagementURL.Host) + mgmClient, err := mgm.NewClient(engineCtx, config.ManagementURL.Host, myPrivateKey, mgmTlsEnabled) if err != nil { return wrapErr(gstatus.Errorf(codes.FailedPrecondition, "failed connecting to Management Service : %s", err)) } @@ -280,7 +279,7 @@ func (c *ConnectClient) run(runCtx context.Context, mobileDependency MobileDepen } c.clientMetrics.UpdateAgentInfo(agentInfo, myPrivateKey.PublicKey().String()) - log.Debugf("connected to the Management service %s", c.config.ManagementURL.Host) + log.Debugf("connected to the Management service %s", config.ManagementURL.Host) defer func() { if err = mgmClient.Close(); err != nil { log.Warnf("failed to close the Management service client %v", err) @@ -289,7 +288,7 @@ func (c *ConnectClient) run(runCtx context.Context, mobileDependency MobileDepen // connect (just a connection, no stream yet) and login to Management Service to get an initial global Netbird config loginStarted := time.Now() - loginResp, err := loginToManagement(engineCtx, mgmClient, publicSSHKey, c.config) + loginResp, err := loginToManagement(engineCtx, mgmClient, publicSSHKey, config) if err != nil { c.clientMetrics.RecordLoginDuration(engineCtx, time.Since(loginStarted), false) log.Debug(err) @@ -349,7 +348,7 @@ func (c *ConnectClient) run(runCtx context.Context, mobileDependency MobileDepen } peerConfig := loginResp.GetPeerConfig() - engineConfig, err := createEngineConfig(myPrivateKey, c.config, peerConfig, logPath) + engineConfig, err := createEngineConfig(myPrivateKey, config, peerConfig, logPath) if err != nil { log.Error(err) return wrapErr(err) @@ -393,7 +392,7 @@ func (c *ConnectClient) run(runCtx context.Context, mobileDependency MobileDepen c.engine = engine c.engineMutex.Unlock() - if err := engine.Start(loginResp.GetNetbirdConfig(), c.config.ManagementURL); err != nil { + if err := engine.Start(loginResp.GetNetbirdConfig(), config.ManagementURL); err != nil { log.Errorf("error while starting Netbird Connection Engine: %s", err) return wrapErr(err) } diff --git a/client/internal/connect_android_default.go b/client/internal/connect_android_default.go index b05e91fec..e22c52ace 100644 --- a/client/internal/connect_android_default.go +++ b/client/internal/connect_android_default.go @@ -7,6 +7,7 @@ import ( "github.com/netbirdio/netbird/client/internal/dns" "github.com/netbirdio/netbird/client/internal/listener" + "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/internal/stdnet" ) @@ -64,8 +65,9 @@ func init() { // dependencies so the engine's existing Android code paths work unchanged. // Applications that need P2P ICE or real DNS should replace this by // setting androidRunOverride before calling Start(). - androidRunOverride = func(c *ConnectClient, runningChan chan struct{}, logPath string) error { + androidRunOverride = func(c *ConnectClient, config *profilemanager.Config, runningChan chan struct{}, logPath string) error { return c.runOnAndroidEmbed( + config, noopIFaceDiscover{}, noopNetworkChangeListener{}, []netip.AddrPort{}, diff --git a/client/internal/connect_android_embed.go b/client/internal/connect_android_embed.go index c96a0b41d..c91937759 100644 --- a/client/internal/connect_android_embed.go +++ b/client/internal/connect_android_embed.go @@ -7,6 +7,7 @@ import ( "github.com/netbirdio/netbird/client/internal/dns" "github.com/netbirdio/netbird/client/internal/listener" + "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/internal/stdnet" ) @@ -15,6 +16,7 @@ import ( // It provides complete MobileDependency so the engine's existing // Android code paths work unchanged. func (c *ConnectClient) runOnAndroidEmbed( + config *profilemanager.Config, iFaceDiscover stdnet.ExternalIFaceDiscover, networkChangeListener listener.NetworkChangeListener, dnsAddresses []netip.AddrPort, @@ -28,5 +30,5 @@ func (c *ConnectClient) runOnAndroidEmbed( HostDNSAddresses: dnsAddresses, DnsReadyListener: dnsReadyListener, } - return c.sup.start(mobileDependency, runningChan, logPath) + return c.sup.start(config, mobileDependency, runningChan, logPath) } diff --git a/client/internal/connect_lifecycle.go b/client/internal/connect_lifecycle.go index 4ad010253..090acd701 100644 --- a/client/internal/connect_lifecycle.go +++ b/client/internal/connect_lifecycle.go @@ -3,6 +3,8 @@ package internal import ( "context" "errors" + + "github.com/netbirdio/netbird/client/internal/profilemanager" ) // errAlreadyRunning is returned when a start is requested while a run is already @@ -24,6 +26,7 @@ const ( // - for opStop it receives nil once the in-flight run has fully unwound. type lifecycleCmd struct { op lifecycleOp + config *profilemanager.Config mobileDep MobileDependency runningChan chan struct{} logPath string @@ -36,8 +39,9 @@ type runEndResult struct { err error } -// runFunc executes a single client run bound to the supervisor-owned context. -type runFunc func(ctx context.Context, mobileDep MobileDependency, runningChan chan struct{}, logPath string) error +// runFunc executes a single client run bound to the supervisor-owned context, +// with the config supplied by the start request. +type runFunc func(ctx context.Context, config *profilemanager.Config, mobileDep MobileDependency, runningChan chan struct{}, logPath string) error // supervisor serializes start/stop of a single client run. Every request goes // through cmdCh and is handled one at a time by the loop goroutine, so two @@ -98,10 +102,10 @@ func (s *supervisor) handleStart(cmd lifecycleCmd) { s.runCancel = cancel s.curExecOp = &cmd - go func(ctx context.Context, m MobileDependency, rc chan struct{}, lp string) { - err := s.run(ctx, m, rc, lp) + go func(ctx context.Context, cfg *profilemanager.Config, m MobileDependency, rc chan struct{}, lp string) { + err := s.run(ctx, cfg, m, rc, lp) s.runEnded <- runEndResult{err: err} - }(runCtx, cmd.mobileDep, cmd.runningChan, cmd.logPath) + }(runCtx, cmd.config, cmd.mobileDep, cmd.runningChan, cmd.logPath) } func (s *supervisor) handleStop(cmd lifecycleCmd) { @@ -150,8 +154,8 @@ func (s *supervisor) shutdown() { // startAsync enqueues a start without blocking. If done is non-nil it receives // the run's end result (or errAlreadyRunning on rejection, or the context error // on shutdown). -func (s *supervisor) startAsync(mobileDep MobileDependency, runningChan chan struct{}, logPath string, done chan error) { - cmd := lifecycleCmd{op: opStart, mobileDep: mobileDep, runningChan: runningChan, logPath: logPath, done: done} +func (s *supervisor) startAsync(config *profilemanager.Config, mobileDep MobileDependency, runningChan chan struct{}, logPath string, done chan error) { + cmd := lifecycleCmd{op: opStart, config: config, mobileDep: mobileDep, runningChan: runningChan, logPath: logPath, done: done} select { case s.cmdCh <- cmd: case <-s.ctx.Done(): @@ -161,9 +165,9 @@ func (s *supervisor) startAsync(mobileDep MobileDependency, runningChan chan str // start enqueues a start and blocks until the run terminates, preserving the // blocking contract of the legacy Run entry points. -func (s *supervisor) start(mobileDep MobileDependency, runningChan chan struct{}, logPath string) error { +func (s *supervisor) start(config *profilemanager.Config, mobileDep MobileDependency, runningChan chan struct{}, logPath string) error { done := make(chan error, 1) - s.startAsync(mobileDep, runningChan, logPath, done) + s.startAsync(config, mobileDep, runningChan, logPath, done) select { case err := <-done: return err diff --git a/client/ios/NetBirdSDK/client.go b/client/ios/NetBirdSDK/client.go index bafbb0031..80877c56f 100644 --- a/client/ios/NetBirdSDK/client.go +++ b/client/ios/NetBirdSDK/client.go @@ -161,8 +161,8 @@ func (c *Client) Run(fd int32, interfaceName string, envList *EnvList) error { c.onHostDnsFn = func([]string) {} cfg.WgIface = interfaceName - c.connectClient = internal.NewConnectClient(ctx, cfg, c.recorder) - return c.connectClient.RunOniOS(fd, c.networkChangeListener, c.dnsManager, c.stateFile) + c.connectClient = internal.NewConnectClient(ctx, c.recorder) + return c.connectClient.RunOniOS(cfg, fd, c.networkChangeListener, c.dnsManager, c.stateFile) } // Stop the internal client and free the resources diff --git a/client/server/mdm.go b/client/server/mdm.go index 0da0ec5d1..c47c211d4 100644 --- a/client/server/mdm.go +++ b/client/server/mdm.go @@ -159,8 +159,9 @@ func (s *Server) restartEngineForMDMLocked() error { s.clientRunning = true s.clientRunningChan = make(chan struct{}) s.clientGiveUpChan = make(chan struct{}) + client := s.newSessionClient(ctx) log.Info("MDM restart: spawning connectWithRetryRuns with re-resolved config") - go s.connectWithRetryRuns(ctx, config, s.statusRecorder, s.clientRunningChan, s.clientGiveUpChan) + go s.connectWithRetryRuns(ctx, client, config, s.statusRecorder, s.clientRunningChan, s.clientGiveUpChan) s.publishConfigChangedEvent("mdm") return nil } diff --git a/client/server/server.go b/client/server/server.go index 1db2aa7a2..55da13b7b 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -235,7 +235,8 @@ func (s *Server) Start() error { s.clientRunning = true s.clientRunningChan = make(chan struct{}) s.clientGiveUpChan = make(chan struct{}) - go s.connectWithRetryRuns(ctx, config, s.statusRecorder, s.clientRunningChan, s.clientGiveUpChan) + client := s.newSessionClient(ctx) + go s.connectWithRetryRuns(ctx, client, config, s.statusRecorder, s.clientRunningChan, s.clientGiveUpChan) s.publishConfigChangedEvent("startup") return nil } @@ -252,7 +253,7 @@ func (s *Server) Start() error { // of clientGiveUpChan. The defer does NOT touch s.mutex; the daemon's // "intent" (clientRunning) is maintained by the RPC handlers, not by this // goroutine. -func (s *Server) connectWithRetryRuns(ctx context.Context, profileConfig *profilemanager.Config, statusRecorder *peer.Status, runningChan chan struct{}, giveUpChan chan struct{}) { +func (s *Server) connectWithRetryRuns(ctx context.Context, client *internal.ConnectClient, profileConfig *profilemanager.Config, statusRecorder *peer.Status, runningChan chan struct{}, giveUpChan chan struct{}) { defer func() { if giveUpChan != nil { close(giveUpChan) @@ -260,7 +261,7 @@ func (s *Server) connectWithRetryRuns(ctx context.Context, profileConfig *profil }() if s.config.DisableAutoConnect { - if err := s.connect(ctx, s.config, s.statusRecorder, runningChan); err != nil { + if err := s.connect(client, s.config, runningChan); err != nil { log.Debugf("run client connection exited with error: %v", err) } log.Tracef("client connection exited") @@ -289,7 +290,7 @@ func (s *Server) connectWithRetryRuns(ctx context.Context, profileConfig *profil }() runOperation := func() error { - err := s.connect(ctx, profileConfig, statusRecorder, runningChan) + err := s.connect(client, profileConfig, runningChan) if err != nil { log.Debugf("run client connection exited with error: %v. Will retry in the background", err) return err @@ -837,7 +838,8 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR s.clientRunningChan = make(chan struct{}) s.clientGiveUpChan = make(chan struct{}) - go s.connectWithRetryRuns(ctx, s.config, s.statusRecorder, s.clientRunningChan, s.clientGiveUpChan) + client := s.newSessionClient(ctx) + go s.connectWithRetryRuns(ctx, client, s.config, s.statusRecorder, s.clientRunningChan, s.clientGiveUpChan) s.publishConfigChangedEvent("up_rpc") s.mutex.Unlock() @@ -1752,14 +1754,25 @@ func (s *Server) GetFeatures(ctx context.Context, msg *proto.GetFeaturesRequest) return features, nil } -func (s *Server) connect(client *internal.ConnectClient, runningChan chan struct{}) error { +func (s *Server) connect(client *internal.ConnectClient, config *profilemanager.Config, runningChan chan struct{}) error { log.Tracef("running client connection") - if err := client.Run(runningChan, s.logFile); err != nil { + if err := client.Run(config, runningChan, s.logFile); err != nil { return err } return nil } +// newSessionClient builds a ConnectClient for a connection session and records +// it as the active client. Config is supplied per Run (see connect), not at +// construction, so the same client can be reused across reconnects. +func (s *Server) newSessionClient(ctx context.Context) *internal.ConnectClient { + client := internal.NewConnectClient(ctx, s.statusRecorder) + client.SetUpdateManager(s.updateManager) + client.SetSyncResponsePersistence(s.persistSyncResponse) + s.connectClient = client + return client +} + // MDM authority: when the platform-native MDM source sets a kill switch // key (regardless of true/false value), that value wins. The CLI flag // supplied at service install time is the fallback used only when the diff --git a/client/server/server_connect_test.go b/client/server/server_connect_test.go index 0c6e03a4a..7bbbe0d14 100644 --- a/client/server/server_connect_test.go +++ b/client/server/server_connect_test.go @@ -22,7 +22,7 @@ func newTestServer() *Server { } func newDummyConnectClient(ctx context.Context) *internal.ConnectClient { - return internal.NewConnectClient(ctx, nil, nil) + return internal.NewConnectClient(ctx, nil) } // TestConnectSetsClientWithMutex validates that connect() sets s.connectClient diff --git a/client/server/server_test.go b/client/server/server_test.go index 66e0fcc4c..29ba7c3fb 100644 --- a/client/server/server_test.go +++ b/client/server/server_test.go @@ -114,7 +114,8 @@ func TestConnectWithRetryRuns(t *testing.T) { t.Setenv(maxRetryTimeVar, "5s") t.Setenv(retryMultiplierVar, "1") - s.connectWithRetryRuns(ctx, config, s.statusRecorder, nil, nil) + client := s.newSessionClient(ctx) + s.connectWithRetryRuns(ctx, client, config, s.statusRecorder, nil, nil) if counter < 3 { t.Fatalf("expected counter > 2, got %d", counter) }