diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml index 42f740e9b54..4259f1b3e12 100644 --- a/.github/workflows/golang-test-linux.yml +++ b/.github/workflows/golang-test-linux.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: arch: [ '386','amd64' ] - store: [ 'jsonfile', 'sqlite' ] + store: [ 'jsonfile', 'sqlite', 'postgres'] runs-on: ubuntu-latest steps: - name: Install Go diff --git a/.golangci.yaml b/.golangci.yaml index 3c5f4d5b8f2..44b03d0e10a 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -130,3 +130,10 @@ issues: - path: mock\.go linters: - nilnil + # Exclude specific deprecation warnings for grpc methods + - linters: + - staticcheck + text: "grpc.DialContext is deprecated" + - linters: + - staticcheck + text: "grpc.WithBlock is deprecated" diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index b4a1a61c87c..7eb8e4e600d 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -5,7 +5,7 @@ We as members, contributors, and leaders pledge to make participation in our community a harassment-free experience for everyone, regardless of age, body size, visible or invisible disability, ethnicity, sex characteristics, gender -identity and expression, level of experience, education, socio-economic status, +identity and expression, level of experience, education, socioeconomic status, nationality, personal appearance, race, caste, color, religion, or sexual identity and orientation. diff --git a/client/android/client.go b/client/android/client.go index d0efb47ed27..d937e132e35 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -57,15 +57,17 @@ type Client struct { ctxCancel context.CancelFunc ctxCancelLock *sync.Mutex deviceName string + uiVersion string networkChangeListener listener.NetworkChangeListener } // NewClient instantiate a new Client -func NewClient(cfgFile, deviceName string, tunAdapter TunAdapter, iFaceDiscover IFaceDiscover, networkChangeListener NetworkChangeListener) *Client { +func NewClient(cfgFile, deviceName string, uiVersion string, tunAdapter TunAdapter, iFaceDiscover IFaceDiscover, networkChangeListener NetworkChangeListener) *Client { net.SetAndroidProtectSocketFn(tunAdapter.ProtectSocket) return &Client{ cfgFile: cfgFile, deviceName: deviceName, + uiVersion: uiVersion, tunAdapter: tunAdapter, iFaceDiscover: iFaceDiscover, recorder: peer.NewRecorder(""), @@ -88,6 +90,9 @@ func (c *Client) Run(urlOpener URLOpener, dns *DNSList, dnsReadyListener DnsRead var ctx context.Context //nolint ctxWithValues := context.WithValue(context.Background(), system.DeviceNameCtxKey, c.deviceName) + //nolint + ctxWithValues = context.WithValue(ctxWithValues, system.UiVersionCtxKey, c.uiVersion) + c.ctxCancelLock.Lock() ctx, c.ctxCancel = context.WithCancel(ctxWithValues) defer c.ctxCancel() diff --git a/client/cmd/debug.go b/client/cmd/debug.go index 4deff11a6ff..761fc8b7260 100644 --- a/client/cmd/debug.go +++ b/client/cmd/debug.go @@ -3,13 +3,14 @@ package cmd import ( "context" "fmt" - "strings" "time" "github.com/spf13/cobra" "google.golang.org/grpc/status" + "github.com/netbirdio/netbird/client/internal" "github.com/netbirdio/netbird/client/proto" + "github.com/netbirdio/netbird/client/server" ) var debugCmd = &cobra.Command{ @@ -86,7 +87,7 @@ func setLogLevel(cmd *cobra.Command, args []string) error { defer conn.Close() client := proto.NewDaemonServiceClient(conn) - level := parseLogLevel(args[0]) + level := server.ParseLogLevel(args[0]) if level == proto.LogLevel_UNKNOWN { return fmt.Errorf("unknown log level: %s. Available levels are: panic, fatal, error, warn, info, debug, trace\n", args[0]) } @@ -102,27 +103,6 @@ func setLogLevel(cmd *cobra.Command, args []string) error { return nil } -func parseLogLevel(level string) proto.LogLevel { - switch strings.ToLower(level) { - case "panic": - return proto.LogLevel_PANIC - case "fatal": - return proto.LogLevel_FATAL - case "error": - return proto.LogLevel_ERROR - case "warn": - return proto.LogLevel_WARN - case "info": - return proto.LogLevel_INFO - case "debug": - return proto.LogLevel_DEBUG - case "trace": - return proto.LogLevel_TRACE - default: - return proto.LogLevel_UNKNOWN - } -} - func runForDuration(cmd *cobra.Command, args []string) error { duration, err := time.ParseDuration(args[0]) if err != nil { @@ -137,18 +117,33 @@ func runForDuration(cmd *cobra.Command, args []string) error { client := proto.NewDaemonServiceClient(conn) + stat, err := client.Status(cmd.Context(), &proto.StatusRequest{}) + if err != nil { + return fmt.Errorf("failed to get status: %v", status.Convert(err).Message()) + } + + restoreUp := stat.Status == string(internal.StatusConnected) || stat.Status == string(internal.StatusConnecting) + + initialLogLevel, err := client.GetLogLevel(cmd.Context(), &proto.GetLogLevelRequest{}) + if err != nil { + return fmt.Errorf("failed to get log level: %v", status.Convert(err).Message()) + } + if _, err := client.Down(cmd.Context(), &proto.DownRequest{}); err != nil { return fmt.Errorf("failed to down: %v", status.Convert(err).Message()) } cmd.Println("Netbird down") - _, err = client.SetLogLevel(cmd.Context(), &proto.SetLogLevelRequest{ - Level: proto.LogLevel_TRACE, - }) - if err != nil { - return fmt.Errorf("failed to set log level to trace: %v", status.Convert(err).Message()) + initialLevelTrace := initialLogLevel.GetLevel() >= proto.LogLevel_TRACE + if !initialLevelTrace { + _, err = client.SetLogLevel(cmd.Context(), &proto.SetLogLevelRequest{ + Level: proto.LogLevel_TRACE, + }) + if err != nil { + return fmt.Errorf("failed to set log level to TRACE: %v", status.Convert(err).Message()) + } + cmd.Println("Log level set to trace.") } - cmd.Println("Log level set to trace.") time.Sleep(1 * time.Second) @@ -175,10 +170,22 @@ func runForDuration(cmd *cobra.Command, args []string) error { } cmd.Println("Netbird down") - // TODO reset log level - time.Sleep(1 * time.Second) + if restoreUp { + if _, err := client.Up(cmd.Context(), &proto.UpRequest{}); err != nil { + return fmt.Errorf("failed to up: %v", status.Convert(err).Message()) + } + cmd.Println("Netbird up") + } + + if !initialLevelTrace { + if _, err := client.SetLogLevel(cmd.Context(), &proto.SetLogLevelRequest{Level: initialLogLevel.GetLevel()}); err != nil { + return fmt.Errorf("failed to restore log level: %v", status.Convert(err).Message()) + } + cmd.Println("Log level restored to", initialLogLevel.GetLevel()) + } + cmd.Println("Creating debug bundle...") resp, err := client.DebugBundle(cmd.Context(), &proto.DebugBundleRequest{ diff --git a/client/cmd/down.go b/client/cmd/down.go index d906059ca3a..1837b13da5f 100644 --- a/client/cmd/down.go +++ b/client/cmd/down.go @@ -2,9 +2,10 @@ package cmd import ( "context" - "github.com/netbirdio/netbird/util" "time" + "github.com/netbirdio/netbird/util" + log "github.com/sirupsen/logrus" "github.com/spf13/cobra" diff --git a/client/cmd/testutil.go b/client/cmd/testutil.go index 2f92e1c03dc..35fd7c53762 100644 --- a/client/cmd/testutil.go +++ b/client/cmd/testutil.go @@ -14,6 +14,7 @@ import ( "google.golang.org/grpc" "github.com/netbirdio/management-integrations/integrations" + clientProto "github.com/netbirdio/netbird/client/proto" client "github.com/netbirdio/netbird/client/server" mgmtProto "github.com/netbirdio/netbird/management/proto" @@ -69,10 +70,11 @@ func startManagement(t *testing.T, config *mgmt.Config) (*grpc.Server, net.Liste t.Fatal(err) } s := grpc.NewServer() - store, err := mgmt.NewStoreFromJson(config.Datadir, nil) + store, cleanUp, err := mgmt.NewTestStoreFromJson(config.Datadir) if err != nil { t.Fatal(err) } + t.Cleanup(cleanUp) peersUpdateManager := mgmt.NewPeersUpdateManager(nil) eventStore := &activity.InMemoryEventStore{} diff --git a/client/firewall/create_linux.go b/client/firewall/create_linux.go index 22128f5a4d7..d0bc1b37b3e 100644 --- a/client/firewall/create_linux.go +++ b/client/firewall/create_linux.go @@ -42,20 +42,20 @@ func NewFirewall(context context.Context, iface IFaceMapper) (firewall.Manager, switch check() { case IPTABLES: - log.Debug("creating an iptables firewall manager") + log.Info("creating an iptables firewall manager") fm, errFw = nbiptables.Create(context, iface) if errFw != nil { log.Errorf("failed to create iptables manager: %s", errFw) } case NFTABLES: - log.Debug("creating an nftables firewall manager") + log.Info("creating an nftables firewall manager") fm, errFw = nbnftables.Create(context, iface) if errFw != nil { log.Errorf("failed to create nftables manager: %s", errFw) } default: errFw = fmt.Errorf("no firewall manager found") - log.Debug("no firewall manager found, try to use userspace packet filtering firewall") + log.Info("no firewall manager found, trying to use userspace packet filtering firewall") } if iface.IsUserspaceBind() { @@ -93,16 +93,58 @@ func SupportsIPv6() bool { // check returns the firewall type based on common lib checks. It returns UNKNOWN if no firewall is found. func check() FWType { - nf := nftables.Conn{} - if _, err := nf.ListChains(); err == nil && os.Getenv(SKIP_NFTABLES_ENV) != "true" { - return NFTABLES + useIPTABLES := false + var iptablesChains []string + ip, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) + if err == nil && isIptablesClientAvailable(ip) { + major, minor, _ := ip.GetIptablesVersion() + // use iptables when its version is lower than 1.8.0 which doesn't work well with our nftables manager + if major < 1 || (major == 1 && minor < 8) { + return IPTABLES + } + + useIPTABLES = true + + iptablesChains, err = ip.ListChains("filter") + if err != nil { + log.Errorf("failed to list iptables chains: %s", err) + useIPTABLES = false + } } - ip, err := iptables.NewWithProtocol(iptables.ProtocolIPv4) - if err != nil { - return UNKNOWN + nf := nftables.Conn{} + if chains, err := nf.ListChains(); err == nil && os.Getenv(SKIP_NFTABLES_ENV) != "true" { + if !useIPTABLES { + return NFTABLES + } + + // search for chains where table is filter + // if we find one, we assume that nftables manager can be used with iptables + for _, chain := range chains { + if chain.Table.Name == "filter" { + return NFTABLES + } + } + + // check tables for the following constraints: + // 1. there is no chain in nftables for the filter table and there is at least one chain in iptables, we assume that nftables manager can not be used + // 2. there is no tables or more than one table, we assume that nftables manager can be used + // 3. there is only one table and its name is filter, we assume that nftables manager can not be used, since there was no chain in it + // 4. if we find an error we log and continue with iptables check + nbTablesList, err := nf.ListTables() + switch { + case err == nil && len(iptablesChains) > 0: + return IPTABLES + case err == nil && len(nbTablesList) != 1: + return NFTABLES + case err == nil && len(nbTablesList) == 1 && nbTablesList[0].Name == "filter": + return IPTABLES + case err != nil: + log.Errorf("failed to list nftables tables on fw manager discovery: %s", err) + } } - if isIptablesClientAvailable(ip) { + + if useIPTABLES { return IPTABLES } diff --git a/client/internal/connect.go b/client/internal/connect.go index 0c84609325b..05df9bdeaa9 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net" "runtime" "runtime/debug" "strings" @@ -91,6 +92,9 @@ func (c *ConnectClient) RunOniOS( networkChangeListener listener.NetworkChangeListener, dnsManager dns.IosDnsManager, ) error { + // Set GC percent to 5% to reduce memory usage as iOS only allows 50MB of memory for the extension. + debug.SetGCPercent(5) + mobileDependency := MobileDependency{ FileDescriptor: fileDescriptor, NetworkChangeListener: networkChangeListener, @@ -328,6 +332,15 @@ func createEngineConfig(key wgtypes.Key, config *Config, peerConfig *mgmProto.Pe engineConf.PreSharedKey = &preSharedKey } + port, err := freePort(config.WgPort) + if err != nil { + return nil, err + } + if port != config.WgPort { + log.Infof("using %d as wireguard port: %d is in use", port, config.WgPort) + } + engineConf.WgPort = port + return engineConf, nil } @@ -377,3 +390,20 @@ func statusRecorderToSignalConnStateNotifier(statusRecorder *peer.Status) signal notifier, _ := sri.(signal.ConnStateNotifier) return notifier } + +func freePort(start int) (int, error) { + addr := net.UDPAddr{} + if start == 0 { + start = iface.DefaultWgPort + } + for x := start; x <= 65535; x++ { + addr.Port = x + conn, err := net.ListenUDP("udp", &addr) + if err != nil { + continue + } + conn.Close() + return x, nil + } + return 0, errors.New("no free ports") +} diff --git a/client/internal/connect_test.go b/client/internal/connect_test.go new file mode 100644 index 00000000000..6f4a6bbb7c7 --- /dev/null +++ b/client/internal/connect_test.go @@ -0,0 +1,57 @@ +package internal + +import ( + "net" + "testing" +) + +func Test_freePort(t *testing.T) { + tests := []struct { + name string + port int + want int + wantErr bool + }{ + { + name: "available", + port: 51820, + want: 51820, + wantErr: false, + }, + { + name: "notavailable", + port: 51830, + want: 51831, + wantErr: false, + }, + { + name: "noports", + port: 65535, + want: 0, + wantErr: true, + }, + } + for _, tt := range tests { + + c1, err := net.ListenUDP("udp", &net.UDPAddr{Port: 51830}) + if err != nil { + t.Errorf("freePort error = %v", err) + } + c2, err := net.ListenUDP("udp", &net.UDPAddr{Port: 65535}) + if err != nil { + t.Errorf("freePort error = %v", err) + } + t.Run(tt.name, func(t *testing.T) { + got, err := freePort(tt.port) + if (err != nil) != tt.wantErr { + t.Errorf("freePort() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("freePort() = %v, want %v", got, tt.want) + } + }) + c1.Close() + c2.Close() + } +} diff --git a/client/internal/dns/server.go b/client/internal/dns/server.go index 158c4fb253a..5910985ed7d 100644 --- a/client/internal/dns/server.go +++ b/client/internal/dns/server.go @@ -553,9 +553,7 @@ func (s *DefaultServer) upstreamCallbacks( if nsGroup.Primary { s.currentConfig.RouteAll = true - if runtime.GOOS == "android" { - s.service.RegisterMux(nbdns.RootZone, handler) - } + s.service.RegisterMux(nbdns.RootZone, handler) } if err := s.hostManager.applyDNSConfig(s.currentConfig); err != nil { l.WithError(err).Error("reactivate temporary disabled nameserver group, DNS update apply") diff --git a/client/internal/dns/upstream.go b/client/internal/dns/upstream.go index cc31559fab1..e82c98fbcb7 100644 --- a/client/internal/dns/upstream.go +++ b/client/internal/dns/upstream.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "net" - "runtime" "sync" "sync/atomic" "time" @@ -260,13 +259,10 @@ func (u *upstreamResolverBase) disable(err error) { return } - // todo test the deactivation logic, it seems to affect the client - if runtime.GOOS != "ios" { - log.Warnf("Upstream resolving is Disabled for %v", reactivatePeriod) - u.deactivate(err) - u.disabled = true - go u.waitUntilResponse() - } + log.Warnf("Upstream resolving is Disabled for %v", reactivatePeriod) + u.deactivate(err) + u.disabled = true + go u.waitUntilResponse() } func (u *upstreamResolverBase) testNameserver(server string) error { diff --git a/client/internal/engine.go b/client/internal/engine.go index 92f0f7dc4f4..e0a92eab1c5 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -134,7 +134,7 @@ type Engine struct { // networkSerial is the latest CurrentSerial (state ID) of the network sent by the Management service networkSerial uint64 - networkWatcher *networkmonitor.NetworkWatcher + networkMonitor *networkmonitor.NetworkMonitor sshServerFunc func(hostKeyPEM []byte, addr string) (nbssh.Server, error) sshServer nbssh.Server @@ -151,6 +151,8 @@ type Engine struct { signalProbe *Probe relayProbe *Probe wgProbe *Probe + + wgConnWorker sync.WaitGroup } // Peer is an instance of the Connection Peer @@ -213,7 +215,6 @@ func NewEngineWithProbes( networkSerial: 0, sshServerFunc: nbssh.DefaultSSHServer, statusRecorder: statusRecorder, - networkWatcher: networkmonitor.New(), mgmProbe: mgmProbe, signalProbe: signalProbe, relayProbe: relayProbe, @@ -230,7 +231,10 @@ func (e *Engine) Stop() error { } // stopping network monitor first to avoid starting the engine again - e.networkWatcher.Stop() + if e.networkMonitor != nil { + e.networkMonitor.Stop() + } + log.Info("Network monitor: stopped") err := e.removeAllPeers() if err != nil { @@ -244,6 +248,7 @@ func (e *Engine) Stop() error { time.Sleep(500 * time.Millisecond) e.close() + e.wgConnWorker.Wait() log.Infof("stopped Netbird Engine") return nil } @@ -260,7 +265,7 @@ func (e *Engine) Start() error { } e.ctx, e.cancel = context.WithCancel(e.clientCtx) - e.wgProxyFactory = wgproxy.NewFactory(e.clientCtx, e.config.WgPort) + e.wgProxyFactory = wgproxy.NewFactory(e.ctx, e.config.WgPort) wgIface, err := e.newWgIface() if err != nil { @@ -345,20 +350,8 @@ func (e *Engine) Start() error { e.receiveManagementEvents() e.receiveProbeEvents() - if e.config.NetworkMonitor { - // starting network monitor at the very last to avoid disruptions - go e.networkWatcher.Start(e.ctx, func() { - log.Infof("Network monitor detected network change, restarting engine") - if err := e.Stop(); err != nil { - log.Errorf("Failed to stop engine: %v", err) - } - if err := e.Start(); err != nil { - log.Errorf("Failed to start engine: %v", err) - } - }) - } else { - log.Infof("Network monitor is disabled, not starting") - } + // starting network monitor at the very last to avoid disruptions + e.startNetworkMonitor() return nil } @@ -907,18 +900,25 @@ func (e *Engine) addNewPeer(peerConfig *mgmProto.RemotePeerConfig) error { log.Warnf("error adding peer %s to status recorder, got error: %v", peerKey, err) } + e.wgConnWorker.Add(1) go e.connWorker(conn, peerKey) } return nil } func (e *Engine) connWorker(conn *peer.Conn, peerKey string) { + defer e.wgConnWorker.Done() for { // randomize starting time a bit min := 500 max := 2000 - time.Sleep(time.Duration(rand.Intn(max-min)+min) * time.Millisecond) + duration := time.Duration(rand.Intn(max-min)+min) * time.Millisecond + select { + case <-e.ctx.Done(): + return + case <-time.After(duration): + } // if peer has been removed -> give up if !e.peerExists(peerKey) { @@ -1427,3 +1427,26 @@ func (e *Engine) probeSTUNs() []relay.ProbeResult { func (e *Engine) probeTURNs() []relay.ProbeResult { return relay.ProbeAll(e.ctx, relay.ProbeTURN, e.TURNs) } + +func (e *Engine) startNetworkMonitor() { + if !e.config.NetworkMonitor { + log.Infof("Network monitor is disabled, not starting") + return + } + + e.networkMonitor = networkmonitor.New() + go func() { + err := e.networkMonitor.Start(e.ctx, func() { + log.Infof("Network monitor detected network change, restarting engine") + if err := e.Stop(); err != nil { + log.Errorf("Failed to stop engine: %v", err) + } + if err := e.Start(); err != nil { + log.Errorf("Failed to start engine: %v", err) + } + }) + if err != nil && !errors.Is(err, networkmonitor.ErrStopped) { + log.Errorf("Network monitor: %v", err) + } + }() +} diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 4b6e54d3303..f79354b51aa 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -229,6 +229,7 @@ func TestEngine_UpdateNetworkMap(t *testing.T) { t.Fatal(err) } engine.udpMux = bind.NewUniversalUDPMuxDefault(bind.UniversalUDPMuxParams{UDPConn: conn}) + engine.ctx = ctx type testCase struct { name string @@ -408,6 +409,7 @@ func TestEngine_Sync(t *testing.T) { WgPrivateKey: key, WgPort: 33100, }, MobileDependency{}, peer.NewRecorder("https://mgm")) + engine.ctx = ctx engine.dnsServer = &dns.MockServer{ UpdateDNSServerFunc: func(serial uint64, update nbdns.Config) error { return nil }, @@ -567,6 +569,7 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) { WgPrivateKey: key, WgPort: 33100, }, MobileDependency{}, peer.NewRecorder("https://mgm")) + engine.ctx = ctx newNet, err := stdnet.NewNet() if err != nil { t.Fatal(err) @@ -737,6 +740,8 @@ func TestEngine_UpdateNetworkMapWithDNSUpdate(t *testing.T) { WgPrivateKey: key, WgPort: 33100, }, MobileDependency{}, peer.NewRecorder("https://mgm")) + engine.ctx = ctx + newNet, err := stdnet.NewNet() if err != nil { t.Fatal(err) @@ -1005,7 +1010,9 @@ func createEngine(ctx context.Context, cancel context.CancelFunc, setupKey strin WgPort: wgPort, } - return NewEngine(ctx, cancel, signalClient, mgmtClient, conf, MobileDependency{}, peer.NewRecorder("https://mgm")), nil + e, err := NewEngine(ctx, cancel, signalClient, mgmtClient, conf, MobileDependency{}, peer.NewRecorder("https://mgm")), nil + e.ctx = ctx + return e, err } func startSignal() (*grpc.Server, string, error) { @@ -1044,7 +1051,7 @@ func startManagement(dataDir string) (*grpc.Server, string, error) { return nil, "", err } s := grpc.NewServer(grpc.KeepaliveEnforcementPolicy(kaep), grpc.KeepaliveParams(kasp)) - store, err := server.NewStoreFromJson(config.Datadir, nil) + store, _, err := server.NewTestStoreFromJson(config.Datadir) if err != nil { return nil, "", err } diff --git a/client/internal/networkmonitor/monitor.go b/client/internal/networkmonitor/monitor.go index 71cf031bafa..5475455c6cd 100644 --- a/client/internal/networkmonitor/monitor.go +++ b/client/internal/networkmonitor/monitor.go @@ -2,14 +2,20 @@ package networkmonitor import ( "context" + "errors" + "sync" ) -// NetworkWatcher watches for changes in network configuration. -type NetworkWatcher struct { +var ErrStopped = errors.New("monitor has been stopped") + +// NetworkMonitor watches for changes in network configuration. +type NetworkMonitor struct { cancel context.CancelFunc + wg sync.WaitGroup + mu sync.Mutex } // New creates a new network monitor. -func New() *NetworkWatcher { - return &NetworkWatcher{} +func New() *NetworkMonitor { + return &NetworkMonitor{} } diff --git a/client/internal/networkmonitor/monitor_bsd.go b/client/internal/networkmonitor/monitor_bsd.go index e15c08d7e43..de4209f5d48 100644 --- a/client/internal/networkmonitor/monitor_bsd.go +++ b/client/internal/networkmonitor/monitor_bsd.go @@ -31,7 +31,7 @@ func checkChange(ctx context.Context, nexthopv4 netip.Addr, intfv4 *net.Interfac for { select { case <-ctx.Done(): - return ctx.Err() + return ErrStopped default: buf := make([]byte, 2048) n, err := unix.Read(fd, buf) @@ -63,7 +63,7 @@ func checkChange(ctx context.Context, nexthopv4 netip.Addr, intfv4 *net.Interfac } log.Infof("Network monitor: monitored interface (%s) is down.", ifinfo.Name) - callback() + go callback() // handle route changes case unix.RTM_ADD, syscall.RTM_DELETE: @@ -84,11 +84,11 @@ func checkChange(ctx context.Context, nexthopv4 netip.Addr, intfv4 *net.Interfac switch msg.Type { case unix.RTM_ADD: log.Infof("Network monitor: default route changed: via %s, interface %s", route.Gw, intf) - callback() + go callback() case unix.RTM_DELETE: if intfv4 != nil && route.Gw.Compare(nexthopv4) == 0 || intfv6 != nil && route.Gw.Compare(nexthopv6) == 0 { log.Infof("Network monitor: default route removed: via %s, interface %s", route.Gw, intf) - callback() + go callback() } } } diff --git a/client/internal/networkmonitor/monitor_generic.go b/client/internal/networkmonitor/monitor_generic.go index 329246c8f48..97cfbc2ca83 100644 --- a/client/internal/networkmonitor/monitor_generic.go +++ b/client/internal/networkmonitor/monitor_generic.go @@ -5,6 +5,7 @@ package networkmonitor import ( "context" "errors" + "fmt" "net" "net/netip" "runtime/debug" @@ -15,20 +16,18 @@ import ( "github.com/netbirdio/netbird/client/internal/routemanager" ) -// Start begins watching for network changes and calls the callback function and stops when a change is detected. -func (nw *NetworkWatcher) Start(ctx context.Context, callback func()) { - if nw.cancel != nil { - log.Warn("Network monitor: already running, stopping previous watcher") - nw.Stop() - } - +// Start begins monitoring network changes. When a change is detected, it calls the callback asynchronously and returns. +func (nw *NetworkMonitor) Start(ctx context.Context, callback func()) (err error) { if ctx.Err() != nil { - log.Info("Network monitor: not starting, context is already cancelled") - return + return ctx.Err() } + nw.mu.Lock() ctx, nw.cancel = context.WithCancel(ctx) - defer nw.Stop() + nw.mu.Unlock() + + nw.wg.Add(1) + defer nw.wg.Done() var nexthop4, nexthop6 netip.Addr var intf4, intf6 *net.Interface @@ -56,27 +55,30 @@ func (nw *NetworkWatcher) Start(ctx context.Context, callback func()) { expBackOff := backoff.WithContext(backoff.NewExponentialBackOff(), ctx) if err := backoff.Retry(operation, expBackOff); err != nil { - log.Errorf("Network monitor: failed to get default next hops: %v", err) - return + return fmt.Errorf("failed to get default next hops: %w", err) } // recover in case sys ops panic defer func() { if r := recover(); r != nil { - log.Errorf("Network monitor: panic occurred: %v, stack trace: %s", r, string(debug.Stack())) + err = fmt.Errorf("panic occurred: %v, stack trace: %s", r, string(debug.Stack())) } }() - if err := checkChange(ctx, nexthop4, intf4, nexthop6, intf6, callback); err != nil && !errors.Is(err, context.Canceled) { - log.Errorf("Network monitor: failed to start: %v", err) + if err := checkChange(ctx, nexthop4, intf4, nexthop6, intf6, callback); err != nil { + return fmt.Errorf("check change: %w", err) } + + return nil } // Stop stops the network monitor. -func (nw *NetworkWatcher) Stop() { +func (nw *NetworkMonitor) Stop() { + nw.mu.Lock() + defer nw.mu.Unlock() + if nw.cancel != nil { nw.cancel() - nw.cancel = nil - log.Info("Network monitor: stopped") + nw.wg.Wait() } } diff --git a/client/internal/networkmonitor/monitor_linux.go b/client/internal/networkmonitor/monitor_linux.go index f39f1235cc2..3f93c6ac6f9 100644 --- a/client/internal/networkmonitor/monitor_linux.go +++ b/client/internal/networkmonitor/monitor_linux.go @@ -36,7 +36,7 @@ func checkChange(ctx context.Context, nexthopv4 netip.Addr, intfv4 *net.Interfac for { select { case <-ctx.Done(): - return ctx.Err() + return ErrStopped // handle interface state changes case update := <-linkChan: @@ -47,12 +47,12 @@ func checkChange(ctx context.Context, nexthopv4 netip.Addr, intfv4 *net.Interfac switch update.Header.Type { case syscall.RTM_DELLINK: log.Infof("Network monitor: monitored interface (%s) is gone", update.Link.Attrs().Name) - callback() + go callback() return nil case syscall.RTM_NEWLINK: if (update.IfInfomsg.Flags&syscall.IFF_RUNNING) == 0 && update.Link.Attrs().OperState == netlink.OperDown { log.Infof("Network monitor: monitored interface (%s) is down.", update.Link.Attrs().Name) - callback() + go callback() return nil } } @@ -67,12 +67,12 @@ func checkChange(ctx context.Context, nexthopv4 netip.Addr, intfv4 *net.Interfac // triggered on added/replaced routes case syscall.RTM_NEWROUTE: log.Infof("Network monitor: default route changed: via %s, interface %d", route.Gw, route.LinkIndex) - callback() + go callback() return nil case syscall.RTM_DELROUTE: if intfv4 != nil && route.Gw.Equal(nexthopv4.AsSlice()) || intfv6 != nil && route.Gw.Equal(nexthop6.AsSlice()) { log.Infof("Network monitor: default route removed: via %s, interface %d", route.Gw, route.LinkIndex) - callback() + go callback() return nil } } diff --git a/client/internal/networkmonitor/monitor_mobile.go b/client/internal/networkmonitor/monitor_mobile.go index 988f296bbcd..c81fad16c07 100644 --- a/client/internal/networkmonitor/monitor_mobile.go +++ b/client/internal/networkmonitor/monitor_mobile.go @@ -4,8 +4,9 @@ package networkmonitor import "context" -func (nw *NetworkWatcher) Start(context.Context, func()) { +func (nw *NetworkMonitor) Start(context.Context, func()) error { + return nil } -func (nw *NetworkWatcher) Stop() { +func (nw *NetworkMonitor) Stop() { } diff --git a/client/internal/networkmonitor/monitor_windows.go b/client/internal/networkmonitor/monitor_windows.go index f6c5d963f2d..b8d9c6de77d 100644 --- a/client/internal/networkmonitor/monitor_windows.go +++ b/client/internal/networkmonitor/monitor_windows.go @@ -48,10 +48,10 @@ func checkChange(ctx context.Context, nexthopv4 netip.Addr, intfv4 *net.Interfac for { select { case <-ctx.Done(): - return ctx.Err() + return ErrStopped case <-ticker.C: if changed(nexthopv4, intfv4, neighborv4, nexthopv6, intfv6, neighborv6) { - callback() + go callback() return nil } } diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index b50eb25f90d..fbfc10406a1 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -487,6 +487,10 @@ func (conn *Conn) configureConnection(remoteConn net.Conn, remoteWgPort int, rem return nil, err } + if runtime.GOOS == "ios" { + runtime.GC() + } + if conn.onConnected != nil { conn.onConnected(conn.config.Key, remoteRosenpassPubKey, ipNet.IP.String(), remoteRosenpassAddr) } diff --git a/client/internal/relay/relay.go b/client/internal/relay/relay.go index 84fd72e49c9..4542a37febb 100644 --- a/client/internal/relay/relay.go +++ b/client/internal/relay/relay.go @@ -170,7 +170,7 @@ func ProbeAll( var wg sync.WaitGroup for i, uri := range relays { - ctx, cancel := context.WithTimeout(ctx, 1*time.Second) + ctx, cancel := context.WithTimeout(ctx, 2*time.Second) defer cancel() wg.Add(1) diff --git a/client/internal/routemanager/systemops_darwin.go b/client/internal/routemanager/systemops_darwin.go index 017dc6c28a2..ee4196a0ca5 100644 --- a/client/internal/routemanager/systemops_darwin.go +++ b/client/internal/routemanager/systemops_darwin.go @@ -43,11 +43,6 @@ func routeCmd(action string, prefix netip.Prefix, nexthop netip.Addr, intf *net. } if prefix.Addr().Is6() { inet = "-inet6" - // Special case for IPv6 split default route, pointing to the wg interface fails - // TODO: Remove once we have IPv6 support on the interface - if prefix.Bits() == 1 { - intf = &net.Interface{Name: "lo0"} - } } args := []string{"-n", action, inet, network} diff --git a/client/internal/templates/pkce-auth-msg.html b/client/internal/templates/pkce-auth-msg.html index efd1e06a390..4825c48e734 100644 --- a/client/internal/templates/pkce-auth-msg.html +++ b/client/internal/templates/pkce-auth-msg.html @@ -1,7 +1,7 @@ - +