mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-12 17:59:06 +02:00
fix_debug_upload_url_from_mgmt
225
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 riccardom
|
c71fd1d841 |
[management,client] Default to NetBird's upload service when nothing is configured
The previous commit made a peer with no destination — no MDM override, no URL
named by the caller, nothing published by its management server — refuse to
upload and keep the bundle local unless it was enrolled with NetBird's cloud.
That closed the reported data-boundary concern, but it broke the default for
everyone who uploads a bundle as part of their day: a self-hosted user opening
a support ticket got a refusal where the command used to work.
Product decision (NetBird's, not the reporter's): the knob to keep bundles
inside your own infrastructure is what this branch provides, and it is enough.
The default stays the service NetBird runs, self-hosted included. An admin who
needs the bundles to stay in-house configures the destination; until then the
everyday flow keeps working.
So ResolveUploadURL drops the cloud check, the sentinel error and the
managementURL argument, and never fails:
MDM > explicitly named URL > published by management > NetBird's service
Nothing observable changes for a deployment that configures nothing, which also
removes two edge cases the fail-closed default had: a peer still enrolled on the
legacy api.wiretrustee.com host would have been classified self-hosted and
refused, and an upgrade would have silently stopped uploads for self-hosted
deployments relying on them. The privilege gate is unaffected — a host other
than the default one still requires a privileged caller, so pointing the CLI
somewhere other than what management published needs root.
|
||
|
riccardom
|
fcb9b02451 |
[client] Require privilege to relax TLS on a resolved upload destination
requirePrivilegeForUploadURL returned early on an empty URL, which was correct while an empty URL meant "do not upload": there was no destination for --upload-bundle-insecure to weaken. Now an empty URL means "use the destination the management server published", so an unprivileged local caller could send upload=true with an empty URL and uploadInsecure=true and have the root daemon PUT the bundle to that destination with TLS verification disabled. Gate the insecure flag on the empty-URL path too. A named URL keeps its existing order, so a malformed one still reports InvalidArgument rather than a privilege error. |
||
|
riccardom
|
9f6d17b9e8 |
[management,client] Take the debug-bundle upload destination from management
The debug-bundle paths that upload without a human picking a destination compiled the vendor endpoint in: the mobile clients and the desktop UI hold `https://upload.debug.netbird.io/upload-url` as a constant, the CLI defaults its flag to it, and the remote job falls back to it when nothing else is set. A self-hosted deployment therefore shipped peer logs, routes, DNS and firewall state to NetBird-run infrastructure without its operator ever configuring that, and had no way to point those paths anywhere else. #7147 and #7153 gave the remote job a per-job URL and an MDM override, but neither reaches the mobile, UI or CLI paths, and both fail open when unset. Publish the destination from the management server instead, on the channel that already carries stun/turn/signal/relay/flow/metrics: - `NetbirdConfig.debug.upload_url`, sourced from the new account setting `debug_bundle_upload_url` (REST + dashboard) and falling back to the new `DebugUpload.URL` in the management server config, which a self-hosted install can set once so a fresh account is not left on the vendor default. Both are validated as https-with-host where they are written; a change fans out to connected peers rather than waiting for the next login. - One resolver on the client, `debug.ResolveUploadURL`, used by every path: MDM override > explicitly named URL > destination published by management > the NetBird service, but only for a peer enrolled with NetBird's cloud. Anything else fails closed with ErrNoUploadDestination and the bundle stays local, which is the behaviour change: a self-hosted deployment that names no upload service no longer uploads at all. - The engine keeps the published value (`Engine.DebugUploadURL`) so the bundle paths, which run off the engine loop, do not have to read it back out of the opt-in sync-response store. - The daemon request grows `upload`, so "upload to wherever this deployment says" is expressible; an empty `uploadURL` no longer has to mean "no upload". The privilege gate is unchanged and still applies only to a URL the local caller named — a destination published by management is the operator naming their own service. - The desktop UI stops carrying a vendor URL of its own and sends the intent. Reported privately as GHSA-hf99-43rj-h577. |
||
|
Riccardo ManfrinandZoltan Papp
|
e14006ddc1 |
[client] mobile MDM bridge — iOS + Android setMDMPolicyFetcher entrypoint (#6435)
* MDM Android mobile wiring * Removes dead code * Removes static vars * Now we need to apply MDM in the GetConfig * You now need to explicitly call these around * Adds iOS wiring * Resolve merge conflicts from main - login.go: keep both new imports (mdm + nbnet + server) - ios/NetBirdSDK/client.go: additive struct-field merge (mdmLoader + stateMu/connectClient/config) - setconfig_mdm_test.go: adopt new withMDMPolicy(t, s, policy) signature; fix stray old-signature call in TestSetConfig_MDMAllow_ManagementURLPortNormalized * Convey MDM overlay config to Debug Bundle output Aligns to other clients OSes behavior * Solved conflict in client.go * Fixup helper withMDMPolicy -> configWithMDM * Fixup after merge * Resolve merge conflicts * [client] Move MDM enforcement logic into a shared Go layer (#7319) The mobile bridges only carried the policy fetcher, leaving every enforcement decision to the native apps: the desktop derived its UI restrictions in the Wails service layer, the daemon kept the conflict machinery in the server package, and both mobile bridges duplicated the JSON fetch adapter. Anything the native side had to reimplement was a place for iOS and Android to drift apart. Enforcement now lives in client/mdm and is consumed identically by all three platforms: - conflicts.go holds the value-aware conflict checks lifted out of the daemon, so the same normalization (canonical URLs, PSK sentinel echo) applies wherever a config change is validated. - restrictions.go derives the UI enforcement snapshot from a policy and renders it in the JSON shape the desktop frontend already consumes. The service-layer types become aliases, keeping one source of truth. - jsonloader.go replaces the adapter that was copy-pasted into both bridges. - changedetector.go moves change detection off the native side: the caller forwards the OS notification and asks whether the managed configuration actually changed, instead of diffing dictionaries itself. The mobile bridges gain the enforcement the daemon already had. The Preferences getters resolve managed keys from the policy, so a naive UI shows the enforced value; Commit rejects a staged change that diverges from a managed key; NewAuth resolves the managed management URL before persisting the config and overlays the policy on it, so a login can no longer run against a URL the policy forbids. Android's profile mutations fail closed when disableProfiles is set. NewAuth takes the fetcher as a required argument rather than keeping a policy-blind overload: the apps consume this code as a submodule, so a compile error at the bump is the point. The mobile PSK getter is replaced by a presence check — the key has no reason to cross the bridge, and not returning it means the native side needs no redaction sentinel of its own. * [client] Resolve the main merge conflicts in the MDM integration The merge commit was recorded with the conflict markers still in the tree. Resolve them so the branch builds again: - client/ios/NetBirdSDK: keep both the mdm and mobile imports, and keep the mdmLoader/mdmDetector fields next to main's stateMu documentation. - client/server/mdm.go: drop the conflict helpers main added locally, they already live in the client/mdm package on this branch, and keep the new checks main introduced (allowRemoteJobs, enableLocalMetrics, localMetricsAddress) as calls into the package-level helpers. - client/mdm/conflicts.go: add ConflictStringPtr, the presence-aware string check main needs for the optional localMetricsAddress field. - Port the two tests main added over the per-Server loader helper and the configWithMDM helper, both of which replaced the package-level policy injection this branch removed. * [client] Reject explicit empty PSK when MDM enforces a pre-shared key The SetConfig, Login and mobile Commit conflict checks collapsed the PSK to a plain string, so an explicit empty value was indistinguishable from an unset field and slipped past the MDM gate, clearing the persisted key. Carry the optional field as a pointer through ConflictStringPtr, treating only the redaction sentinel as a no-op echo. ConflictString had no other callers and is removed. * [client] Apply MDM overlay on the preloaded iOS config in Run Run only overlaid the MDM policy when the config was loaded from file, so the tvOS path fed by SetConfigFromJSON started with unmanaged settings. Apply the overlay after the config source is selected, as the other resolution sites already do. * [client] Gate non-active profile logout behind the MDM profiles switch The mobile ProfileManager let LogoutProfile clear credentials of any profile even when disableProfiles was enforced. Follow the daemon's validateProfileLogout semantics: logging out of the active profile is a plain logout and stays allowed, logging out of any other profile is profile management and is rejected under the policy. * [client] Resolve the managed management URL through the MDM overlay on mobile NewAuth on Android and iOS replaced the caller URL with the raw policy value before persisting, so a malformed managed URL failed config validation and blocked the login instead of being skipped with a warning like the overlay does. Preferences.GetManagementURL likewise echoed the raw policy string to the native UI even when the overlay had rejected it. Follow the daemon: persist the caller URL, overlay the policy on the resolved config, and report the overlaid ManagementURL as the effective value. * [client] Clean up MDM review leftovers Drop the unused ChangeDetector.Current, point the stale LoadPolicy comment references at Loader.Load, and move the profileEmail godoc back above its function. * [client] Check remote jobs and local metrics keys in the mobile MDM conflict gate MDMConflicts skipped allowRemoteJobs, enableLocalMetrics and localMetricsAddress even though the overlay applies all three and the daemon gate already checks them, so a mobile Commit could persist values diverging from the enforced policy. Align the list with the daemon. * [client] Silence the deprecated PreSharedKey lint in the login conflict test The legacy LoginRequest.PreSharedKey field is deliberately exercised by the test, matching the nolint already carried by the production path. * [client] Publish the mobile MDM loader and detector atomically SetMDMPolicyFetcher wrote the loader and change detector as two plain fields that Run, the OS-change callback and the restrictions getter read from other threads without synchronization. Hold both behind a single atomic pointer so a registration is published as one unit and readers always observe a matching loader and detector pair; Preferences gets the same treatment for its loader. Exported signatures are unchanged. * [client] Report the MDM-overlaid remote jobs value from mobile Preferences GetRemoteJobsAllowed returned the staged or persisted value even when the policy manages allowRemoteJobs, so the native settings UI could show a value the Commit gate would reject. Resolve it through the overlay like GetManagementURL does. * [client] Stop persisting the MDM-overlaid config after mobile logins NewAuth already writes the config through UpdateOrCreateConfig before the MDM policy is overlaid, and the login itself never mutates the Config. The post-login WriteOutConfig calls therefore only rewrote the same file with the enforced ManagementURL and PreSharedKey in it, so a removed or changed policy kept acting through the persisted values. * [client] Document that the MDM overlay on Config is not reversible ApplyMDMPolicy promised that an empty Policy clears a prior overlay, but applyMDMPolicy only resets the enforcement metadata and the runtime-only upload URL; the enforced ManagementURL, PreSharedKey and flags stay. Every lifecycle owner resolves the base Config again before applying, so state that contract instead of the reversibility that was never implemented. * [client] Re-resolve the tvOS preloaded config before every MDM overlay The iOS Client kept the config parsed from SetConfigFromJSON and applied the MDM overlay onto that same instance on every Run, IsLoginRequired and DebugBundle, so a key removed from the policy stayed enforced. Store the JSON instead and parse it per load through one loadConfig path. Auth serialized the overlaid config from GetConfigJSON, which tvOS then persisted to UserDefaults and fed back as the preload. Keep the resolved config as the base, run the login on a JSON round-trip copy with the overlay, and return the base from GetConfigJSON. * [client] Serve the MDM-managed management URL without touching the config file on mobile Preferences.GetManagementURL resolved a managed URL by reading and overlaying the persisted config, so a corrupt file or the tvOS sandbox turned an enforced URL into a read error. Return the canonical managed value directly, the same string BuildRestrictions already hands to the UI, and only fall back to the staged or persisted value when MDM does not manage the key. NewAuth validated the caller-supplied management URL before the overlay ran, so a malformed or echoed value blocked or persisted under an MDM policy that already dictates the URL. Ignore the caller value while the key is managed; the login runs against the overlay either way. * [client] Align the MDM loader docs with the fetcher precedence and make disableAdvancedView a tristate NewLoader, PolicyFetcher and the darwin/windows loadPlatform docs claimed the fetcher is unused on desktop, while every loader returns its values when one is injected. That precedence is the seam the server tests rely on across platforms, so the docs now describe it; production desktop callers still pass nil and keep the registry / plist authoritative. Fields.DisableAdvancedView collapsed "managed and false" into the same JSON as "not managed", unlike AllowServerSSH and the daemon's optional proto field. Carry it as a *bool so the UIs can tell the two apart; the desktop reflect loop skips pointer fields already, and the mobile decoders treat null as not managed. * [client] Clean up MDM review nits - ResolveConflicts treats a managed key whose ConflictCheck has no Check as a conflict instead of dereferencing nil. - Ticker.Run and ChangeDetector.Changed share policyChanged so the diff semantics and the log line cannot drift apart. - TestLoader_NilFetcherReturnsEmpty skips on windows/darwin, where a nil fetcher reads the real registry / plist. - The profilemanager test loader checks GetInt before GetBool so integer keys survive the round trip, and the PSK tests use the exported redaction sentinel. * [client] Fix int policy values coercing to bool in the MDM test helper withMDMPolicy rebuilt the policy map by trying GetString, then GetBool, then GetInt. Policy.GetBool accepts native ints (non-zero means true), so an int-valued key such as wireguardPort round-tripped through the helper as the bool true and GetInt was never reached. Try GetInt before GetBool, as the profilemanager helper already does; GetInt does not coerce bools, so booleans still fall through to GetBool. No test sets an int key today, so this was latent: the first test to exercise the wireguardPort conflict gate would have seen ConflictInt64 report a conflict for every value, including a matching one. --------- Co-authored-by: Zoltan Papp <zoltan.pmail@gmail.com> |
||
|
Riccardo Manfrin
|
7b22d55bf6 |
[client] Bind the cached SSH JWT to the local caller that obtained it (#7378)
* [client] Bind the cached SSH JWT to the local caller that obtained it Record the identity that obtained the token and return it only to that same identity, comparing the account alone: the group set and the elevation flag describe what a token may do rather than who it belongs to, and the same user may call once elevated and once not. A control channel that carries no caller identity gets a miss on read and stores nothing on write, matching how the other ipcauth consumers fail closed. Clear the entry when the session it speaks for ends: logout, down and profile switch. * [client] Cover the profile-switch path of the SSH JWT cache The cache being correct buys nothing if a handler around it forgets to clear it, and SwitchProfile had no test at all. Point the profile globals at a temp dir holding a single default profile, which is the one ActiveProfileState.FilePath resolves without consulting the current OS user, and call SwitchProfile with no request so neither the switch itself nor the profile-list event is involved. * [client] Report the SSH JWT cache in the no-identity startup warning daemonServerOptions already warns once, at startup, about what a control channel with no caller identity gives up. Name the SSH JWT cache there too, on both the TCP and the no-peer-identity-primitive paths. The per-request logs in cachedJWT and WaitJWTToken drop to Debug: the condition is expected and handled on such a channel, the caller simply re-authenticates, and repeating it on every SSH authentication buried the one message that is actionable. * [client] Stop the local-metrics manager leaking out of the profile test localmetrics.NewManager runs a goroutine until its context is done, and the test handed it context.Background(), so the manager outlived the test and stayed in the test binary for every case that followed. * [client] Keep the cached SSH JWT across a down/up cycle Clearing the cache in cleanupConnection also caught Down, which ends the connection and not the session: the peer stays enrolled, `up` reconnects without going back to the IdP, and the token still belongs to the same NetBird identity. With a long cache TTL that cost the owner a fresh device-code flow for nothing, since the owner binding is what keeps the token away from other local accounts. Clear it on the two paths where the session really ends and the next one may belong to a different NetBird user: profile logout when the profile is the active one, and active-profile logout. SwitchProfile already cleared it on its own. * [client] Resolve the merge conflict in the profile-logout cleanup main extracted the inline profile-logout cleanup into cleanupAfterProfileLogout, which this branch had edited in place to clear the SSH JWT cache. Take main's helper and move the clear inside it. The helper returns early when the profile that was deregistered is not the active one, so the cache is still only cleared when the session that owns the token actually ends. * [client] Do not cache an SSH JWT obtained under a session that ended WaitJWTToken polls the IdP with s.mutex released, and that wait can run for as long as the user takes in the browser. A logout or a profile switch in the meantime clears the cache, but the poll then completed and stored its token anyway, so the entry the next session read belonged to the previous one. Give the cache a generation that clear advances. WaitJWTToken takes the generation before the wait and hands it back to store, which keeps the token only while the generation still matches. The two mutexes are distinct, so this was never a data race and the race detector could not have found it: the window is between two separately locked sections. * [client] Make the profile-switch test switch a profile SwitchProfile with a nil request skips switchProfileIfNeeded, so the test only covered the no-op path and would have passed with profile-transition invalidation broken. Create a second profile and name it in the request, then assert the active profile actually moved before checking the cache. Also correct the comment on the Down test: the logout handlers do call cleanupConnection. What changed is that clearing the cache is no longer one of the things cleanupConnection does. * [client] Take the SSH JWT cache generation when the flow is created WaitJWTToken read the generation after validating the device code, but the flow it belongs to is created earlier, in RequestJWTAuth, and SwitchProfile does not reset s.oauthAuthFlow. A profile switch between the two therefore advanced the generation before it was ever read: the guard compared the new session against itself and let the token through, which is the case it exists to stop. Record the generation on the flow when RequestJWTAuth creates it, and read it from there. The whole span from the request to the IdP answering now counts as one session for the cache. * [client] Correct two test comments the clear-on-Down change invalidated Moving the clear out of cleanupConnection left two comments describing the old behaviour: newTestServer said cleanupConnection clears the cache, and the comment above TestJWTCache_ClearDropsTheEntry listed Down among the callers of clear. Neither is true any more. * [client] Read the SSH JWT cache generation before the IdP round trip RequestJWTAuth read the generation where it stored the flow, which is after RequestAuthInfo has talked to the IdP. A logout or a profile switch during that call advanced the generation first, so the flow recorded the new session's value and the later store was accepted: the window moved rather than closed. Read it with the config, under the same s.mutex section. SwitchProfile holds that mutex across its own clear(), so the config and the generation cannot be torn apart by a switch. |
||
|
Theodor Midtlien
|
e5c0cdf958 | [client] Stay connected with login command (#7384) | ||
|
Maycon Santos
|
ebc259e30b |
[management,client] Gate remote jobs behind an admin opt-in with MDM support (#7153)
This introduces a disabled-by-default allow-remote-jobs setting that controls whether the management server may run jobs (such as debug bundles) on a peer. The flag propagates end to end: through client configuration, the daemon SetConfig and Login requests, authentication, and system info, up to management, where it is stored on the peer and exposed on the peers API as remote_jobs_allowed. The client refuses any management-requested job unless the peer has opted in. Because enabling remote jobs crosses the user-to-root boundary, turning it on requires privilege, mirroring the SSH-server gate. Administrators can enforce the setting through MDM policy on both macOS and Windows, and MDM can also override the debug-bundle upload URL. The change ships policy documentation and generated profile templates, and adds configuration, conflict, and enforcement tests covering the opt-in, privilege, and MDM paths. |
||
|
Riccardo Manfrin
|
c170905bc9 |
[client] Allow logging out of the active profile when profiles are disabled (#7360)
* [client] Allow logging out of the active profile when profiles are disabled
A profile-addressed logout was refused outright when the profiles feature is
disabled: handleProfileLogout ran validateProfileOperation, which returned
Unavailable ("profiles are disabled, you cannot use this feature without
profiles enabled") before looking at which profile was targeted.
The desktop UI always addresses logout by profile — both the profile menu and
the session-expiration dialog send the active profile's ID — so a client with
profiles disabled could not log out at all; only a plain `netbird logout`,
which takes the profile-less path, still worked. Logging out of the profile the
daemon is already running is a deregistration, not profile management, and with
profiles disabled there is a single profile anyway, so every profile-addressed
logout is by definition an active-profile logout.
Replace validateProfileOperation with validateProfileLogout, which skips the
profiles-disabled check when the target is the active profile and keeps gating
logout of any other profile. This mirrors switchProfileIfNeeded, which already
gates only the branch that actually manages profiles. The dropped
allowActiveProfile parameter was always true, leaving canRemoveProfile
unreachable, so both are removed.
* [client] Compare the username and propagate state errors on profile logout
Review follow-ups on the logout gate:
Propagate the GetActiveProfileState failure instead of discarding it. A failed
lookup made the target look non-active, so a caller with profiles disabled got
"profiles are disabled" in place of the real error.
Compare the username along with the ID when deciding whether the target is the
active profile, matching switchProfileIfNeeded. Legacy profile IDs are display
names, so two users can hold the same ID in their own profile directories, and
an ID-only match let one user's logout pass the gate against the other user's
active profile. The default profile is shared and carries no username, so it
keeps matching on the ID alone.
Re-read the active profile before the connection teardown rather than reusing
the pre-flight snapshot. Login switches profiles under guardedConfigMu, which
the logout path does not hold, so a login that landed while the deregistration
was in flight would otherwise lose its fresh connection to a stale flag.
* [client] Address review on the profile logout gate
Pass the username down to logoutFromProfile and reuse the running config only
when the target is the active profile for that username. On an ID-only match a
legacy profile ID shared between two users made the connected-client path
deregister the active peer while its connection stayed up, which the gate fix
alone did not cover.
Split the setup-key-less branch of Login into beginSSOLogin, with the
reuse-the-pending-flow decision in pendingOAuthFlowResponse. Login's cognitive
complexity drops from 37 to 21 (gocognit), clearing the SonarQube report on
this file with no behaviour change.
Point the test fixture at an https URL, since the profiles a gated logout must
not touch only need to be unreachable, not plaintext.
|
||
|
Viktor Liu
|
63c26be72f | [client] Add local Prometheus metrics endpoint (#6689) | ||
|
Pascal FischerandDmitri Dolguikh
|
e06c17cf59 |
[management] network map from nmap data type (#6919)
Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> Co-authored-by: Dmitri Dolguikh <dmitri.external@netbird.io> |
||
|
Viktor Liu
|
e4b8bf39d2 |
[client] Fix staticcheck findings from the updated golangci-lint (#7266)
* Fix staticcheck findings reported by the updated golangci-lint * Skip the receive error log when the local context is done |
||
|
dmitri-netbird
|
a144e8c144 |
[client, management] switch to go.uber.org/mock (#7253)
* switch to go.uber.org/mock/gomock Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> * updated go:generate commands + regenerated mocks Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> * update go:generate mockgen commands Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> * removed duplicate import Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> * fix go:generate Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> --------- Signed-off-by: Dmitri Dolguikh <dmitri.external@netbird.io> |
||
|
Viktor Liu
|
e290769df1 | [client] Take the graphical session answer from the caller instead of the daemon environment (#7187) | ||
|
Viktor Liu
|
5584f8ef0a | [client] Add strict anonymization level and MAC anonymization to debug bundles (#7102) | ||
|
Zoltan Papp
|
f2d13b884a |
[client] Fix session expired relogin (#7055)
## Describe your changes After the SSO session expires, the daemon tears the engine down permanently (management returns `PermissionDenied` → `runCancel()` → the retry loop exits for good). The "Session expired" dialog's Login button still drove the extend-session flow, which requires a live engine: the user completed the full browser SSO + 2FA round trip only to get `Failed to extend the session — engine is not initialised`, with no way out other than quitting and relaunching the client. Reproduce: 1. Log in on a desktop client with session expiration enabled (e.g. 16h TTL). 2. Let the session expire (e.g. leave the machine asleep overnight). 3. Wake it, click **Login** on the "Session expired" dialog, complete SSO + 2FA. 4. The error dialog appears and every retry fails the same way. Changes: - The expired branch of the session-expiration dialog now emits `trigger-login`, driving the full `Login → SSO → Up` sequence that rebuilds the client, instead of the extend flow (an expired session can no longer be extended). - `RequestExtendAuthSession` fails fast when the engine is already gone, so the browser/2FA round trip is not wasted on a doomed extend. - The expired tray row navigated the main window to `/#/login`, a route that does not exist and fell through to the main page without starting a login; it now emits `trigger-login` as well. ## Issue ticket number and link <!-- Required for anything that changes behavior. Link the issue (or the validated discussion it came from) that the NetBird team already agreed on. See https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#ticket-first-pr-second --> ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] I ran and tested this change locally — I did not rely on CI to find out whether it works - [ ] This PR has a single purpose (not a fix + refactor + feature in one) - [ ] This change is a trivial fix, **OR** it links an issue the NetBird team agreed on beforehand. Changes to the public API, gRPC protocols, functionality behavior, CLI / service flags, or new features always need that agreement first. See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#ticket-first-pr-second). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved session extension handling when the client engine is unavailable by prompting users to log in again. - Updated expired-session behavior to trigger the standard login flow, providing a more consistent sign-in experience. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Zoltan Papp
|
78c1c2fc32 |
[client] Probe the daemon login with IsLoginRequired (#7052)
## Describe your changes
Probe the daemon login with IsLoginRequired
The Login probe attemptLogin(ctx, "", "") on an unregistered peer ends
in registerPeer with no setup key and no JWT, which fails locally with
InvalidArgument before reaching Management. Since #6983 classified that
as StatusLoginFailed and returned early, every setup-key enrolment and
every expired-session SSO re-login aborted before using its credentials,
breaking all netbird-cloud e2e runs from commit
|
||
|
Viktor Liu
|
e90be36cd5 | [client] Don't ask for an SSO login when the login never reached management (#6983) | ||
|
Viktor Liu
|
feecb993f4 | [client] Restrict debug bundle log path and upload destinations (#6975) | ||
|
Viktor Liu
|
aed60a2432 | [client] Fix daemon lock order inversion between SetConfig and login (#6978) | ||
|
Viktor Liu
|
0f5d2d91fb | [client] Authorize daemon IPC callers by their local identity (#6967) | ||
|
Zoltan Papp
|
2ef457be95 |
[client] Unify route selection in the route manager (#6928)
## Describe your changes Move route select/deselect handling from the daemon server into exported routemanager methods (SelectRoutes, DeselectRoutes, SelectAllRoutes, DeselectAllRoutes) so every consumer shares one implementation: v4/v6 exit-pair expansion, exit-node mutual exclusion, and selection triggering. Previously the exit-node exclusivity lived only in the daemon's SelectNetworks RPC, so the Android and iOS bindings could leave two exit nodes selected until the next network map reconciliation. Both bindings now call the shared manager methods and enforce exclusivity at toggle time, matching the desktop behavior. ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6928"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1787772098&installation_model_id=427504&pr_number=6928&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6928&signature=31ad59e1483e1582cd447a8db2fe21e5309230e631cbd0cad0f977cd15fb7b9b"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith-bot</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Route selection/deselection is now handled through shared route-manager APIs for both individual routes and “all routes”. * Exit-node selections automatically enforce mutual exclusivity while keeping non-exit routes unaffected. * **Bug Fixes** * Unknown or unavailable route IDs now return errors, and exclusivity is preserved even when some route IDs fail. * **Tests** * Added route-selection tests covering exclusivity (including IPv4/IPv6), select-all behavior, partial errors, and invalid IDs. * **Refactor / Chores** * Simplified Android, iOS, and server routing flows to delegate to the shared manager; updated mocks and removed redundant routing command logic/dependencies. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Zoltan Papp
|
6fc05efa6c |
[client] Disconnect daemon on GUI quit via async Down (#6796)
The tray Quit menu now disconnects the daemon before exiting instead of only tearing down the GUI. A new DownAsync RPC lets the daemon start the teardown and return immediately: beginDown cancels the connection under the mutex (so it cannot reconnect), then finishDown (the retry-goroutine wait and status reset) runs on a background goroutine. handleQuit aborts any in-flight profile switch first (so a queued Up cannot reconnect during teardown) and calls DownAsync so quitting never blocks on the engine shutdown. ## Describe your changes ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [x] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6796"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1786785261&installation_id=146802194&pr_number=6796&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6796&signature=4fc95ebfed320240170c8318a2bc8750acfa5ad131454e3011c43bc8a5a09ed1"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>/codesmith</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved shutdown reliability by continuing teardown even when stopping the service fails (stop errors are logged but not returned). * Refined connection shutdown to return “service not up” errors directly for clearer, more immediate RPC behavior. * Prevented shutdown hangs by making the tray Quit disconnect time-bounded (5 seconds). * Ensured any in-flight profile switch is cancelled before exiting, with quit serialized to avoid races. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
David Fry
|
e70a69bbcf |
[client] Restore residual state in foreground mode before login (#6707)
* Improved residual state restoration during foreground startup and foreground login, ensuring consistent recovery with stale states. * Foreground flows now initialize advanced routing so stale routes are bypassed during login. |
||
|
Maycon Santos
|
8e02154bf5 |
[client] Add SSO login flow timing instrumentation (#6717)
Users reported long delays between finishing browser authentication and the client connecting. Logs could not attribute the time: the PKCE and device flows were silent between issuing the auth URL and returning the token, and nothing recorded when the GUI issued the Up request after WaitSSOLogin completed. Add log lines covering the full chain: PKCE callback arrival and token exchange duration, device-flow polling and approval timing, GUI-side brackets around WaitSSOLogin and Up, daemon-side Up arrival and WaitSSOLogin return, and a frontend stall detector that reports when webview timers were suspended (macOS App Nap / hidden-window throttling), which delays the WaitSSOLogin-to-Up handoff. |
||
|
|
91acb8147c |
[management,client] 0.75.0 release with new desktop UI (#6473)
- **Wails v3 application** (`client/ui`) with a React + TypeScript + Tailwind frontend replacing the Fyne UI: main connection view, exit-node switcher, networks/peers browser with detail panels, profile management, settings (general, network, SSH, security, troubleshooting, appearance), debug-bundle creation, and a first-run welcome flow. - **Internationalization**: go-i18n bundle with 9 locales (en, de, es, fr, hu, it, pt, ru, zh-CN) shared between the tray and the frontend. - **New system tray** implementation with per-platform theme-aware icons, including a native XEmbed host for Linux (`xembed_tray_linux.c`) and a Linux theme watcher. - **Session handling**: auth session watcher (`client/internal/auth/sessionwatch`), pending login flow, session-expiration dialog and tray notifications, and `netbird login` improvements. - **Daemon API extensions** (`daemon.proto`): status stream subscription, event stream, networks/exit-node selection endpoints, and richer full status — with probe throttling on the daemon side to protect against UI-driven request storms. - **UI preferences store** persisted per profile, autostart management via the daemon (single source of truth in HKCU on Windows). - **Build system**: Taskfile-based builds per platform (macOS, Linux, Windows), Docker cross-compilation images, MSIX/NSIS/nfpm/AppImage packaging, and a new `frontend-ui` CI workflow. Co-authored-by: Zoltan Papp <zoltan.pmail@gmail.com> Co-authored-by: Eduard Gert <kontakt@eduardgert.de> Co-authored-by: braginini <bangvalo@gmail.com> Co-authored-by: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Co-authored-by: riccardom <riccardomanfrin@gmail.com> |
||
|
Riccardo Manfrin
|
c9d387bd0d |
[client] fix MDM managementURL conflict on default-port URL echo (#6672)
* Adds failing test * Fixes Management URL normalized compare on MDM |
||
|
Viktor Liu
|
1d8b5f6e5c | [client] Make lazy connections opt-out via NB_LAZY_CONN (#6617) | ||
|
Zoltan Pappandcoderabbitai[bot]
|
2d7b309004 |
[client] Categorize privileged tests behind a build tag and run them in Docker (#6425)
* [client] categorize root/system-mutating tests behind a privileged build tag Tests that need root or mutate host state (nftables/iptables/DNS, TUN/WireGuard interfaces, routes, eBPF, SSH/service install) are now gated behind a //go:build privileged tag. The default `go test ./client/...` runs as a non-root user with no sudo and leaves host networking untouched; mixed files were split so pure-logic tests stay in the default suite. A self-hosting ory/dockertest/v4 harness (client/testutil/privileged) runs the privileged suite inside a --privileged --cap-add=NET_ADMIN container via `make test-privileged`; a DOCKER_CI=true guard skips the spawn when already inside the container. Added `make test-unit` for the host-safe run. * [client] add PRIV_RUN/PRIV_PKGS filters to the privileged test harness The dockertest harness now reads two optional env vars when building the in-container `go test` command: PRIV_RUN adds a -run test-name filter and PRIV_PKGS overrides the package list. Both empty reproduce the full privileged suite, so CI and `make test-privileged` behave as before. Lets a developer run a single privileged test in the container, e.g.: PRIV_RUN=TestNftablesManager PRIV_PKGS=./client/firewall/nftables/... make test-privileged * [client] fix unused-helper lint after the privileged test split Splitting privileged tests into *_privileged_test.go left their shared helpers in the untagged files, so in the default (no-tag) build they had no callers and golangci-lint flagged them as unused. Moved the privileged-only helpers into the privileged files next to their callers (generateDummyHandler; createEngine/startSignal/startManagement/getConnectedPeers/ getPeers + kaep/kasp; (*mockDaemon).setJWTToken). Annotated the shared routing-test fixtures that must stay untagged for cross-platform compilation with //nolint:unused (systemops_bsd expected* vars, ensureIPv6DefaultRoute on bsd/windows, loopbackIfaceWindows), matching the existing linux variant. * [client] fix privileged test CI failures and run the harness on macOS The host-safe unit run dropped sudo but two privileged test groups were never tagged, and the Docker privileged job silently never ran the suite: - Gate the ssh/server PrivilegeDropper command-construction tests behind the privileged tag (they require root to target a different UID); split them into executor_unix_privileged_test.go. - Tag sharedsock raw-socket tests privileged (need CAP_NET_RAW). - Fix the Docker job command: nested single quotes around the build tags closed the sh -c wrapper early, dropping the go list package set and the privileged tag, so go test ran on the empty repo root. Use double quotes. Make the self-hosting harness usable from a dev Mac: - Build it on darwin as well as linux; it only drives Docker. - Resolve the active docker context endpoint into DOCKER_HOST when the default /var/run/docker.sock is absent (Docker Desktop, Colima, OrbStack). - Rename the misspelled containerGoModache constant to containerGoModCache. * Update client/internal/engine_privileged_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update client/internal/routemanager/systemops/systemops_linux_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update client/internal/routemanager/systemops/systemops_windows_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update client/server/server_privileged_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * [ci] Run privileged-tagged tests on darwin, windows and freebsd The privileged build tag split moved root/system-mutating tests behind //go:build privileged, but only the linux docker job was given the tag. The native darwin (sudo), windows (PsExec64 -s) and freebsd VM runners already have the required privileges, so add the privileged tag there too to keep CI running the same set of tests as before the split. * [ci] Exclude dockertest harness from the darwin privileged run The privileged tag now compiles client/testutil/privileged on darwin, whose TestRunPrivilegedSuiteInDocker spawns a container the macOS runner has no Docker for. Exclude the harness package from the darwin list, matching the linux job, so the privileged tests run in place without a container spawn. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |
||
|
Zoltan Papp
|
ac9529ea8c |
[client] Fix engine lifecyrcle race (#6443)
* [client] always clean up on Engine.Start failure via defer The rosenpass init paths (NewManager/Run) returned without calling e.close(), leaking the WireGuard interface and other partially initialized state on failure. Per-branch cleanup was easy to miss when adding new early returns. Convert Start to a named error return and tear down via a single defer that calls e.close() whenever err != nil, removing the scattered per-branch close() calls (including the redundant one in initFirewall). * [client] make Engine single-use and guard against double Start Create the run context once in NewEngine instead of in Start. This keeps e.cancel valid for the engine's whole lifetime, so Stop can cancel a Start that is blocked waiting on the network while holding syncMsgMux: Stop now cancels before taking the lock, unblocking that Start so it can release the mutex. Reject re-entry into Start: a non-nil wgInterface means a prior Start already ran (ErrEngineAlreadyStarted), and a cancelled run context means the engine was stopped (ErrEngineAlreadyStopped). Both checks run before the cleanup defer so a duplicate call cannot tear down the running engine's state. * [client] let engine context unblock WaitStreamConnected WaitStreamConnected only watched the signal client's own context, which derives from the parent engineCtx rather than the engine's run context. A Start blocked here (signal stream not yet up) could therefore not be released by Engine.Stop, since Stop only cancels the engine's run context. Pass a context into WaitStreamConnected and select on it too, and have the engine pass e.ctx, so Stop cancelling e.ctx unblocks a parked Start. Update the Client interface, the mock, and callers accordingly. * [client] fix Start/Stop race by making the run loop own engine shutdown ConnectClient.Stop stopped the engine directly while the run loop's backoff cycle could still be starting an engine, so Engine.close raced Engine.Start (e.g. firewall setup reading wgInterface while close nils it). embed.Client.Start's rollback only avoided a deadlock by cancelling before Stop; the race itself remained and was caught by -race. Make the run loop the sole owner of engine shutdown: derive the run context in NewConnectClient, and have Stop cancel it and wait for the loop to exit (skipping the wait when the loop never ran) instead of calling engine.Stop. The loop now always stops the engine on its way out, dropping the unsynchronised wgInterface check it used to guard that call. Self-calls from within the loop use runCancel to avoid waiting on themselves. embed keeps a defensive pre-Stop cancel(); the daemon's cleanupConnection gets a TODO to adopt Stop() rather than stopping the engine in parallel. * [client] init context state in engine tests Engine tests built the engine context with context.WithCancel( context.Background()), omitting CtxInitState. Now that the run context is created in the constructor, the wgIfaceMonitor goroutine can reach triggerClientRestart during teardown, which calls CtxGetState and panics on the missing state. Real entry points (up, embed, service) always CtxInitState; only the tests skipped it. * [client] interrupt connect backoff on context cancel The run loop retried with a raw ExponentialBackOff, so a backoff sleep ignored context cancellation. Now that ConnectClient.Stop waits for the run loop to exit, a cancel landing during a sleep would block Stop for the full interval (up to MaxInterval). Wrap the backoff with the run context so Retry returns promptly on cancel; the retry budget itself (MaxElapsedTime) is unchanged. * [client] bound WaitStreamConnected in signal client tests The tests waited on WaitStreamConnected with context.Background() and the client's own context was also Background, so a stream that never connects would hang until the suite timeout. Pass a 5s timeout context and assert StreamConnected afterwards so the tests fail fast with a clear reason. * [client] fix WaitStreamConnected stale-channel race The StreamConnected check and the wait-channel creation took the mutex separately, so notifyStreamConnected could set the status and close/clear connectedCh in between: the waiter then created a fresh channel nobody would ever close and blocked forever. Also, the status read was unlocked while notify wrote it under the mutex (a data race). Do the check and the channel fetch in one locked section; drop the now-unused getStreamStatusChan helper. Pre-existing bug, not introduced by this branch. * [client] abort Start if context cancelled while waiting for signal stream receiveSignalEvents blocks in WaitStreamConnected until the signal stream connects or the context is cancelled. If Stop cancelled e.ctx while Start was parked there, Start kept going: it started the remaining subsystems on a cancelled context and marked a shutting-down engine as started. Return the context error from receiveSignalEvents and propagate it from Start, so the deferred cleanup runs and the cancellation reaches the caller. * [client] clean up all started components on Start failure Start's failure defer only called close(), which covers the wg interface, firewall, rosenpass and port forwarding but leaves connMgr, srWatcher, route/DNS/flow/state managers and the monitor goroutines running. A late failure (e.g. the context-cancelled check after the signal stream) thus leaked them. Extract Stop's locked teardown into stopLocked (caller holds syncMsgMux, does not wait on shutdownWg) and call it from both Stop and Start's defer. The defer also cancels the run context first so goroutines started before the failure unwind. Teardown order is unchanged. |
||
|
Theodor Midtlien
|
ee360963f9 |
[client] Migrate profile identity from display name to ID and allow renaming of profiles (#6367)
* Migrate to profile ids * Migrate android profile manager * Clean up * Fix review * Add ID type * Fix test and runes in ShortID() * Fix profile switch on up and android comments * Revert android profile to string id * Fix feedback * Fix UI feedback * Fix id assignment * Add renaming of profiles * Fix review * Remove ui binary * Fix getProfileConfigPath not validating id * Change resolve handle order and fix server merge problems * Fix mdm test |
||
|
Riccardo Manfrin
|
2bcea9d582 |
[client] add MDM configuration profile support (Windows registry + macOS plist) (#6374)
* Initial scaffolding * Applies MDM override * Unit tests * Helpers business logic * Return error if trying to modify any config that is gated by MDM * Add ManagedFields to returned config over GetConfig * Adds initial 101 MDM policy business logic testing * gRPC MDM changes * MDM Name scoping for clarity * Implements windows loading of MDM policy * Adds missing WGPort config * Cleanup setupKey to align to linear * Align split tunnel code * Adds some log * Prefix every log with MDM * Adds debug config cobra command This can be useful for troubleshooting and checking config now that its resolution is not trivial defaults > config > env cars > CLI/UI > MDM * Adds MDM 1m diff checker & reloader * Adds also up/start after cancel * Publishes event for UI to sync upon MDM changes * Add events to resync UI to actual config This also provide fixup for UI no aligning to changed config when coming from cli up with config flags. * UI behavior conflicts relaxation UI sends full config snapshot with all values. It doesn't make sense to block it if the values are aligned with the values constrained by the MDM policy. It's just simplier to allow values that are compliant. (this goes for the CLI as well at this point) * Lock toggle Settngs * Advanced Settings locking * Fixup presharedkey * Apply MDM locks * Toggle gray in/out for Advanced Settings * Adds support for disabling of Profiles and UpdateSettings feature flags * Adds Gate Login as well when --disable-update-settings=true is given to service This commit tries to settle things with an old PR-4237 which had relaxed the case where the SetConfig returned an `Unavailable` code error. Under this circumnstance the PR allowed the upFunc to just emit a warning and progress further with the login gRPC. Since the login call is consuming the --management-url coming from the `up` command, it might be possible to abuse the "Unavailable" code to inject a management URL that is different from the configured one even though the --disable-update-settings is set to true (?) * Evaluate disable-update-settings errors only when there's an actual override * [UI] Fixup advanced Settings * [UI] Fixup for preshared key * [UI] Fixup for profile enable/disable toggle We need to align the initial state to evaluate the delta in case. The initial state has to be "true" since the profile starts visible. Then we receive MDM and transition the cache bool value to the actual MDM imposed state * Enforces disable networks * [UI] Aligns to "enable/disable once on change only" * Fixup: MDM wins. always * Removes --disable-advanced-settings It was a typo in our meetings. the actual thing is --disable-update-settings * [PROTO] Removes --disable-advanced-settings * [UI] Removes --disable-advanced-settings * Pins feat profile retrieval to notif event * [UI] Fix for "hide" not working when propagating to parent with children * Adds dep for reading plist files * Introduces support for darwing plist loading * Tests MDM config reload via ticker * [PROVISIONING] ADMX/ADML/PS/bash scripts/templates * CI fixes - Add docstrings to `mdm_integration` - refactor for cognitive complexity - mod tidy * Linting * Add docstrings to `mdm_integration` * nil,nil is no policy and no error. Allow it * nil,nil is no policy and no error. Allow it * exclude MDM profile adminstrated keys data from debug bundle * Fixes Rosenpass left disable after MDM unlock * Partial revert coderabbit added docstrings * Renaming fix * Avoid locking on clientRunning bool when the connection is aborted for whatever reason We want to just signal this through the giveUpChan, we will manage the signal from the waiter side and in case set it to false there. THis way we avoid locking, which should allow the MDM down+wait_for_term_chan_signal_+up procedure clientRunning is used to signal two different conditions here: 1. the initialization procedure is over (we have an engine) 2. the connection being up (or being attempted) Probably these two functionalities should not alias, and the failure of the second condition (because of any error) should just drive a reconnection (currently it's not happening, and we silently go idle). OR, mor probably, the two things are the SAME and there should not exist a case where we did the "Up" initialization and connection attempt but we are not still attempting it. * Moves test helper at te very bottom * Addresses github comments * No lock no copy * Prevents engine not stopping within 10 secs from being paired by another instance We instead juts SKIP updating the policy, so 1. the MDM ticker will kick in 1 minute time, 2. find the policy misaligned, 3. enter the onMDMPolicyChange, 4. find the s.clientRunning == true (because it is set to false only in server cleanupConnection, and not by s.actCancel()) 5. call s.actCancel() again if not nil 6. immediately return from <-s.clientGiveUpChan 7. finally call s.restartEngineForMDMLocked() * Since we ARE running there should be a config If the config was cancelled midflight, connect will abort later on * DisableAutoConnect should not stop a running connection. DisableAutoConnect should just avoid the connection attempts *when the service starts*. If we are started and we are up and running, DisableAutoConnect should not kick in. Another PR will follow about this topic * Removes unused vars * Moves callback into Run method arg * align comment to removal of DisableAutoConnect DisableAutoConnect should just avoid the connection attempts *when the service starts*. If we are started and we are up and running, DisableAutoConnect should not kick in * Removes unused managed_fields data. This was initially used to drive the UI but approach changed to reload config/features upon notifications which makes this data redundant. * Reorder stuff * Unexport unrequired vars/functions PoliciesEqual → policiesEqual AllKeys → allKeys * Adds list of MDM managed fields in the debug bundle |
||
|
Theodor Midtlien
|
512899d82d |
[client] Prevent corruption from competing log rotation and improve debug bundle (#6214)
* Adds heuristic to detect an edge case on Linux where a system has configured logrotate as a separate service to rotate log files which would mangle our client log files. If we detect logrotate being configured for netbird, we disable our rotation. * Adds new env var to disable log rotation: NB_LOG_DISABLE_ROTATION * Adds compressed and plain logrotate files to debug bundle. * Replaces lumberjack with timberjack (maintained fork with bug fixes and extra features). * Clarifies which daemon version is running in the bundle stats. * Change logging for client service status to console |
||
|
Bethuel Mmbaga
|
14af179556 | [management] Refactor management server bootstrap (#6256) | ||
|
Viktor Liu
|
1224d6e1ee | [client] Persist management URL and pre-shared key overrides on login (#6065) | ||
|
Viktor Liu
|
205ebcfda2 | [management, client] Add IPv6 overlay support (#5631) | ||
|
Viktor Liu
|
057d651d2e | [client, proxy] Add packet capture to debug bundle and CLI (#5891) | ||
|
Bethuel Mmbaga
|
df197d5001 | [management] Prevent JWT reuse during peer login (#6002) | ||
|
Viktor Liu
|
407e9d304b | [client] Move macOS sleep detection into the daemon (purego) (#5926) | ||
|
Viktor Liu
|
2e0e3a3601 | [client] Replace exclusion routes with scoped default + IP_BOUND_IF on macOS (#5918) | ||
|
Maycon Santos
|
53b04e512a |
[management] Reuse a single cache store across all management server consumers (#5889)
* Add support for legacy IDP cache environment variable * Centralize cache store creation to reuse a single Redis connection pool Each cache consumer (IDP cache, token store, PKCE store, secrets manager, EDR validator) was independently calling NewStore, creating separate Redis clients with their own connection pools — up to 1400 potential connections from a single management server process. Introduce a shared CacheStore() singleton on BaseServer that creates one store at boot and injects it into all consumers. Consumer constructors now receive a store.StoreInterface instead of creating their own. For Redis mode, all consumers share one connection pool (1000 max conns). For in-memory mode, all consumers share one GoCache instance. * Update management-integrations module to latest version * sync go.sum * Export `GetAddrFromEnv` to allow reuse across packages * Update management-integrations module version in go.mod and go.sum * Update management-integrations module version in go.mod and go.sum |
||
|
Viktor Liu
|
6b540d145c | [client] Add --disable-networks flag to block network selection (#5896) | ||
|
Viktor Liu
|
94a36cb53e | [client] Handle UPnP routers that only support permanent leases (#5826) | ||
|
Viktor Liu
|
d33cd4c95b | [client] Add NAT-PMP/UPnP support (#5202) | ||
|
Viktor Liu
|
aba5d6f0d2 | [client] Error out on netbird expose when block inbound is enabled (#5818) | ||
|
Zoltan PappandViktor Liu
|
91f0d5cefd |
[client] Feature/client metrics (#5512)
* Add client metrics * Add client metrics system with OpenTelemetry and VictoriaMetrics support Implements a comprehensive client metrics system to track peer connection stages and performance. The system supports multiple backend implementations (OpenTelemetry, VictoriaMetrics, and no-op) and tracks detailed connection stage durations from creation through WireGuard handshake. Key changes: - Add metrics package with pluggable backend implementations - Implement OpenTelemetry metrics backend - Implement VictoriaMetrics metrics backend - Add no-op metrics implementation for disabled state - Track connection stages: creation, semaphore, signaling, connection ready, and WireGuard handshake - Move WireGuard watcher functionality to conn.go - Refactor engine to integrate metrics tracking - Add metrics export endpoint in debug server * Add signaling metrics tracking for initial and reconnection attempts * Reset connection stage timestamps during reconnections to exclude unnecessary metrics tracking * Delete otel lib from client * Update unit tests * Invoke callback on handshake success in WireGuard watcher * Add Netbird version tracking to client metrics Integrate Netbird version into VictoriaMetrics backend and metrics labels. Update `ClientMetrics` constructor and metric name formatting to include version information. * Add sync duration tracking to client metrics Introduce `RecordSyncDuration` for measuring sync message processing time. Update all metrics implementations (VictoriaMetrics, no-op) to support the new method. Refactor `ClientMetrics` to use `AgentInfo` for static agent data. * Remove no-op metrics implementation and simplify ClientMetrics constructor Eliminate unused `noopMetrics` and refactor `ClientMetrics` to always use the VictoriaMetrics implementation. Update associated logic to reflect these changes. * Add total duration tracking for connection attempts Calculate total duration for both initial connections and reconnections, accounting for different timestamp scenarios. Update `Export` method to include Prometheus HELP comments. * Add metrics push support to VictoriaMetrics integration * [client] anchor connection metrics to first signal received * Remove creation_to_semaphore connection stage metric The semaphore queuing stage (Created → SemaphoreAcquired) is no longer tracked. Connection metrics now start from SignalingReceived. Updated docs and Grafana dashboard accordingly. * [client] Add remote push config for metrics with version-based eligibility Introduce remoteconfig.Manager that fetches a remote JSON config to control metrics push interval and restrict pushing to a specific agent version range. When NB_METRICS_INTERVAL is set, remote config is bypassed entirely for local override. * [client] Add WASM-compatible NewClientMetrics implementation Replace NewClientMetrics in metrics.go with a WASM-specific stub in metrics_js.go, returning nil for compatibility with JS builds. Simplify method usage for WASM targets. * Add missing file * Update default case in DeploymentType.String to return "unknown" instead of "selfhosted" * [client] Rework metrics to use timestamped samples instead of histograms Replace cumulative Prometheus histograms with timestamped point-in-time samples that are pushed once and cleared. This fixes metrics for sparse events (connections/syncs that happen once at startup) where rate() and increase() produced incorrect or empty results. Changes: - Switch from VictoriaMetrics histogram library to raw Prometheus text format with explicit millisecond timestamps - Reset samples after successful push (no resending stale data) - Rename connection_to_handshake → connection_to_wg_handshake - Add netbird_peer_connection_count metric for ICE vs Relay tracking - Simplify dashboard: point-based scatter plots, donut pie chart - Add maxStalenessInterval=1m to VictoriaMetrics to prevent forward-fill - Fix deployment_type Unknown returning "selfhosted" instead of "unknown" - Fix inverted shouldPush condition in push.go * [client] Add InfluxDB metrics backend alongside VictoriaMetrics Add influxdb.go with timestamped line protocol export for sparse one-shot events. Restore victoria.go to use proper Prometheus histograms. Update Grafana dashboards, add InfluxDB datasource, and update docs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [client] Fix metrics issues and update dev docker setup - Fix StopPush not clearing push state, preventing restart - Fix race condition reading currentConnPriority without lock in recordConnectionMetrics - Fix stale comment referencing old metrics server URL - Update docker-compose for InfluxDB: add scoped tokens, .env config, init scripts - Rename docker-compose.victoria.yml to docker-compose.yml * [client] Add anonymised peer tracking to pushed metrics Introduce peer_id and connection_pair_id tags to InfluxDB metrics. Public keys are hashed (truncated SHA-256) for anonymisation. The connection pair ID is deterministic regardless of which side computes it, enabling deduplication of reconnections in the ICE vs Relay dashboard. Also pin Grafana to v11.6.0 for file-based provisioning and fix datasource UID references. * Remove unused dependencies from go.mod and go.sum * Refactor InfluxDB ingest pipeline: extract validation logic - Move line validation logic to `validateLine` and `validateField` helper functions. - Improve error handling with structured validation and clearer separation of concerns. - Add stderr redirection for error messages in `create-tokens.sh`. * Set non-root user in Dockerfile for Ingest service * Fix Windows CI: command line too long * Remove Victoria metrics * Add hashed peer ID as Authorization header in metrics push * Revert influxdb in docker compose * Enable gzip compression and authorization validation for metrics push and ingest * Reducate code of complexity * Update debug documentation to include metrics.txt description * Increase `maxBodySize` limit to 50 MB and update gzip reader wrapping logic * Refactor deployment type detection to use URL parsing for improved accuracy * Update readme * Throttle remote config retries on fetch failure * Preserve first WG handshake timestamp, ignore rekeys * Skip adding empty metrics.txt to debug bundle in debug mode * Update default metrics server URL to https://ingest.netbird.io * Atomic metrics export-and-reset to prevent sample loss between Export and Reset calls * Fix doc * Refactor Push configuration to improve clarity and enforce minimum push interval * Remove `minPushInterval` and update push interval validation logic * Revert ExportAndReset, it is acceptable data loss * Fix metrics review issues: rename env var, remove stale infra, add tests - Rename NB_METRICS_ENABLED to NB_METRICS_PUSH_ENABLED to clarify that collection is always active (for debug bundles) and only push is opt-in - Change default config URL from staging to production (ingest.netbird.io) - Delete broken Prometheus dashboard (used non-existent metric names) - Delete unused VictoriaMetrics datasource config - Replace committed .env with .env.example containing placeholder values - Wire Grafana admin credentials through env vars in docker-compose - Make metricsStages a pointer to prevent reset-vs-write race on reconnect - Fix typed-nil interface in debug bundle path (GetClientMetrics) - Use deterministic field order in InfluxDB Export (sorted keys) - Replace Authorization header with X-Peer-ID for metrics push - Fix ingest server timeout to use time.Second instead of float - Fix gzip double-close, stale comments, trim log levels - Add tests for influxdb.go and MetricsStages * Add login duration metric, ingest tag validation, and duration bounds - Add netbird_login measurement recording login/auth duration to management server, with success/failure result tag - Validate InfluxDB tags against per-measurement allowlists in ingest server to prevent arbitrary tag injection - Cap all duration fields (*_seconds) at 300s instead of only total_seconds - Add ingest server tests for tag/field validation, bounds, and auth * Add arch tag to all metrics * Fix Grafana dashboard: add arch to drop columns, add login panels * Validate NB_METRICS_SERVER_URL is an absolute HTTP(S) URL * Address review comments: fix README wording, update stale comments * Clarify env var precedence does not bypass remote config eligibility * Remove accidentally committed pprof files --------- Co-authored-by: Viktor Liu <viktor@netbird.io> |
||
|
Viktor Liu
|
3e6baea405 | [management,proxy,client] Add L4 capabilities (TLS/TCP/UDP) (#5530) | ||
|
Zoltan Papp
|
fe9b844511 |
[client] refactor auto update workflow (#5448)
Auto-update logic moved out of the UI into a dedicated updatemanager.Manager service that runs in the connection layer. The UI no longer polls or checks for updates independently. The update manager supports three modes driven by the management server's auto-update policy: No policy set by mgm: checks GitHub for the latest version and notifies the user (previous behavior, now centralized) mgm enforces update: the "About" menu triggers installation directly instead of just downloading the file — user still initiates the action mgm forces update: installation proceeds automatically without user interaction updateManager lifecycle is now owned by daemon, giving the daemon server direct control via a new TriggerUpdate RPC Introduces EngineServices struct to group external service dependencies passed to NewEngine, reducing its argument count from 11 to 4 |
||
|
Maycon Santos
|
15aa6bae1b |
[client] Fix exit node menu not refreshing on Windows (#5553)
* [client] Fix exit node menu not refreshing on Windows TrayOpenedCh is not implemented in the systray library on Windows, so exit nodes were never refreshed after the initial connect. Combined with the management sync not having populated routes yet when the Connected status fires, this caused the exit node menu to remain empty permanently after disconnect/reconnect cycles. Add a background poller on Windows that refreshes exit nodes while connected, with fast initial polling to catch routes from management sync followed by a steady 10s interval. On macOS/Linux, TrayOpenedCh continues to handle refreshes on each tray open. Also fix a data race on connectClient assignment in the server's connect() method and add nil checks in CleanState/DeleteState to prevent panics when connectClient is nil. * Remove unused exitNodeIDs * Remove unused exitNodeState struct |
||
|
Zoltan Papp
|
3acd86e346 |
[client] "reset connection" error on wake from sleep (#5522)
Capture engine reference before actCancel() in cleanupConnection(). After actCancel(), the connectWithRetryRuns goroutine sets engine to nil, causing connectClient.Stop() to skip shutdown. This allows the goroutine to set ErrResetConnection on the shared state after Down() clears it, causing the next Up() to fail. |
||
|
Zoltan Papp
|
c2c4d9d336 |
[client] Fix Server mutex held across waitForUp in Up() (#5460)
Up() acquired s.mutex with a deferred unlock, then called waitForUp() while still holding the lock. waitForUp() blocks for up to 50 seconds waiting on clientRunningChan/clientGiveUpChan, starving all concurrent gRPC calls that require the same mutex (Status, ListProfiles, etc.). Replace the deferred unlock with explicit s.mutex.Unlock() on every early-return path and immediately before waitForUp(), matching the pattern already used by the clientRunning==true branch. |
