diff --git a/.github/workflows/agent-network-e2e.yml b/.github/workflows/agent-network-e2e.yml index d78e3bbd3..88b98293d 100644 --- a/.github/workflows/agent-network-e2e.yml +++ b/.github/workflows/agent-network-e2e.yml @@ -5,6 +5,13 @@ on: schedule: - cron: "0 3 * * *" workflow_dispatch: + inputs: + bedrock_model: + description: >- + Bedrock inference-profile id to drive the matrix with, exactly as + AWS issues it. Leave empty for the Sonnet 4.6 default. + required: false + default: "" concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -51,6 +58,9 @@ jobs: # token (and URL, for gateways) is unset, so partial coverage is fine. OPENAI_TOKEN: ${{ secrets.E2E_OPENAI_TOKEN }} ANTHROPIC_TOKEN: ${{ secrets.E2E_ANTHROPIC_TOKEN }} + # Moonshot AI platform key (platform.kimi.ai); drives both Kimi wire + # shapes (OpenAI /v1 and Anthropic /anthropic) through kimi_api. + KIMI_TOKEN: ${{ secrets.E2E_KIMI_TOKEN }} VERCEL_URL: ${{ secrets.E2E_VERCEL_URL }} VERCEL_TOKEN: ${{ secrets.E2E_VERCEL_TOKEN }} OPENROUTER_URL: ${{ secrets.E2E_OPENROUTER_URL }} @@ -59,6 +69,8 @@ jobs: CLOUDFLARE_TOKEN: ${{ secrets.E2E_CLOUDFLARE_TOKEN }} AWS_BEARER_TOKEN_BEDROCK: ${{ secrets.E2E_AWS_BEARER_TOKEN_BEDROCK }} AWS_REGION: ${{ secrets.E2E_AWS_REGION }} + # Bedrock model override: dispatch input wins, then the repo variable, else the test default. + AWS_BEDROCK_MODEL: ${{ inputs.bedrock_model || vars.E2E_AWS_BEDROCK_MODEL }} # Vertex (Anthropic-on-Vertex): SA + project required; region defaults # to "global", model to a pinned claude snapshot. GOOGLE_VERTEX_SA_BASE64: ${{ secrets.E2E_GOOGLE_VERTEX_SA_BASE64 }} diff --git a/.github/workflows/frontend-ui.yml b/.github/workflows/frontend-ui.yml index bb5bb4528..552ccef29 100644 --- a/.github/workflows/frontend-ui.yml +++ b/.github/workflows/frontend-ui.yml @@ -86,7 +86,7 @@ jobs: ${{ runner.os }}-pnpm- - name: Install dependencies - run: pnpm install --frozen-lockfile + run: pnpm install --frozen-lockfile --ignore-scripts - name: Generate Wails bindings run: pnpm run bindings diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index b444a9900..586e1235b 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -45,7 +45,7 @@ jobs: display_name: Linux name: ${{ matrix.display_name }} runs-on: ${{ matrix.os }} - timeout-minutes: 15 + timeout-minutes: 25 steps: - name: Checkout code uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -79,4 +79,4 @@ jobs: skip-cache: true skip-save-cache: true cache-invalidation-interval: 0 - args: --timeout=12m + args: --timeout=20m diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 0753b1012..8dd05a192 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -273,8 +273,8 @@ dockers_v2: - netbirdio/netbird - ghcr.io/netbirdio/netbird tags: - - "v{{ .Version }}-rootless" - - "{{ if eq .Env.SKIP_PUBLISH \"false\" }}latest{{ end }}" + - "{{ .Version }}-rootless" + - "{{ if eq .Env.SKIP_PUBLISH \"false\" }}rootless-latest{{ end }}" dockerfile: client/Dockerfile-rootless extra_files: - client/netbird-entrypoint.sh diff --git a/.goreleaser_ui.yaml b/.goreleaser_ui.yaml index 197fcd440..1157e6379 100644 --- a/.goreleaser_ui.yaml +++ b/.goreleaser_ui.yaml @@ -24,6 +24,8 @@ builds: ldflags: - -s -w -X github.com/netbirdio/netbird/version.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.CommitDate}} -X main.builtBy=goreleaser mod_timestamp: "{{ .CommitTimestamp }}" + tags: + - production - id: netbird-ui-windows-amd64 dir: client/ui @@ -39,6 +41,8 @@ builds: - -s -w -X github.com/netbirdio/netbird/version.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.CommitDate}} -X main.builtBy=goreleaser - -H windowsgui mod_timestamp: "{{ .CommitTimestamp }}" + tags: + - production - id: netbird-ui-windows-arm64 dir: client/ui @@ -55,6 +59,8 @@ builds: - -s -w -X github.com/netbirdio/netbird/version.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.CommitDate}} -X main.builtBy=goreleaser - -H windowsgui mod_timestamp: "{{ .CommitTimestamp }}" + tags: + - production archives: - id: linux-arch diff --git a/.goreleaser_ui_darwin.yaml b/.goreleaser_ui_darwin.yaml index 96e15371a..47b991344 100644 --- a/.goreleaser_ui_darwin.yaml +++ b/.goreleaser_ui_darwin.yaml @@ -29,6 +29,8 @@ builds: ldflags: - -s -w -X github.com/netbirdio/netbird/version.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.CommitDate}} -X main.builtBy=goreleaser mod_timestamp: "{{ .CommitTimestamp }}" + tags: + - production universal_binaries: - id: netbird-ui-darwin diff --git a/client/android/client.go b/client/android/client.go index 99ccdf393..b3a845818 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "slices" + "strings" "sync" "time" @@ -247,6 +248,9 @@ func (c *Client) DebugBundle(platformFiles PlatformFiles, anonymize bool) (strin deps.SyncResponse = resp if e := cc.Engine(); e != nil { + deps.RefreshStatus = func() { + e.RunHealthProbes(context.Background(), true) + } if cm := e.GetClientMetrics(); cm != nil { deps.ClientMetrics = cm } @@ -296,6 +300,13 @@ func (c *Client) SetInfoLogLevel() { // PeersList return with the list of the PeerInfos func (c *Client) PeersList() *PeerInfoArray { + // The recorder only caches transfer counters and handshake times; nothing + // refreshes them on its own, so without this they read as zero. The desktop + // daemon does the same before serving a full peer status. + if err := c.recorder.RefreshWireGuardStats(); err != nil { + log.Debugf("failed to refresh WireGuard stats: %v", err) + } + fullStatus := c.recorder.GetFullStatus() peerInfos := make([]PeerInfo, len(fullStatus.Peers)) @@ -306,6 +317,20 @@ func (c *Client) PeersList() *PeerInfoArray { FQDN: p.FQDN, ConnStatus: int(p.ConnStatus), Routes: PeerRoutes{routes: maps.Keys(p.GetRoutes())}, + + PubKey: p.PubKey, + Latency: formatDuration(p.Latency), + LatencyMs: p.Latency.Milliseconds(), + BytesRx: p.BytesRx, + BytesTx: p.BytesTx, + ConnStatusUpdate: formatTime(p.ConnStatusUpdate), + Relayed: p.Relayed, + RosenpassEnabled: p.RosenpassEnabled, + LastWireguardHandshake: formatTime(p.LastWireguardHandshake), + LocalIceCandidateType: p.LocalIceCandidateType, + RemoteIceCandidateType: p.RemoteIceCandidateType, + LocalIceCandidateEndpoint: p.LocalIceCandidateEndpoint, + RemoteIceCandidateEndpoint: p.RemoteIceCandidateEndpoint, } peerInfos[n] = pi } @@ -436,10 +461,6 @@ func (c *Client) RemoveConnectionListener() { c.recorder.RemoveConnectionListener() } -func (c *Client) toggleRoute(command routeCommand) error { - return command.toggleRoute() -} - func (c *Client) getRouteManager() (routemanager.Manager, error) { client := c.getConnectClient() if client == nil { @@ -459,22 +480,22 @@ func (c *Client) getRouteManager() (routemanager.Manager, error) { return manager, nil } -func (c *Client) SelectRoute(route string) error { +func (c *Client) SelectRoute(id string) error { manager, err := c.getRouteManager() if err != nil { return err } - return c.toggleRoute(selectRouteCommand{route: route, manager: manager}) + return manager.SelectRoutes([]route.NetID{route.NetID(id)}, true) } -func (c *Client) DeselectRoute(route string) error { +func (c *Client) DeselectRoute(id string) error { manager, err := c.getRouteManager() if err != nil { return err } - return c.toggleRoute(deselectRouteCommand{route: route, manager: manager}) + return manager.DeselectRoutes([]route.NetID{route.NetID(id)}) } // getNetworkDomainsFromRoute extracts domains from a route and enriches each domain @@ -509,3 +530,28 @@ func exportEnvList(list *EnvList) { } } } + +// formatDuration renders a duration for display, trimming the fractional part +// to two digits so latencies read as "12.34ms" rather than "12.345678ms". +func formatDuration(d time.Duration) string { + ds := d.String() + dotIndex := strings.Index(ds, ".") + if dotIndex == -1 { + return ds + } + + endIndex := min(dotIndex+3, len(ds)) + + // Skip the remaining digits so only the unit suffix is appended back. + unitStart := endIndex + for unitStart < len(ds) && ds[unitStart] >= '0' && ds[unitStart] <= '9' { + unitStart++ + } + return ds[:endIndex] + ds[unitStart:] +} + +// formatTime renders a timestamp in UTC using a fixed layout. The zero time is +// passed through as-is so the UI can recognise it and show "never" instead. +func formatTime(t time.Time) string { + return t.UTC().Format("2006-01-02 15:04:05") +} diff --git a/client/android/peer_notifier.go b/client/android/peer_notifier.go index c2595e574..f525055bb 100644 --- a/client/android/peer_notifier.go +++ b/client/android/peer_notifier.go @@ -12,12 +12,30 @@ const ( ) // PeerInfo describe information about the peers. It designed for the UI usage +// +// The fields below ConnStatus back the peer detail screen. Durations and times +// are pre-formatted into strings so the UI does not have to know Go's layouts; +// Latency is additionally exposed as LatencyMs for colour coding. type PeerInfo struct { IP string IPv6 string FQDN string ConnStatus int Routes PeerRoutes + + PubKey string + Latency string + LatencyMs int64 + BytesRx int64 + BytesTx int64 + ConnStatusUpdate string + Relayed bool + RosenpassEnabled bool + LastWireguardHandshake string + LocalIceCandidateType string + RemoteIceCandidateType string + LocalIceCandidateEndpoint string + RemoteIceCandidateEndpoint string } func (p *PeerInfo) GetPeerRoutes() *PeerRoutes { diff --git a/client/android/profile_manager.go b/client/android/profile_manager.go index 87c001396..9a051137c 100644 --- a/client/android/profile_manager.go +++ b/client/android/profile_manager.go @@ -189,6 +189,19 @@ func (pm *ProfileManager) LogoutProfile(id string) error { return nil } +// RenameProfile changes a profile's display name. The profile ID, and therefore +// its on-disk filename, is left untouched: only the "name" field of the config +// is rewritten. This works for the default profile too, whose config lives in +// netbird.cfg rather than under profiles/. +func (pm *ProfileManager) RenameProfile(id string, newName string) error { + if err := pm.serviceMgr.RenameProfile(profilemanager.ID(id), androidUsername, newName); err != nil { + return fmt.Errorf("failed to rename profile: %w", err) + } + + log.Infof("renamed profile %s to: %s", id, newName) + return nil +} + // RemoveProfile deletes a profile func (pm *ProfileManager) RemoveProfile(id string) error { // Use ServiceManager (removes profile from profiles/ directory) diff --git a/client/android/route_command.go b/client/android/route_command.go deleted file mode 100644 index 5e7357335..000000000 --- a/client/android/route_command.go +++ /dev/null @@ -1,70 +0,0 @@ -//go:build android - -package android - -import ( - "fmt" - - log "github.com/sirupsen/logrus" - "golang.org/x/exp/maps" - - "github.com/netbirdio/netbird/client/internal/routemanager" - "github.com/netbirdio/netbird/route" -) - -func executeRouteToggle(id string, manager routemanager.Manager, - operationName string, - routeOperation func(routes []route.NetID, allRoutes []route.NetID) error) error { - netID := route.NetID(id) - routes := []route.NetID{netID} - - routesMap := manager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - - log.Debugf("%s with ids: %v", operationName, routes) - - if err := routeOperation(routes, maps.Keys(routesMap)); err != nil { - log.Debugf("error when %s: %s", operationName, err) - return fmt.Errorf("error %s: %w", operationName, err) - } - - manager.TriggerSelection(manager.GetClientRoutes()) - - return nil -} - -type routeCommand interface { - toggleRoute() error -} - -type selectRouteCommand struct { - route string - manager routemanager.Manager -} - -func (s selectRouteCommand) toggleRoute() error { - routeSelector := s.manager.GetRouteSelector() - if routeSelector == nil { - return fmt.Errorf("no route selector available") - } - - routeOperation := func(routes []route.NetID, allRoutes []route.NetID) error { - return routeSelector.SelectRoutes(routes, true, allRoutes) - } - - return executeRouteToggle(s.route, s.manager, "selecting route", routeOperation) -} - -type deselectRouteCommand struct { - route string - manager routemanager.Manager -} - -func (d deselectRouteCommand) toggleRoute() error { - routeSelector := d.manager.GetRouteSelector() - if routeSelector == nil { - return fmt.Errorf("no route selector available") - } - - return executeRouteToggle(d.route, d.manager, "deselecting route", routeSelector.DeselectRoutes) -} diff --git a/client/iface/iface_test.go b/client/iface/iface_test.go index 8ff2bbb54..89c8cd16e 100644 --- a/client/iface/iface_test.go +++ b/client/iface/iface_test.go @@ -464,6 +464,8 @@ func Test_RemovePeer(t *testing.T) { } func Test_ConnectPeers(t *testing.T) { + t.Setenv("NB_DISABLE_EBPF_WG_PROXY", "true") + peer1ifaceName := fmt.Sprintf("utun%d", WgIntNumber+400) peer1wgIP := netip.MustParsePrefix("10.99.99.17/30") peer1Key, _ := wgtypes.GeneratePrivateKey() @@ -505,12 +507,8 @@ func Test_ConnectPeers(t *testing.T) { t.Fatal(err) } - localIP, err := getLocalIP() - if err != nil { - t.Fatal(err) - } - - peer1endpoint, err := net.ResolveUDPAddr("udp", fmt.Sprintf("%s:%d", localIP, peer1wgPort)) + localIP1 := "127.0.0.1" + peer1endpoint, err := net.ResolveUDPAddr("udp", fmt.Sprintf("%s:%d", localIP1, peer1wgPort)) if err != nil { t.Fatal(err) } @@ -546,7 +544,8 @@ func Test_ConnectPeers(t *testing.T) { t.Fatal(err) } - peer2endpoint, err := net.ResolveUDPAddr("udp", fmt.Sprintf("%s:%d", localIP, peer2wgPort)) + localIP2 := "127.0.0.1" + peer2endpoint, err := net.ResolveUDPAddr("udp", fmt.Sprintf("%s:%d", localIP2, peer2wgPort)) if err != nil { t.Fatal(err) } @@ -569,17 +568,17 @@ func Test_ConnectPeers(t *testing.T) { if err != nil { t.Fatal(err) } - // todo: investigate why in some tests execution we need 30s + // The peers use userspace WireGuard (stdnet transport). A tight busy-loop + // here starves the wireguard-go goroutines that process the handshake, so + // poll on a ticker instead and yield the CPU between checks. WireGuard also + // only retries a lost handshake initiation every REKEY_TIMEOUT (5s), which + // is why the overall wait can occasionally stretch to tens of seconds. timeout := 30 * time.Second timeoutChannel := time.After(timeout) + ticker := time.NewTicker(500 * time.Millisecond) + defer ticker.Stop() for { - select { - case <-timeoutChannel: - t.Fatalf("waiting for peer handshake timeout after %s", timeout.String()) - default: - } - peer, gpErr := getPeer(peer1ifaceName, peer2Key.PublicKey().String()) if gpErr != nil { t.Fatal(gpErr) @@ -588,6 +587,12 @@ func Test_ConnectPeers(t *testing.T) { t.Log("peers successfully handshake") break } + + select { + case <-timeoutChannel: + t.Fatalf("waiting for peer handshake timeout after %s", timeout.String()) + case <-ticker.C: + } } } @@ -615,28 +620,3 @@ func getPeer(ifaceName, peerPubKey string) (wgtypes.Peer, error) { } return wgtypes.Peer{}, fmt.Errorf("peer not found") } - -func getLocalIP() (string, error) { - // Get all interfaces - addrs, err := net.InterfaceAddrs() - if err != nil { - return "", err - } - - for _, addr := range addrs { - ipNet, ok := addr.(*net.IPNet) - if !ok { - continue - } - if ipNet.IP.IsLoopback() { - continue - } - - if ipNet.IP.To4() == nil { - continue - } - return ipNet.IP.String(), nil - } - - return "", fmt.Errorf("no local IP found") -} diff --git a/client/internal/auth/auth.go b/client/internal/auth/auth.go index 51f56b644..153727a6c 100644 --- a/client/internal/auth/auth.go +++ b/client/internal/auth/auth.go @@ -351,6 +351,7 @@ func (a *Auth) setSystemInfoFlags(info *system.Info) { a.config.BlockLANAccess, a.config.BlockInbound, a.config.DisableIPv6, + a.config.SyncMessageVersion, a.config.EnableSSHRoot, a.config.EnableSSHSFTP, a.config.EnableSSHLocalPortForwarding, diff --git a/client/internal/auth/sessionwatch/watcher.go b/client/internal/auth/sessionwatch/watcher.go index e75a7022e..e685c28d0 100644 --- a/client/internal/auth/sessionwatch/watcher.go +++ b/client/internal/auth/sessionwatch/watcher.go @@ -24,11 +24,7 @@ import ( ) const ( - // Skew tolerates a small clock difference between the management - // server and this peer before treating a deadline as "in the past". - // Slightly above typical NTP drift; tight enough that the UI doesn't - // paint a stale expiry as if it were valid. - Skew = 30 * time.Second + maxPastHorizon = 30 * 24 * time.Hour // maxDeadlineHorizon caps how far in the future an accepted deadline // can sit. A timestamp beyond this is almost certainly a protocol @@ -57,7 +53,7 @@ var ( ErrDeadlineTooFarFuture = errors.New("session deadline too far in the future") // ErrDeadlineInPast is returned by Update when the supplied deadline - // is more than Skew in the past. + // is more than maxPastHorizon in the past. ErrDeadlineInPast = errors.New("session deadline in the past") ) @@ -66,15 +62,14 @@ var ( // for deadline change/clear, PublishEvent for the two warnings); tests pass // a fake recorder so the same surface is observable without an engine. // -// The watcher is the single owner of the deadline propagated to the -// recorder: every set, clear, sanity-check rejection and Close routes the -// value through SetSessionExpiresAt, so the SubscribeStatus snapshot the UI -// reads can never drift from the watcher's timer state. (SetSessionExpiresAt -// fans out its own state-change notification, so no separate notify is -// needed.) The recorder is server-scoped and outlives this engine-scoped -// watcher — without the Close-time clear a teardown (Down, or the Down+Up of -// a profile switch) would leave the next session showing the previous one's -// stale "expires in" value. +// While the watcher runs, it owns the deadline propagated to the recorder: +// every set, clear and sanity-check rejection routes the value through +// SetSessionExpiresAt, so the SubscribeStatus snapshot the UI reads can +// never drift from the watcher's timer state. (SetSessionExpiresAt fans +// out its own state-change notification, so no separate notify is needed.) +// The recorder is server-scoped and outlives this engine-scoped watcher; +// Close deliberately leaves the recorder value in place so transient engine +// restarts don't blank it — the client run loop clears it on real teardown. // // PublishEvent's signature mirrors peer.Status.PublishEvent: the watcher // composes the metadata internally so the wire format (MetaSession*) is @@ -135,10 +130,13 @@ func NewWithLeads(lead, final time.Duration, recorder StatusRecorder) *Watcher { // was disabled). // // Same-value updates are no-ops. A different non-zero value cancels any -// pending timer, resets the "already fired" guard, and arms a new one. +// pending timer, resets the "already fired" guards, and — when the +// deadline lies in the future — arms fresh warning timers. A deadline +// already in the past (within maxPastHorizon) is recorded as-is with no +// timers: the session has expired and consumers render it that way. // // Returns one of the sentinel Err* values when the deadline fails the -// sanity checks (pre-epoch, far future, or in the past beyond Skew). +// sanity checks (pre-epoch, far future, or past beyond maxPastHorizon). // In every error case the watcher first clears its state so it stays // consistent with what the caller will push into its other sinks (e.g. // applySessionDeadline forces a zero deadline into the status recorder @@ -163,7 +161,7 @@ func (w *Watcher) Update(deadline time.Time) error { case deadline.After(now.Add(maxDeadlineHorizon)): w.clearLocked() return fmt.Errorf("%w: %v", ErrDeadlineTooFarFuture, deadline) - case deadline.Before(now.Add(-Skew)): + case deadline.Before(now.Add(-maxPastHorizon)): w.clearLocked() return fmt.Errorf("%w: %v (now=%v)", ErrDeadlineInPast, deadline, now) } @@ -183,7 +181,9 @@ func (w *Watcher) Update(deadline time.Time) error { w.finalFiredAt = time.Time{} w.dismissedAt = time.Time{} - w.armTimerLocked(deadline) + if deadline.After(now) { + w.armTimerLocked(deadline) + } recorder := w.recorder w.mu.Unlock() if recorder != nil { @@ -227,30 +227,25 @@ func (w *Watcher) Dismiss() { log.Infof("auth session final-warning dismissed for deadline %s", w.current.Format(time.RFC3339)) } -// Close stops any pending timer and drops the deadline on the status -// recorder. Update calls after Close are ignored. Clearing the recorder -// here is what keeps a teardown (Down, or the Down+Up of a profile switch) -// from leaving the next session showing this one's stale "expires in" -// value — the recorder is server-scoped and outlives this engine-scoped -// watcher, so nothing else drops the anchor on teardown. +// Close stops any pending timer. Update calls after Close are ignored. +// The recorder keeps its deadline: the watcher is engine-scoped and closes +// on every engine restart (network change, sleep/wake, stream errors) +// while the SSO deadline stays valid across those, so clearing here would +// blank the UI's "expires in" row on every transient reconnect. The +// client run loop clears the server-scoped recorder when it exits for +// real (Down, profile switch, permanent login failure). func (w *Watcher) Close() { w.mu.Lock() + defer w.mu.Unlock() if w.closed { - w.mu.Unlock() return } w.closed = true w.stopTimerLocked() - hadDeadline := !w.current.IsZero() w.current = time.Time{} w.firedAt = time.Time{} w.finalFiredAt = time.Time{} w.dismissedAt = time.Time{} - recorder := w.recorder - w.mu.Unlock() - if recorder != nil && hadDeadline { - recorder.SetSessionExpiresAt(time.Time{}) - } } // clearLocked drops the tracked deadline and notifies the recorder so diff --git a/client/internal/auth/sessionwatch/watcher_test.go b/client/internal/auth/sessionwatch/watcher_test.go index da2b6add6..4b49a94b6 100644 --- a/client/internal/auth/sessionwatch/watcher_test.go +++ b/client/internal/auth/sessionwatch/watcher_test.go @@ -224,11 +224,13 @@ func TestNewDeadlineCancelsPriorTimer(t *testing.T) { func TestRefreshAfterFireArmsNewWarning(t *testing.T) { r := &fakeRecorder{} - lead := 30 * time.Millisecond + lead := 150 * time.Millisecond w := newWatcher(lead, r) defer w.Close() - first := time.Now().Add(50 * time.Millisecond) + // Warning fires ~20ms in; the deadline itself stays 150ms away so the + // replacement below lands well before it. + first := time.Now().Add(170 * time.Millisecond) _ = w.Update(first) // Wait for stateChange + warning of the first cycle. @@ -306,7 +308,29 @@ func TestUpdateRejectsTooFarFuture(t *testing.T) { } } -func TestUpdateInPastClearsDeadline(t *testing.T) { +func TestUpdateRecentPastRecordedAsExpired(t *testing.T) { + r := &fakeRecorder{} + w := newWatcher(50*time.Millisecond, r) + defer w.Close() + + d := time.Now().Add(-1 * time.Hour) + if err := w.Update(d); err != nil { + t.Fatalf("recent-past Update should succeed, got %v", err) + } + if !w.Deadline().Equal(d) { + t.Fatalf("expected deadline to be recorded, got %v want %v", w.Deadline(), d) + } + if got := r.deadline(); !got.Equal(d) { + t.Fatalf("recorder deadline = %v, want %v", got, d) + } + + time.Sleep(80 * time.Millisecond) + if n := countWhere(r.snapshot(), func(e event) bool { return e.kind == publish }); n != 0 { + t.Fatalf("no warning events may fire for an already-past deadline, got %+v", r.snapshot()) + } +} + +func TestUpdateAncientPastRejected(t *testing.T) { r := &fakeRecorder{} w := newWatcher(50*time.Millisecond, r) defer w.Close() @@ -318,12 +342,12 @@ func TestUpdateInPastClearsDeadline(t *testing.T) { // Drain the stateChange from the seed. waitForEvents(t, r, 1) - err := w.Update(time.Now().Add(-1 * time.Hour)) + err := w.Update(time.Now().Add(-31 * 24 * time.Hour)) if !errors.Is(err, ErrDeadlineInPast) { t.Fatalf("want ErrDeadlineInPast, got %v", err) } if !w.Deadline().IsZero() { - t.Fatalf("in-past update must clear the deadline, got %v", w.Deadline()) + t.Fatalf("rejected ancient-past update must clear the deadline, got %v", w.Deadline()) } events := waitForEvents(t, r, 2) if events[1].kind != stateChange { @@ -331,39 +355,25 @@ func TestUpdateInPastClearsDeadline(t *testing.T) { } } -func TestUpdateWithinSkewAccepted(t *testing.T) { - r := &fakeRecorder{} - w := newWatcher(50*time.Millisecond, r) - defer w.Close() - - // 5 seconds in the past is within the 30s Skew tolerance — accept it. - d := time.Now().Add(-5 * time.Second) - if err := w.Update(d); err != nil { - t.Fatalf("within-skew Update should succeed, got %v", err) - } - if !w.Deadline().Equal(d) { - t.Fatalf("expected deadline to be applied, got %v want %v", w.Deadline(), d) - } -} - func TestCloseSilencesUpdates(t *testing.T) { r := &fakeRecorder{} w := newWatcher(50*time.Millisecond, r) w.Close() - _ = w.Update(time.Now().Add(time.Hour)) - - time.Sleep(20 * time.Millisecond) + if err := w.Update(time.Now().Add(time.Hour)); err != nil { + t.Fatalf("Update after Close: want nil, got %v", err) + } if got := r.snapshot(); len(got) != 0 { t.Fatalf("expected no events after Close, got %+v", got) } } -// TestCloseClearsRecorderDeadline pins the profile-switch fix: a watcher -// holding a live deadline must zero the recorder on Close so the next -// engine's watcher (and the UI reading the shared server-scoped recorder) -// doesn't start out showing the previous session's stale "expires in". -func TestCloseClearsRecorderDeadline(t *testing.T) { +// TestCloseKeepsRecorderDeadline pins the reconnect-flap fix: the watcher +// closes on every engine restart (network change, sleep/wake) while the +// SSO deadline stays valid across those, so Close must leave the +// server-scoped recorder's value in place. The client run loop clears the +// recorder when it exits for real. +func TestCloseKeepsRecorderDeadline(t *testing.T) { r := &fakeRecorder{} w := newWatcher(time.Hour, r) @@ -377,8 +387,8 @@ func TestCloseClearsRecorderDeadline(t *testing.T) { w.Close() - if got := r.deadline(); !got.IsZero() { - t.Fatalf("recorder deadline after Close = %v, want zero", got) + if got := r.deadline(); !got.Equal(d) { + t.Fatalf("recorder deadline after Close = %v, want %v", got, d) } } diff --git a/client/internal/conn_mgr.go b/client/internal/conn_mgr.go index 77d1e6ca5..ad0f00c5d 100644 --- a/client/internal/conn_mgr.go +++ b/client/internal/conn_mgr.go @@ -34,6 +34,8 @@ const ( // - Handling connection establishment based on peer signaling // // The implementation is not thread-safe; it is protected by engine.syncMsgMux. +// The only exception is ActivatePeer, which is safe for concurrent use so the +// DNS warm-up path can call it without contending on the engine mutex. type ConnMgr struct { peerStore *peerstore.Store statusRecorder *peer.Status @@ -42,12 +44,26 @@ type ConnMgr struct { rosenpassEnabled bool lazyConnMgr *manager.Manager + // lazyConnMgrMu guards the lazyConnMgr pointer for readers outside the + // engine loop (ActivatePeer). Writers hold it in addition to + // engine.syncMsgMux; all other reads stay under engine.syncMsgMux only. + lazyConnMgrMu sync.RWMutex + + // reconcileRoutedIPs re-applies a peer's routed allowed IPs after its lazy wake endpoint is + // (re)armed (Mode A at arm time). Injected by the engine; nil disables the reconcile. + reconcileRoutedIPs func(peerKey string) error wg sync.WaitGroup lazyCtx context.Context lazyCtxCancel context.CancelFunc } +// SetRoutedIPsReconciler injects the callback used to re-apply a peer's routed allowed IPs when +// its lazy wake endpoint is (re)armed. Must be called before the lazy manager starts. +func (e *ConnMgr) SetRoutedIPsReconciler(fn func(peerKey string) error) { + e.reconcileRoutedIPs = fn +} + func NewConnMgr(engineConfig *EngineConfig, statusRecorder *peer.Status, peerStore *peerstore.Store, iface lazyconn.WGIface) *ConnMgr { e := &ConnMgr{ peerStore: peerStore, @@ -238,12 +254,20 @@ func (e *ConnMgr) RemovePeerConn(peerKey string) { conn.Log.Infof("removed peer from lazy conn manager") } +// ActivatePeer wakes an idle lazy connection. Unlike the rest of ConnMgr it is +// safe for concurrent use: the lazy manager pointer is read under lazyConnMgrMu +// and the manager itself is internally synchronized, so callers outside the +// engine loop (DNS warm-up) do not need engine.syncMsgMux. func (e *ConnMgr) ActivatePeer(ctx context.Context, conn *peer.Conn) { - if !e.isStartedWithLazyMgr() { + e.lazyConnMgrMu.RLock() + lazyConnMgr := e.lazyConnMgr + started := lazyConnMgr != nil && e.lazyCtxCancel != nil + e.lazyConnMgrMu.RUnlock() + if !started { return } - if found := e.lazyConnMgr.ActivatePeer(conn.GetKey()); found { + if found := lazyConnMgr.ActivatePeer(conn.GetKey()); found { if err := conn.Open(ctx); err != nil { conn.Log.Errorf("failed to open connection: %v", err) } @@ -268,16 +292,22 @@ func (e *ConnMgr) Close() { e.lazyCtxCancel() e.wg.Wait() + + e.lazyConnMgrMu.Lock() e.lazyConnMgr = nil + e.lazyConnMgrMu.Unlock() } func (e *ConnMgr) initLazyManager(engineCtx context.Context) { cfg := manager.Config{ InactivityThreshold: inactivityThresholdEnv(), + ReconcileAllowedIPs: e.reconcileRoutedIPs, } - e.lazyConnMgr = manager.NewManager(cfg, engineCtx, e.peerStore, e.iface) + e.lazyConnMgrMu.Lock() + e.lazyConnMgr = manager.NewManager(cfg, engineCtx, e.peerStore, e.iface) e.lazyCtx, e.lazyCtxCancel = context.WithCancel(engineCtx) + e.lazyConnMgrMu.Unlock() e.wg.Add(1) go func() { @@ -316,7 +346,10 @@ func (e *ConnMgr) closeManager(ctx context.Context) { e.lazyCtxCancel() e.wg.Wait() + + e.lazyConnMgrMu.Lock() e.lazyConnMgr = nil + e.lazyConnMgrMu.Unlock() for _, peerID := range e.peerStore.PeersPubKey() { e.peerStore.PeerConnOpen(ctx, peerID) @@ -352,11 +385,20 @@ func inactivityThresholdEnv() *time.Duration { return nil } - parsedMinutes, err := strconv.Atoi(envValue) - if err != nil || parsedMinutes <= 0 { - return nil + // Documented format: a Go duration such as "30m" or "1h". + if d, err := time.ParseDuration(envValue); err == nil { + if d <= 0 { + return nil + } + return &d } - d := time.Duration(parsedMinutes) * time.Minute - return &d + // Backwards compatibility: a bare integer used to be interpreted as minutes. + if parsedMinutes, err := strconv.Atoi(envValue); err == nil && parsedMinutes > 0 { + d := time.Duration(parsedMinutes) * time.Minute + return &d + } + + log.Warnf("invalid %s value %q: expected a Go duration such as 30m or 1h", lazyconn.EnvInactivityThreshold, envValue) + return nil } diff --git a/client/internal/conn_mgr_test.go b/client/internal/conn_mgr_test.go index 5e2c53e35..ac5d6f2c8 100644 --- a/client/internal/conn_mgr_test.go +++ b/client/internal/conn_mgr_test.go @@ -1,10 +1,21 @@ package internal import ( + "context" + "net" + "net/netip" "os" + "sync" "testing" + "time" + "golang.zx2c4.com/wireguard/wgctrl/wgtypes" + + "github.com/netbirdio/netbird/client/iface/wgaddr" "github.com/netbirdio/netbird/client/internal/lazyconn" + "github.com/netbirdio/netbird/client/internal/peer" + "github.com/netbirdio/netbird/client/internal/peerstore" + "github.com/netbirdio/netbird/monotime" ) func TestResolveLazyForce(t *testing.T) { @@ -38,3 +49,93 @@ func TestResolveLazyForce(t *testing.T) { }) } } + +type mockLazyWGIface struct{} + +func (mockLazyWGIface) RemovePeer(string) error { return nil } +func (mockLazyWGIface) UpdatePeer(string, []netip.Prefix, time.Duration, *net.UDPAddr, *wgtypes.Key) error { + return nil +} +func (mockLazyWGIface) IsUserspaceBind() bool { return false } +func (mockLazyWGIface) Address() wgaddr.Address { return wgaddr.Address{} } +func (mockLazyWGIface) LastActivities() map[string]monotime.Time { return nil } +func (mockLazyWGIface) MTU() uint16 { return 1280 } + +// TestConnMgr_ActivatePeerConcurrentWithLifecycle exercises ActivatePeer from +// non-engine goroutines (the DNS warm-up path) racing the manager lifecycle, +// which stays on the engine loop. Run with -race: it fails if ActivatePeer +// still requires engine.syncMsgMux for safety. +func TestConnMgr_ActivatePeerConcurrentWithLifecycle(t *testing.T) { + t.Setenv(lazyconn.EnvLazyConn, "on") + + status := peer.NewRecorder("https://mgm") + store := peerstore.NewConnStore() + connMgr := NewConnMgr(&EngineConfig{}, status, store, mockLazyWGIface{}) + + conn := newTestPeerConn(t, "peerA") + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + connMgr.Start(ctx) + + done := make(chan struct{}) + var wg sync.WaitGroup + for range 4 { + wg.Add(1) + go func() { + defer wg.Done() + for { + select { + case <-done: + return + default: + connMgr.ActivatePeer(ctx, conn) + } + } + }() + } + + // Let the activators spin against the started manager, then tear it down + // underneath them and let them spin against the stopped manager. + time.Sleep(100 * time.Millisecond) + connMgr.Close() + time.Sleep(50 * time.Millisecond) + + close(done) + wg.Wait() +} + +func TestInactivityThresholdEnv(t *testing.T) { + tests := []struct { + name string + val string + want *time.Duration + }{ + {name: "unset", val: "", want: nil}, + {name: "go duration minutes", val: "30m", want: durPtr(30 * time.Minute)}, + {name: "go duration hours", val: "1h", want: durPtr(time.Hour)}, + {name: "go duration seconds", val: "90s", want: durPtr(90 * time.Second)}, + {name: "bare integer is minutes (backwards compat)", val: "5", want: durPtr(5 * time.Minute)}, + {name: "zero duration", val: "0s", want: nil}, + {name: "zero integer", val: "0", want: nil}, + {name: "negative duration", val: "-5m", want: nil}, + {name: "garbage", val: "abc", want: nil}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(lazyconn.EnvInactivityThreshold, tc.val) + got := inactivityThresholdEnv() + switch { + case tc.want == nil && got != nil: + t.Fatalf("want nil, got %v", *got) + case tc.want != nil && got == nil: + t.Fatalf("want %v, got nil", *tc.want) + case tc.want != nil && *got != *tc.want: + t.Fatalf("want %v, got %v", *tc.want, *got) + } + }) + } +} + +func durPtr(d time.Duration) *time.Duration { return &d } diff --git a/client/internal/connect.go b/client/internal/connect.go index c2fc2fd73..87126b222 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -34,6 +34,7 @@ import ( "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/internal/statemanager" "github.com/netbirdio/netbird/client/internal/stdnet" + "github.com/netbirdio/netbird/client/internal/tunnelnotifier" "github.com/netbirdio/netbird/client/internal/updater" "github.com/netbirdio/netbird/client/internal/updater/installer" nbnet "github.com/netbirdio/netbird/client/net" @@ -136,10 +137,13 @@ func (c *ConnectClient) RunOniOS( // Set GC percent to 5% to reduce memory usage as iOS only allows 50MB of memory for the extension. debug.SetGCPercent(5) + notifier := tunnelnotifier.New(networkChangeListener, dnsManager) + defer notifier.Close() + mobileDependency := MobileDependency{ FileDescriptor: fileDescriptor, - NetworkChangeListener: networkChangeListener, - DnsManager: dnsManager, + NetworkChangeListener: notifier, + DnsManager: notifier, StateFilePath: stateFilePath, TempDir: cacheDir, } @@ -257,7 +261,10 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan log.Errorf("failed to clean up temporary installer file: %v", err) } - defer c.statusRecorder.ClientStop() + defer func() { + c.statusRecorder.SetSessionExpiresAt(time.Time{}) + c.statusRecorder.ClientStop() + }() operation := func() error { // if context cancelled we not start new backoff cycle if c.ctx.Err() != nil { @@ -618,6 +625,7 @@ func createEngineConfig(key wgtypes.Key, config *profilemanager.Config, peerConf BlockLANAccess: config.BlockLANAccess, BlockInbound: config.BlockInbound, DisableIPv6: config.DisableIPv6, + SyncMessageVersion: config.SyncMessageVersion, LazyConnection: lazyconn.ParseState(config.LazyConnection), @@ -693,6 +701,7 @@ func loginToManagement(ctx context.Context, client mgm.Client, pubSSHKey []byte, config.BlockLANAccess, config.BlockInbound, config.DisableIPv6, + config.SyncMessageVersion, config.EnableSSHRoot, config.EnableSSHSFTP, config.EnableSSHLocalPortForwarding, diff --git a/client/internal/debug/debug.go b/client/internal/debug/debug.go index 0e506ccd7..2de1023e9 100644 --- a/client/internal/debug/debug.go +++ b/client/internal/debug/debug.go @@ -676,6 +676,7 @@ func (g *BundleGenerator) addCommonConfigFields(configContent *strings.Builder) configContent.WriteString(fmt.Sprintf("BlockLANAccess: %v\n", g.internalConfig.BlockLANAccess)) configContent.WriteString(fmt.Sprintf("BlockInbound: %v\n", g.internalConfig.BlockInbound)) configContent.WriteString(fmt.Sprintf("DisableIPv6: %v\n", g.internalConfig.DisableIPv6)) + configContent.WriteString(fmt.Sprintf("SyncMessageVersion: %v\n", g.internalConfig.SyncMessageVersion)) if g.internalConfig.DisableNotifications != nil { configContent.WriteString(fmt.Sprintf("DisableNotifications: %v\n", *g.internalConfig.DisableNotifications)) diff --git a/client/internal/debug/debug_test.go b/client/internal/debug/debug_test.go index 8286f6852..7fe93a5c1 100644 --- a/client/internal/debug/debug_test.go +++ b/client/internal/debug/debug_test.go @@ -887,6 +887,8 @@ func TestAddConfig_AllFieldsCovered(t *testing.T) { ClientCertKeyPath: "/tmp/key", LazyConnection: "on", MTU: 1280, + DisableIPv6: true, + SyncMessageVersion: func(v int) *int { return &v }(1), } for _, anonymize := range []bool{false, true} { diff --git a/client/internal/dns/local/local.go b/client/internal/dns/local/local.go index d0268186c..fef35fd41 100644 --- a/client/internal/dns/local/local.go +++ b/client/internal/dns/local/local.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "net/netip" + "os" "slices" "strings" "sync" @@ -36,7 +37,43 @@ type resolver interface { // record is left alone (it points at something outside our mesh, e.g. // a non-peer upstream). type PeerConnectivity interface { - IsConnectedByIP(ip string) (known, connected bool) + IsConnectedByIP(ip netip.Addr) (known, connected bool) +} + +// PeerActivator wakes lazy-connection peers on demand. The local resolver calls +// it with the tunnel IPs an answer points at, so a peer that is idle (lazily +// disconnected) starts connecting at DNS-resolution time rather than racing the +// client's first request packet. nil disables warm-up. +type PeerActivator interface { + // ActivatePeersByIP triggers wake-up for the peer(s) owning addrs and blocks + // until one is connected or ctx (a short per-query budget) expires. It is a + // fast no-op for unknown or already-connected addresses. + ActivatePeersByIP(ctx context.Context, addrs []netip.Addr) +} + +const ( + defaultLazyWarmupTimeout = 2 * time.Second + envLazyWarmupTimeout = "NB_DNS_LAZY_WARMUP_TIMEOUT" +) + +// lazyWarmupTimeoutFromEnv returns the per-query budget for waking a +// lazy-connection peer a DNS answer points at. Tunable via +// NB_DNS_LAZY_WARMUP_TIMEOUT (a Go duration). Parsed once at construction time. +func lazyWarmupTimeoutFromEnv() time.Duration { + v := os.Getenv(envLazyWarmupTimeout) + if v == "" { + return defaultLazyWarmupTimeout + } + d, err := time.ParseDuration(v) + if err != nil { + log.Warnf("invalid %s value %q, using default %s: %v", envLazyWarmupTimeout, v, defaultLazyWarmupTimeout, err) + return defaultLazyWarmupTimeout + } + if d <= 0 { + log.Warnf("non-positive %s value %q, using default %s", envLazyWarmupTimeout, v, defaultLazyWarmupTimeout) + return defaultLazyWarmupTimeout + } + return d } type Resolver struct { @@ -51,6 +88,12 @@ type Resolver struct { // filter and preserves the legacy "return whatever is registered" // behaviour for callers that never wire a status source. peerConn PeerConnectivity + // peerActivator, when non-nil, is called at resolution time to warm the + // lazy connection to the peer(s) an answer points at. nil disables warm-up. + peerActivator PeerActivator + // warmupTimeout is the per-query budget for the lazy-connection warm-up + // wait, resolved from the environment once at construction time. + warmupTimeout time.Duration ctx context.Context cancel context.CancelFunc @@ -59,11 +102,12 @@ type Resolver struct { func NewResolver() *Resolver { ctx, cancel := context.WithCancel(context.Background()) return &Resolver{ - records: make(map[dns.Question][]dns.RR), - domains: make(map[domain.Domain]struct{}), - zones: make(map[domain.Domain]bool), - ctx: ctx, - cancel: cancel, + records: make(map[dns.Question][]dns.RR), + domains: make(map[domain.Domain]struct{}), + zones: make(map[domain.Domain]bool), + warmupTimeout: lazyWarmupTimeoutFromEnv(), + ctx: ctx, + cancel: cancel, } } @@ -76,6 +120,14 @@ func (d *Resolver) SetPeerConnectivity(p PeerConnectivity) { d.peerConn = p } +// SetPeerActivator wires the DNS-time lazy-connection warm-up. Pass nil to +// disable. Safe to call multiple times; the latest value wins. +func (d *Resolver) SetPeerActivator(a PeerActivator) { + d.mu.Lock() + defer d.mu.Unlock() + d.peerActivator = a +} + func (d *Resolver) MatchSubdomains() bool { return true } @@ -122,6 +174,9 @@ func (d *Resolver) ServeDNS(w dns.ResponseWriter, r *dns.Msg) { replyMessage.RecursionAvailable = true result := d.lookupRecords(logger, question) + // Warm before filtering: activation flips a lazily-idle target to connected, + // which then lets it survive the disconnected-peer filter below. + d.warmLazyPeers(question, result.records) result.records = d.filterDisconnectedPeerAnswers(logger, question, result.records) replyMessage.Authoritative = !result.hasExternalData replyMessage.Answer = result.records @@ -495,8 +550,8 @@ func (d *Resolver) filterDisconnectedPeerAnswers(logger *log.Entry, question dns kept := make([]dns.RR, 0, len(records)) var dropped int for _, rr := range records { - ip := extractRecordIP(rr) - if ip == "" { + ip, ok := extractRecordAddr(rr) + if !ok { kept = append(kept, rr) continue } @@ -518,22 +573,57 @@ func (d *Resolver) filterDisconnectedPeerAnswers(logger *log.Entry, question dns return kept } -// extractRecordIP returns the dotted-decimal / colon-hex IP carried by -// an A or AAAA record, or "" for any other record type. -func extractRecordIP(rr dns.RR) string { +// warmLazyPeers triggers lazy-connection wake-up for the peers a resolved +// answer points at and waits briefly for one to connect, so the caller's first +// request doesn't race the connection establishment. Warm-up is scoped to +// match-only (non-authoritative) zones — the synthesized private-service zones +// and user-created zones whose records point at specific peers. The account's +// peer zone is authoritative, so plain peer-name lookups never trigger warm-up; +// otherwise resolving any peer's name would wake its idle connection, defeating +// laziness mesh-wide. No-op when no activator is wired (lazy connections +// disabled) or the answer carries no peer IPs. +func (d *Resolver) warmLazyPeers(question dns.Question, records []dns.RR) { + if len(records) < 2 { + return + } + d.mu.RLock() + activator := d.peerActivator + var nonAuth, found bool + if activator != nil { + nonAuth, found = d.findZone(question.Name) + } + d.mu.RUnlock() + if activator == nil || !found || !nonAuth { + return + } + + var addrs []netip.Addr + for _, rr := range records { + if addr, ok := extractRecordAddr(rr); ok { + addrs = append(addrs, addr) + } + } + if len(addrs) == 0 { + return + } + + ctx, cancel := context.WithTimeout(d.ctx, d.warmupTimeout) + defer cancel() + activator.ActivatePeersByIP(ctx, addrs) +} + +// extractRecordAddr returns the IP address carried by an A or AAAA record. +// ok is false for any other record type or a record with no address. +func extractRecordAddr(rr dns.RR) (netip.Addr, bool) { switch r := rr.(type) { case *dns.A: - if r.A == nil { - return "" - } - return r.A.String() + addr, ok := netip.AddrFromSlice(r.A) + return addr.Unmap(), ok case *dns.AAAA: - if r.AAAA == nil { - return "" - } - return r.AAAA.String() + addr, ok := netip.AddrFromSlice(r.AAAA) + return addr.Unmap(), ok } - return "" + return netip.Addr{}, false } // Update replaces all zones and their records diff --git a/client/internal/dns/local/local_test.go b/client/internal/dns/local/local_test.go index 9b7dac231..89e896c0a 100644 --- a/client/internal/dns/local/local_test.go +++ b/client/internal/dns/local/local_test.go @@ -37,8 +37,8 @@ type mockPeerConnectivity struct { byIP map[string]struct{ known, connected bool } } -func (m mockPeerConnectivity) IsConnectedByIP(ip string) (known, connected bool) { - v, ok := m.byIP[ip] +func (m mockPeerConnectivity) IsConnectedByIP(ip netip.Addr) (known, connected bool) { + v, ok := m.byIP[ip.String()] if !ok { return false, false } diff --git a/client/internal/dns/local/warmup_test.go b/client/internal/dns/local/warmup_test.go new file mode 100644 index 000000000..0e77aa963 --- /dev/null +++ b/client/internal/dns/local/warmup_test.go @@ -0,0 +1,204 @@ +package local + +import ( + "context" + "net" + "net/netip" + "sync" + "testing" + "time" + + "github.com/miekg/dns" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/internal/dns/test" + nbdns "github.com/netbirdio/netbird/dns" +) + +// recordingActivator records the addresses it was asked to warm and returns +// immediately, so ServeDNS is not blocked by the test. +type recordingActivator struct { + mu sync.Mutex + called bool + addrs []netip.Addr +} + +func (r *recordingActivator) ActivatePeersByIP(_ context.Context, addrs []netip.Addr) { + r.mu.Lock() + defer r.mu.Unlock() + r.called = true + r.addrs = append(r.addrs, addrs...) +} + +func serveA(t *testing.T, resolver *Resolver, name string) *dns.Msg { + t.Helper() + var resp *dns.Msg + w := &test.MockResponseWriter{WriteMsgFunc: func(m *dns.Msg) error { resp = m; return nil }} + resolver.ServeDNS(w, new(dns.Msg).SetQuestion(name, dns.TypeA)) + return resp +} + +// serviceZone registers rec in a match-only (non-authoritative) zone, the shape +// the synthesized private-service zones arrive in. +func serviceZone(t *testing.T, resolver *Resolver, zone string, records ...nbdns.SimpleRecord) { + t.Helper() + resolver.Update([]nbdns.CustomZone{{ + Domain: zone, + Records: records, + NonAuthoritative: true, + }}) +} + +func TestLocalResolver_WarmsLazyPeerOnResolve(t *testing.T) { + // Warm-up fires only for multi-record answers (the HA / round-robin shape of + // the synthesized private-service zones), so register two peer targets. + const name = "svc.proxy.netbird.cloud." + recs := []nbdns.SimpleRecord{ + {Name: name, Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}, + {Name: name, Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.8"}, + } + resolver := NewResolver() + serviceZone(t, resolver, "proxy.netbird.cloud", recs...) + + act := &recordingActivator{} + resolver.SetPeerActivator(act) + + resp := serveA(t, resolver, name) + require.NotNil(t, resp, "resolver must answer") + require.NotEmpty(t, resp.Answer, "answer must carry the A records") + + act.mu.Lock() + defer act.mu.Unlock() + assert.True(t, act.called, "activator must be invoked for a multi-record service-zone answer") + assert.Contains(t, act.addrs, netip.MustParseAddr("100.64.0.7"), "activator must receive the first peer IP") + assert.Contains(t, act.addrs, netip.MustParseAddr("100.64.0.8"), "activator must receive the second peer IP") +} + +func TestLocalResolver_NoWarmupForSingleRecord(t *testing.T) { + // A single-record answer does not trigger warm-up; the resolver only warms + // multi-record answers. + rec := nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"} + resolver := NewResolver() + serviceZone(t, resolver, "proxy.netbird.cloud", rec) + + act := &recordingActivator{} + resolver.SetPeerActivator(act) + + resp := serveA(t, resolver, rec.Name) + require.NotNil(t, resp, "resolver must answer") + require.NotEmpty(t, resp.Answer, "answer must carry the A record") + + act.mu.Lock() + defer act.mu.Unlock() + assert.False(t, act.called, "activator must not be invoked for a single-record answer") +} + +func TestLocalResolver_NoActivatorNoWarmup(t *testing.T) { + // With no activator wired the resolver behaves exactly as before. + rec := nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"} + resolver := NewResolver() + serviceZone(t, resolver, "proxy.netbird.cloud", rec) + + resp := serveA(t, resolver, rec.Name) + require.NotNil(t, resp, "resolver must still answer without an activator") + require.NotEmpty(t, resp.Answer, "answer must carry the A record") +} + +func TestLocalResolver_NoWarmupForMissingRecord(t *testing.T) { + // A query that resolves to nothing must not invoke the activator (no IPs). + resolver := NewResolver() + serviceZone(t, resolver, "proxy.netbird.cloud", + nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}) + + act := &recordingActivator{} + resolver.SetPeerActivator(act) + + serveA(t, resolver, "absent.proxy.netbird.cloud.") + + act.mu.Lock() + defer act.mu.Unlock() + assert.False(t, act.called, "activator must not be invoked when there is no answer") +} + +func TestLocalResolver_NoWarmupInAuthoritativeZone(t *testing.T) { + // The account's peer zone is authoritative; resolving a peer's name there + // must not wake its lazy connection — warm-up is scoped to match-only + // (non-authoritative) zones such as the synthesized private-service zones. + // Use a multi-record answer so the authoritative-zone scoping is the only + // reason warm-up is skipped, not the single-record guard. + const name = "peer.netbird.cloud." + recs := []nbdns.SimpleRecord{ + {Name: name, Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.9"}, + {Name: name, Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.10"}, + } + resolver := NewResolver() + resolver.Update([]nbdns.CustomZone{{ + Domain: "netbird.cloud", + Records: recs, + }}) + + act := &recordingActivator{} + resolver.SetPeerActivator(act) + + resp := serveA(t, resolver, name) + require.NotNil(t, resp, "resolver must answer") + require.NotEmpty(t, resp.Answer, "answer must carry the A records") + + act.mu.Lock() + defer act.mu.Unlock() + assert.False(t, act.called, "activator must not be invoked for authoritative-zone answers") +} + +func TestLazyWarmupTimeoutFromEnv(t *testing.T) { + tests := []struct { + name string + value string + envSet bool + want time.Duration + }{ + {name: "unset uses default", want: defaultLazyWarmupTimeout}, + {name: "valid overrides", value: "5s", envSet: true, want: 5 * time.Second}, + {name: "invalid falls back", value: "not-a-duration", envSet: true, want: defaultLazyWarmupTimeout}, + {name: "negative falls back", value: "-1s", envSet: true, want: defaultLazyWarmupTimeout}, + {name: "zero falls back", value: "0s", envSet: true, want: defaultLazyWarmupTimeout}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envSet { + t.Setenv(envLazyWarmupTimeout, tt.value) + } + assert.Equal(t, tt.want, lazyWarmupTimeoutFromEnv()) + assert.Equal(t, tt.want, NewResolver().warmupTimeout, "constructor must resolve the timeout once") + }) + } +} + +func TestExtractRecordAddr(t *testing.T) { + t.Run("A record yields unmapped v4", func(t *testing.T) { + // net.ParseIP returns the 16-byte v4-in-v6 form, the same shape + // miekg/dns stores after parsing an A record; the extracted address + // must compare equal to a plain v4 netip.Addr. + addr, ok := extractRecordAddr(&dns.A{A: net.ParseIP("100.64.0.7")}) + require.True(t, ok) + assert.True(t, addr.Is4()) + assert.Equal(t, netip.MustParseAddr("100.64.0.7"), addr) + }) + + t.Run("AAAA record yields v6", func(t *testing.T) { + addr, ok := extractRecordAddr(&dns.AAAA{AAAA: net.ParseIP("fd00::1")}) + require.True(t, ok) + assert.Equal(t, netip.MustParseAddr("fd00::1"), addr) + }) + + t.Run("A record without address", func(t *testing.T) { + _, ok := extractRecordAddr(&dns.A{}) + assert.False(t, ok) + }) + + t.Run("non-address record", func(t *testing.T) { + _, ok := extractRecordAddr(&dns.CNAME{Target: "target.netbird.cloud."}) + assert.False(t, ok) + }) +} diff --git a/client/internal/dns/mock_server.go b/client/internal/dns/mock_server.go index 31fedd9e5..b19862c2f 100644 --- a/client/internal/dns/mock_server.go +++ b/client/internal/dns/mock_server.go @@ -8,6 +8,7 @@ import ( "github.com/miekg/dns" dnsconfig "github.com/netbirdio/netbird/client/internal/dns/config" + "github.com/netbirdio/netbird/client/internal/dns/local" nbdns "github.com/netbirdio/netbird/dns" "github.com/netbirdio/netbird/route" "github.com/netbirdio/netbird/shared/management/domain" @@ -92,6 +93,11 @@ func (m *MockServer) SetFirewall(Firewall) { // Mock implementation - no-op } +// SetPeerActivator mock implementation of SetPeerActivator from Server interface +func (m *MockServer) SetPeerActivator(local.PeerActivator) { + // Mock implementation - no-op +} + // BeginBatch mock implementation of BeginBatch from Server interface func (m *MockServer) BeginBatch() { // Mock implementation - no-op diff --git a/client/internal/dns/server.go b/client/internal/dns/server.go index 7556c66cc..f79454457 100644 --- a/client/internal/dns/server.go +++ b/client/internal/dns/server.go @@ -82,6 +82,7 @@ type Server interface { PopulateManagementDomain(mgmtURL *url.URL) error SetRouteSources(selected, active func() route.HAMap) SetFirewall(Firewall) + SetPeerActivator(local.PeerActivator) } type nsGroupsByDomain struct { @@ -491,6 +492,13 @@ func (s *DefaultServer) SetFirewall(fw Firewall) { } } +// SetPeerActivator wires the DNS-time lazy-connection warm-up on the local +// resolver. Injected after the connection manager exists (it does not at +// DNS-server construction time). Pass nil to disable. +func (s *DefaultServer) SetPeerActivator(a local.PeerActivator) { + s.localResolver.SetPeerActivator(a) +} + // Stop stops the server func (s *DefaultServer) Stop() { s.ctxCancel() @@ -1435,11 +1443,11 @@ type localPeerConnectivity struct { // IsConnectedByIP looks the IP up in the peerstore and surfaces both // the known and connected bits. Used by Resolver.filterDisconnectedPeerAnswers. -func (l localPeerConnectivity) IsConnectedByIP(ip string) (known, connected bool) { +func (l localPeerConnectivity) IsConnectedByIP(ip netip.Addr) (known, connected bool) { if l.status == nil { return false, false } - state, ok := l.status.PeerStateByIP(ip) + state, ok := l.status.PeerStateByIP(ip.String()) if !ok { return false, false } diff --git a/client/internal/dns_peer_activator.go b/client/internal/dns_peer_activator.go new file mode 100644 index 000000000..c283d6251 --- /dev/null +++ b/client/internal/dns_peer_activator.go @@ -0,0 +1,76 @@ +package internal + +import ( + "context" + "net/netip" + "time" + + "github.com/netbirdio/netbird/client/internal/peer" + "github.com/netbirdio/netbird/client/internal/peerstore" +) + +const dnsActivationPollInterval = 50 * time.Millisecond + +// dnsPeerActivator wakes lazy-connection peers from the DNS resolution path. It +// implements dns/local.PeerActivator. DNS queries run on their own goroutines, +// so it only touches state that is safe for concurrent use — ConnMgr.ActivatePeer, +// peerstore.Store and peer.Status — and never takes the engine's syncMsgMux, +// keeping DNS resolution from contending with network-map processing. +type dnsPeerActivator struct { + connMgr *ConnMgr + peerStore *peerstore.Store + status *peer.Status + // ctx is the engine's long-lived context. The connection dial is tied to it + // (not the per-query DNS wait budget) so a handshake that outlasts the wait + // still completes in the background rather than being cancelled at the deadline. + ctx context.Context +} + +// ActivatePeersByIP triggers wake-up for the peer(s) owning addrs and waits +// until one is connected or ctx (the per-query DNS wait budget) expires. +// Activation itself is tied to the engine's long-lived context so the dial +// survives a wait that times out. Unknown or already-connected addresses are +// skipped, so the steady-state (warm) path adds no latency. +func (a *dnsPeerActivator) ActivatePeersByIP(ctx context.Context, addrs []netip.Addr) { + if a == nil || a.connMgr == nil { + return + } + + var pending []string + for _, addr := range addrs { + ip := addr.String() + st, ok := a.status.PeerStateByIP(ip) + if !ok || st.ConnStatus == peer.StatusConnected { + continue + } + conn, ok := a.peerStore.PeerConn(st.PubKey) + if !ok { + continue + } + a.connMgr.ActivatePeer(a.ctx, conn) + pending = append(pending, ip) + } + + if len(pending) == 0 { + return + } + a.waitConnected(ctx, pending) +} + +// waitConnected blocks until any of ips reports a connected peer or ctx expires. +func (a *dnsPeerActivator) waitConnected(ctx context.Context, ips []string) { + ticker := time.NewTicker(dnsActivationPollInterval) + defer ticker.Stop() + for { + for _, ip := range ips { + if st, ok := a.status.PeerStateByIP(ip); ok && st.ConnStatus == peer.StatusConnected { + return + } + } + select { + case <-ctx.Done(): + return + case <-ticker.C: + } + } +} diff --git a/client/internal/dns_peer_activator_test.go b/client/internal/dns_peer_activator_test.go new file mode 100644 index 000000000..8c3b75e59 --- /dev/null +++ b/client/internal/dns_peer_activator_test.go @@ -0,0 +1,129 @@ +package internal + +import ( + "context" + "net/netip" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/internal/peer" + "github.com/netbirdio/netbird/client/internal/peerstore" +) + +func newTestPeerConn(t *testing.T, key string) *peer.Conn { + t.Helper() + conn, err := peer.NewConn(peer.ConnConfig{ + Key: key, + LocalKey: "local", + WgConfig: peer.WgConfig{ + AllowedIps: []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32")}, + }, + }, peer.ServiceDependencies{}) + require.NoError(t, err) + return conn +} + +func newTestDNSPeerActivator(t *testing.T) (*dnsPeerActivator, *peer.Status, *peerstore.Store) { + t.Helper() + status := peer.NewRecorder("https://mgm") + store := peerstore.NewConnStore() + // ConnMgr without Start: the lazy manager is nil, so ActivatePeer is a + // no-op — these tests exercise the activator's skip/wait logic. + connMgr := NewConnMgr(&EngineConfig{}, status, store, nil) + return &dnsPeerActivator{ + connMgr: connMgr, + peerStore: store, + status: status, + ctx: context.Background(), + }, status, store +} + +func TestDNSPeerActivator_NilSafe(t *testing.T) { + var a *dnsPeerActivator + a.ActivatePeersByIP(context.Background(), []netip.Addr{netip.MustParseAddr("100.64.0.1")}) +} + +// TestDNSPeerActivator_SkipsUnknownAndConnectedPeers verifies the steady-state +// (warm) path adds no latency: already-connected and unknown addresses never +// enter the wait loop. +func TestDNSPeerActivator_SkipsUnknownAndConnectedPeers(t *testing.T) { + a, status, store := newTestDNSPeerActivator(t) + + require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", "fd00::1")) + require.NoError(t, status.UpdatePeerState(peer.State{PubKey: "peerA", ConnStatus: peer.StatusConnected})) + store.AddPeerConn("peerA", newTestPeerConn(t, "peerA")) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + start := time.Now() + a.ActivatePeersByIP(ctx, []netip.Addr{ + netip.MustParseAddr("100.64.0.1"), // known, connected -> skipped + netip.MustParseAddr("fd00::1"), // known via IPv6, connected -> skipped + netip.MustParseAddr("100.64.0.99"), // unknown -> skipped + }) + require.Less(t, time.Since(start), time.Second, "no pending peer must mean no wait") +} + +// TestDNSPeerActivator_WaitsForPendingPeerToConnect verifies the wait loop +// returns as soon as a pending peer reports connected, well before the +// per-query budget expires. +func TestDNSPeerActivator_WaitsForPendingPeerToConnect(t *testing.T) { + a, status, store := newTestDNSPeerActivator(t) + + require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", "")) + store.AddPeerConn("peerA", newTestPeerConn(t, "peerA")) + + go func() { + time.Sleep(150 * time.Millisecond) + _ = status.UpdatePeerState(peer.State{PubKey: "peerA", ConnStatus: peer.StatusConnected}) + }() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + start := time.Now() + a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")}) + elapsed := time.Since(start) + + require.GreaterOrEqual(t, elapsed, 100*time.Millisecond, "must wait for the pending peer") + require.Less(t, elapsed, 5*time.Second, "must return on connect, not at the deadline") +} + +// TestDNSPeerActivator_ReturnsAtBudgetWhenPeerStaysIdle verifies a peer that +// never connects releases the DNS response at the per-query budget instead of +// blocking it indefinitely. +func TestDNSPeerActivator_ReturnsAtBudgetWhenPeerStaysIdle(t *testing.T) { + a, status, store := newTestDNSPeerActivator(t) + + require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", "")) + store.AddPeerConn("peerA", newTestPeerConn(t, "peerA")) + + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + + start := time.Now() + a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")}) + elapsed := time.Since(start) + + require.GreaterOrEqual(t, elapsed, 250*time.Millisecond, "must wait out the budget for a pending peer") + require.Less(t, elapsed, 5*time.Second, "must not block past the budget") +} + +// TestDNSPeerActivator_NoWaitWithoutPeerConn verifies a known-but-idle peer +// with no connection object in the store is not waited on: there is nothing to +// activate, so waiting could only ever time out. +func TestDNSPeerActivator_NoWaitWithoutPeerConn(t *testing.T) { + a, status, _ := newTestDNSPeerActivator(t) + + require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", "")) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + start := time.Now() + a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")}) + require.Less(t, time.Since(start), time.Second, "peer without a conn must not be waited on") +} diff --git a/client/internal/ebpf/ebpf/bpf_bpfeb.o b/client/internal/ebpf/ebpf/bpf_bpfeb.o index 6e9cda44a..7433ad740 100644 Binary files a/client/internal/ebpf/ebpf/bpf_bpfeb.o and b/client/internal/ebpf/ebpf/bpf_bpfeb.o differ diff --git a/client/internal/ebpf/ebpf/bpf_bpfel.o b/client/internal/ebpf/ebpf/bpf_bpfel.o index 6338f4774..779f43a00 100644 Binary files a/client/internal/ebpf/ebpf/bpf_bpfel.o and b/client/internal/ebpf/ebpf/bpf_bpfel.o differ diff --git a/client/internal/ebpf/ebpf/src/dns_fwd.c b/client/internal/ebpf/ebpf/src/dns_fwd.c index 5f3fbcc32..9f8de2001 100644 --- a/client/internal/ebpf/ebpf/src/dns_fwd.c +++ b/client/internal/ebpf/ebpf/src/dns_fwd.c @@ -52,11 +52,14 @@ int xdp_dns_fwd(struct iphdr *ip, struct udphdr *udp) { if (udp->dest == GENERAL_DNS_PORT && ip->daddr == dns_ip) { udp->dest = dns_port; + // Clear the now-stale checksum; zero means "not computed" for IPv4. + udp->check = 0; return XDP_PASS; } if (udp->source == dns_port && ip->saddr == dns_ip) { udp->source = GENERAL_DNS_PORT; + udp->check = 0; return XDP_PASS; } diff --git a/client/internal/ebpf/ebpf/src/wg_proxy.c b/client/internal/ebpf/ebpf/src/wg_proxy.c index 88fea65cf..5e7474928 100644 --- a/client/internal/ebpf/ebpf/src/wg_proxy.c +++ b/client/internal/ebpf/ebpf/src/wg_proxy.c @@ -50,5 +50,11 @@ int xdp_wg_proxy(struct iphdr *ip, struct udphdr *udp) { __be16 new_dst_port = htons(proxy_port); udp->dest = new_dst_port; udp->source = new_src_port; + + // The ports are covered by the UDP checksum. This is an IPv4 loopback hop + // and the payload is already integrity-protected, so clear the checksum (a + // zero UDP checksum means "not computed" for IPv4) rather than leave a + // stale value the kernel would drop as UDP_CSUM. + udp->check = 0; return XDP_PASS; } diff --git a/client/internal/engine.go b/client/internal/engine.go index 1d00ed0d2..617892e43 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -64,7 +64,10 @@ import ( "github.com/netbirdio/netbird/route" mgm "github.com/netbirdio/netbird/shared/management/client" "github.com/netbirdio/netbird/shared/management/domain" + sharedgrpc "github.com/netbirdio/netbird/shared/management/grpc" + nbnetworkmap "github.com/netbirdio/netbird/shared/management/networkmap" mgmProto "github.com/netbirdio/netbird/shared/management/proto" + types "github.com/netbirdio/netbird/shared/management/types" "github.com/netbirdio/netbird/shared/netiputil" auth "github.com/netbirdio/netbird/shared/relay/auth/hmac" relayClient "github.com/netbirdio/netbird/shared/relay/client" @@ -147,6 +150,7 @@ type EngineConfig struct { BlockLANAccess bool BlockInbound bool DisableIPv6 bool + SyncMessageVersion *int // LazyConnection is the MDM-sourced lazy-connection override; StateUnset defers to // the env var and management feature flag. @@ -220,6 +224,13 @@ type Engine struct { // networkSerial is the latest CurrentSerial (state ID) of the network sent by the Management service networkSerial uint64 + // latestComponents is the most-recent NetworkMapComponents decoded from + // a NetworkMapEnvelope (capability=3 peers only). Held alongside the + // NetworkMap that Calculate() produced from it so future incremental + // updates have a base to apply changes against. nil for legacy-format + // peers. Guarded by syncMsgMux. + latestComponents *types.NetworkMapComponents + networkMonitor *networkmonitor.NetworkMonitor sshServer sshServer @@ -652,8 +663,24 @@ func (e *Engine) Start(netbirdConfig *mgmProto.NetbirdConfig, mgmtURL *url.URL) iceCfg := e.createICEConfig() e.connMgr = NewConnMgr(e.config, e.statusRecorder, e.peerStore, wgIface) + e.connMgr.SetRoutedIPsReconciler(func(peerKey string) error { + if e.routeManager == nil { + return nil + } + return e.routeManager.ReconcilePeerAllowedIPs(peerKey) + }) e.connMgr.Start(e.ctx) + // Wire DNS-time lazy-connection warm-up now that the connection manager + // exists (it does not at DNS-server construction time). A DNS answer that + // points at an idle peer then wakes it before the client's first request. + e.dnsServer.SetPeerActivator(&dnsPeerActivator{ + connMgr: e.connMgr, + peerStore: e.peerStore, + status: e.statusRecorder, + ctx: e.ctx, + }) + e.srWatcher = guard.NewSRWatcher(e.signal, e.relayManager, e.mobileDep.IFaceDiscover, iceCfg) e.srWatcher.Start(peer.IsForceRelayed()) @@ -963,8 +990,12 @@ func (e *Engine) handleSync(update *mgmProto.SyncResponse) error { e.ApplySessionDeadline(update.GetSessionExpiresAt()) - if update.NetworkMap != nil && update.NetworkMap.PeerConfig != nil { - e.handleAutoUpdateVersion(update.NetworkMap.PeerConfig.AutoUpdate) + // Envelope sync responses carry PeerConfig at the top level; legacy + // NetworkMap syncs carry it under NetworkMap.PeerConfig. + if pc := update.GetPeerConfig(); pc != nil { + e.handleAutoUpdateVersion(pc.GetAutoUpdate()) + } else if nm := update.GetNetworkMap(); nm != nil && nm.GetPeerConfig() != nil { + e.handleAutoUpdateVersion(nm.GetPeerConfig().GetAutoUpdate()) } done := e.phase("netbird_config") @@ -974,12 +1005,47 @@ func (e *Engine) handleSync(update *mgmProto.SyncResponse) error { return err } + // Decode the network map from either the components envelope or the + // legacy proto.NetworkMap before the posture-check gating below, so the + // "is there a network map" decision covers both wire shapes. + var ( + nm *mgmProto.NetworkMap + components *types.NetworkMapComponents + ) + if version := update.GetVersion(); version == int32(sharedgrpc.ComponentNetworkMap) { + // Components-format peer: decode the envelope back to typed + // components, run Calculate() locally, and convert to the wire + // NetworkMap shape the rest of the engine consumes. Components are + // retained so future incremental updates can apply deltas instead + // of doing a full reconstruction. + envelope := update.GetNetworkMapEnvelope() + if envelope == nil { + return fmt.Errorf("received a SyncReponse indicating use of components network map, but components are missing") + } + + localKey := e.config.WgPrivateKey.PublicKey().String() + dnsName := "" + if pc := update.GetPeerConfig(); pc != nil { + // PeerConfig.Fqdn = "." — extract the + // shared domain by stripping the peer's own label prefix. Falls + // back to empty if the FQDN doesn't have the expected shape. + dnsName = extractDNSDomainFromFQDN(pc.GetFqdn()) + } + result, err := nbnetworkmap.EnvelopeToNetworkMap(e.ctx, envelope, localKey, dnsName) + if err != nil { + return fmt.Errorf("decode network map envelope: %w", err) + } + nm = result.NetworkMap + components = result.Components + } else { + nm = update.GetNetworkMap() + } + // Posture checks are bound to the network map presence: // NetworkMap != nil, checks present -> apply the received checks // NetworkMap != nil, checks nil -> posture checks were removed, clear them // NetworkMap == nil -> config-only update (e.g. relay token rotation), // leave the previously applied checks untouched - nm := update.GetNetworkMap() if nm == nil { return nil } @@ -992,6 +1058,14 @@ func (e *Engine) handleSync(update *mgmProto.SyncResponse) error { } done = e.phase("persist") + // Only retain the components view when the server sent the envelope + // path. A legacy proto.NetworkMap means components == nil; writing it + // here would clobber a previously-cached snapshot, breaking the + // incremental-delta base on a future envelope sync. + if components != nil { + e.latestComponents = components + } + e.persistSyncResponse(update) done() @@ -1005,6 +1079,19 @@ func (e *Engine) handleSync(update *mgmProto.SyncResponse) error { return nil } +// extractDNSDomainFromFQDN returns the trailing dotted domain part of the +// receiving peer's FQDN — the same value the management server fills as +// dnsName when it builds the legacy NetworkMap. "peer42.netbird.cloud" → +// "netbird.cloud". An empty string is returned for unrecognized formats. +func extractDNSDomainFromFQDN(fqdn string) string { + for i := 0; i < len(fqdn); i++ { + if fqdn[i] == '.' && i+1 < len(fqdn) { + return fqdn[i+1:] + } + } + return "" +} + // updateNetbirdConfig applies the management-provided NetBird configuration: // STUN/TURN and relay servers, flow logging and DNS settings. A nil config is a no-op, // which is the case for sync updates carrying only a network map. @@ -1164,6 +1251,7 @@ func (e *Engine) applyInfoFlags(info *system.Info) { e.config.BlockLANAccess, e.config.BlockInbound, e.config.DisableIPv6, + e.config.SyncMessageVersion, e.config.EnableSSHRoot, e.config.EnableSSHSFTP, e.config.EnableSSHLocalPortForwarding, @@ -2032,6 +2120,7 @@ func (e *Engine) readInitialSettings() ([]*route.Route, *nbdns.Config, bool, err e.config.BlockLANAccess, e.config.BlockInbound, e.config.DisableIPv6, + e.config.SyncMessageVersion, e.config.EnableSSHRoot, e.config.EnableSSHSFTP, e.config.EnableSSHLocalPortForwarding, diff --git a/client/internal/engine_session_deadline_test.go b/client/internal/engine_session_deadline_test.go index 6127e5bb0..5a67f103a 100644 --- a/client/internal/engine_session_deadline_test.go +++ b/client/internal/engine_session_deadline_test.go @@ -75,4 +75,14 @@ func TestApplySessionDeadline_ThreeState(t *testing.T) { require.True(t, e.statusRecorder.GetSessionExpiresAt().IsZero(), "invalid timestamp must clear the deadline") }) + + t.Run("recently expired timestamp stays visible as expired", func(t *testing.T) { + e := newEngine() + expired := time.Now().Add(-5 * time.Minute).UTC().Truncate(time.Second) + + e.ApplySessionDeadline(timestamppb.New(expired)) + + require.True(t, e.statusRecorder.GetSessionExpiresAt().Equal(expired), + "recently-expired deadline must stay on the recorder so consumers render it as expired") + }) } diff --git a/client/internal/lazyconn/manager/manager.go b/client/internal/lazyconn/manager/manager.go index 3868e37e8..b7424bb2f 100644 --- a/client/internal/lazyconn/manager/manager.go +++ b/client/internal/lazyconn/manager/manager.go @@ -29,6 +29,11 @@ type managedPeer struct { type Config struct { InactivityThreshold *time.Duration + // ReconcileAllowedIPs re-applies a peer's routed allowed IPs after its wake endpoint is + // armed. The activity listener creates the wake peer with the overlay /32 only; without the + // routed prefixes WireGuard would not steer subnet-bound traffic to the wake endpoint, so an + // idle routing peer could never be woken by that traffic. Optional; nil disables the reconcile. + ReconcileAllowedIPs func(peerKey string) error } // Manager manages lazy connections @@ -56,6 +61,9 @@ type Manager struct { peerToHAGroups map[string][]route.HAUniqueID // peer ID -> HA groups they belong to haGroupToPeers map[route.HAUniqueID][]string // HA group -> peer IDs in the group routesMu sync.RWMutex + + // reconcileAllowedIPs re-applies a peer's routed allowed IPs after its wake endpoint is armed. + reconcileAllowedIPs func(peerKey string) error } // NewManager creates a new lazy connection manager @@ -73,6 +81,7 @@ func NewManager(config Config, engineCtx context.Context, peerStore *peerstore.S activityManager: activity.NewManager(wgIface), peerToHAGroups: make(map[string][]route.HAUniqueID), haGroupToPeers: make(map[route.HAUniqueID][]string), + reconcileAllowedIPs: config.ReconcileAllowedIPs, } if wgIface.IsUserspaceBind() { @@ -201,7 +210,7 @@ func (m *Manager) AddPeer(peerCfg lazyconn.PeerConfig) (bool, error) { return false, nil } - if err := m.activityManager.MonitorPeerActivity(peerCfg); err != nil { + if err := m.armActivityListener(peerCfg); err != nil { return false, err } @@ -288,7 +297,7 @@ func (m *Manager) DeactivatePeer(peerID peerid.ConnID) { m.inactivityManager.RemovePeer(mp.peerCfg.PublicKey) - if err := m.activityManager.MonitorPeerActivity(*mp.peerCfg); err != nil { + if err := m.armActivityListener(*mp.peerCfg); err != nil { mp.peerCfg.Log.Errorf("failed to create activity monitor: %v", err) return } @@ -465,6 +474,31 @@ func (m *Manager) close() { } // shouldDeferIdleForHA checks if peer should stay connected due to HA group requirements +// armRoutedAllowedIPs re-applies the peer's routed allowed IPs onto its freshly armed wake +// endpoint. The activity listener creates the wake peer with the overlay /32 only, so without +// this the routed prefixes would be missing and traffic to a routed subnet could not wake the +// idle routing peer. It is a no-op when no reconciler is configured. +// armActivityListener (re)arms the peer's wake endpoint via the activity manager and then +// re-applies its routed allowed IPs, so traffic to a routed subnet can wake an idle routing +// peer. The routed prefixes must be re-applied after the wake endpoint exists because the +// listener creates it with the overlay /32 only. +func (m *Manager) armActivityListener(peerCfg lazyconn.PeerConfig) error { + if err := m.activityManager.MonitorPeerActivity(peerCfg); err != nil { + return err + } + m.armRoutedAllowedIPs(&peerCfg) + return nil +} + +func (m *Manager) armRoutedAllowedIPs(peerCfg *lazyconn.PeerConfig) { + if m.reconcileAllowedIPs == nil { + return + } + if err := m.reconcileAllowedIPs(peerCfg.PublicKey); err != nil { + peerCfg.Log.Errorf("failed to reconcile routed allowed IPs on wake endpoint: %v", err) + } +} + func (m *Manager) shouldDeferIdleForHA(inactivePeers map[string]struct{}, peerID string) bool { m.routesMu.RLock() defer m.routesMu.RUnlock() @@ -577,7 +611,7 @@ func (m *Manager) onPeerInactivityTimedOut(peerIDs map[string]struct{}) { mp.peerCfg.Log.Infof("start activity monitor") - if err := m.activityManager.MonitorPeerActivity(*mp.peerCfg); err != nil { + if err := m.armActivityListener(*mp.peerCfg); err != nil { mp.peerCfg.Log.Errorf("failed to create activity monitor: %v", err) continue } diff --git a/client/internal/mobile_dependency.go b/client/internal/mobile_dependency.go index 310d61a25..0234432b1 100644 --- a/client/internal/mobile_dependency.go +++ b/client/internal/mobile_dependency.go @@ -11,12 +11,14 @@ import ( // MobileDependency collect all dependencies for mobile platform type MobileDependency struct { - // Android only - TunAdapter device.TunAdapter - IFaceDiscover stdnet.ExternalIFaceDiscover + // Android and iOS NetworkChangeListener listener.NetworkChangeListener - HostDNSAddresses []netip.AddrPort - DnsReadyListener dns.ReadyListener + + // Android only + TunAdapter device.TunAdapter + IFaceDiscover stdnet.ExternalIFaceDiscover + HostDNSAddresses []netip.AddrPort + DnsReadyListener dns.ReadyListener // iOS only DnsManager dns.IosDnsManager diff --git a/client/internal/peer/status.go b/client/internal/peer/status.go index a987482fe..423ce9b23 100644 --- a/client/internal/peer/status.go +++ b/client/internal/peer/status.go @@ -813,19 +813,14 @@ func (d *Status) SetSessionExpiresAt(deadline time.Time) { } // GetSessionExpiresAt returns the most recently recorded SSO session deadline, -// or the zero value when no deadline is tracked. A deadline that has already -// slipped into the past reports as "none": once the session has expired it is -// no longer a meaningful countdown, and the sessionwatch.Watcher does not -// arm a timer at the deadline itself to clear it (only the two pre-expiry -// warnings). Without this guard the UI would keep painting a stale -// "expires in …" against a moment that has passed until the next login, -// extend, or teardown rewrote the value. +// or the zero value when no deadline is tracked. A deadline in the past is +// returned as-is: it means the session has expired, and consumers (tray row, +// CLI status) render it as "expired" rather than hiding it — masking it as +// "none" would blank the UI at the exact moment it should say the session +// ended. func (d *Status) GetSessionExpiresAt() time.Time { d.mux.Lock() defer d.mux.Unlock() - if !d.sessionExpiresAt.IsZero() && d.sessionExpiresAt.Before(time.Now()) { - return time.Time{} - } return d.sessionExpiresAt } diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index ed2f21999..a110e4102 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -96,6 +96,7 @@ type ConfigInput struct { BlockLANAccess *bool BlockInbound *bool DisableIPv6 *bool + SyncMessageVersion *int DisableNotifications *bool @@ -137,6 +138,7 @@ type Config struct { BlockLANAccess bool BlockInbound bool DisableIPv6 bool + SyncMessageVersion *int DisableNotifications *bool @@ -587,6 +589,12 @@ func (config *Config) apply(input ConfigInput) (updated bool, err error) { updated = true } + if input.SyncMessageVersion != nil && *input.SyncMessageVersion != *config.SyncMessageVersion { + log.Infof("setting SyncMessageVersion to %v", *input.SyncMessageVersion) + *config.SyncMessageVersion = *input.SyncMessageVersion + updated = true + } + if input.DisableNotifications != nil && (config.DisableNotifications == nil || *input.DisableNotifications != *config.DisableNotifications) { if *input.DisableNotifications { log.Infof("disabling notifications") diff --git a/client/internal/routemanager/dnsinterceptor/handler.go b/client/internal/routemanager/dnsinterceptor/handler.go index b784cc274..f92300bfd 100644 --- a/client/internal/routemanager/dnsinterceptor/handler.go +++ b/client/internal/routemanager/dnsinterceptor/handler.go @@ -95,7 +95,7 @@ func (d *DnsInterceptor) RemoveRoute() error { // AllowedIPs should use real IPs if d.currentPeerKey != "" { - if _, err := d.allowedIPsRefcounter.Decrement(prefix); err != nil { + if _, err := d.allowedIPsRefcounter.Decrement(prefix, d.currentPeerKey); err != nil { merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %v", prefix, err)) } } @@ -172,7 +172,7 @@ func (d *DnsInterceptor) removeAllowedIP(realPrefix netip.Prefix) error { } // AllowedIPs use real IPs - if _, err := d.allowedIPsRefcounter.Decrement(realPrefix); err != nil { + if _, err := d.allowedIPsRefcounter.Decrement(realPrefix, d.currentPeerKey); err != nil { return fmt.Errorf("remove allowed IP %s: %v", realPrefix, err) } @@ -205,7 +205,7 @@ func (d *DnsInterceptor) RemoveAllowedIPs() error { for _, prefixes := range d.interceptedDomains { for _, prefix := range prefixes { // AllowedIPs use real IPs - if _, err := d.allowedIPsRefcounter.Decrement(prefix); err != nil { + if _, err := d.allowedIPsRefcounter.Decrement(prefix, d.currentPeerKey); err != nil { merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %v", prefix, err)) } } diff --git a/client/internal/routemanager/dynamic/route.go b/client/internal/routemanager/dynamic/route.go index f0efd7b22..bb3b1c59c 100644 --- a/client/internal/routemanager/dynamic/route.go +++ b/client/internal/routemanager/dynamic/route.go @@ -135,7 +135,7 @@ func (r *Route) RemoveAllowedIPs() error { var merr *multierror.Error for _, domainPrefixes := range r.dynamicDomains { for _, prefix := range domainPrefixes { - if _, err := r.allowedIPsRefcounter.Decrement(prefix); err != nil { + if _, err := r.allowedIPsRefcounter.Decrement(prefix, r.currentPeerKey); err != nil { merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %w", prefix, err)) } } @@ -185,7 +185,7 @@ func (r *Route) startResolver(ctx context.Context) { } func (r *Route) update(ctx context.Context) error { - resolved, err := r.resolveDomains() + resolved, err := r.resolveDomains(ctx) if err != nil { if len(resolved) == 0 { return fmt.Errorf("resolve domains: %w", err) @@ -199,9 +199,9 @@ func (r *Route) update(ctx context.Context) error { return nil } -func (r *Route) resolveDomains() (domainMap, error) { +func (r *Route) resolveDomains(ctx context.Context) (domainMap, error) { results := make(chan resolveResult) - go r.resolve(results) + go r.resolve(ctx, results) resolved := domainMap{} var merr *multierror.Error @@ -217,7 +217,7 @@ func (r *Route) resolveDomains() (domainMap, error) { return resolved, nberrors.FormatErrorOrNil(merr) } -func (r *Route) resolve(results chan resolveResult) { +func (r *Route) resolve(ctx context.Context, results chan resolveResult) { var wg sync.WaitGroup for _, d := range r.route.Domains { @@ -225,10 +225,10 @@ func (r *Route) resolve(results chan resolveResult) { go func(domain domain.Domain) { defer wg.Done() - ips, err := r.getIPsFromResolver(domain) + ips, err := r.getIPsFromResolver(ctx, domain) if err != nil { log.Tracef("Failed to resolve domain %s with private resolver: %v", domain.SafeString(), err) - ips, err = net.LookupIP(domain.PunycodeString()) + ips, err = lookupHostIPs(ctx, domain) if err != nil { results <- resolveResult{domain: domain, err: fmt.Errorf("resolve d %s: %w", domain.SafeString(), err)} return @@ -320,7 +320,7 @@ func (r *Route) removeRoutes(prefixes []netip.Prefix) ([]netip.Prefix, error) { merr = multierror.Append(merr, fmt.Errorf("remove dynamic route for IP %s: %w", prefix, err)) } if r.currentPeerKey != "" { - if _, err := r.allowedIPsRefcounter.Decrement(prefix); err != nil { + if _, err := r.allowedIPsRefcounter.Decrement(prefix, r.currentPeerKey); err != nil { merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %s: %w", prefix, err)) } } @@ -364,6 +364,20 @@ func determinePrefixChanges(oldPrefixes, newPrefixes []netip.Prefix) (toAdd, toR return } +// lookupHostIPs resolves d via the system resolver, honoring ctx cancellation. +func lookupHostIPs(ctx context.Context, d domain.Domain) ([]net.IP, error) { + addrs, err := net.DefaultResolver.LookupIPAddr(ctx, d.PunycodeString()) + if err != nil { + return nil, err + } + + ips := make([]net.IP, 0, len(addrs)) + for _, addr := range addrs { + ips = append(ips, addr.IP) + } + return ips, nil +} + func combinePrefixes(oldPrefixes, removedPrefixes, addedPrefixes []netip.Prefix) []netip.Prefix { prefixSet := make(map[netip.Prefix]struct{}) for _, prefix := range oldPrefixes { diff --git a/client/internal/routemanager/dynamic/route_generic.go b/client/internal/routemanager/dynamic/route_generic.go index 56fd63fba..8bc2dd3df 100644 --- a/client/internal/routemanager/dynamic/route_generic.go +++ b/client/internal/routemanager/dynamic/route_generic.go @@ -3,11 +3,12 @@ package dynamic import ( + "context" "net" "github.com/netbirdio/netbird/shared/management/domain" ) -func (r *Route) getIPsFromResolver(domain domain.Domain) ([]net.IP, error) { - return net.LookupIP(domain.PunycodeString()) +func (r *Route) getIPsFromResolver(ctx context.Context, domain domain.Domain) ([]net.IP, error) { + return lookupHostIPs(ctx, domain) } diff --git a/client/internal/routemanager/dynamic/route_ios.go b/client/internal/routemanager/dynamic/route_ios.go index 1ae281d56..6a3d262b8 100644 --- a/client/internal/routemanager/dynamic/route_ios.go +++ b/client/internal/routemanager/dynamic/route_ios.go @@ -3,6 +3,7 @@ package dynamic import ( + "context" "fmt" "net" "time" @@ -16,7 +17,7 @@ import ( const dialTimeout = 10 * time.Second -func (r *Route) getIPsFromResolver(domain domain.Domain) ([]net.IP, error) { +func (r *Route) getIPsFromResolver(ctx context.Context, domain domain.Domain) ([]net.IP, error) { privateClient, err := nbdns.GetClientPrivate(r.wgInterface, r.resolverAddr.Addr(), dialTimeout) if err != nil { return nil, fmt.Errorf("error while creating private client: %s", err) @@ -32,7 +33,7 @@ func (r *Route) getIPsFromResolver(domain domain.Domain) ([]net.IP, error) { msg := new(dns.Msg) msg.SetQuestion(fqdn, qtype) - response, _, err := nbdns.ExchangeWithFallback(nil, privateClient, msg, r.resolverAddr.String()) + response, _, err := nbdns.ExchangeWithFallback(ctx, privateClient, msg, r.resolverAddr.String()) if err != nil { if queryErr == nil { queryErr = fmt.Errorf("DNS query for %s (type %d) after %s: %w", domain.SafeString(), qtype, time.Since(startTime), err) diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go index 66b24cc5a..2ab7e2a85 100644 --- a/client/internal/routemanager/manager.go +++ b/client/internal/routemanager/manager.go @@ -52,6 +52,10 @@ type Manager interface { UpdateRoutes(updateSerial uint64, serverRoutes map[route.ID]*route.Route, clientRoutes route.HAMap, useNewDNSRoute bool) error ClassifyRoutes(newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap) TriggerSelection(route.HAMap) + SelectRoutes(ids []route.NetID, appendRoute bool) error + DeselectRoutes(ids []route.NetID) error + SelectAllRoutes() + DeselectAllRoutes() GetRouteSelector() *routeselector.RouteSelector GetClientRoutes() route.HAMap GetSelectedClientRoutes() route.HAMap @@ -61,6 +65,7 @@ type Manager interface { InitialRouteRange() []string SetFirewall(firewall.Manager) error SetDNSForwarderPort(port uint16) + ReconcilePeerAllowedIPs(peerKey string) error Stop(stateManager *statemanager.Manager) } @@ -215,7 +220,7 @@ func (m *DefaultManager) setupRefCounters(useNoop bool) { ) } - m.allowedIPsRefCounter = refcounter.New( + m.allowedIPsRefCounter = refcounter.NewAllowedIPs( func(prefix netip.Prefix, peerKey string) (string, error) { // save peerKey to use it in the remove function return peerKey, m.wgInterface.AddAllowedIP(peerKey, prefix) @@ -232,6 +237,30 @@ func (m *DefaultManager) setupRefCounters(useNoop bool) { ) } +// ReconcilePeerAllowedIPs re-applies every routed allowed IP currently tracked for the peer +// onto the WireGuard device. The allowed-IP refcounter only calls its AddFunc (which pushes to +// the device) on a prefix's 0->1 transition, so a peer whose device entry was rebuilt without a +// matching refcounter change — e.g. a lazy connection cycling through idle->wake, which recreates +// the WireGuard peer with the overlay /32 only — ends up missing routed prefixes the refcounter +// still considers installed, and nothing retries. Calling this when the peer's WireGuard entry is +// (re)created restores convergence. It is add-only and idempotent: AddAllowedIP is update-only, so +// prefixes are re-added to an existing peer and an absent peer is left untouched. +func (m *DefaultManager) ReconcilePeerAllowedIPs(peerKey string) error { + if m.allowedIPsRefCounter == nil { + return nil + } + + return m.allowedIPsRefCounter.ReapplyMatching( + func(out string) bool { return out == peerKey }, + func(prefix netip.Prefix) error { + if err := m.wgInterface.AddAllowedIP(peerKey, prefix); err != nil { + return fmt.Errorf("add allowed IP %s for peer %s: %w", prefix, peerKey, err) + } + return nil + }, + ) +} + // Init sets up the routing func (m *DefaultManager) Init() error { m.routeSelector = m.initSelector() @@ -775,7 +804,7 @@ func (m *DefaultManager) collectExitNodeInfo(clientRoutes route.HAMap) exitNodeI var info exitNodeInfo for haID, routes := range clientRoutes { - if !m.isExitNodeRoute(routes) { + if !isExitNodeRoutes(routes) { continue } @@ -795,13 +824,6 @@ func (m *DefaultManager) collectExitNodeInfo(clientRoutes route.HAMap) exitNodeI return info } -func (m *DefaultManager) isExitNodeRoute(routes []*route.Route) bool { - if len(routes) == 0 { - return false - } - return route.IsV4DefaultRoute(routes[0].Network) || route.IsV6DefaultRoute(routes[0].Network) -} - func (m *DefaultManager) categorizeUserSelection(netID route.NetID, info *exitNodeInfo) { if m.routeSelector.IsSelected(netID) { info.userSelected = append(info.userSelected, netID) diff --git a/client/internal/routemanager/mock.go b/client/internal/routemanager/mock.go index 937314995..cf761091d 100644 --- a/client/internal/routemanager/mock.go +++ b/client/internal/routemanager/mock.go @@ -16,6 +16,8 @@ type MockManager struct { ClassifyRoutesFunc func(routes []*route.Route) (map[route.ID]*route.Route, route.HAMap) UpdateRoutesFunc func(updateSerial uint64, serverRoutes map[route.ID]*route.Route, clientRoutes route.HAMap, useNewDNSRoute bool) error TriggerSelectionFunc func(haMap route.HAMap) + SelectRoutesFunc func(ids []route.NetID, appendRoute bool) error + DeselectRoutesFunc func(ids []route.NetID) error GetRouteSelectorFunc func() *routeselector.RouteSelector GetClientRoutesFunc func() route.HAMap GetSelectedClientRoutesFunc func() route.HAMap @@ -55,6 +57,30 @@ func (m *MockManager) TriggerSelection(networks route.HAMap) { } } +// SelectRoutes mock implementation of SelectRoutes from Manager interface +func (m *MockManager) SelectRoutes(ids []route.NetID, appendRoute bool) error { + if m.SelectRoutesFunc != nil { + return m.SelectRoutesFunc(ids, appendRoute) + } + return nil +} + +// DeselectRoutes mock implementation of DeselectRoutes from Manager interface +func (m *MockManager) DeselectRoutes(ids []route.NetID) error { + if m.DeselectRoutesFunc != nil { + return m.DeselectRoutesFunc(ids) + } + return nil +} + +// SelectAllRoutes mock implementation of SelectAllRoutes from Manager interface +func (m *MockManager) SelectAllRoutes() { +} + +// DeselectAllRoutes mock implementation of DeselectAllRoutes from Manager interface +func (m *MockManager) DeselectAllRoutes() { +} + // GetRouteSelector mock implementation of GetRouteSelector from Manager interface func (m *MockManager) GetRouteSelector() *routeselector.RouteSelector { if m.GetRouteSelectorFunc != nil { @@ -112,6 +138,11 @@ func (m *MockManager) SetFirewall(firewall.Manager) error { func (m *MockManager) SetDNSForwarderPort(port uint16) { } +// ReconcilePeerAllowedIPs mock implementation of ReconcilePeerAllowedIPs from Manager interface +func (m *MockManager) ReconcilePeerAllowedIPs(peerKey string) error { + return nil +} + // Stop mock implementation of Stop from Manager interface func (m *MockManager) Stop(stateManager *statemanager.Manager) { if m.StopFunc != nil { diff --git a/client/internal/routemanager/notifier/notifier_ios.go b/client/internal/routemanager/notifier/notifier_ios.go index d0888f3a1..c91a76551 100644 --- a/client/internal/routemanager/notifier/notifier_ios.go +++ b/client/internal/routemanager/notifier/notifier_ios.go @@ -3,7 +3,6 @@ package notifier import ( - "container/list" "net/netip" "slices" "sort" @@ -16,20 +15,12 @@ import ( type Notifier struct { mu sync.Mutex - cond *sync.Cond currentPrefixes []string listener listener.NetworkChangeListener - queue *list.List - closed bool } func NewNotifier() *Notifier { - n := &Notifier{ - queue: list.New(), - } - n.cond = sync.NewCond(&n.mu) - go n.deliverLoop() - return n + return &Notifier{} } func (n *Notifier) SetListener(listener listener.NetworkChangeListener) { @@ -59,44 +50,19 @@ func (n *Notifier) OnNewPrefixes(prefixes []netip.Prefix) { sort.Strings(newNets) n.mu.Lock() + defer n.mu.Unlock() if slices.Equal(n.currentPrefixes, newNets) { - n.mu.Unlock() return } n.currentPrefixes = newNets - routes := strings.Join(n.currentPrefixes, ",") - n.queue.PushBack(routes) - n.cond.Signal() - n.mu.Unlock() + if n.listener != nil { + n.listener.OnNetworkChanged(strings.Join(n.currentPrefixes, ",")) + } } func (n *Notifier) Close() { - n.mu.Lock() - n.closed = true - n.cond.Signal() - n.mu.Unlock() } func (n *Notifier) GetInitialRouteRanges() []string { return nil } - -func (n *Notifier) deliverLoop() { - for { - n.mu.Lock() - for n.queue.Len() == 0 && !n.closed { - n.cond.Wait() - } - if n.closed && n.queue.Len() == 0 { - n.mu.Unlock() - return - } - routes := n.queue.Remove(n.queue.Front()).(string) - l := n.listener - n.mu.Unlock() - - if l != nil { - l.OnNetworkChanged(routes) - } - } -} diff --git a/client/internal/routemanager/reconcile_test.go b/client/internal/routemanager/reconcile_test.go new file mode 100644 index 000000000..c6806a6cd --- /dev/null +++ b/client/internal/routemanager/reconcile_test.go @@ -0,0 +1,90 @@ +//go:build !windows + +package routemanager + +import ( + "net" + "net/netip" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.zx2c4.com/wireguard/tun/netstack" + + "github.com/netbirdio/netbird/client/iface/device" + "github.com/netbirdio/netbird/client/iface/wgaddr" + "github.com/netbirdio/netbird/client/internal/routemanager/refcounter" +) + +// reconcileWGMock is a minimal iface.WGIface that only records AddAllowedIP calls; every other +// method is an inert stub because ReconcilePeerAllowedIPs exercises none of them. +type reconcileWGMock struct { + mu sync.Mutex + adds map[string][]netip.Prefix +} + +func (m *reconcileWGMock) AddAllowedIP(peerKey string, allowedIP netip.Prefix) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.adds == nil { + m.adds = map[string][]netip.Prefix{} + } + m.adds[peerKey] = append(m.adds[peerKey], allowedIP) + return nil +} + +func (m *reconcileWGMock) added(peerKey string) []netip.Prefix { + m.mu.Lock() + defer m.mu.Unlock() + return m.adds[peerKey] +} + +func (m *reconcileWGMock) RemoveAllowedIP(string, netip.Prefix) error { return nil } +func (m *reconcileWGMock) Name() string { return "utun-test" } +func (m *reconcileWGMock) Address() wgaddr.Address { return wgaddr.Address{} } +func (m *reconcileWGMock) ToInterface() *net.Interface { return nil } +func (m *reconcileWGMock) IsUserspaceBind() bool { return false } +func (m *reconcileWGMock) GetFilter() device.PacketFilter { return nil } +func (m *reconcileWGMock) GetDevice() *device.FilteredDevice { return nil } +func (m *reconcileWGMock) GetNet() *netstack.Net { return nil } + +// TestReconcilePeerAllowedIPs verifies the declarative reconcile re-applies every routed prefix +// tracked for the peer (self-heal, independent of refcount level) and stays scoped to that peer. +func TestReconcilePeerAllowedIPs(t *testing.T) { + wg := &reconcileWGMock{} + m := &DefaultManager{wgInterface: wg} + m.allowedIPsRefCounter = refcounter.NewAllowedIPs( + func(_ netip.Prefix, peerKey string) (string, error) { return peerKey, nil }, + func(netip.Prefix, string) error { return nil }, + ) + + peerA1 := netip.MustParsePrefix("10.0.0.0/24") + peerA2 := netip.MustParsePrefix("10.1.0.0/24") + peerB1 := netip.MustParsePrefix("10.2.0.0/24") + + for prefix, peer := range map[netip.Prefix]string{peerA1: "peerA", peerA2: "peerA", peerB1: "peerB"} { + _, err := m.allowedIPsRefCounter.Increment(prefix, peer) + require.NoError(t, err) + } + // Extra reference: reconcile must still re-apply the prefix even though its refcount never + // hit 0 again (the exact case the plain incremental path skips). + _, err := m.allowedIPsRefCounter.Increment(peerA1, "peerA") + require.NoError(t, err) + + require.NoError(t, m.ReconcilePeerAllowedIPs("peerA")) + + assert.ElementsMatch(t, []netip.Prefix{peerA1, peerA2}, wg.added("peerA"), + "reconcile must re-apply all routed prefixes of the peer") + assert.Empty(t, wg.added("peerB"), "reconcile must not touch another peer's prefixes") +} + +// TestReconcilePeerAllowedIPsNoCounter verifies reconcile is a safe no-op before the refcounter is +// set up. +func TestReconcilePeerAllowedIPsNoCounter(t *testing.T) { + wg := &reconcileWGMock{} + m := &DefaultManager{wgInterface: wg} + + require.NoError(t, m.ReconcilePeerAllowedIPs("peerA")) + assert.Empty(t, wg.added("peerA")) +} diff --git a/client/internal/routemanager/refcounter/allowedips.go b/client/internal/routemanager/refcounter/allowedips.go new file mode 100644 index 000000000..6d682e8a9 --- /dev/null +++ b/client/internal/routemanager/refcounter/allowedips.go @@ -0,0 +1,206 @@ +package refcounter + +import ( + "errors" + "fmt" + "net/netip" + "sort" + "sync" + + "github.com/hashicorp/go-multierror" + + nberrors "github.com/netbirdio/netbird/client/errors" +) + +// allowedIPsEntry holds the per-peer reference counts for a single prefix and which peer is +// currently installed in WireGuard. WireGuard allows a prefix on exactly one peer, so at most +// one peer is active at a time even when several peers reference the prefix. +type allowedIPsEntry struct { + // peers maps a peerKey to the number of references holding the prefix for that peer. + peers map[string]int + // active is the peerKey currently installed in WireGuard for this prefix ("" if none). + active string + // total is the sum of all per-peer reference counts (kept in sync with peers). + total int +} + +// AllowedIPsRefCounter is a peer-aware reference counter for WireGuard AllowedIPs. +// +// The generic Counter keys only by prefix and remembers a single Out value set by the first +// caller, which it never changes. That is wrong for AllowedIPs: two independent watchers (or +// multiple resolved domains) can reference the same prefix through different peers, and when the +// peer currently installed in WireGuard releases its last reference the prefix must be handed over +// to a surviving peer instead of being left pointing at the released one. +// +// It calls add/remove (which program WireGuard) only on the transitions that matter: +// - add on the first reference for a prefix, or when swapping the active peer; +// - remove on the last reference for a prefix, or on the old peer during a swap. +type AllowedIPsRefCounter struct { + mu sync.Mutex + entries map[netip.Prefix]*allowedIPsEntry + add AddFunc[netip.Prefix, string, string] + remove RemoveFunc[netip.Prefix, string] +} + +// NewAllowedIPs creates a new peer-aware AllowedIPs reference counter. +// add programs a prefix on a peer in WireGuard and returns the peerKey to store as the active peer. +// remove unprograms the prefix from the given peer. +func NewAllowedIPs(add AddFunc[netip.Prefix, string, string], remove RemoveFunc[netip.Prefix, string]) *AllowedIPsRefCounter { + return &AllowedIPsRefCounter{ + entries: map[netip.Prefix]*allowedIPsEntry{}, + add: add, + remove: remove, + } +} + +// Increment adds a reference to prefix for peerKey. WireGuard is programmed only for the first +// reference to a prefix; while a different peer is already installed the prefix is left with it +// (first peer wins, HA at the WireGuard layer is not possible) and only the reference count is kept. +func (rm *AllowedIPsRefCounter) Increment(prefix netip.Prefix, peerKey string) (Ref[string], error) { + rm.mu.Lock() + defer rm.mu.Unlock() + + e, ok := rm.entries[prefix] + if !ok { + e = &allowedIPsEntry{peers: map[string]int{}} + rm.entries[prefix] = e + } + + logCallerF("Increasing allowed IP ref count for prefix %v peer %s [peer %d -> %d, total %d -> %d, active %q]", + prefix, peerKey, e.peers[peerKey], e.peers[peerKey]+1, e.total, e.total+1, e.active) + + // Program WireGuard only when nothing is installed yet for this prefix. + if e.active == "" { + out, err := rm.add(prefix, peerKey) + if errors.Is(err, ErrIgnore) { + if e.total == 0 { + delete(rm.entries, prefix) + } + return Ref[string]{Count: e.total, Out: e.active}, nil + } + if err != nil { + if e.total == 0 { + delete(rm.entries, prefix) + } + return Ref[string]{}, fmt.Errorf("failed to add allowed IP %v for peer %s: %w", prefix, peerKey, err) + } + e.active = out + } + + e.peers[peerKey]++ + e.total++ + + return Ref[string]{Count: e.total, Out: e.active}, nil +} + +// Decrement removes a reference to prefix for peerKey. When the peer currently installed in +// WireGuard releases its last reference, the prefix is swapped to a surviving peer if one exists, +// otherwise it is removed from WireGuard. +func (rm *AllowedIPsRefCounter) Decrement(prefix netip.Prefix, peerKey string) (Ref[string], error) { + rm.mu.Lock() + defer rm.mu.Unlock() + + e, ok := rm.entries[prefix] + if !ok { + logCallerF("No allowed IP reference found for prefix %v", prefix) + return Ref[string]{}, nil + } + + if e.peers[peerKey] > 0 { + logCallerF("Decreasing allowed IP ref count for prefix %v peer %s [peer %d -> %d, total %d -> %d, active %q]", + prefix, peerKey, e.peers[peerKey], e.peers[peerKey]-1, e.total, e.total-1, e.active) + e.peers[peerKey]-- + e.total-- + if e.peers[peerKey] == 0 { + delete(e.peers, peerKey) + } + } else { + logCallerF("No allowed IP reference found for prefix %v peer %s", prefix, peerKey) + } + + // If the peer currently installed in WireGuard still holds references, nothing to reprogram. + // Keying the check on the active peer (not the one just released) makes this self-healing: + // a prior swap whose remove/add failed leaves e.active pointing at a peer with no references, + // and this retries the hand-off on the next Decrement instead of getting stuck. + if e.active != "" && e.peers[e.active] > 0 { + return Ref[string]{Count: e.total, Out: e.active}, nil + } + + // Detach the stale/gone active peer from WireGuard before reprogramming. + if e.active != "" { + if err := rm.remove(prefix, e.active); err != nil { + return Ref[string]{Count: e.total, Out: e.active}, fmt.Errorf("remove allowed IP %v for peer %s: %w", prefix, e.active, err) + } + e.active = "" + } + + // Hand the prefix over to a surviving peer, or drop the entry when none remain. + if survivor, ok := pickSurvivor(e.peers); ok { + out, err := rm.add(prefix, survivor) + if err != nil { + return Ref[string]{Count: e.total, Out: ""}, fmt.Errorf("swap allowed IP %v to peer %s: %w", prefix, survivor, err) + } + e.active = out + return Ref[string]{Count: e.total, Out: e.active}, nil + } + + delete(rm.entries, prefix) + return Ref[string]{Count: 0, Out: ""}, nil +} + +// Flush removes all prefixes from WireGuard and clears the counter. +func (rm *AllowedIPsRefCounter) Flush() error { + rm.mu.Lock() + defer rm.mu.Unlock() + + var merr *multierror.Error + for prefix, e := range rm.entries { + if e.active == "" { + continue + } + logCallerF("Flushing allowed IP for prefix %v peer %s", prefix, e.active) + if err := rm.remove(prefix, e.active); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove allowed IP %v for peer %s: %w", prefix, e.active, err)) + } + } + + clear(rm.entries) + + return nberrors.FormatErrorOrNil(merr) +} + +// ReapplyMatching calls apply for every prefix whose currently installed (active) peer satisfies +// pred, holding the lock for the whole pass. It is used to re-push allowed IPs onto a peer whose +// WireGuard entry was rebuilt (e.g. a lazy connection cycling idle->wake) without a matching +// refcounter change, which would otherwise leave the prefix installed in the counter but missing +// on the device. Only the active peer is considered — a prefix that lost its installed peer to a +// failed swap is skipped here and reconciled by the next Increment/Decrement. +func (rm *AllowedIPsRefCounter) ReapplyMatching(pred func(out string) bool, apply func(key netip.Prefix) error) error { + rm.mu.Lock() + defer rm.mu.Unlock() + + var merr *multierror.Error + for prefix, e := range rm.entries { + if e.active != "" && pred(e.active) { + if err := apply(prefix); err != nil { + merr = multierror.Append(merr, err) + } + } + } + return nberrors.FormatErrorOrNil(merr) +} + +// pickSurvivor deterministically selects a peer still referencing the prefix. WireGuard cannot do +// multipath for a single prefix, so any surviving peer is a valid winner; the choice is made stable +// (lowest peerKey) for predictable behavior and testability. +func pickSurvivor(peers map[string]int) (string, bool) { + if len(peers) == 0 { + return "", false + } + keys := make([]string, 0, len(peers)) + for k := range peers { + keys = append(keys, k) + } + sort.Strings(keys) + return keys[0], true +} diff --git a/client/internal/routemanager/refcounter/allowedips_test.go b/client/internal/routemanager/refcounter/allowedips_test.go new file mode 100644 index 000000000..835142083 --- /dev/null +++ b/client/internal/routemanager/refcounter/allowedips_test.go @@ -0,0 +1,241 @@ +package refcounter + +import ( + "errors" + "net/netip" + "testing" +) + +// fakeWG models WireGuard's cryptokey routing: a prefix can be installed on exactly one peer. +// failAdd/failRemove make the next add/remove fail once, to exercise the self-healing error paths. +type fakeWG struct { + installed map[netip.Prefix]string + adds int + removes int + failAdd bool + failRemove bool +} + +func newFakeWG() *fakeWG { + return &fakeWG{installed: map[netip.Prefix]string{}} +} + +func (f *fakeWG) counter() *AllowedIPsRefCounter { + return NewAllowedIPs( + func(prefix netip.Prefix, peerKey string) (string, error) { + if f.failAdd { + f.failAdd = false + return "", errors.New("add failed") + } + f.adds++ + f.installed[prefix] = peerKey + return peerKey, nil + }, + func(prefix netip.Prefix, peerKey string) error { + if f.failRemove { + f.failRemove = false + return errors.New("remove failed") + } + f.removes++ + // only clear if this peer is the one installed, mirroring wg semantics + if f.installed[prefix] == peerKey { + delete(f.installed, prefix) + } + return nil + }, + ) +} + +func mustPrefix(t *testing.T, s string) netip.Prefix { + t.Helper() + p, err := netip.ParsePrefix(s) + if err != nil { + t.Fatalf("parse prefix %q: %v", s, err) + } + return p +} + +func mustIncrement(t *testing.T, c *AllowedIPsRefCounter, p netip.Prefix, peer string) Ref[string] { + t.Helper() + ref, err := c.Increment(p, peer) + if err != nil { + t.Fatalf("Increment(%v, %s): %v", p, peer, err) + } + return ref +} + +func mustDecrement(t *testing.T, c *AllowedIPsRefCounter, p netip.Prefix, peer string) Ref[string] { + t.Helper() + ref, err := c.Decrement(p, peer) + if err != nil { + t.Fatalf("Decrement(%v, %s): %v", p, peer, err) + } + return ref +} + +// TestAllowedIPs_SwapOnActivePeerRemoval reproduces the reported bug: two networks with the same +// prefix routed by different peers. Removing the network whose peer is installed must hand the +// prefix over to the surviving peer instead of leaving it on the removed one. +func TestAllowedIPs_SwapOnActivePeerRemoval(t *testing.T) { + f := newFakeWG() + c := f.counter() + p := mustPrefix(t, "10.44.8.0/24") + + mustIncrement(t, c, p, "peerA") + mustIncrement(t, c, p, "peerB") + // First peer wins while both are present. + if got := f.installed[p]; got != "peerA" { + t.Fatalf("expected peerA installed, got %q", got) + } + + // Remove the active peer's network -> must swap to peerB. + mustDecrement(t, c, p, "peerA") + if got := f.installed[p]; got != "peerB" { + t.Fatalf("BUG: prefix stuck on removed peer, want peerB got %q", got) + } + + // Remove the last one -> prefix gone. + mustDecrement(t, c, p, "peerB") + if _, ok := f.installed[p]; ok { + t.Fatalf("expected prefix removed, still installed on %q", f.installed[p]) + } +} + +// TestAllowedIPs_RemoveNonActivePeer removing a non-installed peer must not touch WireGuard. +func TestAllowedIPs_RemoveNonActivePeer(t *testing.T) { + f := newFakeWG() + c := f.counter() + p := mustPrefix(t, "10.44.8.0/24") + + mustIncrement(t, c, p, "peerA") + mustIncrement(t, c, p, "peerB") + removesBefore := f.removes + + mustDecrement(t, c, p, "peerB") + if f.installed[p] != "peerA" { + t.Fatalf("active peer must stay peerA, got %q", f.installed[p]) + } + if f.removes != removesBefore { + t.Fatalf("removing a non-active peer must not call wg remove") + } +} + +// TestAllowedIPs_SamePeerMultipleRefs two references via the same peer must keep the prefix until +// the last reference is released (the reason the per-peer count must be an int, not a set). +func TestAllowedIPs_SamePeerMultipleRefs(t *testing.T) { + f := newFakeWG() + c := f.counter() + p := mustPrefix(t, "10.44.8.0/24") + + mustIncrement(t, c, p, "peerA") + mustIncrement(t, c, p, "peerA") + if f.adds != 1 { + t.Fatalf("expected a single wg add for the same peer, got %d", f.adds) + } + + mustDecrement(t, c, p, "peerA") + if f.installed[p] != "peerA" { + t.Fatalf("prefix must stay while a reference remains, got %q", f.installed[p]) + } + if f.removes != 0 { + t.Fatalf("no wg remove expected while a reference remains, got %d", f.removes) + } + + mustDecrement(t, c, p, "peerA") + if _, ok := f.installed[p]; ok { + t.Fatalf("prefix must be removed after last reference") + } +} + +// TestAllowedIPs_RefCountAndActive checks the Ref returned to callers (used for the HA-disabled log). +func TestAllowedIPs_RefCountAndActive(t *testing.T) { + f := newFakeWG() + c := f.counter() + p := mustPrefix(t, "10.44.8.0/24") + + ref := mustIncrement(t, c, p, "peerA") + if ref.Count != 1 || ref.Out != "peerA" { + t.Fatalf("want {1, peerA}, got {%d, %q}", ref.Count, ref.Out) + } + ref = mustIncrement(t, c, p, "peerB") + if ref.Count != 2 || ref.Out != "peerA" { + t.Fatalf("want {2, peerA}, got {%d, %q}", ref.Count, ref.Out) + } +} + +// TestAllowedIPs_Flush removes everything installed and clears the counter. +func TestAllowedIPs_Flush(t *testing.T) { + f := newFakeWG() + c := f.counter() + p1 := mustPrefix(t, "10.44.8.0/24") + p2 := mustPrefix(t, "10.44.9.0/24") + + mustIncrement(t, c, p1, "peerA") + mustIncrement(t, c, p2, "peerB") + + if err := c.Flush(); err != nil { + t.Fatal(err) + } + if len(f.installed) != 0 { + t.Fatalf("expected all prefixes removed, got %v", f.installed) + } + // After flush, a fresh increment must add again. + mustIncrement(t, c, p1, "peerC") + if f.installed[p1] != "peerC" { + t.Fatalf("counter not reset after flush") + } +} + +// TestAllowedIPs_SelfHealAfterSwapAddError ensures a failed add during a swap does not permanently +// strand the prefix: the next Decrement (or Increment) must retry and install a surviving peer. +func TestAllowedIPs_SelfHealAfterSwapAddError(t *testing.T) { + f := newFakeWG() + c := f.counter() + p := mustPrefix(t, "10.44.8.0/24") + + mustIncrement(t, c, p, "peerA") + mustIncrement(t, c, p, "peerB") + mustIncrement(t, c, p, "peerC") + + // Removing the active peerA triggers a swap to a survivor; make the add fail once. + f.failAdd = true + if _, err := c.Decrement(p, "peerA"); err == nil { + t.Fatalf("expected error from failed swap add") + } + if _, ok := f.installed[p]; ok { + t.Fatalf("nothing should be installed after a failed swap add, got %q", f.installed[p]) + } + + // A later Decrement of a non-active survivor must retry the hand-off (self-heal), not stay stuck. + ref := mustDecrement(t, c, p, "peerC") + if got := f.installed[p]; got == "" { + t.Fatalf("self-heal failed: prefix left unrouted after add recovered") + } + if ref.Out == "" { + t.Fatalf("expected an active peer after self-heal, got empty") + } +} + +// TestAllowedIPs_SelfHealAfterRemoveError ensures a failed remove during a swap is retried instead +// of leaving e.active stuck on a peer that no longer holds references. +func TestAllowedIPs_SelfHealAfterRemoveError(t *testing.T) { + f := newFakeWG() + c := f.counter() + p := mustPrefix(t, "10.44.8.0/24") + + mustIncrement(t, c, p, "peerA") + mustIncrement(t, c, p, "peerB") + + // Releasing active peerA must detach it (remove) then add peerB; fail the remove once. + f.failRemove = true + if _, err := c.Decrement(p, "peerA"); err == nil { + t.Fatalf("expected error from failed remove") + } + + // Next Decrement of the non-active survivor retries: removes stale peerA, installs peerB. + mustDecrement(t, c, p, "peerB") + // peerB had only one ref, so after retry the prefix is fully released. + if _, ok := f.installed[p]; ok { + t.Fatalf("expected prefix released after self-heal, still on %q", f.installed[p]) + } +} diff --git a/client/internal/routemanager/refcounter/refcounter.go b/client/internal/routemanager/refcounter/refcounter.go index 27a724f50..917120275 100644 --- a/client/internal/routemanager/refcounter/refcounter.go +++ b/client/internal/routemanager/refcounter/refcounter.go @@ -94,6 +94,26 @@ func (rm *Counter[Key, I, O]) Get(key Key) (Ref[O], bool) { return ref, ok } +// ReapplyMatching calls apply for every key whose stored Out satisfies pred, holding the +// counter lock for the whole pass. Running apply under the lock keeps it atomic with respect +// to Increment/Decrement: a prefix dropped to zero is removed from the map (and had its +// RemoveFunc called) before this pass observes it, so a stale key can never be re-applied. +// pred and apply are invoked under the lock, so they must not call back into the counter. +func (rm *Counter[Key, I, O]) ReapplyMatching(pred func(out O) bool, apply func(key Key) error) error { + rm.mu.Lock() + defer rm.mu.Unlock() + + var merr *multierror.Error + for key, ref := range rm.refCountMap { + if pred(ref.Out) { + if err := apply(key); err != nil { + merr = multierror.Append(merr, err) + } + } + } + return nberrors.FormatErrorOrNil(merr) +} + // Increment increments the reference count for the given key. // If this is the first reference to the key, the AddFunc is called. func (rm *Counter[Key, I, O]) Increment(key Key, in I) (Ref[O], error) { diff --git a/client/internal/routemanager/refcounter/refcounter_test.go b/client/internal/routemanager/refcounter/refcounter_test.go new file mode 100644 index 000000000..79a99c388 --- /dev/null +++ b/client/internal/routemanager/refcounter/refcounter_test.go @@ -0,0 +1,47 @@ +package refcounter + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestReapplyMatching verifies ReapplyMatching invokes apply for exactly the keys whose stored +// Out satisfies the predicate (no duplicates for multiply-referenced keys) — the primitive +// ReconcilePeerAllowedIPs relies on to re-apply a single peer's routed prefixes. +func TestReapplyMatching(t *testing.T) { + rc := New[netip.Prefix, string, string]( + func(_ netip.Prefix, peerKey string) (string, error) { return peerKey, nil }, + func(netip.Prefix, string) error { return nil }, + ) + + peerA1 := netip.MustParsePrefix("10.0.0.0/24") + peerA2 := netip.MustParsePrefix("10.1.0.0/24") + peerB1 := netip.MustParsePrefix("10.2.0.0/24") + + for prefix, peer := range map[netip.Prefix]string{peerA1: "peerA", peerA2: "peerA", peerB1: "peerB"} { + _, err := rc.Increment(prefix, peer) + require.NoError(t, err) + } + // a second reference must not make the key applied twice + _, err := rc.Increment(peerA1, "peerA") + require.NoError(t, err) + + var applied []netip.Prefix + err = rc.ReapplyMatching( + func(out string) bool { return out == "peerA" }, + func(key netip.Prefix) error { applied = append(applied, key); return nil }, + ) + require.NoError(t, err) + assert.ElementsMatch(t, []netip.Prefix{peerA1, peerA2}, applied) + + var none []netip.Prefix + err = rc.ReapplyMatching( + func(out string) bool { return out == "missing" }, + func(key netip.Prefix) error { none = append(none, key); return nil }, + ) + require.NoError(t, err) + assert.Empty(t, none) +} diff --git a/client/internal/routemanager/refcounter/types.go b/client/internal/routemanager/refcounter/types.go index aadac3e25..7da0e17e3 100644 --- a/client/internal/routemanager/refcounter/types.go +++ b/client/internal/routemanager/refcounter/types.go @@ -5,5 +5,7 @@ import "net/netip" // RouteRefCounter is a Counter for Route, it doesn't take any input on Increment and doesn't use any output on Decrement type RouteRefCounter = Counter[netip.Prefix, struct{}, struct{}] -// AllowedIPsRefCounter is a Counter for AllowedIPs, it takes a peer key on Increment and passes it back to Decrement -type AllowedIPsRefCounter = Counter[netip.Prefix, string, string] +// AllowedIPsRefCounter tracks WireGuard AllowedIPs per prefix. Unlike the generic Counter it is peer-aware: +// a prefix can be claimed by several peers at once and WireGuard allows a given prefix on exactly one peer, +// so the counter records the per-peer reference count and swaps the installed peer when the active one is released. +// See allowedips.go. diff --git a/client/internal/routemanager/selection.go b/client/internal/routemanager/selection.go new file mode 100644 index 000000000..6d5feec79 --- /dev/null +++ b/client/internal/routemanager/selection.go @@ -0,0 +1,138 @@ +package routemanager + +import ( + "fmt" + "slices" + + "github.com/hashicorp/go-multierror" + log "github.com/sirupsen/logrus" + "golang.org/x/exp/maps" + + nberrors "github.com/netbirdio/netbird/client/errors" + "github.com/netbirdio/netbird/route" +) + +// SelectRoutes selects the routes with the given network IDs and applies the +// new selection. V4/v6 exit-node pairs are expanded automatically. Exit nodes +// are mutually exclusive: if the selection activates an exit node, every other +// available exit node is deselected so two can't be active at once. With +// appendRoute=false the previous selection is replaced instead of extended. +func (m *DefaultManager) SelectRoutes(ids []route.NetID, appendRoute bool) error { + if err := m.selectRoutes(ids, appendRoute); err != nil { + return err + } + m.TriggerSelection(m.GetClientRoutes()) + return nil +} + +// DeselectRoutes removes the routes with the given network IDs from the +// selection and applies the change. V4/v6 exit-node pairs are expanded +// automatically. +func (m *DefaultManager) DeselectRoutes(ids []route.NetID) error { + if err := m.deselectRoutes(ids); err != nil { + return err + } + m.TriggerSelection(m.GetClientRoutes()) + return nil +} + +func (m *DefaultManager) deselectRoutes(ids []route.NetID) error { + routesMap := m.GetClientRoutesWithNetID() + routes := route.ExpandV6ExitPairs(slices.Clone(ids), routesMap) + + log.Debugf("deselecting routes with ids: %v", routes) + + if err := m.routeSelector.DeselectRoutes(routes, maps.Keys(routesMap)); err != nil { + return fmt.Errorf("deselect routes: %w", err) + } + + return nil +} + +// SelectAllRoutes selects every available route and applies the selection. +// Exit nodes stay mutually exclusive: at most one remains active. +func (m *DefaultManager) SelectAllRoutes() { + m.selectAllRoutes() + m.TriggerSelection(m.GetClientRoutes()) +} + +func (m *DefaultManager) selectAllRoutes() { + m.routeSelector.SelectAllRoutes() + + // Select-all wipes every explicit selection, so exit nodes fall back to + // management's auto-apply flags — which may mark several at once. + // Reconcile immediately so at most one exit node stays active instead of + // waiting for the next network map to enforce it. + m.mux.Lock() + defer m.mux.Unlock() + m.updateRouteSelectorFromManagement(m.clientRoutes) +} + +// DeselectAllRoutes deselects every route and applies the change. +func (m *DefaultManager) DeselectAllRoutes() { + m.routeSelector.DeselectAllRoutes() + m.TriggerSelection(m.GetClientRoutes()) +} + +func (m *DefaultManager) selectRoutes(ids []route.NetID, appendRoute bool) error { + routesMap := m.GetClientRoutesWithNetID() + routes := route.ExpandV6ExitPairs(slices.Clone(ids), routesMap) + allIDs := maps.Keys(routesMap) + + log.Debugf("selecting routes with ids: %v", routes) + + // A partial failure (e.g. an unknown ID in the request) still selects the + // valid routes, so exclusivity below must run regardless of the error. + var merr *multierror.Error + if err := m.routeSelector.SelectRoutes(routes, appendRoute, allIDs); err != nil { + merr = multierror.Append(merr, fmt.Errorf("select routes: %w", err)) + } + + // Exit nodes are mutually exclusive: if this selection activates an + // exit node, deselect every other available exit node so two can't be + // selected at once. Non-exit route selections are left untouched. + if requestActivatesExitNode(routes, routesMap) { + if others := otherExitNodeIDs(routesMap, routes); len(others) > 0 { + if err := m.routeSelector.DeselectRoutes(others, allIDs); err != nil { + merr = multierror.Append(merr, fmt.Errorf("deselect sibling exit nodes: %w", err)) + } + } + } + + return nberrors.FormatErrorOrNil(merr) +} + +func isExitNodeRoutes(routes []*route.Route) bool { + return len(routes) > 0 && (route.IsV4DefaultRoute(routes[0].Network) || route.IsV6DefaultRoute(routes[0].Network)) +} + +// requestActivatesExitNode reports whether any requested NetID maps to an exit +// node (default route) in the current route table. +func requestActivatesExitNode(requested []route.NetID, routesMap map[route.NetID][]*route.Route) bool { + for _, id := range requested { + if isExitNodeRoutes(routesMap[id]) { + return true + } + } + return false +} + +// otherExitNodeIDs returns every available exit-node NetID that is not in the +// requested set — the siblings to deselect so a single exit node stays active. +func otherExitNodeIDs(routesMap map[route.NetID][]*route.Route, requested []route.NetID) []route.NetID { + keep := make(map[route.NetID]struct{}, len(requested)) + for _, id := range requested { + keep[id] = struct{}{} + } + var others []route.NetID + for id, routes := range routesMap { + if !isExitNodeRoutes(routes) { + continue + } + if _, ok := keep[id]; ok { + continue + } + others = append(others, id) + } + return others +} diff --git a/client/internal/routemanager/selection_test.go b/client/internal/routemanager/selection_test.go new file mode 100644 index 000000000..6066b5661 --- /dev/null +++ b/client/internal/routemanager/selection_test.go @@ -0,0 +1,129 @@ +package routemanager + +import ( + "net/netip" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/internal/routeselector" + "github.com/netbirdio/netbird/route" +) + +func v6ExitRoute(netID, peer string) *route.Route { + return &route.Route{ + NetID: route.NetID(netID), + Network: netip.MustParsePrefix("::/0"), + Peer: peer, + } +} + +func newSelectionTestManager() *DefaultManager { + return &DefaultManager{ + routeSelector: routeselector.NewRouteSelector(), + clientRoutes: route.HAMap{ + "exitA|0.0.0.0/0": {exitRoute("exitA", "p1", true)}, + "exitA-v6|::/0": {v6ExitRoute("exitA-v6", "p1")}, + "exitB|0.0.0.0/0": {exitRoute("exitB", "p2", true)}, + "lan|192.168.1.0/24": {{NetID: "lan", Network: netip.MustParsePrefix("192.168.1.0/24"), Peer: "p3"}}, + }, + } +} + +func TestSelectRoutes_ExitNodeExclusivity(t *testing.T) { + m := newSelectionTestManager() + + // Selecting an exit node selects its v6 pair and deselects the sibling. + require.NoError(t, m.selectRoutes([]route.NetID{"exitA"}, true)) + assert.True(t, m.routeSelector.IsSelected("exitA"), "exitA should be selected") + assert.True(t, m.routeSelector.IsSelected("exitA-v6"), "the v6 pair follows its v4 base") + assert.False(t, m.routeSelector.IsSelected("exitB"), "the sibling exit node must be deselected") + + // Switching to the sibling deselects the previous exit node and its v6 pair. + require.NoError(t, m.selectRoutes([]route.NetID{"exitB"}, true)) + assert.True(t, m.routeSelector.IsSelected("exitB"), "exitB should now be selected") + assert.False(t, m.routeSelector.IsSelected("exitA"), "the previous exit node must be deselected") + assert.False(t, m.routeSelector.IsSelected("exitA-v6"), "the previous exit node's v6 pair must be deselected") + assert.True(t, m.routeSelector.IsSelected("lan"), "non-exit route selection is untouched") + + // Selecting a non-exit route leaves the active exit node alone. + require.NoError(t, m.selectRoutes([]route.NetID{"lan"}, true)) + assert.True(t, m.routeSelector.IsSelected("exitB"), "selecting a non-exit route keeps the exit node") + + // Deselecting the active exit node turns every exit node off. + require.NoError(t, m.deselectRoutes([]route.NetID{"exitB"})) + assert.False(t, m.routeSelector.IsSelected("exitB"), "exitB should be deselected") + assert.False(t, m.routeSelector.IsSelected("exitA"), "exitA stays deselected") + assert.True(t, m.routeSelector.IsSelected("lan"), "non-exit route selection is untouched") +} + +func TestSelectRoutes_PartialErrorStillEnforcesExclusivity(t *testing.T) { + // The unknown ID must be reported, but the valid exit node in the same + // request is still selected — so its sibling must still be deselected. + // Both orderings are covered: processing must continue past the invalid + // ID wherever it sits in the request. + requests := map[string][]route.NetID{ + "invalid id first": {"missing", "exitB"}, + "invalid id last": {"exitB", "missing"}, + } + + for name, ids := range requests { + t.Run(name, func(t *testing.T) { + m := newSelectionTestManager() + + require.NoError(t, m.selectRoutes([]route.NetID{"exitA"}, true)) + + err := m.selectRoutes(ids, true) + assert.Error(t, err, "unknown id must be reported") + assert.True(t, m.routeSelector.IsSelected("exitB"), "valid exit node from the request is selected") + assert.False(t, m.routeSelector.IsSelected("exitA"), "sibling exit node must be deselected despite the error") + assert.False(t, m.routeSelector.IsSelected("exitA-v6"), "sibling's v6 pair must be deselected too") + }) + } +} + +func TestSelectAllRoutes_KeepsSingleExitNode(t *testing.T) { + // Both exit nodes are marked for auto-apply by management + // (SkipAutoApply=false), the state where select-all could turn on two at + // once without the immediate reconciliation. + m := &DefaultManager{ + routeSelector: routeselector.NewRouteSelector(), + clientRoutes: route.HAMap{ + "exitA|0.0.0.0/0": {exitRoute("exitA", "p1", false)}, + "exitB|0.0.0.0/0": {exitRoute("exitB", "p2", false)}, + "lan|192.168.1.0/24": {{NetID: "lan", Network: netip.MustParsePrefix("192.168.1.0/24"), Peer: "p3"}}, + }, + } + + require.NoError(t, m.selectRoutes([]route.NetID{"exitB"}, true)) + + m.selectAllRoutes() + + assert.True(t, m.routeSelector.IsSelected("lan"), "non-exit routes are all selected") + assert.True(t, m.routeSelector.IsSelected("exitA"), "the deterministic management pick stays active") + assert.False(t, m.routeSelector.IsSelected("exitB"), "select-all must not leave a second exit node active") +} + +func TestSelectRoutes_UnknownRoute(t *testing.T) { + m := newSelectionTestManager() + + assert.Error(t, m.selectRoutes([]route.NetID{"missing"}, true), "selecting an unavailable route must fail") + assert.Error(t, m.deselectRoutes([]route.NetID{"missing"}), "deselecting an unavailable route must fail") +} + +func TestExitNodeSelectionHelpers(t *testing.T) { + routesMap := map[route.NetID][]*route.Route{ + "exitA": {{Network: netip.MustParsePrefix("0.0.0.0/0")}}, + "exitB": {{Network: netip.MustParsePrefix("::/0")}}, + "lan": {{Network: netip.MustParsePrefix("192.168.0.0/16")}}, + } + + assert.True(t, requestActivatesExitNode([]route.NetID{"exitA"}, routesMap), "v4 default route is an exit node") + assert.True(t, requestActivatesExitNode([]route.NetID{"exitB"}, routesMap), "v6 default route is an exit node") + assert.False(t, requestActivatesExitNode([]route.NetID{"lan"}, routesMap), "lan route is not an exit node") + assert.False(t, requestActivatesExitNode([]route.NetID{"missing"}, routesMap), "unknown id is not an exit node") + + others := otherExitNodeIDs(routesMap, []route.NetID{"exitB"}) + assert.ElementsMatch(t, []route.NetID{"exitA"}, others, "only the other exit node is a sibling; the lan route is ignored") +} diff --git a/client/internal/routemanager/static/route.go b/client/internal/routemanager/static/route.go index d480fdf00..8ba03d090 100644 --- a/client/internal/routemanager/static/route.go +++ b/client/internal/routemanager/static/route.go @@ -15,6 +15,11 @@ type Route struct { route *route.Route routeRefCounter *refcounter.RouteRefCounter allowedIPsRefcounter *refcounter.AllowedIPsRefCounter + // currentPeerKey is the routing peer this watcher currently has the prefix installed on + // (the HA winner elected by the watcher). It can differ from route.Peer and change on + // failover, so it is recorded on AddAllowedIPs and used on RemoveAllowedIPs to decrement + // the exact peer that was incremented. + currentPeerKey string } func NewRoute(params common.HandlerParams) *Route { @@ -52,12 +57,15 @@ func (r *Route) AddAllowedIPs(peerKey string) error { ref.Out, ) } + r.currentPeerKey = peerKey return nil } func (r *Route) RemoveAllowedIPs() error { - if _, err := r.allowedIPsRefcounter.Decrement(r.route.Network); err != nil { - return err + var err error + if _, decErr := r.allowedIPsRefcounter.Decrement(r.route.Network, r.currentPeerKey); decErr != nil { + err = fmt.Errorf("remove allowed IP %s: %w", r.route.Network, decErr) } - return nil + r.currentPeerKey = "" + return err } diff --git a/client/internal/routemanager/sysctl/sysctl_linux.go b/client/internal/routemanager/sysctl/sysctl_linux.go index f96a57f37..46b7c9fb7 100644 --- a/client/internal/routemanager/sysctl/sysctl_linux.go +++ b/client/internal/routemanager/sysctl/sysctl_linux.go @@ -20,6 +20,8 @@ const ( rpFilterPath = "net.ipv4.conf.all.rp_filter" rpFilterInterfacePath = "net.ipv4.conf.%s.rp_filter" srcValidMarkPath = "net.ipv4.conf.all.src_valid_mark" + percentEscape = "%25" + dotEscape = "%2E" ) type iface interface { @@ -56,7 +58,11 @@ func Setup(wgIface iface) (map[string]int, error) { continue } - i := fmt.Sprintf(rpFilterInterfacePath, intf.Name) + // Escape '%' and '.' so they survive the dot-to-slash conversion in Set() + safeName := strings.ReplaceAll(intf.Name, "%", percentEscape) + safeName = strings.ReplaceAll(safeName, ".", dotEscape) + + i := fmt.Sprintf(rpFilterInterfacePath, safeName) oldVal, err := Set(i, 2, true) if err != nil { result = multierror.Append(result, err) @@ -70,7 +76,11 @@ func Setup(wgIface iface) (map[string]int, error) { // Set sets a sysctl configuration, if onlyIfOne is true it will only set the new value if it's set to 1 func Set(key string, desiredValue int, onlyIfOne bool) (int, error) { - path := fmt.Sprintf("/proc/sys/%s", strings.ReplaceAll(key, ".", "/")) + path := strings.ReplaceAll(key, ".", "/") + // Unescape interface dots and percent signs + path = strings.ReplaceAll(path, dotEscape, ".") + path = strings.ReplaceAll(path, percentEscape, "%") + path = fmt.Sprintf("/proc/sys/%s", path) currentValue, err := os.ReadFile(path) if err != nil { return -1, fmt.Errorf("read sysctl %s: %w", key, err) diff --git a/client/internal/tunnelnotifier/notifier.go b/client/internal/tunnelnotifier/notifier.go new file mode 100644 index 000000000..b62923a6e --- /dev/null +++ b/client/internal/tunnelnotifier/notifier.go @@ -0,0 +1,124 @@ +package tunnelnotifier + +import ( + "container/list" + "sync" + + "github.com/netbirdio/netbird/client/internal/dns" + "github.com/netbirdio/netbird/client/internal/listener" +) + +type eventKind int + +const ( + eventRoutes eventKind = iota + eventIfaceIP + eventIfaceIPv6 + eventDNS +) + +var ( + _ listener.NetworkChangeListener = (*Notifier)(nil) + _ dns.IosDnsManager = (*Notifier)(nil) +) + +type event struct { + kind eventKind + payload string +} + +type Notifier struct { + mu sync.Mutex + cond *sync.Cond + queue *list.List + closed bool + done chan struct{} + + listener listener.NetworkChangeListener + dnsManager dns.IosDnsManager +} + +func New(l listener.NetworkChangeListener, dm dns.IosDnsManager) *Notifier { + n := &Notifier{ + queue: list.New(), + done: make(chan struct{}), + listener: l, + dnsManager: dm, + } + n.cond = sync.NewCond(&n.mu) + go n.deliverLoop() + return n +} + +func (n *Notifier) OnNetworkChanged(routes string) { + n.enqueue(event{kind: eventRoutes, payload: routes}) +} + +func (n *Notifier) SetInterfaceIP(ip string) { + n.enqueue(event{kind: eventIfaceIP, payload: ip}) +} + +func (n *Notifier) SetInterfaceIPv6(ip string) { + n.enqueue(event{kind: eventIfaceIPv6, payload: ip}) +} + +func (n *Notifier) ApplyDns(config string) { + n.enqueue(event{kind: eventDNS, payload: config}) +} + +// Close stops accepting new events and blocks until the delivery loop has +// drained all queued events and exited. +func (n *Notifier) Close() { + n.mu.Lock() + n.closed = true + n.cond.Signal() + n.mu.Unlock() + <-n.done +} + +func (n *Notifier) enqueue(ev event) { + n.mu.Lock() + defer n.mu.Unlock() + if n.closed { + return + } + n.queue.PushBack(ev) + n.cond.Signal() +} + +func (n *Notifier) deliverLoop() { + defer close(n.done) + for { + n.mu.Lock() + for n.queue.Len() == 0 && !n.closed { + n.cond.Wait() + } + if n.closed && n.queue.Len() == 0 { + n.mu.Unlock() + return + } + ev := n.queue.Remove(n.queue.Front()).(event) + l := n.listener + dm := n.dnsManager + n.mu.Unlock() + + switch ev.kind { + case eventRoutes: + if l != nil { + l.OnNetworkChanged(ev.payload) + } + case eventIfaceIP: + if l != nil { + l.SetInterfaceIP(ev.payload) + } + case eventIfaceIPv6: + if l != nil { + l.SetInterfaceIPv6(ev.payload) + } + case eventDNS: + if dm != nil { + dm.ApplyDns(ev.payload) + } + } + } +} diff --git a/client/internal/tunnelnotifier/notifier_test.go b/client/internal/tunnelnotifier/notifier_test.go new file mode 100644 index 000000000..ffbcdc15c --- /dev/null +++ b/client/internal/tunnelnotifier/notifier_test.go @@ -0,0 +1,192 @@ +package tunnelnotifier + +import ( + "fmt" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type call struct { + kind string + payload string +} + +type recorder struct { + mu sync.Mutex + calls []call + inFlight atomic.Int32 + overlap atomic.Bool + delay time.Duration +} + +func (r *recorder) record(kind, payload string) { + if r.inFlight.Add(1) != 1 { + r.overlap.Store(true) + } + if r.delay > 0 { + time.Sleep(r.delay) + } + r.mu.Lock() + r.calls = append(r.calls, call{kind: kind, payload: payload}) + r.mu.Unlock() + r.inFlight.Add(-1) +} + +func (r *recorder) count() int { + r.mu.Lock() + defer r.mu.Unlock() + return len(r.calls) +} + +func (r *recorder) snapshot() []call { + r.mu.Lock() + defer r.mu.Unlock() + out := make([]call, len(r.calls)) + copy(out, r.calls) + return out +} + +type fakeListener struct { + rec *recorder +} + +func (f *fakeListener) OnNetworkChanged(routes string) { + f.rec.record("routes", routes) +} + +func (f *fakeListener) SetInterfaceIP(ip string) { + f.rec.record("ip", ip) +} + +func (f *fakeListener) SetInterfaceIPv6(ip string) { + f.rec.record("ipv6", ip) +} + +type fakeDNSManager struct { + rec *recorder +} + +func (f *fakeDNSManager) ApplyDns(config string) { + f.rec.record("dns", config) +} + +func TestFIFOOrder(t *testing.T) { + rec := &recorder{} + n := New(&fakeListener{rec: rec}, &fakeDNSManager{rec: rec}) + defer n.Close() + + n.SetInterfaceIP("10.0.0.1") + n.SetInterfaceIPv6("fd00::1") + n.ApplyDns(`{"domains":[]}`) + n.OnNetworkChanged("10.0.0.0/8,192.168.0.0/16") + n.ApplyDns(`{"domains":["example.com"]}`) + + require.Eventually(t, func() bool { return rec.count() == 5 }, time.Second, time.Millisecond) + + expected := []call{ + {kind: "ip", payload: "10.0.0.1"}, + {kind: "ipv6", payload: "fd00::1"}, + {kind: "dns", payload: `{"domains":[]}`}, + {kind: "routes", payload: "10.0.0.0/8,192.168.0.0/16"}, + {kind: "dns", payload: `{"domains":["example.com"]}`}, + } + assert.Equal(t, expected, rec.snapshot()) +} + +func TestNoOverlappingCalls(t *testing.T) { + rec := &recorder{delay: 100 * time.Microsecond} + n := New(&fakeListener{rec: rec}, &fakeDNSManager{rec: rec}) + defer n.Close() + + const producers = 8 + const perProducer = 25 + + var wg sync.WaitGroup + for i := 0; i < producers; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + for j := 0; j < perProducer; j++ { + payload := fmt.Sprintf("%d-%d", id, j) + switch j % 4 { + case 0: + n.OnNetworkChanged(payload) + case 1: + n.SetInterfaceIP(payload) + case 2: + n.SetInterfaceIPv6(payload) + case 3: + n.ApplyDns(payload) + } + } + }(i) + } + wg.Wait() + + require.Eventually(t, func() bool { return rec.count() == producers*perProducer }, 5*time.Second, time.Millisecond) + assert.False(t, rec.overlap.Load()) +} + +func TestDNSAndRoutesInterleaved(t *testing.T) { + rec := &recorder{delay: 100 * time.Microsecond} + n := New(&fakeListener{rec: rec}, &fakeDNSManager{rec: rec}) + defer n.Close() + + const events = 50 + + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + for i := 0; i < events; i++ { + n.ApplyDns(fmt.Sprintf("dns-%d", i)) + } + }() + go func() { + defer wg.Done() + for i := 0; i < events; i++ { + n.OnNetworkChanged(fmt.Sprintf("routes-%d", i)) + } + }() + wg.Wait() + + require.Eventually(t, func() bool { return rec.count() == 2*events }, 5*time.Second, time.Millisecond) + assert.False(t, rec.overlap.Load()) + + var dnsSeen, routesSeen int + for _, c := range rec.snapshot() { + switch c.kind { + case "dns": + assert.Equal(t, fmt.Sprintf("dns-%d", dnsSeen), c.payload) + dnsSeen++ + case "routes": + assert.Equal(t, fmt.Sprintf("routes-%d", routesSeen), c.payload) + routesSeen++ + } + } + assert.Equal(t, events, dnsSeen) + assert.Equal(t, events, routesSeen) +} + +func TestCloseDrainsQueue(t *testing.T) { + rec := &recorder{delay: time.Millisecond} + n := New(&fakeListener{rec: rec}, &fakeDNSManager{rec: rec}) + + const events = 20 + for i := 0; i < events; i++ { + n.OnNetworkChanged(fmt.Sprintf("routes-%d", i)) + } + n.Close() + + require.Equal(t, events, rec.count(), "Close must not return before all queued events are delivered") + + n.OnNetworkChanged("after-close") + n.ApplyDns("after-close") + time.Sleep(50 * time.Millisecond) + assert.Equal(t, events, rec.count()) +} diff --git a/client/ios/NetBirdSDK/client.go b/client/ios/NetBirdSDK/client.go index 359a83556..9289a3910 100644 --- a/client/ios/NetBirdSDK/client.go +++ b/client/ios/NetBirdSDK/client.go @@ -13,7 +13,6 @@ import ( "time" log "github.com/sirupsen/logrus" - "golang.org/x/exp/maps" "github.com/netbirdio/netbird/client/internal" "github.com/netbirdio/netbird/client/internal/auth" @@ -233,6 +232,9 @@ func (c *Client) DebugBundle(anonymize bool) (string, error) { deps.SyncResponse = resp if e := cc.Engine(); e != nil { + deps.RefreshStatus = func() { + e.RunHealthProbes(context.Background(), true) + } if cm := e.GetClientMetrics(); cm != nil { deps.ClientMetrics = cm } @@ -634,23 +636,18 @@ func (c *Client) SelectRoute(id string) error { } routeManager := engine.GetRouteManager() - routeSelector := routeManager.GetRouteSelector() if id == "All" { log.Debugf("select all routes") - routeSelector.SelectAllRoutes() - } else { - log.Debugf("select route with id: %s", id) - routes := toNetIDs([]string{id}) - routesMap := routeManager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - if err := routeSelector.SelectRoutes(routes, true, maps.Keys(routesMap)); err != nil { - log.Debugf("error when selecting routes: %s", err) - return fmt.Errorf("select routes: %w", err) - } + routeManager.SelectAllRoutes() + return nil } - routeManager.TriggerSelection(routeManager.GetClientRoutes()) - return nil + log.Debugf("select route with id: %s", id) + if err := routeManager.SelectRoutes(toNetIDs([]string{id}), true); err != nil { + log.Debugf("error when selecting routes: %s", err) + return err + } + return nil } func (c *Client) DeselectRoute(id string) error { @@ -664,21 +661,17 @@ func (c *Client) DeselectRoute(id string) error { } routeManager := engine.GetRouteManager() - routeSelector := routeManager.GetRouteSelector() if id == "All" { log.Debugf("deselect all routes") - routeSelector.DeselectAllRoutes() - } else { - log.Debugf("deselect route with id: %s", id) - routes := toNetIDs([]string{id}) - routesMap := routeManager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - if err := routeSelector.DeselectRoutes(routes, maps.Keys(routesMap)); err != nil { - log.Debugf("error when deselecting routes: %s", err) - return fmt.Errorf("deselect routes: %w", err) - } + routeManager.DeselectAllRoutes() + return nil + } + + log.Debugf("deselect route with id: %s", id) + if err := routeManager.DeselectRoutes(toNetIDs([]string{id})); err != nil { + log.Debugf("error when deselecting routes: %s", err) + return err } - routeManager.TriggerSelection(routeManager.GetClientRoutes()) return nil } diff --git a/client/ios/NetBirdSDK/version.go b/client/ios/NetBirdSDK/version.go new file mode 100644 index 000000000..606ad18e2 --- /dev/null +++ b/client/ios/NetBirdSDK/version.go @@ -0,0 +1,12 @@ +//go:build ios + +package NetBirdSDK + +import "github.com/netbirdio/netbird/version" + +// GoClientVersion returns the NetBird Go client version that was baked into +// the framework at compile time via +// -ldflags "-X github.com/netbirdio/netbird/version.version=". +func GoClientVersion() string { + return version.NetbirdVersion() +} diff --git a/client/server/network.go b/client/server/network.go index c38715256..c390b8180 100644 --- a/client/server/network.go +++ b/client/server/network.go @@ -8,7 +8,6 @@ import ( "sort" "strings" - "golang.org/x/exp/maps" "google.golang.org/grpc/codes" gstatus "google.golang.org/grpc/status" @@ -161,30 +160,11 @@ func (s *Server) SelectNetworks(_ context.Context, req *proto.SelectNetworksRequ return nil, fmt.Errorf("no route manager") } - routeSelector := routeManager.GetRouteSelector() if req.GetAll() { - routeSelector.SelectAllRoutes() - } else { - routes := toNetIDs(req.GetNetworkIDs()) - routesMap := routeManager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - netIdRoutes := maps.Keys(routesMap) - if err := routeSelector.SelectRoutes(routes, req.GetAppend(), netIdRoutes); err != nil { - return nil, fmt.Errorf("select routes: %w", err) - } - - // Exit nodes are mutually exclusive: if this selection activates an - // exit node, deselect every other available exit node so two can't be - // selected at once. Non-exit route selections are left untouched. - if requestActivatesExitNode(routes, routesMap) { - if others := otherExitNodeIDs(routesMap, routes); len(others) > 0 { - if err := routeSelector.DeselectRoutes(others, netIdRoutes); err != nil { - return nil, fmt.Errorf("deselect sibling exit nodes: %w", err) - } - } - } + routeManager.SelectAllRoutes() + } else if err := routeManager.SelectRoutes(toNetIDs(req.GetNetworkIDs()), req.GetAppend()); err != nil { + return nil, err } - routeManager.TriggerSelection(routeManager.GetClientRoutes()) s.statusRecorder.PublishEvent( proto.SystemEvent_INFO, @@ -224,19 +204,11 @@ func (s *Server) DeselectNetworks(_ context.Context, req *proto.SelectNetworksRe return nil, fmt.Errorf("no route manager") } - routeSelector := routeManager.GetRouteSelector() if req.GetAll() { - routeSelector.DeselectAllRoutes() - } else { - routes := toNetIDs(req.GetNetworkIDs()) - routesMap := routeManager.GetClientRoutesWithNetID() - routes = route.ExpandV6ExitPairs(routes, routesMap) - netIdRoutes := maps.Keys(routesMap) - if err := routeSelector.DeselectRoutes(routes, netIdRoutes); err != nil { - return nil, fmt.Errorf("deselect routes: %w", err) - } + routeManager.DeselectAllRoutes() + } else if err := routeManager.DeselectRoutes(toNetIDs(req.GetNetworkIDs())); err != nil { + return nil, err } - routeManager.TriggerSelection(routeManager.GetClientRoutes()) s.statusRecorder.PublishEvent( proto.SystemEvent_INFO, @@ -261,37 +233,3 @@ func toNetIDs(routes []string) []route.NetID { return netIDs } -func isExitNodeRoutes(routes []*route.Route) bool { - return len(routes) > 0 && (route.IsV4DefaultRoute(routes[0].Network) || route.IsV6DefaultRoute(routes[0].Network)) -} - -// requestActivatesExitNode reports whether any requested NetID maps to an exit -// node (default route) in the current route table. -func requestActivatesExitNode(requested []route.NetID, routesMap map[route.NetID][]*route.Route) bool { - for _, id := range requested { - if isExitNodeRoutes(routesMap[id]) { - return true - } - } - return false -} - -// otherExitNodeIDs returns every available exit-node NetID that is not in the -// requested set — the siblings to deselect so a single exit node stays active. -func otherExitNodeIDs(routesMap map[route.NetID][]*route.Route, requested []route.NetID) []route.NetID { - keep := make(map[route.NetID]struct{}, len(requested)) - for _, id := range requested { - keep[id] = struct{}{} - } - var others []route.NetID - for id, routes := range routesMap { - if !isExitNodeRoutes(routes) { - continue - } - if _, ok := keep[id]; ok { - continue - } - others = append(others, id) - } - return others -} diff --git a/client/server/network_exitnode_test.go b/client/server/network_exitnode_test.go deleted file mode 100644 index 1c0ba0ecb..000000000 --- a/client/server/network_exitnode_test.go +++ /dev/null @@ -1,26 +0,0 @@ -package server - -import ( - "net/netip" - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/netbirdio/netbird/route" -) - -func TestExitNodeSelectionHelpers(t *testing.T) { - routesMap := map[route.NetID][]*route.Route{ - "exitA": {{Network: netip.MustParsePrefix("0.0.0.0/0")}}, - "exitB": {{Network: netip.MustParsePrefix("::/0")}}, - "lan": {{Network: netip.MustParsePrefix("192.168.0.0/16")}}, - } - - assert.True(t, requestActivatesExitNode([]route.NetID{"exitA"}, routesMap), "v4 default route is an exit node") - assert.True(t, requestActivatesExitNode([]route.NetID{"exitB"}, routesMap), "v6 default route is an exit node") - assert.False(t, requestActivatesExitNode([]route.NetID{"lan"}, routesMap), "lan route is not an exit node") - assert.False(t, requestActivatesExitNode([]route.NetID{"missing"}, routesMap), "unknown id is not an exit node") - - others := otherExitNodeIDs(routesMap, []route.NetID{"exitB"}) - assert.ElementsMatch(t, []route.NetID{"exitA"}, others, "only the other exit node is a sibling; the lan route is ignored") -} diff --git a/client/server/server.go b/client/server/server.go index 2b919d58d..8047006fe 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -1081,7 +1081,10 @@ func (s *Server) Down(ctx context.Context, _ *proto.DownRequest) (*proto.DownRes if err := s.cleanupConnection(); err != nil { s.mutex.Unlock() - // todo review to update the status in case any type of error + if errors.Is(err, ErrServiceNotUp) { + log.Debugf("Down called while service not up: %v", err) + return nil, err + } log.Errorf("failed to shut down properly: %v", err) return nil, err } @@ -1154,7 +1157,7 @@ func (s *Server) cleanupConnection() error { // making the run loop the sole owner of engine shutdown. if engine != nil { if err := engine.Stop(); err != nil { - return err + log.Errorf("failed to stop engine during cleanup: %v", err) } } diff --git a/client/system/info.go b/client/system/info.go index 1838204b8..daeabca13 100644 --- a/client/system/info.go +++ b/client/system/info.go @@ -79,13 +79,15 @@ type Info struct { EnableSSHLocalPortForwarding bool EnableSSHRemotePortForwarding bool DisableSSHAuth bool + + SyncMessageVersion *int } func (i *Info) SetFlags( rosenpassEnabled, rosenpassPermissive bool, serverSSHAllowed *bool, disableClientRoutes, disableServerRoutes, - disableDNS, disableFirewall, blockLANAccess, blockInbound, disableIPv6 bool, + disableDNS, disableFirewall, blockLANAccess, blockInbound, disableIPv6 bool, syncMessageVersion *int, enableSSHRoot, enableSSHSFTP, enableSSHLocalPortForwarding, enableSSHRemotePortForwarding *bool, disableSSHAuth *bool, ) { @@ -103,6 +105,8 @@ func (i *Info) SetFlags( i.BlockInbound = blockInbound i.DisableIPv6 = disableIPv6 + i.SyncMessageVersion = syncMessageVersion + if enableSSHRoot != nil { i.EnableSSHRoot = *enableSSHRoot } diff --git a/client/ui/autostart_default.go b/client/ui/autostart_default.go index bf1b16a97..162922579 100644 --- a/client/ui/autostart_default.go +++ b/client/ui/autostart_default.go @@ -51,7 +51,7 @@ func autostartDisabledByMDM(policy *mdm.Policy) bool { // netbirdFootprintExists reports whether the machine already carries NetBird // daemon config or state, meaning this is not a genuinely fresh install. It is // the update-safety gate for the autostart default: upgrading users always -// have a footprint, so an update can never trigger a login-item write. +// have a footprint, so an update can never trigger a autostart entry write. func netbirdFootprintExists() bool { candidates := []string{ profilemanager.DefaultConfigPath, @@ -69,9 +69,23 @@ func netbirdFootprintExists() bool { // applyAutostartDefault runs the one-time launch-on-login default for genuinely // fresh installs. The autostartInitialized marker is persisted before any // enable attempt so a crash mid-flow degrades to "never enabled" instead of -// retrying login-item writes on every launch. A user's later disable in +// retrying autostart entry writes on every launch. A user's later disable in // Settings is never overridden: the marker guarantees at-most-once, ever. func applyAutostartDefault(ctx context.Context, autostart *services.Autostart, prefs *preferences.Store, prefsFileExisted bool) { + mdmDisabled := autostartDisabledByMDM(mdm.LoadPolicy()) + + if mdmDisabled { + if enabled, err := autostart.IsEnabled(ctx); err != nil { + log.Warnf("MDM disableAutostart: read autostart state: %v", err) + } else if enabled { + if err := autostart.SetEnabled(ctx, false); err != nil { + log.Warnf("MDM disableAutostart: force off failed: %v", err) + } else { + log.Info("MDM disableAutostart enforced: autostart turned off") + } + } + } + priorFootprint := netbirdFootprintExists() || prefsFileExisted if prefs.Get().AutostartInitialized { @@ -84,7 +98,7 @@ func applyAutostartDefault(ctx context.Context, autostart *services.Autostart, p state := autostartDefaultState{ supported: autostart.Supported(ctx), - mdmDisabled: autostartDisabledByMDM(mdm.LoadPolicy()), + mdmDisabled: mdmDisabled, priorInstall: priorFootprint, } enable, reason := shouldEnableAutostartDefault(state) diff --git a/client/ui/frontend/src/components/empty-state/DaemonOutdatedOverlay.tsx b/client/ui/frontend/src/components/empty-state/DaemonOutdatedOverlay.tsx index 4ee8c2740..e8e7108eb 100644 --- a/client/ui/frontend/src/components/empty-state/DaemonOutdatedOverlay.tsx +++ b/client/ui/frontend/src/components/empty-state/DaemonOutdatedOverlay.tsx @@ -1,10 +1,13 @@ +import { useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import { AlertTriangleIcon, DownloadIcon } from "lucide-react"; import { Browser } from "@wailsio/runtime"; +import { Version } from "@bindings/services"; import { Button } from "@/components/buttons/Button"; import { useStatus } from "@/contexts/StatusContext.tsx"; const RELEASES_URL = "https://github.com/netbirdio/netbird/releases/latest"; +const RC_RELEASES_URL = "https://pkgs.netbird.io/releases/rc"; function openUrl(url: string) { Browser.OpenURL(url).catch(() => globalThis.open(url, "_blank")); @@ -12,7 +15,26 @@ function openUrl(url: string) { export const DaemonOutdatedOverlay = () => { const { t } = useTranslation(); - const { isDaemonOutdated } = useStatus(); + const { status, isDaemonOutdated } = useStatus(); + + const [guiVersion, setGuiVersion] = useState("-"); + const clientVersion = status?.daemonVersion ?? "—"; + + const isRc = /-rc/i.test(guiVersion) || /-rc/i.test(clientVersion); + const downloadUrl = isRc ? RC_RELEASES_URL : RELEASES_URL; + + useEffect(() => { + if (!isDaemonOutdated) return; + let cancelled = false; + Version.GUI() + .then((v) => { + if (!cancelled) setGuiVersion(v); + }) + .catch((err) => console.error("[DaemonOutdatedOverlay] GUI version error", err)); + return () => { + cancelled = true; + }; + }, [isDaemonOutdated]); if (!isDaemonOutdated) return null; @@ -38,10 +60,37 @@ export const DaemonOutdatedOverlay = () => {

{t("daemon.outdated.description")}

+
+

+ {clientVersion === "development" ? ( + + {t("settings.about.clientName")}{" "} + + {t("settings.about.development")} + + + ) : ( + t("settings.about.client", { version: clientVersion }) + )} +

+

+ {guiVersion === "development" ? ( + + {t("settings.about.guiName")}{" "} + + {t("settings.about.development")} + + + ) : ( + t("settings.about.gui", { version: guiVersion }) + )} +

+
+
-
diff --git a/client/ui/frontend/src/contexts/ProfileContext.tsx b/client/ui/frontend/src/contexts/ProfileContext.tsx index 4dd3eaa7a..62377f1bc 100644 --- a/client/ui/frontend/src/contexts/ProfileContext.tsx +++ b/client/ui/frontend/src/contexts/ProfileContext.tsx @@ -28,6 +28,7 @@ type ProfileContextValue = { loaded: boolean; refresh: () => Promise; switchProfile: (id: string) => Promise; + switchProfileNoConnect: (id: string) => Promise; addProfile: (name: string) => Promise; removeProfile: (id: string) => Promise; renameProfile: (id: string, newName: string) => Promise; @@ -112,6 +113,16 @@ export const ProfileProvider = ({ children }: { children: ReactNode }) => { [username, refresh], ); + // Manage-profiles variant: switches without connecting, so the user can + // still adjust the management URL before bringing the connection up. + const switchProfileNoConnect = useCallback( + async (id: string) => { + await ProfileSwitcher.SwitchActiveNoConnect({ profileName: id, username }); + await refresh(); + }, + [username, refresh], + ); + // addProfile creates a profile by display name and returns the // daemon-generated ID, so the caller can immediately address it by ID. const addProfile = useCallback( @@ -158,6 +169,7 @@ export const ProfileProvider = ({ children }: { children: ReactNode }) => { loaded, refresh, switchProfile, + switchProfileNoConnect, addProfile, removeProfile, renameProfile, @@ -171,6 +183,7 @@ export const ProfileProvider = ({ children }: { children: ReactNode }) => { loaded, refresh, switchProfile, + switchProfileNoConnect, addProfile, removeProfile, renameProfile, diff --git a/client/ui/frontend/src/modules/auto-update/UpdateVersionCard.tsx b/client/ui/frontend/src/modules/auto-update/UpdateVersionCard.tsx index 4fb5e2586..4861c492a 100644 --- a/client/ui/frontend/src/modules/auto-update/UpdateVersionCard.tsx +++ b/client/ui/frontend/src/modules/auto-update/UpdateVersionCard.tsx @@ -2,6 +2,7 @@ import { type ReactNode } from "react"; import { useTranslation } from "react-i18next"; import { Browser } from "@wailsio/runtime"; import { DownloadIcon, NotepadText } from "lucide-react"; +import { Update as UpdateSvc } from "@bindings/services"; import { Button } from "@/components/buttons/Button"; import { useClientVersion } from "@/contexts/ClientVersionContext"; import { cn } from "@/lib/cn"; @@ -14,6 +15,12 @@ function openUrl(url: string) { }); } +function openInstallerDownload() { + UpdateSvc.DownloadURL() + .then(openUrl) + .catch(() => openUrl(GITHUB_RELEASES)); +} + export function UpdateVersionCard() { const { t } = useTranslation(); const { updateVersion, enforced, triggerUpdate } = useClientVersion(); @@ -37,11 +44,7 @@ export function UpdateVersionCard() { {t("update.card.installNow")} ) : ( - diff --git a/client/ui/frontend/src/modules/profiles/ProfilesTab.tsx b/client/ui/frontend/src/modules/profiles/ProfilesTab.tsx index c1ce2e449..97261ccc9 100644 --- a/client/ui/frontend/src/modules/profiles/ProfilesTab.tsx +++ b/client/ui/frontend/src/modules/profiles/ProfilesTab.tsx @@ -45,7 +45,7 @@ export function ProfilesTab() { activeProfileId, loaded, username, - switchProfile, + switchProfileNoConnect, addProfile, removeProfile, renameProfile, @@ -100,7 +100,7 @@ export function ProfilesTab() { confirmLabel: t("profile.switch.confirm"), }); if (!ok) return; - await guarded(i18next.t("profile.error.switchTitle"), () => switchProfile(id)); + await guarded(i18next.t("profile.error.switchTitle"), () => switchProfileNoConnect(id)); }; const handleDeregister = async (id: string, name: string) => { @@ -129,14 +129,13 @@ export function ProfilesTab() { await guarded(i18next.t("profile.error.createTitle"), async () => { const id = await addProfile(name); // SetConfig is keyed by the new profile's ID, so it writes the - // not-yet-active profile. Write before switching so any reconnect - // targets the right deployment. + // not-yet-active profile before the switch makes it current. if (!isNetbirdCloud(managementUrl)) { await SettingsSvc.SetConfig( new SetConfigParams({ profileName: id, username, managementUrl }), ); } - await switchProfile(id); + await switchProfileNoConnect(id); }); }; diff --git a/client/ui/frontend/src/modules/session/SessionExpirationDialog.tsx b/client/ui/frontend/src/modules/session/SessionExpirationDialog.tsx index 2ceb958d4..10e71babb 100644 --- a/client/ui/frontend/src/modules/session/SessionExpirationDialog.tsx +++ b/client/ui/frontend/src/modules/session/SessionExpirationDialog.tsx @@ -73,6 +73,13 @@ export default function SessionExpirationDialog() { let offCancel: (() => void) | undefined; + // Return the dialog to its interactive state and dismiss the browser popup + const resetDialog = () => { + offCancel?.(); + WindowManager.CloseBrowserLogin().catch(console.error); + setBusy(false); + }; + try { const start = await Session.RequestExtend({ hint: "" }); const uri = start.verificationUriComplete || start.verificationUri; @@ -105,25 +112,22 @@ export default function SessionExpirationDialog() { if (outcome.kind === "cancel") { waitPromise.cancel?.(); waitPromise.catch(() => {}); + resetDialog(); return; } // Another surface owns this flow; keep the dialog open to retry. if (outcome.result.preempted) { + resetDialog(); return; } - - // Close before the popup so the restore can't flash this window back. - WindowManager.CloseSessionExpiration().catch(console.error); + WindowManager.CloseRenewFlow().catch(console.error); } catch (e) { + resetDialog(); await errorDialog({ Title: t("sessionExpiration.extendFailedTitle"), Message: formatErrorMessage(e), }); - } finally { - offCancel?.(); - WindowManager.CloseBrowserLogin().catch(console.error); - setBusy(false); } }, [busy, t]); @@ -139,12 +143,11 @@ export default function SessionExpirationDialog() { }); WindowManager.CloseSessionExpiration().catch(console.error); } catch (e) { + setBusy(false); await errorDialog({ Title: t("sessionExpiration.logoutFailedTitle"), Message: formatErrorMessage(e), }); - } finally { - setBusy(false); } }, [busy, t]); diff --git a/client/ui/frontend/src/modules/welcome/WelcomeStepTray.tsx b/client/ui/frontend/src/modules/welcome/WelcomeStepTray.tsx index fe06abc20..5a8b0d015 100644 --- a/client/ui/frontend/src/modules/welcome/WelcomeStepTray.tsx +++ b/client/ui/frontend/src/modules/welcome/WelcomeStepTray.tsx @@ -22,6 +22,9 @@ type WelcomeStepTrayProps = { export function WelcomeStepTray({ onContinue }: Readonly) { const { t } = useTranslation(); const trayScreenshot = trayScreenshotForOS(); + // macOS has no tray — the icon sits in the menu bar, so the copy says so. + const titleKey = isMacOS() ? "welcome.titleMac" : "welcome.title"; + const descriptionKey = isMacOS() ? "welcome.descriptionMac" : "welcome.description"; return ( <> @@ -36,9 +39,9 @@ export function WelcomeStepTray({ onContinue }: Readonly)
- {t("welcome.title")} + {t(titleKey)} - {t("welcome.description")} + {t(descriptionKey)}
diff --git a/client/ui/i18n/locales/de/common.json b/client/ui/i18n/locales/de/common.json index 5e0d8096d..5e91e8d88 100644 --- a/client/ui/i18n/locales/de/common.json +++ b/client/ui/i18n/locales/de/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "Suchen Sie NetBird in der Taskleiste" }, + "welcome.titleMac": { + "message": "Suchen Sie NetBird in der Menüleiste" + }, "welcome.description": { "message": "NetBird läuft in Ihrer Taskleiste. Klicken Sie auf das Symbol, um sich zu verbinden, Profile zu wechseln oder die Einstellungen zu öffnen." }, + "welcome.descriptionMac": { + "message": "NetBird läuft in Ihrer Menüleiste. Klicken Sie auf das Symbol, um sich zu verbinden, Profile zu wechseln oder die Einstellungen zu öffnen." + }, "welcome.continue": { "message": "Weiter" }, @@ -1293,10 +1299,13 @@ "message": "Dokumentation" }, "daemon.outdated.title": { - "message": "NetBird-Dienst ist veraltet" + "message": "NetBird Client ist veraltet" }, "daemon.outdated.description": { - "message": "Aktualisieren Sie den NetBird-Dienst, um diese App zu verwenden." + "message": "Die neue GUI ist nicht mit Ihrem älteren Client kompatibel. Aktualisieren Sie Ihren Client, um die neue Anwendung zu verwenden." + }, + "daemon.outdated.download": { + "message": "Neueste Version herunterladen" }, "error.jwt_clock_skew": { "message": "Anmeldung fehlgeschlagen: Die Uhr dieses Geräts ist nicht mit dem Server synchron. Bitte synchronisieren Sie die Systemuhr und versuchen Sie es erneut." diff --git a/client/ui/i18n/locales/en/common.json b/client/ui/i18n/locales/en/common.json index 42d40ec30..24bbc67ce 100644 --- a/client/ui/i18n/locales/en/common.json +++ b/client/ui/i18n/locales/en/common.json @@ -1377,11 +1377,19 @@ }, "welcome.title": { "message": "Look for NetBird in your tray", - "description": "Heading on the first onboarding step, pointing the user to the tray icon. 'tray' = system tray / menu bar." + "description": "Heading on the first onboarding step, pointing the user to the tray icon. Shown on Windows and Linux; macOS uses welcome.titleMac." + }, + "welcome.titleMac": { + "message": "Look for NetBird in your menu bar", + "description": "Heading on the first onboarding step on macOS, pointing the user to the menu bar icon. Use your language's Apple term for the macOS menu bar." }, "welcome.description": { "message": "NetBird lives in your tray. Click the icon to connect, switch profiles, or open settings.", - "description": "Body of the first onboarding step explaining the tray icon." + "description": "Body of the first onboarding step explaining the tray icon. Shown on Windows and Linux; macOS uses welcome.descriptionMac." + }, + "welcome.descriptionMac": { + "message": "NetBird lives in your menu bar. Click the icon to connect, switch profiles, or open settings.", + "description": "Body of the first onboarding step on macOS explaining the menu bar icon. Use your language's Apple term for the macOS menu bar." }, "welcome.continue": { "message": "Continue", @@ -1724,12 +1732,16 @@ "description": "Documentation link on the daemon-unavailable overlay." }, "daemon.outdated.title": { - "message": "NetBird Service Is Outdated", - "description": "Title of the overlay shown when the NetBird background service is too old to drive this UI." + "message": "NetBird Client Is Outdated", + "description": "Title of the overlay shown when the NetBird client (daemon) is too old to drive this UI." }, "daemon.outdated.description": { - "message": "Update the NetBird service to use this app.", - "description": "Body of the daemon-outdated overlay telling the user to upgrade the service." + "message": "The new GUI isn't compatible with the older NetBird client. Update your client to use the new application.", + "description": "Body of the daemon-outdated overlay explaining that the GUI is newer than the client and the client must be updated." + }, + "daemon.outdated.download": { + "message": "Download Latest", + "description": "Button on the daemon-outdated overlay that opens the download page for the latest release." }, "error.jwt_clock_skew": { "message": "Sign-in failed: this device's clock is out of sync with the server. Please sync your system clock and try again.", diff --git a/client/ui/i18n/locales/es/common.json b/client/ui/i18n/locales/es/common.json index 47faee61f..c036e4f75 100644 --- a/client/ui/i18n/locales/es/common.json +++ b/client/ui/i18n/locales/es/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "Busque NetBird en su bandeja del sistema" }, + "welcome.titleMac": { + "message": "Busque NetBird en su barra de menús" + }, "welcome.description": { "message": "NetBird reside en su bandeja del sistema. Haga clic en el icono para conectarse, cambiar de perfil o abrir la configuración." }, + "welcome.descriptionMac": { + "message": "NetBird reside en su barra de menús. Haga clic en el icono para conectarse, cambiar de perfil o abrir la configuración." + }, "welcome.continue": { "message": "Continuar" }, @@ -1293,10 +1299,13 @@ "message": "Documentación" }, "daemon.outdated.title": { - "message": "El servicio de NetBird está desactualizado" + "message": "NetBird Client está desactualizado" }, "daemon.outdated.description": { - "message": "Actualice el servicio de NetBird para usar esta aplicación." + "message": "La nueva GUI no es compatible con su cliente anterior. Actualice su cliente para usar la nueva aplicación." + }, + "daemon.outdated.download": { + "message": "Descargar la última versión" }, "error.jwt_clock_skew": { "message": "Error al iniciar sesión: el reloj de este dispositivo no está sincronizado con el servidor. Sincronice el reloj del sistema e inténtelo de nuevo." diff --git a/client/ui/i18n/locales/fr/common.json b/client/ui/i18n/locales/fr/common.json index be0836e93..c6b91fb25 100644 --- a/client/ui/i18n/locales/fr/common.json +++ b/client/ui/i18n/locales/fr/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "Cherchez NetBird dans votre barre d’état système" }, + "welcome.titleMac": { + "message": "Cherchez NetBird dans votre barre des menus" + }, "welcome.description": { "message": "NetBird se trouve dans votre barre d’état système. Cliquez sur l’icône pour vous connecter, changer de profil ou ouvrir les paramètres." }, + "welcome.descriptionMac": { + "message": "NetBird se trouve dans votre barre des menus. Cliquez sur l’icône pour vous connecter, changer de profil ou ouvrir les paramètres." + }, "welcome.continue": { "message": "Continuer" }, @@ -1293,10 +1299,13 @@ "message": "Documentation" }, "daemon.outdated.title": { - "message": "Le service NetBird est obsolète" + "message": "Le Client NetBird est obsolète" }, "daemon.outdated.description": { - "message": "Mettez à jour le service NetBird pour utiliser cette application." + "message": "La nouvelle GUI n'est pas compatible avec votre ancien client. Mettez à jour votre client pour utiliser la nouvelle application." + }, + "daemon.outdated.download": { + "message": "Télécharger la dernière version" }, "error.jwt_clock_skew": { "message": "Échec de la connexion : l’horloge de cet appareil n’est pas synchronisée avec le serveur. Veuillez synchroniser l’horloge de votre système et réessayer." diff --git a/client/ui/i18n/locales/hu/common.json b/client/ui/i18n/locales/hu/common.json index b54918364..dd5a1af6c 100644 --- a/client/ui/i18n/locales/hu/common.json +++ b/client/ui/i18n/locales/hu/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "Keresse a NetBirdöt a tálcán" }, + "welcome.titleMac": { + "message": "Keresse a NetBirdöt a menüsorban" + }, "welcome.description": { "message": "A NetBird a tálcán fut. Kattintson az ikonra a csatlakozáshoz, profilváltáshoz vagy a beállítások megnyitásához." }, + "welcome.descriptionMac": { + "message": "A NetBird a menüsorban fut. Kattintson az ikonra a csatlakozáshoz, profilváltáshoz vagy a beállítások megnyitásához." + }, "welcome.continue": { "message": "Folytatás" }, @@ -1293,10 +1299,13 @@ "message": "Dokumentáció" }, "daemon.outdated.title": { - "message": "A NetBird szolgáltatás elavult" + "message": "A NetBird Kliens elavult" }, "daemon.outdated.description": { - "message": "Frissítsd a NetBird szolgáltatást az alkalmazás használatához." + "message": "Az új GUI nem kompatibilis a régebbi klienseddel. Frissítsd a klienst az új alkalmazás használatához." + }, + "daemon.outdated.download": { + "message": "Legújabb letöltése" }, "error.jwt_clock_skew": { "message": "A bejelentkezés sikertelen: az eszköz órája eltér a szerverétől. Kérjük, szinkronizálja a rendszer óráját, majd próbálja újra." diff --git a/client/ui/i18n/locales/it/common.json b/client/ui/i18n/locales/it/common.json index 603364fa2..7a2eb610c 100644 --- a/client/ui/i18n/locales/it/common.json +++ b/client/ui/i18n/locales/it/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "Cerchi NetBird nella tray" }, + "welcome.titleMac": { + "message": "Cerchi NetBird nella barra dei menu" + }, "welcome.description": { "message": "NetBird risiede nella tray. Clicchi sull'icona per connettersi, cambiare profilo o aprire le impostazioni." }, + "welcome.descriptionMac": { + "message": "NetBird risiede nella barra dei menu. Clicchi sull'icona per connettersi, cambiare profilo o aprire le impostazioni." + }, "welcome.continue": { "message": "Continua" }, @@ -1293,10 +1299,13 @@ "message": "Documentazione" }, "daemon.outdated.title": { - "message": "Il servizio NetBird è obsoleto" + "message": "NetBird Client è obsoleto" }, "daemon.outdated.description": { - "message": "Aggiorna il servizio NetBird per usare questa app." + "message": "La nuova GUI non è compatibile con il tuo client precedente. Aggiorna il client per usare la nuova applicazione." + }, + "daemon.outdated.download": { + "message": "Scarica l'ultima versione" }, "error.jwt_clock_skew": { "message": "Accesso non riuscito: l'orologio di questo dispositivo non è sincronizzato con il server. Sincronizzi l'orologio di sistema e riprovi." diff --git a/client/ui/i18n/locales/ja/common.json b/client/ui/i18n/locales/ja/common.json index cd54bce17..326c825bf 100644 --- a/client/ui/i18n/locales/ja/common.json +++ b/client/ui/i18n/locales/ja/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "トレイの NetBird を確認してください" }, + "welcome.titleMac": { + "message": "メニューバーの NetBird を確認してください" + }, "welcome.description": { "message": "NetBird はトレイに常駐します。アイコンをクリックして、接続、プロファイルの切り替え、設定を開くことができます。" }, + "welcome.descriptionMac": { + "message": "NetBird はメニューバーに常駐します。アイコンをクリックして、接続、プロファイルの切り替え、設定を開くことができます。" + }, "welcome.continue": { "message": "続ける" }, diff --git a/client/ui/i18n/locales/pt/common.json b/client/ui/i18n/locales/pt/common.json index 2ed0a94c5..37b02d5a8 100644 --- a/client/ui/i18n/locales/pt/common.json +++ b/client/ui/i18n/locales/pt/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "Procure o NetBird na sua bandeja" }, + "welcome.titleMac": { + "message": "Procure o NetBird na sua barra de menus" + }, "welcome.description": { "message": "O NetBird fica na sua bandeja. Clique no ícone para conectar, alternar perfis ou abrir as configurações." }, + "welcome.descriptionMac": { + "message": "O NetBird fica na sua barra de menus. Clique no ícone para conectar, alternar perfis ou abrir as configurações." + }, "welcome.continue": { "message": "Continuar" }, @@ -1293,10 +1299,13 @@ "message": "Documentação" }, "daemon.outdated.title": { - "message": "O serviço NetBird está desatualizado" + "message": "O NetBird Client está desatualizado" }, "daemon.outdated.description": { - "message": "Atualize o serviço NetBird para usar este aplicativo." + "message": "A nova GUI não é compatível com o seu cliente mais antigo. Atualize o seu cliente para usar o novo aplicativo." + }, + "daemon.outdated.download": { + "message": "Baixar a versão mais recente" }, "error.jwt_clock_skew": { "message": "Falha no login: o relógio deste dispositivo está fora de sincronia com o servidor. Sincronize o relógio do sistema e tente novamente." diff --git a/client/ui/i18n/locales/ru/common.json b/client/ui/i18n/locales/ru/common.json index 6ba7de8cc..b9ae59df2 100644 --- a/client/ui/i18n/locales/ru/common.json +++ b/client/ui/i18n/locales/ru/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "Найдите NetBird в системном трее" }, + "welcome.titleMac": { + "message": "Найдите NetBird в строке меню" + }, "welcome.description": { "message": "NetBird находится в системном трее. Нажмите на значок, чтобы подключиться, переключить профиль или открыть настройки." }, + "welcome.descriptionMac": { + "message": "NetBird находится в строке меню. Нажмите на значок, чтобы подключиться, переключить профиль или открыть настройки." + }, "welcome.continue": { "message": "Продолжить" }, @@ -1293,10 +1299,13 @@ "message": "Документация" }, "daemon.outdated.title": { - "message": "Служба NetBird устарела" + "message": "Клиент NetBird устарел" }, "daemon.outdated.description": { - "message": "Обновите службу NetBird, чтобы использовать это приложение." + "message": "Новый GUI несовместим с вашим более старым клиентом. Обновите клиент, чтобы использовать новое приложение." + }, + "daemon.outdated.download": { + "message": "Скачать последнюю версию" }, "error.jwt_clock_skew": { "message": "Не удалось войти: часы этого устройства рассинхронизированы с сервером. Синхронизируйте системные часы и повторите попытку." diff --git a/client/ui/i18n/locales/zh-CN/common.json b/client/ui/i18n/locales/zh-CN/common.json index 609344fc0..2141a770d 100644 --- a/client/ui/i18n/locales/zh-CN/common.json +++ b/client/ui/i18n/locales/zh-CN/common.json @@ -1034,9 +1034,15 @@ "welcome.title": { "message": "在托盘中查找 NetBird" }, + "welcome.titleMac": { + "message": "在菜单栏中查找 NetBird" + }, "welcome.description": { "message": "NetBird 驻留在您的托盘中。点击图标即可连接、切换配置文件或打开设置。" }, + "welcome.descriptionMac": { + "message": "NetBird 驻留在您的菜单栏中。点击图标即可连接、切换配置文件或打开设置。" + }, "welcome.continue": { "message": "继续" }, @@ -1293,10 +1299,13 @@ "message": "文档" }, "daemon.outdated.title": { - "message": "NetBird 服务版本过旧" + "message": "NetBird 客户端版本过旧" }, "daemon.outdated.description": { - "message": "请更新 NetBird 服务以使用此应用。" + "message": "新版 GUI 与您较旧的客户端不兼容。请更新客户端以使用新应用。" + }, + "daemon.outdated.download": { + "message": "下载最新版本" }, "error.jwt_clock_skew": { "message": "登录失败：此设备的时钟与服务器不同步。请同步您的系统时钟后重试。" diff --git a/client/ui/main.go b/client/ui/main.go index 4889bad79..9a3e17743 100644 --- a/client/ui/main.go +++ b/client/ui/main.go @@ -281,6 +281,9 @@ func newApplication(onSecondInstance func()) *application.App { Linux: application.LinuxOptions{ ProgramName: "netbird", }, + Windows: application.WindowsOptions{ + WndProcInterceptor: endSessionInterceptor(), + }, SingleInstance: &application.SingleInstanceOptions{ UniqueID: "io.netbird.ui", OnSecondInstanceLaunch: func(_ application.SecondInstanceData) { @@ -367,6 +370,9 @@ func newMainWindow(app *application.App, prefStore *preferences.Store) *applicat // Hide instead of quit on close; "really quit" is reached via tray -> Quit. window.RegisterHook(events.Common.WindowClosing, func(e *application.WindowEvent) { + if services.ShuttingDown() { + return + } e.Cancel() window.Hide() }) diff --git a/client/ui/services/autostart.go b/client/ui/services/autostart.go index f7e3aeea0..98e893f04 100644 --- a/client/ui/services/autostart.go +++ b/client/ui/services/autostart.go @@ -10,7 +10,7 @@ import ( "github.com/wailsapp/wails/v3/pkg/application" ) -// Autostart facade over Wails' AutostartManager. The OS login-item registration +// Autostart facade over Wails' AutostartManager. The OS autostart entry registration // is the single source of truth; nothing is mirrored to preferences. type Autostart struct { mgr *application.AutostartManager diff --git a/client/ui/services/profileswitcher.go b/client/ui/services/profileswitcher.go index c27b62d92..727b2473f 100644 --- a/client/ui/services/profileswitcher.go +++ b/client/ui/services/profileswitcher.go @@ -12,13 +12,15 @@ import ( "github.com/netbirdio/netbird/client/internal/profilemanager" ) -// ProfileSwitcher holds the reconnect policy shared by the tray and React -// frontend so both flip profiles identically. The policy keys off prevStatus -// from DaemonFeed.Get at SwitchActive entry: +// ProfileSwitcher holds the switch policy shared by the tray and React +// frontend so both flip profiles identically. SwitchActive (plain selection: +// header dropdown, tray submenu) always connects after the switch; +// SwitchActiveNoConnect (manage-profiles screen) never does, so the user can +// still adjust the management URL before connecting. prevStatus from +// DaemonFeed.Get at entry only decides the teardown: // -// Connected/Connecting → Switch + Down + Up; optimistic Connecting paint. -// NeedsLogin/LoginFailed/SessionExpired → Switch + Down; clear stale error for re-login. -// Idle → Switch only. +// Connected/Connecting/NeedsLogin/LoginFailed/SessionExpired → Down first. +// Idle → no Down. type ProfileSwitcher struct { profiles *Profiles connection *Connection @@ -29,29 +31,40 @@ func NewProfileSwitcher(profiles *Profiles, connection *Connection, feed *Daemon return &ProfileSwitcher{profiles: profiles, connection: connection, feed: feed} } -// SwitchActive switches to the named profile applying the reconnect policy. +// SwitchActive switches to the named profile and always connects afterwards. func (s *ProfileSwitcher) SwitchActive(ctx context.Context, p ProfileRef) error { + return s.switchActive(ctx, p, true) +} + +// SwitchActiveNoConnect switches to the named profile without connecting, +// tearing down any existing connection first. +func (s *ProfileSwitcher) SwitchActiveNoConnect(ctx context.Context, p ProfileRef) error { + return s.switchActive(ctx, p, false) +} + +func (s *ProfileSwitcher) switchActive(ctx context.Context, p ProfileRef, connect bool) error { prevStatus := "" - if st, err := s.feed.Get(ctx); err == nil { - prevStatus = st.Status - } else { - log.Warnf("profileswitcher: get status: %v", err) + if s.feed != nil { + if st, err := s.feed.Get(ctx); err == nil { + prevStatus = st.Status + } else { + log.Warnf("profileswitcher: get status: %v", err) + } } - wasActive := strings.EqualFold(prevStatus, StatusConnected) || - strings.EqualFold(prevStatus, StatusConnecting) - needsDown := wasActive || + needsDown := strings.EqualFold(prevStatus, StatusConnected) || + strings.EqualFold(prevStatus, StatusConnecting) || strings.EqualFold(prevStatus, StatusNeedsLogin) || strings.EqualFold(prevStatus, StatusLoginFailed) || strings.EqualFold(prevStatus, StatusSessionExpired) - log.Infof("profileswitcher: switch profile=%q prevStatus=%q wasActive=%v needsDown=%v", - p.ProfileName, prevStatus, wasActive, needsDown) + log.Infof("profileswitcher: switch profile=%q prevStatus=%q connect=%v needsDown=%v", + p.ProfileName, prevStatus, connect, needsDown) - // Optimistic Connecting paint only when wasActive: those prevStatuses emit - // stale Connected + transient Idle pushes during Down that must be - // suppressed until Up resumes the stream (see DaemonFeed suppression table). - if wasActive { + // Optimistic Connecting paint plus stale-push suppression during Down (see + // DaemonFeed suppression table); also arms the login-watch that pops + // browser-login when the new profile turns out to need SSO. + if connect && s.feed != nil { s.feed.BeginProfileSwitch() } @@ -76,9 +89,9 @@ func (s *ProfileSwitcher) SwitchActive(ctx context.Context, p ProfileRef) error } } - if wasActive { + if connect { if err := s.connection.Up(ctx, UpParams(p)); err != nil { - return fmt.Errorf("reconnect %q: %w", p.ProfileName, err) + return fmt.Errorf("connect %q: %w", p.ProfileName, err) } } diff --git a/client/ui/services/shutdown.go b/client/ui/services/shutdown.go new file mode 100644 index 000000000..0da51940c --- /dev/null +++ b/client/ui/services/shutdown.go @@ -0,0 +1,24 @@ +package services + +import "sync/atomic" + +var ( + sessionEnding atomic.Bool + quitting atomic.Bool +) + +func BeginSessionEnd() { + sessionEnding.Store(true) +} + +func AbortSessionEnd() { + sessionEnding.Store(false) +} + +func BeginShutdown() { + quitting.Store(true) +} + +func ShuttingDown() bool { + return sessionEnding.Load() || quitting.Load() +} diff --git a/client/ui/services/update.go b/client/ui/services/update.go index 753177d45..b743b9858 100644 --- a/client/ui/services/update.go +++ b/client/ui/services/update.go @@ -10,6 +10,7 @@ import ( "github.com/netbirdio/netbird/client/proto" "github.com/netbirdio/netbird/client/ui/updater" + "github.com/netbirdio/netbird/version" ) // UpdateResult mirrors TriggerUpdateResponse. @@ -33,6 +34,12 @@ func (s *Update) GetState() updater.State { return s.holder.Get() } +// DownloadURL returns the platform-appropriate installer download link for +// manual (non-enforced) updates. +func (s *Update) DownloadURL() string { + return version.DownloadUrl() +} + // Quit exits the app. Scheduled off the calling goroutine so the JS caller's // response returns before the runtime tears down. func (s *Update) Quit() { diff --git a/client/ui/services/windowmanager.go b/client/ui/services/windowmanager.go index 3316dadaa..bac9790b4 100644 --- a/client/ui/services/windowmanager.go +++ b/client/ui/services/windowmanager.go @@ -154,6 +154,9 @@ func NewWindowManager(app *application.App, mainWindow *application.WebviewWindo }) // Hide (not destroy) on close to keep React state; reset to General for a flash-free reopen. s.settings.RegisterHook(events.Common.WindowClosing, func(e *application.WindowEvent) { + if ShuttingDown() { + return + } e.Cancel() s.app.Event.Emit(EventSettingsOpen, "general") s.settings.Hide() @@ -185,37 +188,38 @@ func (s *WindowManager) OpenBrowserLogin(uri string) { startURL = "/#/dialog/browser-login?uri=" + url.QueryEscape(uri) } s.hideOtherWindowsLocked("browser-login") - // Prefer the main window's screen (multi-monitor); falls back to OS-default centering. - var screen *application.Screen - if s.mainWindow != nil { - if sc, err := s.mainWindow.GetScreen(); err == nil { - screen = sc - } - } opts := DialogWindowOptions("browser-login", s.title("window.title.signIn"), startURL, s.linuxIcon) // Not always-on-top: it would obscure the browser tab the user logs in through. opts.AlwaysOnTop = false opts.InitialPosition = application.WindowCentered - opts.Screen = screen + // Open on the active (where users cursor is) display, like the session-expiration dialog. + opts.Screen = s.getScreenBasedOnCursorPosition() s.browserLogin = s.app.Window.NewWithOptions(opts) bl := s.browserLogin - // Red-X close means cancel: emit the event so startLogin() tears down the SSO wait. bl.OnWindowEvent(events.Common.WindowClosing, func(_ *application.WindowEvent) { - s.app.Event.Emit(EventBrowserLoginCancel) s.mu.Lock() - s.browserLogin = nil - s.restoreHiddenWindowsLocked() + // Only a live user red-X still has this registered; programmatic closers + // nil s.browserLogin first and clean up themselves. Guarding here stops a + // stale close event from wiping a replacement popup's state. + userClosed := s.browserLogin == bl + if userClosed { + s.browserLogin = nil + s.restoreHiddenWindowsLocked() + } s.mu.Unlock() + if userClosed { + s.app.Event.Emit(EventBrowserLoginCancel) + } }) - s.centerWhenReady(s.browserLogin) + s.centerOnCursorScreen(s.browserLogin) return } if uri != "" { s.browserLogin.SetURL("/#/dialog/browser-login?uri=" + url.QueryEscape(uri)) } + s.centerOnCursorScreen(s.browserLogin) s.browserLogin.Show() s.browserLogin.Focus() - s.centerWhenReady(s.browserLogin) } // BrowserLoginWindow returns the live SSO popup, or nil. While non-nil it is the @@ -238,6 +242,15 @@ func (s *WindowManager) CloseBrowserLogin() { s.mu.Lock() w := s.browserLogin s.browserLogin = nil + // The WindowClosing hook no-ops on a programmatic close, so restore here — + // but only if a popup was actually open. The frontend calls this even when no + // popup was ever shown (e.g. resetDialog() after an early RequestExtend failure, + // or connection.ts's catch path), and hiddenForLogin is shared with + // OpenInstallProgress, so an unconditional restore could re-show windows a + // still-running install-progress is hiding. + if w != nil { + s.restoreHiddenWindowsLocked() + } s.mu.Unlock() if w != nil { w.Close() @@ -279,6 +292,35 @@ func (s *WindowManager) CloseSessionExpiration() { } } +// CloseRenewFlow tears down the SSO session-renewal UI in a single call: it +// closes the browser-login popup and the session-expiration window together. +func (s *WindowManager) CloseRenewFlow() { + s.mu.Lock() + bl := s.browserLogin + se := s.sessionExpiration + s.browserLogin = nil + s.sessionExpiration = nil + if se != nil { + kept := s.hiddenForLogin[:0] + for _, w := range s.hiddenForLogin { + if w != se { + kept = append(kept, w) + } + } + s.hiddenForLogin = kept + } + s.restoreHiddenWindowsLocked() + s.mu.Unlock() + + // Close after unlock so the re-entrant handlers can take s.mu. + if bl != nil { + bl.Close() + } + if se != nil { + se.Close() + } +} + // OpenInstallProgress shows the install-progress window and hides the rest for the duration // (restored on close). It owns its own result polling since the daemon restarts mid-install. func (s *WindowManager) OpenInstallProgress(version string) { @@ -354,6 +396,9 @@ func (s *WindowManager) CloseWelcome() { // OpenError shows the custom error dialog; title/message are pre-localised and ride in the // start URL. A second error replaces the open one via SetURL. Singleton, destroyed on close. func (s *WindowManager) OpenError(title, message string) { + if ShuttingDown() { + return + } s.mu.Lock() defer s.mu.Unlock() startURL := errorDialogURL(title, message) diff --git a/client/ui/shutdown_other.go b/client/ui/shutdown_other.go new file mode 100644 index 000000000..6e617233f --- /dev/null +++ b/client/ui/shutdown_other.go @@ -0,0 +1,7 @@ +//go:build !windows && !android && !ios && !freebsd && !js + +package main + +func endSessionInterceptor() func(hwnd uintptr, msg uint32, wParam, lParam uintptr) (uintptr, bool) { + return nil +} diff --git a/client/ui/shutdown_windows.go b/client/ui/shutdown_windows.go new file mode 100644 index 000000000..fbb92a518 --- /dev/null +++ b/client/ui/shutdown_windows.go @@ -0,0 +1,36 @@ +//go:build windows + +package main + +import ( + "os" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/ui/services" +) + +const ( + wmQueryEndSession = 0x0011 + wmEndSession = 0x0016 +) + +func endSessionInterceptor() func(hwnd uintptr, msg uint32, wParam, lParam uintptr) (uintptr, bool) { + return func(_ uintptr, msg uint32, wParam, _ uintptr) (uintptr, bool) { + switch msg { + case wmQueryEndSession: + services.BeginSessionEnd() + return 1, true + case wmEndSession: + if wParam == 0 { + services.AbortSessionEnd() + return 0, true + } + log.Info("windows session is ending; exiting immediately") + os.Exit(0) + return 0, true + default: + return 0, false + } + } +} diff --git a/client/ui/tray.go b/client/ui/tray.go index 700d94098..3050d159a 100644 --- a/client/ui/tray.go +++ b/client/ui/tray.go @@ -30,9 +30,10 @@ const ( statusError = "Error" - urlGitHubRepo = "https://github.com/netbirdio/netbird" - urlGitHubReleases = "https://github.com/netbirdio/netbird/releases/latest" - urlDocs = "https://docs.netbird.io" + quitDownTimeout = 5 * time.Second + + urlGitHubRepo = "https://github.com/netbirdio/netbird" + urlDocs = "https://docs.netbird.io" ) // TrayServices bundles the services the tray menu needs, grouped so NewTray @@ -315,8 +316,7 @@ func (t *Tray) relayoutMenu() { if sessionDeadline.IsZero() { t.sessionExpiresItem.SetHidden(true) } else { - remaining := t.formatSessionRemaining(time.Until(sessionDeadline)) - t.sessionExpiresItem.SetLabel(t.loc.T("tray.session.expiresIn", "remaining", remaining)) + t.sessionExpiresItem.SetLabel(t.sessionRowLabel(sessionDeadline)) t.sessionExpiresItem.SetHidden(false) } } @@ -446,11 +446,29 @@ func (t *Tray) buildMenu() *application.Menu { menu.AddSeparator() menu.Add(t.loc.T("tray.menu.quit")). SetAccelerator("CmdOrCtrl+Q"). - OnClick(func(*application.Context) { t.app.Quit() }) + OnClick(func(*application.Context) { t.handleQuit() }) return menu } +func (t *Tray) handleQuit() { + services.BeginShutdown() + t.profileMu.Lock() + if t.switchCancel != nil { + t.switchCancel() + t.switchCancel = nil + } + t.profileMu.Unlock() + t.svc.DaemonFeed.CancelProfileSwitch() + + ctx, cancel := context.WithTimeout(context.Background(), quitDownTimeout) + defer cancel() + if err := t.svc.Connection.Down(ctx); err != nil { + log.Errorf("disconnect on quit: %v", err) + } + t.app.Quit() +} + // handleConnect receives the clicked item from the buildMenu closure — // t.upItem is menuMu-guarded and must not be read here. func (t *Tray) handleConnect(upItem *application.MenuItem) { diff --git a/client/ui/tray_notify.go b/client/ui/tray_notify.go index 6c60e3d4b..d1117b57b 100644 --- a/client/ui/tray_notify.go +++ b/client/ui/tray_notify.go @@ -25,6 +25,9 @@ type sendFn func(notifications.NotificationOptions) error // event-dispatch goroutine that panic is fatal process-wide; recover() turns // it into a logged no-op. func safeSendNotification(send sendFn, what string, opts notifications.NotificationOptions) (err error) { + if services.ShuttingDown() { + return nil + } defer func() { if r := recover(); r != nil { log.Errorf("notify %s: recovered from panic (notification bus unavailable): %v", what, r) diff --git a/client/ui/tray_session.go b/client/ui/tray_session.go index 6b73ddb49..885fdb348 100644 --- a/client/ui/tray_session.go +++ b/client/ui/tray_session.go @@ -64,11 +64,42 @@ func (t *Tray) applySessionExpiry(deadline *time.Time, connected bool) bool { return changed } -// runSessionExpiryTicker recomputes the "Expires in …" row label every 30s. Runs until process exit. +// runSessionExpiryTicker recomputes the "Expires in …" row label until process exit. +// The interval scales with the remaining time: coarse when the deadline is far off, +// down to 10s in the final two minutes so the label doesn't lag the ceiling-rounded +// countdown near expiry. The cached deadline is re-read every iteration, so an extend +// or reconnect that moves it is picked up on the next tick. func (t *Tray) runSessionExpiryTicker() { - tk := time.NewTicker(30 * time.Second) - for range tk.C { + tm := time.NewTimer(sessionRefreshInterval(t.sessionRemaining())) + defer tm.Stop() + for range tm.C { t.refreshSessionExpiresLabel() + tm.Reset(sessionRefreshInterval(t.sessionRemaining())) + } +} + +// sessionRemaining returns the time left on the cached SSO deadline, or 0 when unknown. +func (t *Tray) sessionRemaining() time.Duration { + t.sessionMu.Lock() + deadline := t.sessionExpiresAt + t.sessionMu.Unlock() + if deadline.IsZero() { + return 0 + } + return time.Until(deadline) +} + +// sessionRefreshInterval picks how long to wait before the next label recompute. +func sessionRefreshInterval(remaining time.Duration) time.Duration { + switch { + case remaining <= 0: + return 30 * time.Second + case remaining <= 2*time.Minute: + return 10 * time.Second + case remaining <= time.Hour: + return 30 * time.Second + default: + return time.Minute } } @@ -87,30 +118,39 @@ func (t *Tray) refreshSessionExpiresLabel() { if deadline.IsZero() { return } - remaining := t.formatSessionRemaining(time.Until(deadline)) - item.SetLabel(t.loc.T("tray.session.expiresIn", "remaining", remaining)) + item.SetLabel(t.sessionRowLabel(deadline)) +} + +func (t *Tray) sessionRowLabel(deadline time.Time) string { + remaining := time.Until(deadline) + if remaining <= 0 { + return t.loc.T("tray.status.sessionExpired") + } + return t.loc.T("tray.session.expiresIn", "remaining", t.formatSessionRemaining(remaining)) } // formatSessionRemaining renders d as a localised long-form string picking the largest non-zero unit. +// Each unit is rounded up so the label never claims less time than actually remains, matching the +// upper-bound sense of the sub-minute "less than a minute" fragment. // Singular/plural keys are split per language for proper translation. func (t *Tray) formatSessionRemaining(d time.Duration) string { switch { case d < time.Minute: return t.loc.T("tray.session.unit.lessThanMinute") - case d < time.Hour: - m := int(d / time.Minute) + case d <= 59*time.Minute: + m := ceilDiv(d, time.Minute) if m == 1 { return t.loc.T("tray.session.unit.minute") } return t.loc.T("tray.session.unit.minutes", "count", strconv.Itoa(m)) - case d < 24*time.Hour: - h := int((d + 30*time.Minute) / time.Hour) + case d <= 23*time.Hour: + h := ceilDiv(d, time.Hour) if h == 1 { return t.loc.T("tray.session.unit.hour") } return t.loc.T("tray.session.unit.hours", "count", strconv.Itoa(h)) default: - days := int((d + 12*time.Hour) / (24 * time.Hour)) + days := ceilDiv(d, 24*time.Hour) if days == 1 { return t.loc.T("tray.session.unit.day") } @@ -118,6 +158,11 @@ func (t *Tray) formatSessionRemaining(d time.Duration) string { } } +// ceilDiv divides d by unit rounding up, assuming d > 0. +func ceilDiv(d, unit time.Duration) int { + return int((d + unit - time.Nanosecond) / unit) +} + // registerSessionWarningCategory wires the OS notification category and response handler for the expiry warning. // Errors are swallowed since the worst case is a plain notification without buttons. func (t *Tray) registerSessionWarningCategory() { @@ -252,11 +297,9 @@ func (t *Tray) openSessionExpiration() { } // openSessionExtendFlow opens the SessionExpiration window seeded with the cached deadline's remaining time, -// for the "Expires in …" tray row. No-ops when the deadline is unknown or elapsed. +// for the "Expires in …" tray row. Once the deadline has elapsed the row reads "Session expired" and the +// click routes to the login flow instead. No-op when the deadline is unknown. func (t *Tray) openSessionExtendFlow() { - if t.svc.WindowManager == nil { - return - } t.sessionMu.Lock() deadline := t.sessionExpiresAt t.sessionMu.Unlock() @@ -265,6 +308,14 @@ func (t *Tray) openSessionExtendFlow() { } seconds := int(time.Until(deadline).Seconds()) if seconds <= 0 { + if t.window != nil { + t.window.SetURL("/#/login") + t.window.Show() + t.window.Focus() + } + return + } + if t.svc.WindowManager == nil { return } t.svc.WindowManager.OpenSessionExpiration(seconds) diff --git a/client/ui/tray_update.go b/client/ui/tray_update.go index 2d79cff05..1a377dfa3 100644 --- a/client/ui/tray_update.go +++ b/client/ui/tray_update.go @@ -13,6 +13,7 @@ import ( "github.com/netbirdio/netbird/client/ui/services" "github.com/netbirdio/netbird/client/ui/updater" + "github.com/netbirdio/netbird/version" ) // trayUpdater owns the tray UI that reacts to auto-update. Composed inside Tray. @@ -76,15 +77,15 @@ func (u *trayUpdater) applyLanguage() { u.refreshMenuItem(state) } -// handleClick opens the GitHub releases page when not Enforced, otherwise shows -// the progress page and asks the daemon to start the installer. +// handleClick opens the installer download link when not Enforced, otherwise +// shows the progress page and asks the daemon to start the installer. func (u *trayUpdater) handleClick() { u.mu.Lock() state := u.state u.mu.Unlock() if !state.Enforced { - _ = u.app.Browser.OpenURL(urlGitHubReleases) + _ = u.app.Browser.OpenURL(version.DownloadUrl()) return } diff --git a/combined/cmd/admin.go b/combined/cmd/admin.go new file mode 100644 index 000000000..66fac4ac9 --- /dev/null +++ b/combined/cmd/admin.go @@ -0,0 +1,151 @@ +package cmd + +import ( + "context" + "fmt" + + "github.com/dexidp/dex/storage" + log "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + + "github.com/netbirdio/netbird/formatter/hook" + admincmd "github.com/netbirdio/netbird/management/cmd/admin" + tokencmd "github.com/netbirdio/netbird/management/cmd/token" + nbconfig "github.com/netbirdio/netbird/management/internals/server/config" + "github.com/netbirdio/netbird/management/server/activity" + activitystore "github.com/netbirdio/netbird/management/server/activity/store" + "github.com/netbirdio/netbird/management/server/store" + "github.com/netbirdio/netbird/management/server/types" + "github.com/netbirdio/netbird/util" +) + +// newAdminCommands creates the admin command tree with combined-specific resource openers. +func newAdminCommands() *cobra.Command { + return admincmd.NewCommands(admincmd.Openers{ + Resources: withAdminResources, + Store: withAdminStoreOnly, + IDP: withAdminIDPOnly, + }) +} + +func newLegacyTokenCommand() *cobra.Command { + cmd := tokencmd.NewCommands(tokencmd.StoreOpener(withAdminStoreOnly)) + cmd.Deprecated = "use 'admin token' instead" + return cmd +} + +// withAdminResources loads the combined YAML config, initializes stores, and calls fn. +func withAdminResources(cmd *cobra.Command, fn func(ctx context.Context, resources admincmd.Resources) error) error { + return withAdminConfig(cmd, func(ctx context.Context, cfg *CombinedConfig) error { + mgmtConfig, err := adminManagementConfig(cfg) + if err != nil { + return err + } + + managementStore, err := openAdminStore(ctx, cfg) + if err != nil { + return err + } + defer admincmd.CloseStore(ctx, managementStore) + + idpStorage, idpStorageFile, err := admincmd.OpenIDPStorage(mgmtConfig) + if err != nil { + return err + } + defer admincmd.CloseIDPStorage(idpStorage) + + eventStore, esErr := openAdminEventStore(ctx, cfg, mgmtConfig) + if esErr != nil { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: audit events will not be recorded: %v\n", esErr) + } + if eventStore != nil { + defer func() { + if err := eventStore.Close(ctx); err != nil { + log.Debugf("close activity event store: %v", err) + } + }() + } + + return fn(ctx, admincmd.Resources{Store: managementStore, IDPStorage: idpStorage, IDPStorageFile: idpStorageFile, EventStore: eventStore}) + }) +} + +// withAdminStoreOnly opens only the management store for admin subcommands that do not +// need embedded IdP storage. +func withAdminStoreOnly(cmd *cobra.Command, fn func(ctx context.Context, s store.Store) error) error { + return withAdminConfig(cmd, func(ctx context.Context, cfg *CombinedConfig) error { + managementStore, err := openAdminStore(ctx, cfg) + if err != nil { + return err + } + defer admincmd.CloseStore(ctx, managementStore) + + return fn(ctx, managementStore) + }) +} + +func withAdminIDPOnly(cmd *cobra.Command, fn func(ctx context.Context, idpStorage storage.Storage, storageFile string) error) error { + return withAdminConfig(cmd, func(ctx context.Context, cfg *CombinedConfig) error { + mgmtConfig, err := adminManagementConfig(cfg) + if err != nil { + return err + } + idpStorage, idpStorageFile, err := admincmd.OpenIDPStorage(mgmtConfig) + if err != nil { + return err + } + defer admincmd.CloseIDPStorage(idpStorage) + + return fn(ctx, idpStorage, idpStorageFile) + }) +} + +func withAdminConfig(cmd *cobra.Command, fn func(ctx context.Context, cfg *CombinedConfig) error) error { + if err := util.InitLog("error", "console"); err != nil { + return fmt.Errorf("init log: %w", err) + } + + ctx := context.WithValue(cmd.Context(), hook.ExecutionContextKey, hook.SystemSource) //nolint:staticcheck + + cfg, err := LoadConfig(configPath) + if err != nil { + return fmt.Errorf("load config: %w", err) + } + cfg.ApplyAdminDefaults() + applyServerStoreEnv(cfg.Server.Store) + + return fn(ctx, cfg) +} + +func adminManagementConfig(cfg *CombinedConfig) (*nbconfig.Config, error) { + mgmtConfig, err := cfg.ToManagementConfig() + if err != nil { + return nil, fmt.Errorf("create management config: %w", err) + } + return mgmtConfig, nil +} + +func openAdminStore(ctx context.Context, cfg *CombinedConfig) (store.Store, error) { + managementStore, err := store.NewStore(ctx, types.Engine(cfg.Management.Store.Engine), cfg.Management.DataDir, nil, true) + if err != nil { + return nil, fmt.Errorf("create store: %w", err) + } + return managementStore, nil +} + +func openAdminEventStore(ctx context.Context, cfg *CombinedConfig, config *nbconfig.Config) (activity.Store, error) { + if config.DataStoreEncryptionKey == "" { + return nil, fmt.Errorf("data store encryption key is not configured") + } + if err := applyActivityStoreEnv(cfg.Server.ActivityStore); err != nil { + return nil, fmt.Errorf("configure activity event store: %w", err) + } + eventStore, err := activitystore.NewSqlStore(ctx, config.Datadir, config.DataStoreEncryptionKey) + if err != nil { + return nil, fmt.Errorf("open activity event store: %w", err) + } + if eventStore == nil { + return nil, fmt.Errorf("open activity event store: returned nil store") + } + return eventStore, nil +} diff --git a/combined/cmd/admin_config_test.go b/combined/cmd/admin_config_test.go new file mode 100644 index 000000000..ff7045d38 --- /dev/null +++ b/combined/cmd/admin_config_test.go @@ -0,0 +1,47 @@ +package cmd + +import ( + "context" + "os" + "testing" + + "github.com/stretchr/testify/require" + + nbconfig "github.com/netbirdio/netbird/management/internals/server/config" +) + +func TestApplyAdminDefaultsCopiesServerStoreWithoutExposedAddress(t *testing.T) { + cfg := DefaultConfig() + cfg.Server.ExposedAddress = "" + cfg.Server.DataDir = "/srv/netbird" + cfg.Server.Store = StoreConfig{ + Engine: "postgres", + DSN: "postgres://user:pass@example.com/netbird", + } + + cfg.ApplyAdminDefaults() + + require.Equal(t, "/srv/netbird", cfg.Management.DataDir) + require.Equal(t, "postgres", cfg.Management.Store.Engine) + require.Equal(t, cfg.Server.Store.DSN, cfg.Management.Store.DSN) +} + +func TestOpenAdminEventStoreMissingEncryptionKeyReturnsNilInterface(t *testing.T) { + eventStore, err := openAdminEventStore(context.Background(), &CombinedConfig{}, &nbconfig.Config{}) + require.Error(t, err) + require.Contains(t, err.Error(), "encryption key") + require.Nil(t, eventStore) +} + +func TestApplyServerStoreEnv(t *testing.T) { + t.Setenv("NB_STORE_ENGINE_POSTGRES_DSN", "") + t.Setenv("NB_STORE_ENGINE_MYSQL_DSN", "") + t.Setenv("NB_STORE_ENGINE_SQLITE_FILE", "") + + applyServerStoreEnv(StoreConfig{Engine: "postgres", DSN: "postgres-dsn", File: "store.db"}) + require.Equal(t, "postgres-dsn", os.Getenv("NB_STORE_ENGINE_POSTGRES_DSN")) + require.Equal(t, "store.db", os.Getenv("NB_STORE_ENGINE_SQLITE_FILE")) + + applyServerStoreEnv(StoreConfig{Engine: "mysql", DSN: "mysql-dsn"}) + require.Equal(t, "mysql-dsn", os.Getenv("NB_STORE_ENGINE_MYSQL_DSN")) +} diff --git a/combined/cmd/config.go b/combined/cmd/config.go index fcbc60dc9..7f30cd8a8 100644 --- a/combined/cmd/config.go +++ b/combined/cmd/config.go @@ -6,8 +6,7 @@ import ( "net" "net/netip" "os" - "path" - "path/filepath" + filePath "path/filepath" "strings" "time" @@ -74,6 +73,9 @@ type ServerConfig struct { ActivityStore StoreConfig `yaml:"activityStore"` AuthStore StoreConfig `yaml:"authStore"` ReverseProxy ReverseProxyConfig `yaml:"reverseProxy"` + + SupportedSyncMessageVersions *int `yaml:"supportedSyncMessageVersions,omitempty"` + PerAccountSupportedSyncMessageVersions map[string]int `yaml:"perAccountSupportedSyncMessageVersions,omitempty"` } // TLSConfig contains TLS/HTTPS settings @@ -300,6 +302,19 @@ func (c *CombinedConfig) ApplySimplifiedDefaults() { c.autoConfigureClientSettings(exposedProto, exposedHost, exposedHostPort, hasExternalStuns, hasExternalRelay, hasExternalSignal) } +// ApplyAdminDefaults applies the management settings needed by admin commands even +// when the full server config is invalid and ApplySimplifiedDefaults cannot run. +func (c *CombinedConfig) ApplyAdminDefaults() { + if c.Management.DataDir == "" || c.Management.DataDir == "/var/lib/netbird/" { + c.Management.DataDir = c.Server.DataDir + } + if c.Management.Store.Engine == "" || c.Management.Store.Engine == "sqlite" { + if c.Server.Store.Engine != "" || c.Server.Store.File != "" || c.Server.Store.DSN != "" { + c.Management.Store = c.Server.Store + } + } +} + // applyRelayDefaults configures the relay service if no external relay is configured. func (c *CombinedConfig) applyRelayDefaults(exposedProto, exposedHostPort string, hasExternalRelay, hasExternalStuns bool) { if hasExternalRelay { @@ -577,11 +592,11 @@ func (c *CombinedConfig) buildEmbeddedIdPConfig(mgmt ManagementConfig) (*idp.Emb return nil, fmt.Errorf("authStore.dsn is required when authStore.engine is postgres") } } else { - authStorageFile = path.Join(mgmt.DataDir, "idp.db") + authStorageFile = filePath.Join(mgmt.DataDir, "idp.db") if c.Server.AuthStore.File != "" { authStorageFile = c.Server.AuthStore.File - if !filepath.IsAbs(authStorageFile) { - authStorageFile = filepath.Join(mgmt.DataDir, authStorageFile) + if !filePath.IsAbs(authStorageFile) { + authStorageFile = filePath.Join(mgmt.DataDir, authStorageFile) } } } @@ -696,16 +711,18 @@ func (c *CombinedConfig) ToManagementConfig() (*nbconfig.Config, error) { httpConfig.AuthCallbackURL = callbackURL + types.ProxyCallbackEndpointFull return &nbconfig.Config{ - Stuns: stuns, - Relay: relayConfig, - Signal: signalConfig, - Datadir: mgmt.DataDir, - DataStoreEncryptionKey: mgmt.Store.EncryptionKey, - HttpConfig: httpConfig, - StoreConfig: storeConfig, - ReverseProxy: reverseProxy, - DisableDefaultPolicy: mgmt.DisableDefaultPolicy, - EmbeddedIdP: embeddedIdP, + Stuns: stuns, + Relay: relayConfig, + Signal: signalConfig, + Datadir: mgmt.DataDir, + DataStoreEncryptionKey: mgmt.Store.EncryptionKey, + HttpConfig: httpConfig, + StoreConfig: storeConfig, + ReverseProxy: reverseProxy, + DisableDefaultPolicy: mgmt.DisableDefaultPolicy, + EmbeddedIdP: embeddedIdP, + HighestSupportedSyncMessageVersion: c.Server.SupportedSyncMessageVersions, + PerAccountHighestSupportedSyncMessageVersion: c.Server.PerAccountSupportedSyncMessageVersions, }, nil } @@ -729,7 +746,7 @@ func ApplyEmbeddedIdPConfig(ctx context.Context, cfg *nbconfig.Config, mgmtPort cfg.EmbeddedIdP.Storage.Type = "sqlite3" } if cfg.EmbeddedIdP.Storage.Config.File == "" && cfg.Datadir != "" { - cfg.EmbeddedIdP.Storage.Config.File = path.Join(cfg.Datadir, "idp.db") + cfg.EmbeddedIdP.Storage.Config.File = filePath.Join(cfg.Datadir, "idp.db") } issuer := cfg.EmbeddedIdP.Issuer diff --git a/combined/cmd/root.go b/combined/cmd/root.go index 2b7956f11..5f2564e3a 100644 --- a/combined/cmd/root.go +++ b/combined/cmd/root.go @@ -31,6 +31,7 @@ import ( relayServer "github.com/netbirdio/netbird/relay/server" "github.com/netbirdio/netbird/relay/server/listener" "github.com/netbirdio/netbird/relay/server/listener/ws" + syncgrpc "github.com/netbirdio/netbird/shared/management/grpc" sharedMetrics "github.com/netbirdio/netbird/shared/metrics" "github.com/netbirdio/netbird/shared/relay/auth" "github.com/netbirdio/netbird/shared/signal/proto" @@ -64,7 +65,8 @@ func init() { rootCmd.PersistentFlags().StringVarP(&configPath, "config", "c", "", "path to YAML configuration file (required)") _ = rootCmd.MarkPersistentFlagRequired("config") - rootCmd.AddCommand(newTokenCommands()) + rootCmd.AddCommand(newAdminCommands()) + rootCmd.AddCommand(newLegacyTokenCommand()) } func RootCmd() *cobra.Command { @@ -122,6 +124,37 @@ func execute(cmd *cobra.Command, _ []string) error { } // initializeConfig loads and validates the configuration, then initializes logging. +func applyServerStoreEnv(storeConfig StoreConfig) { + if dsn := storeConfig.DSN; dsn != "" { + switch strings.ToLower(storeConfig.Engine) { + case "postgres": + os.Setenv("NB_STORE_ENGINE_POSTGRES_DSN", dsn) + case "mysql": + os.Setenv("NB_STORE_ENGINE_MYSQL_DSN", dsn) + } + } + if file := storeConfig.File; file != "" { + os.Setenv("NB_STORE_ENGINE_SQLITE_FILE", file) + } +} + +func applyActivityStoreEnv(storeConfig StoreConfig) error { + if engine := storeConfig.Engine; engine != "" { + engineLower := strings.ToLower(engine) + if engineLower == "postgres" && storeConfig.DSN == "" { + return fmt.Errorf("activityStore.dsn is required when activityStore.engine is postgres") + } + os.Setenv("NB_ACTIVITY_EVENT_STORE_ENGINE", engineLower) + if dsn := storeConfig.DSN; dsn != "" { + os.Setenv("NB_ACTIVITY_EVENT_POSTGRES_DSN", dsn) + } + } + if file := storeConfig.File; file != "" { + os.Setenv("NB_ACTIVITY_EVENT_SQLITE_FILE", file) + } + return nil +} + func initializeConfig() error { var err error config, err = LoadConfig(configPath) @@ -137,30 +170,10 @@ func initializeConfig() error { return fmt.Errorf("failed to initialize log: %w", err) } - if dsn := config.Server.Store.DSN; dsn != "" { - switch strings.ToLower(config.Server.Store.Engine) { - case "postgres": - os.Setenv("NB_STORE_ENGINE_POSTGRES_DSN", dsn) - case "mysql": - os.Setenv("NB_STORE_ENGINE_MYSQL_DSN", dsn) - } - } - if file := config.Server.Store.File; file != "" { - os.Setenv("NB_STORE_ENGINE_SQLITE_FILE", file) - } + applyServerStoreEnv(config.Server.Store) - if engine := config.Server.ActivityStore.Engine; engine != "" { - engineLower := strings.ToLower(engine) - if engineLower == "postgres" && config.Server.ActivityStore.DSN == "" { - return fmt.Errorf("activityStore.dsn is required when activityStore.engine is postgres") - } - os.Setenv("NB_ACTIVITY_EVENT_STORE_ENGINE", engineLower) - if dsn := config.Server.ActivityStore.DSN; dsn != "" { - os.Setenv("NB_ACTIVITY_EVENT_POSTGRES_DSN", dsn) - } - } - if file := config.Server.ActivityStore.File; file != "" { - os.Setenv("NB_ACTIVITY_EVENT_SQLITE_FILE", file) + if err := applyActivityStoreEnv(config.Server.ActivityStore); err != nil { + return err } log.Infof("Starting combined NetBird server") @@ -505,6 +518,16 @@ func createManagementServer(cfg *CombinedConfig, mgmtConfig *nbconfig.Config) (m } mgmtPort, _ := strconv.Atoi(portStr) + if err := syncgrpc.ValidateSyncMessageVersion(mgmtConfig.HighestSupportedSyncMessageVersion); err != nil { + return nil, err + } + + for accountId, version := range mgmtConfig.PerAccountHighestSupportedSyncMessageVersion { + if err := syncgrpc.ValidateSyncMessageVersion(&version); err != nil { + return nil, fmt.Errorf("unrecognized sync message version in perAccountSupportedSyncMessageVersions for account %s %w", accountId, err) + } + } + mgmtSrv := newServer( &mgmtServer.Config{ NbConfig: mgmtConfig, diff --git a/combined/cmd/token.go b/combined/cmd/token.go deleted file mode 100644 index 550480062..000000000 --- a/combined/cmd/token.go +++ /dev/null @@ -1,63 +0,0 @@ -package cmd - -import ( - "context" - "fmt" - "os" - "strings" - - log "github.com/sirupsen/logrus" - "github.com/spf13/cobra" - - "github.com/netbirdio/netbird/formatter/hook" - tokencmd "github.com/netbirdio/netbird/management/cmd/token" - "github.com/netbirdio/netbird/management/server/store" - "github.com/netbirdio/netbird/management/server/types" - "github.com/netbirdio/netbird/util" -) - -// newTokenCommands creates the token command tree with combined-specific store opener. -func newTokenCommands() *cobra.Command { - return tokencmd.NewCommands(withTokenStore) -} - -// withTokenStore loads the combined YAML config, initializes the store, and calls fn. -func withTokenStore(cmd *cobra.Command, fn func(ctx context.Context, s store.Store) error) error { - if err := util.InitLog("error", "console"); err != nil { - return fmt.Errorf("init log: %w", err) - } - - ctx := context.WithValue(cmd.Context(), hook.ExecutionContextKey, hook.SystemSource) //nolint:staticcheck - - cfg, err := LoadConfig(configPath) - if err != nil { - return fmt.Errorf("load config: %w", err) - } - - if dsn := cfg.Server.Store.DSN; dsn != "" { - switch strings.ToLower(cfg.Server.Store.Engine) { - case "postgres": - os.Setenv("NB_STORE_ENGINE_POSTGRES_DSN", dsn) - case "mysql": - os.Setenv("NB_STORE_ENGINE_MYSQL_DSN", dsn) - } - } - if file := cfg.Server.Store.File; file != "" { - os.Setenv("NB_STORE_ENGINE_SQLITE_FILE", file) - } - - datadir := cfg.Management.DataDir - engine := types.Engine(cfg.Management.Store.Engine) - - s, err := store.NewStore(ctx, engine, datadir, nil, true) - if err != nil { - return fmt.Errorf("create store: %w", err) - } - defer func() { - if err := s.Close(ctx); err != nil { - log.Debugf("close store: %v", err) - } - }() - - return fn(ctx, s) -} diff --git a/dns/nameserver.go b/dns/nameserver.go index 81c616c50..84e83e2b4 100644 --- a/dns/nameserver.go +++ b/dns/nameserver.go @@ -53,6 +53,7 @@ type NameServerGroup struct { ID string `gorm:"primaryKey"` // AccountID is a reference to Account that this object belongs AccountID string `gorm:"index"` + PublicID string `json:"-"` // Name group name Name string // Description group description diff --git a/docs/agent-networks/01-end-to-end-flows.md b/docs/agent-networks/01-end-to-end-flows.md index 7264f3768..b8891001b 100644 --- a/docs/agent-networks/01-end-to-end-flows.md +++ b/docs/agent-networks/01-end-to-end-flows.md @@ -109,7 +109,7 @@ sequenceDiagram Chk->>Inj: continue Inj->>Inj: inject NetBird identity headers per provider config Inj->>Grd: continue - Grd->>Grd: enforce model allowlist + Grd->>Grd: enforce per-provider allowlist (fail-closed backstop) Grd->>Up: forward (over WireGuard) Up-->>Resp: response (JSON or SSE stream) Resp->>Resp: parse usage tokens, completion @@ -135,6 +135,21 @@ sequenceDiagram (`redact_pii = settings.RedactPii`). Phones, emails, credit cards, PII names — see `redact.go` for the full set. See [`modules/31-proxy-middleware-builtin.md`](modules/31-proxy-middleware-builtin.md). +- The model allowlist is enforced in TWO places. `CheckLLMPolicyLimits` + is authoritative: it resolves the policy that governs this + (provider, caller-groups) and denies (`deny_code = llm_policy.model_blocked`) + when no applicable policy permits the model — so an allowlist scoped to + one group/provider never leaks to another, and an un-guardrailed policy + is genuinely unrestricted. `llm_guardrail` is a per-provider fail-closed + backstop: it only carries an allowlist for a provider every authorising + policy restricts, and blocks unknown/undetermined models even when + management is unreachable. Because that backstop allowlist is the UNION + of every restricting policy's models, per-group narrowing lives only in + the authoritative check: during a `CheckLLMPolicyLimits` outage + `llm_limit_check` fails open, so a caller can reach any model in the + provider's union — a group scoped to model A could reach model B if + another group restricts the same provider to B. This is the documented + fail-open trade-off; a future flag may switch it to fail-closed. - SSE streaming requires special handling on the response side; the parser must handle partial chunks without buffering the whole stream. See [`modules/32-proxy-llm-parsers.md`](modules/32-proxy-llm-parsers.md). diff --git a/docs/agent-networks/modules/21-management-agentnetwork.md b/docs/agent-networks/modules/21-management-agentnetwork.md index b64c1ba20..cc74206e9 100644 --- a/docs/agent-networks/modules/21-management-agentnetwork.md +++ b/docs/agent-networks/modules/21-management-agentnetwork.md @@ -122,7 +122,7 @@ At request time the path is independent: the proxy calls `SelectPolicyForRequest | on_request | 1 | `llm_router` | `{"providers":[{id, models[], upstream_*, auth_header_*, allowed_group_ids[]}]}` | **true** | | on_request | 2 | `llm_limit_check` | `{}` | – | | on_request | 3 | `llm_identity_inject` | `{"providers":[{provider_id, header_pair?, json_metadata?, extra_headers?}]}` | **true** | - | on_request | 4 | `llm_guardrail` | `{"model_allowlist"?, "prompt_capture":{enabled,redact_pii}}` | – | + | on_request | 4 | `llm_guardrail` | `{"provider_allowlists"?: {providerID: []model}, "prompt_capture":{enabled,redact_pii}}` | – | | on_response | 5 | `llm_limit_record` | `{}` (runs LAST at runtime) | – | | on_response | 6 | `cost_meter` | `{}` | – | | on_response | 7 | `llm_response_parser` | `{"capture_completion": , "redact_pii"?: true}` | – | diff --git a/docs/agent-networks/modules/31-proxy-middleware-builtin.md b/docs/agent-networks/modules/31-proxy-middleware-builtin.md index 904de6424..efe1bc4ce 100644 --- a/docs/agent-networks/modules/31-proxy-middleware-builtin.md +++ b/docs/agent-networks/modules/31-proxy-middleware-builtin.md @@ -244,7 +244,7 @@ no mocks. Tests: `TestChain_AllowPath_StampsAttributionAndRecordsCounter` | `llm_router` | `{providers: [{id, models, upstream_scheme, upstream_host, upstream_path?, auth_header_name, auth_header_value, allowed_group_ids}]}` | | `llm_limit_check` | `{}` — pulls `MgmtClient` from `FactoryContext` | | `llm_identity_inject` | `{providers: [{provider_id, header_pair?|json_metadata?, extra_headers?}]}` | -| `llm_guardrail` | `{model_allowlist: []string, prompt_capture: {enabled, redact_pii}}` | +| `llm_guardrail` | `{provider_allowlists: {providerID: []string}, prompt_capture: {enabled, redact_pii}}` — allowlist keyed by resolved provider id; a provider absent from the map is unrestricted (fail-closed backstop; authoritative per-policy/group check is management's `CheckLLMPolicyLimits`) | | `llm_response_parser` | `{redact_pii?, capture_completion?: *bool}` | | `cost_meter` | `{pricing_path?}` (basename inside data-dir; defaults `pricing.yaml`) | | `llm_limit_record` | `{}` — same pattern as `llm_limit_check` | diff --git a/docs/io.netbird.client.plist b/docs/io.netbird.client.plist index 800ecead1..fe10b5b63 100644 --- a/docs/io.netbird.client.plist +++ b/docs/io.netbird.client.plist @@ -66,6 +66,9 @@ disableAutoConnect + disableAutostart + + disableClientRoutes diff --git a/docs/netbird-macos.mobileconfig b/docs/netbird-macos.mobileconfig index 9bf616094..8216dd55d 100644 --- a/docs/netbird-macos.mobileconfig +++ b/docs/netbird-macos.mobileconfig @@ -103,6 +103,8 @@