From d806f25b3353ee734f22166cc3d9bd2acfa110f7 Mon Sep 17 00:00:00 2001 From: riccardom Date: Tue, 9 Jun 2026 12:02:32 +0200 Subject: [PATCH] CI fixes - Add docstrings to `mdm_integration` - refactor for cognitive complexity - mod tidy --- client/cmd/debug.go | 20 ++ client/cmd/root.go | 6 + client/internal/profilemanager/config.go | 53 +++-- client/mdm/policy.go | 33 ++- client/mdm/policy_darwin.go | 6 +- client/mdm/policy_mobile.go | 3 +- client/mdm/policy_other.go | 3 +- client/mdm/policy_windows.go | 71 +++--- client/mdm/ticker.go | 19 +- client/server/mdm.go | 262 +++++++++++++++-------- client/server/server.go | 62 +++--- client/ui/client_ui.go | 8 +- docs/netbird-macos.sh | 12 +- go.mod | 2 +- 14 files changed, 360 insertions(+), 200 deletions(-) mode change 100755 => 100644 docs/netbird-macos.sh diff --git a/client/cmd/debug.go b/client/cmd/debug.go index 215ec3f7b..59ac0cc34 100644 --- a/client/cmd/debug.go +++ b/client/cmd/debug.go @@ -95,6 +95,21 @@ var debugConfigCmd = &cobra.Command{ RunE: debugConfigDump, } +// debugConfigDump implements `netbird debug config`. It resolves the +// active profile, queries the daemon for the effective configuration +// via GetConfig, and prints the resulting GetConfigResponse as JSON +// (via protojson with EmitUnpopulated=true so the output is stable +// across runs and includes zero-valued fields). +// +// Useful for verifying MDM enforcement end-to-end: the response's +// mDMManagedFields array is the single source of truth for "which +// fields is the daemon currently enforcing from the MDM source", and +// every config field side-by-side with that list confirms the +// merge result. Secrets in the response (e.g. PreSharedKey) are +// debugConfigDump requests the daemon for the resolved effective configuration and prints it as indented JSON. +// It resolves the active profile and current OS user, calls DaemonService.GetConfig with those values, and +// marshals the response using protojson with default/zero-valued fields included. +// Returns an error if profile or user lookup fails, the gRPC call fails, or the response cannot be marshaled. func debugConfigDump(cmd *cobra.Command, _ []string) error { pm := profilemanager.NewProfileManager() activeProf, err := pm.GetActiveProfile() @@ -136,6 +151,11 @@ func debugConfigDump(cmd *cobra.Command, _ []string) error { return nil } +// debugBundle requests the daemon to create a debug bundle and prints the resulting +// local file path and, if uploaded, the uploaded file key. +// It uses the package flags (anonymize, system info, log file count, CLI version and +// optional upload URL) to configure the bundle request. Returns an error if the RPC +// fails or if the daemon reports an upload failure reason. func debugBundle(cmd *cobra.Command, _ []string) error { conn, err := getClient(cmd) if err != nil { diff --git a/client/cmd/root.go b/client/cmd/root.go index 117e493de..6c4a2da24 100644 --- a/client/cmd/root.go +++ b/client/cmd/root.go @@ -103,6 +103,12 @@ func Execute() error { return rootCmd.Execute() } +// init initializes package-level defaults and the CLI command tree. +// init sets platform-specific default config and log directory paths and a default daemon address, +// registers persistent flags (daemon address, management/admin URLs, logging, setup key, preshared key, +// hostname, anonymize, config path), attaches top-level and nested subcommands to the root command, +// and configures `up` command specific flags (external IP maps, DNS resolver address, Rosenpass options, +// auto-connect disabling, and lazy connection). func init() { defaultConfigPathDir = "/etc/netbird/" defaultLogFileDir = "/var/log/netbird/" diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index a9da1055d..d2e9c10f1 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -681,36 +681,28 @@ func (config *Config) applyMDMPolicy(policy *mdm.Policy) { } } - if v, ok := policy.GetBool(mdm.KeyAllowServerSSH); ok { - bv := v - config.ServerSSHAllowed = &bv - logApplied(mdm.KeyAllowServerSSH, bv) - } - if v, ok := policy.GetBool(mdm.KeyDisableClientRoutes); ok { - config.DisableClientRoutes = v - logApplied(mdm.KeyDisableClientRoutes, v) - } - if v, ok := policy.GetBool(mdm.KeyDisableServerRoutes); ok { - config.DisableServerRoutes = v - logApplied(mdm.KeyDisableServerRoutes, v) - } - if v, ok := policy.GetBool(mdm.KeyBlockInbound); ok { - config.BlockInbound = v - logApplied(mdm.KeyBlockInbound, v) - } - if v, ok := policy.GetBool(mdm.KeyDisableAutoConnect); ok { - config.DisableAutoConnect = v - logApplied(mdm.KeyDisableAutoConnect, v) - } - if v, ok := policy.GetBool(mdm.KeyRosenpassEnabled); ok { - config.RosenpassEnabled = v - logApplied(mdm.KeyRosenpassEnabled, v) - } - if v, ok := policy.GetBool(mdm.KeyRosenpassPermissive); ok { - config.RosenpassPermissive = v - logApplied(mdm.KeyRosenpassPermissive, v) + // applyBool collapses the per-key "read + set + log" boilerplate + // for every plain bool MDM key into a single helper. Keeps the + // outer function's cognitive complexity below SonarCube's + // threshold; functional behaviour is identical to the inlined + // branches it replaces. + applyBool := func(key string, setter func(bool)) { + v, ok := policy.GetBool(key) + if !ok { + return + } + setter(v) + logApplied(key, v) } + applyBool(mdm.KeyAllowServerSSH, func(v bool) { bv := v; config.ServerSSHAllowed = &bv }) + applyBool(mdm.KeyDisableClientRoutes, func(v bool) { config.DisableClientRoutes = v }) + applyBool(mdm.KeyDisableServerRoutes, func(v bool) { config.DisableServerRoutes = v }) + applyBool(mdm.KeyBlockInbound, func(v bool) { config.BlockInbound = v }) + applyBool(mdm.KeyDisableAutoConnect, func(v bool) { config.DisableAutoConnect = v }) + applyBool(mdm.KeyRosenpassEnabled, func(v bool) { config.RosenpassEnabled = v }) + applyBool(mdm.KeyRosenpassPermissive, func(v bool) { config.RosenpassPermissive = v }) + if v, ok := policy.GetInt(mdm.KeyWireguardPort); ok { // REG_DWORD is 32-bit; UDP port range is 1-65535. Clamp at the // upper bound and reject obviously-invalid values to avoid the @@ -724,7 +716,10 @@ func (config *Config) applyMDMPolicy(policy *mdm.Policy) { } } -// parseURL parses and validates a service URL +// parseURL parses and validates the URL for the named service. +// It requires the URL to use the http or https scheme and, if no port is present, +// appends ":443" for https or ":80" for http. On success it returns the parsed +// *url.URL; on failure it returns a non-nil error. func parseURL(serviceName, serviceURL string) (*url.URL, error) { parsedMgmtURL, err := url.ParseRequestURI(serviceURL) if err != nil { diff --git a/client/mdm/policy.go b/client/mdm/policy.go index a799d1a17..06aa077f7 100644 --- a/client/mdm/policy.go +++ b/client/mdm/policy.go @@ -96,7 +96,9 @@ type Policy struct { } // NewPolicy constructs a Policy from a key→value map. Pass nil or an empty -// map to construct an empty (no-enforcement) Policy. +// NewPolicy constructs a Policy backed by the provided key→value map. +// If values is nil it is replaced with an empty map so the returned *Policy +// is always non-nil and represents no active MDM enforcement when empty. func NewPolicy(values map[string]any) *Policy { if values == nil { values = map[string]any{} @@ -111,7 +113,9 @@ func NewPolicy(values map[string]any) *Policy { // Diagnostic logging differentiates the three states: // - source absent / unsupported platform: trace log only // - source present, zero keys: info "MDM enrolled (no managed keys)" -// - source present, N keys: info "MDM enrolled with N managed keys: [...]" +// LoadPolicy loads MDM-managed configuration from the platform and returns a Policy representing the managed settings. +// If the platform loader fails or returns nil, LoadPolicy returns a non-nil empty Policy. +// When the loaded map contains zero keys it logs that MDM is enrolled with no managed keys; when it contains keys it logs the count and a stable, sorted list of key names. func LoadPolicy() *Policy { values, err := loadPlatformPolicy() if err != nil { @@ -169,6 +173,19 @@ func (p *Policy) GetString(key string) (string, bool) { return s, true } +// boolStringLiterals enumerates the textual boolean encodings the +// platform loaders may produce (Windows REG_SZ "true", iOS / Android +// managed-config booleans-as-strings, etc.). Lookup keeps GetBool flat +// (no nested switch on the string case). +var boolStringLiterals = map[string]bool{ + "true": true, + "1": true, + "yes": true, + "false": false, + "0": false, + "no": false, +} + // GetBool returns the managed value for key coerced to bool, and whether the // key was set. Accepts native bool and string literals "true"/"false"/"1"/"0". func (p *Policy) GetBool(key string) (bool, bool) { @@ -183,12 +200,8 @@ func (p *Policy) GetBool(key string) (bool, bool) { case bool: return t, true case string: - switch t { - case "true", "1", "yes": - return true, true - case "false", "0", "no": - return false, true - } + b, known := boolStringLiterals[t] + return b, known case int: return t != 0, true case int64: @@ -257,6 +270,10 @@ func (p *Policy) GetStringSlice(key string) ([]string, bool) { return nil, false } +// sortedKeys returns the keys of m as a deterministic, lexicographically +// sorted slice. Used internally by Policy.ManagedKeys and LoadPolicy's +// diagnostic log line so callers see a stable key order across runs +// It produces a deterministic ordering for a map regardless of Go's randomized iteration. func sortedKeys(m map[string]any) []string { out := make([]string, 0, len(m)) for k := range m { diff --git a/client/mdm/policy_darwin.go b/client/mdm/policy_darwin.go index 0ec0180c3..b1b50828b 100644 --- a/client/mdm/policy_darwin.go +++ b/client/mdm/policy_darwin.go @@ -37,7 +37,11 @@ const policyPlistPath = "/Library/Managed Preferences/io.netbird.client.plist" // map naturally onto the Policy accessor expectations (GetString / // GetBool / GetInt / GetStringSlice). Unknown top-level keys are // logged and skipped so a stray entry in the payload does not block -// startup. +// loadPlatformPolicy reads the managed-preferences plist at policyPlistPath and returns recognised MDM key/value pairs. +// +// If the plist file does not exist, it returns (nil, nil). It returns a wrapped error on open/stat/decode failures. +// The function refuses to read a world-writable plist and returns an error in that case. +// Top-level plist keys are canonicalized (case-insensitive) to the internal MDM key names; unknown keys are logged and skipped. func loadPlatformPolicy() (map[string]any, error) { f, err := os.Open(policyPlistPath) if err != nil { diff --git a/client/mdm/policy_mobile.go b/client/mdm/policy_mobile.go index a81ea1c57..3dfcf4731 100644 --- a/client/mdm/policy_mobile.go +++ b/client/mdm/policy_mobile.go @@ -6,7 +6,8 @@ package mdm // Kotlin/Java on Android) reads the OS managed-config store and pushes the // resulting dictionary in-process via a gomobile entry point that lands in // Phase 5 / Phase 6. The stub keeps the package compilable for mobile -// build targets. +// loadPlatformPolicy is a stub used on mobile (iOS/Android) builds that returns a nil policy map and no error. +// The actual managed-config policy is supplied by the native platform layer. func loadPlatformPolicy() (map[string]any, error) { return nil, nil } diff --git a/client/mdm/policy_other.go b/client/mdm/policy_other.go index 0f0619fbc..4426144ea 100644 --- a/client/mdm/policy_other.go +++ b/client/mdm/policy_other.go @@ -4,7 +4,8 @@ package mdm // loadPlatformPolicy returns no policy on platforms without an MDM channel // (Linux, FreeBSD). MDM enforcement is off and the client behaves as if -// the feature did not exist. +// loadPlatformPolicy reports that no platform MDM policy is available on non-Windows/Darwin/iOS/Android builds. +// It returns a nil policy map and a nil error to indicate MDM enforcement is not present on this platform. func loadPlatformPolicy() (map[string]any, error) { return nil, nil } diff --git a/client/mdm/policy_windows.go b/client/mdm/policy_windows.go index 6ef380e88..4081a3a3a 100644 --- a/client/mdm/policy_windows.go +++ b/client/mdm/policy_windows.go @@ -31,7 +31,44 @@ const policyRegistryPath = `Software\Policies\NetBird` // - REG_MULTI_SZ -> []string // // Unsupported value types (REG_BINARY, REG_NONE, ...) are skipped with a -// warning so a malformed deployment does not block startup. +// loadPlatformPolicy reads managed NetBird policy values from HKLM\Software\Policies\NetBird. +// If the registry key does not exist it returns (nil, nil). +// It returns a map whose keys are canonical policy names and whose values are coerced from registry types: +// REG_SZ/REG_EXPAND_SZ -> string, REG_DWORD/REG_QWORD -> int64, REG_MULTI_SZ -> []string. +// Unknown value names, unsupported value types, and per-value read errors are skipped and logged; failures opening the key or enumerating values are returned as errors. +func readRegistryValue(k registry.Key, name, canonical string, out map[string]any) { + _, valType, err := k.GetValue(name, nil) + if err != nil { + log.Warnf("MDM stat %s\\%s: %v", policyRegistryPath, name, err) + return + } + switch valType { + case registry.SZ, registry.EXPAND_SZ: + if v, _, err := k.GetStringValue(name); err == nil { + out[canonical] = v + } else { + log.Warnf("MDM read string %s\\%s: %v", policyRegistryPath, name, err) + } + case registry.DWORD, registry.QWORD: + if v, _, err := k.GetIntegerValue(name); err == nil { + // uint64 from the registry API; Policy.GetBool / GetInt + // helpers consume int64, so narrow safely. + out[canonical] = int64(v) + } else { + log.Warnf("MDM read int %s\\%s: %v", policyRegistryPath, name, err) + } + case registry.MULTI_SZ: + if v, _, err := k.GetStringsValue(name); err == nil { + out[canonical] = v + } else { + log.Warnf("MDM read multi-string %s\\%s: %v", policyRegistryPath, name, err) + } + default: + log.Warnf("MDM ignoring unsupported registry value type %d at %s\\%s", + valType, policyRegistryPath, name) + } +} + func loadPlatformPolicy() (map[string]any, error) { k, err := registry.OpenKey(registry.LOCAL_MACHINE, policyRegistryPath, registry.QUERY_VALUE) if err != nil { @@ -63,37 +100,7 @@ func loadPlatformPolicy() (map[string]any, error) { log.Warnf("MDM ignoring unknown registry value %s\\%s", policyRegistryPath, name) continue } - - _, valType, err := k.GetValue(name, nil) - if err != nil { - log.Warnf("MDM stat %s\\%s: %v", policyRegistryPath, name, err) - continue - } - switch valType { - case registry.SZ, registry.EXPAND_SZ: - if v, _, err := k.GetStringValue(name); err == nil { - out[canonical] = v - } else { - log.Warnf("MDM read string %s\\%s: %v", policyRegistryPath, name, err) - } - case registry.DWORD, registry.QWORD: - if v, _, err := k.GetIntegerValue(name); err == nil { - // uint64 from the registry API; Policy.GetBool / GetInt - // helpers consume int64, so narrow safely. - out[canonical] = int64(v) - } else { - log.Warnf("MDM read int %s\\%s: %v", policyRegistryPath, name, err) - } - case registry.MULTI_SZ: - if v, _, err := k.GetStringsValue(name); err == nil { - out[canonical] = v - } else { - log.Warnf("MDM read multi-string %s\\%s: %v", policyRegistryPath, name, err) - } - default: - log.Warnf("MDM ignoring unsupported registry value type %d at %s\\%s", - valType, policyRegistryPath, name) - } + readRegistryValue(k, name, canonical, out) } return out, nil } diff --git a/client/mdm/ticker.go b/client/mdm/ticker.go index b0113ff54..6463c42dc 100644 --- a/client/mdm/ticker.go +++ b/client/mdm/ticker.go @@ -25,7 +25,8 @@ const testReloadInterval = 1 * time.Second // reloadInterval returns the production cadence, or the accelerated test // cadence when running under `go test`. Centralising the choice here keeps -// the prod/test split in one place and out of the ticker's call sites. +// reloadInterval selects the polling interval used to re-read the OS-native MDM policy. +// It returns testReloadInterval when tests are running (testing.Testing() == true) and defaultReloadInterval otherwise. func reloadInterval() time.Duration { if testing.Testing() { return testReloadInterval @@ -52,7 +53,9 @@ type Ticker struct { // NewTicker constructs a Ticker that re-reads the OS-native policy every // reloadInterval() and invokes onChange on any diff. The cadence is owned by // reloadInterval (production default, accelerated under `go test`); callers -// do not supply it. onChange may be nil for a log-only ticker. +// NewTicker creates a Ticker that polls the OS-native MDM policy at the package reload interval and invokes onChange when a policy change is detected. +// If onChange is nil the ticker will only log detected changes. +// The ticker's initial snapshot is populated by loading the current policy. func NewTicker(onChange func(prev, curr *Policy)) *Ticker { return &Ticker{ interval: reloadInterval(), @@ -91,7 +94,7 @@ func (t *Ticker) Run(ctx context.Context) { } // PoliciesEqual reports whether two Policy instances carry the same managed -// key set with identical values. Nil and empty policies compare equal. +// value maps for deep equality. func PoliciesEqual(a, b *Policy) bool { if a.IsEmpty() && b.IsEmpty() { return true @@ -103,7 +106,11 @@ func PoliciesEqual(a, b *Policy) bool { } // diffPolicies returns the keys added in curr, removed from prev, and whose -// value changed. Returned slices are sorted for stable log output. +// diffPolicies reports keys that were added, removed, or changed between two policies. +// The returned slices contain keys present only in `curr` (added), only in `prev` (removed), +// and present in both but whose values differ (changed). Each slice is sorted +// lexicographically for stable logging output; value differences are determined +// using deep equality. func diffPolicies(prev, curr *Policy) (added, removed, changed []string) { prevKeys := mapOf(prev) currKeys := mapOf(curr) @@ -127,7 +134,9 @@ func diffPolicies(prev, curr *Policy) (added, removed, changed []string) { // mapOf returns a (possibly empty, never nil) copy of the underlying values // map of a Policy so callers outside this package can compare across the -// public Policy boundary without touching unexported state. +// mapOf returns a non-nil copy of the given Policy's key/value map. +// If p is nil, mapOf returns an empty map; otherwise it returns a newly +// allocated map containing the same key/value pairs as p.values. func mapOf(p *Policy) map[string]any { if p == nil { return map[string]any{} diff --git a/client/server/mdm.go b/client/server/mdm.go index 1ac010cea..75258944a 100644 --- a/client/server/mdm.go +++ b/client/server/mdm.go @@ -152,6 +152,97 @@ func (s *Server) restartEngineForMDM() error { return nil } +// preSharedKeyRedactedSentinel is the value GetConfig returns in place +// of an actual PSK, so a UI that round-trips the field back to the +// daemon (via SetConfig / Login) can be distinguished from a deliberate +// override. Any incoming PSK that equals this sentinel is treated as +// a no-op echo, never as a conflict with the policy. +const preSharedKeyRedactedSentinel = "**********" + +// conflictCheck is a value-aware comparison between a single field in +// the incoming request and the corresponding MDM-enforced value. It +// runs only when the field was actually set in the request (presence +// already filtered upstream); ok=true reports the policy value, ok=false +// means the policy is silent on the key — both are treated as conflicts +// to be safe (an MDM key declared as managed must hold a value). +type conflictCheck struct { + key string + check func(*mdm.Policy) (match bool) +} + +// conflictBool builds a check for a *bool field on an arbitrary request +// conflictBool builds a conflictCheck for a boolean MDM key. +// If p is nil the returned check treats the field as matching; otherwise the +// check returns true only when the policy contains the key and its boolean +// value equals *p. +func conflictBool(key string, p *bool) conflictCheck { + return conflictCheck{ + key: key, + check: func(pol *mdm.Policy) bool { + if p == nil { + return true // absent → match by definition + } + want, ok := pol.GetBool(key) + return ok && want == *p + }, + } +} + +// conflictString builds a check for a string field. Empty string ("") +// conflictString returns a conflictCheck for the MDM string key identified by `key`. +// If `got` is empty the field is treated as unset and will not be considered a conflict. +// Otherwise the check succeeds only when the policy contains `key` and its value equals `got`. +func conflictString(key, got string) conflictCheck { + return conflictCheck{ + key: key, + check: func(pol *mdm.Policy) bool { + if got == "" { + return true + } + want, ok := pol.GetString(key) + return ok && want == got + }, + } +} + +// conflictInt64 builds a conflictCheck that verifies an *int64 field against the MDM policy key. +// If p is nil the check always matches; otherwise the check requires the policy to contain the key and its integer value to equal *p. +func conflictInt64(key string, p *int64) conflictCheck { + return conflictCheck{ + key: key, + check: func(pol *mdm.Policy) bool { + if p == nil { + return true + } + want, ok := pol.GetInt(key) + return ok && want == *p + }, + } +} + +// resolveConflicts walks a list of per-field checks against the active +// MDM policy and returns the names of keys whose requested value +// diverges from the policy-enforced value. Keys not managed by MDM are +// skipped silently (the gate fires only for keys the admin has actually +// resolveConflicts identifies MDM-managed policy keys whose values differ from the provided checks. +// If the policy is empty, it returns nil. Only keys present in the policy are considered; for each +// check whose predicate returns false the corresponding key is included in the returned slice. +func resolveConflicts(policy *mdm.Policy, checks []conflictCheck) []string { + if policy.IsEmpty() { + return nil + } + var conflicts []string + for _, c := range checks { + if !policy.HasKey(c.key) { + continue + } + if !c.check(policy) { + conflicts = append(conflicts, c.key) + } + } + return conflicts +} + // mdmManagedFieldConflicts returns the names of MDM-managed keys whose // requested value in the SetConfigRequest differs from the MDM-enforced // value. A field set to the same value the policy already enforces is @@ -159,52 +250,39 @@ func (s *Server) restartEngineForMDM() error { // every toggle, so most fields in a typical request match the policy // exactly and must NOT be flagged as conflicts). // -// The redacted PreSharedKey sentinel ("**********") that GetConfig -// returns is recognised and treated as no-op so the UI can safely round- -// trip it without tripping the gate. +// The redacted PreSharedKey sentinel that GetConfig returns is +// recognised and treated as no-op so the UI can safely round-trip it +// mdmManagedFieldConflicts reports which MDM-managed policy keys would be violated by +// the provided SetConfigRequest. +// +// If msg is nil, it returns nil. The function treats the PSK redaction sentinel +// ("**********") as an intentional no-op (equivalent to field not set). Only keys +// present in the supplied policy are considered; returned slice contains the policy +// key names that conflict with the values in msg. func mdmManagedFieldConflicts(msg *proto.SetConfigRequest, policy *mdm.Policy) []string { - if msg == nil || policy.IsEmpty() { + if msg == nil { return nil } - var conflicts []string - mark := func(key string) { conflicts = append(conflicts, key) } - if msg.ManagementUrl != "" && policy.HasKey(mdm.KeyManagementURL) { - if want, ok := policy.GetString(mdm.KeyManagementURL); !ok || want != msg.ManagementUrl { - mark(mdm.KeyManagementURL) - } + // PSK round-trip echo: collapse the sentinel to empty so the + // shared check treats it as "field not set". + pskGot := "" + if msg.OptionalPreSharedKey != nil && *msg.OptionalPreSharedKey != preSharedKeyRedactedSentinel { + pskGot = *msg.OptionalPreSharedKey } - if msg.OptionalPreSharedKey != nil && policy.HasKey(mdm.KeyPreSharedKey) { - // "**********" is the redacted echo from GetConfig — never a real - // override attempt regardless of what the policy holds. - if *msg.OptionalPreSharedKey != "**********" { - if want, ok := policy.GetString(mdm.KeyPreSharedKey); !ok || want != *msg.OptionalPreSharedKey { - mark(mdm.KeyPreSharedKey) - } - } - } - checkBool := func(key string, p *bool) { - if p == nil || !policy.HasKey(key) { - return - } - if want, ok := policy.GetBool(key); !ok || want != *p { - mark(key) - } - } - checkBool(mdm.KeyRosenpassEnabled, msg.RosenpassEnabled) - checkBool(mdm.KeyRosenpassPermissive, msg.RosenpassPermissive) - checkBool(mdm.KeyDisableAutoConnect, msg.DisableAutoConnect) - checkBool(mdm.KeyAllowServerSSH, msg.ServerSSHAllowed) - checkBool(mdm.KeyDisableClientRoutes, msg.DisableClientRoutes) - checkBool(mdm.KeyDisableServerRoutes, msg.DisableServerRoutes) - checkBool(mdm.KeyBlockInbound, msg.BlockInbound) - if msg.WireguardPort != nil && policy.HasKey(mdm.KeyWireguardPort) { - if want, ok := policy.GetInt(mdm.KeyWireguardPort); !ok || want != *msg.WireguardPort { - mark(mdm.KeyWireguardPort) - } - } - return conflicts + return resolveConflicts(policy, []conflictCheck{ + conflictString(mdm.KeyManagementURL, msg.ManagementUrl), + conflictString(mdm.KeyPreSharedKey, pskGot), + conflictBool(mdm.KeyRosenpassEnabled, msg.RosenpassEnabled), + conflictBool(mdm.KeyRosenpassPermissive, msg.RosenpassPermissive), + conflictBool(mdm.KeyDisableAutoConnect, msg.DisableAutoConnect), + conflictBool(mdm.KeyAllowServerSSH, msg.ServerSSHAllowed), + conflictBool(mdm.KeyDisableClientRoutes, msg.DisableClientRoutes), + conflictBool(mdm.KeyDisableServerRoutes, msg.DisableServerRoutes), + conflictBool(mdm.KeyBlockInbound, msg.BlockInbound), + conflictInt64(mdm.KeyWireguardPort, msg.WireguardPort), + }) } // setConfigRequestHasConfigOverrides reports whether the SetConfigRequest @@ -213,7 +291,13 @@ func mdmManagedFieldConflicts(msg *proto.SetConfigRequest, policy *mdm.Policy) [ // setupSetConfigReq in cmd/up.go), so a plain `netbird up` results in a // SetConfig call with every field at its zero value; the gate must skip // such no-op invocations or it would always fire even when the user did -// not pass any --flag. +// setConfigRequestHasConfigOverrides reports whether msg contains any fields that would mutate +// persisted daemon configuration rather than being purely authentication-only. +// It returns false if msg is nil; otherwise it returns true when any configuration-related +// field is present (for example: management/admin URLs, pre-shared key, DNS/NAT lists and +// cleaning flags, interface/port/MTU settings, auto-connect and routing toggles, DNS/firewall/IPv6 +// controls, SSH-related flags, notification/lazy-connection options, or other persistent config +// toggles). func setConfigRequestHasConfigOverrides(msg *proto.SetConfigRequest) bool { if msg == nil { return false @@ -256,7 +340,10 @@ func setConfigRequestHasConfigOverrides(msg *proto.SetConfigRequest) bool { // (as opposed to pure-auth fields like setupKey, hostname, hint, // profileName, username). Used by the Login handler to decide whether // the `--disable-update-settings` / MDM gates must run: a re-auth that -// changes nothing about the configuration is always allowed. +// loginRequestHasConfigOverrides reports whether a LoginRequest includes any fields that would change persisted daemon configuration. +// It returns true when the request carries any configuration-related values (for example: management/admin URLs, pre-shared key, +// DNS or NAT lists/cleanup flags, interface or WireGuard port, connection and policy toggles, route/DNS/firewall/notification flags, +// Rosenpass settings, lazy-connection or block-inbound), and false when the request is nil or contains only authentication/identity fields. func loginRequestHasConfigOverrides(msg *proto.LoginRequest) bool { if msg == nil { return false @@ -290,64 +377,46 @@ func loginRequestHasConfigOverrides(msg *proto.LoginRequest) bool { // loginRequestMDMConflicts mirrors mdmManagedFieldConflicts but for the // LoginRequest surface. Same value-aware semantics: a field set to the // MDM-enforced value is a no-op echo, not a conflict; only a divergent -// value is flagged. PSK has two proto fields (PreSharedKey deprecated -// and OptionalPreSharedKey current); both routes are checked, and the -// "**********" redaction sentinel is accepted as a no-op. +// value is flagged. PSK has two proto fields — PreSharedKey (deprecated) +// and OptionalPreSharedKey (current); either route trips the gate if it +// diverges from the MDM-enforced PSK. The redaction sentinel is treated +// loginRequestMDMConflicts reports MDM-managed keys that conflict between a LoginRequest and an active MDM policy. +// +// It returns a slice of policy keys that are managed by the given policy and whose values in the request +// differ from the policy. If msg is nil or the policy has no managed keys, it returns nil. The function +// prefers OptionalPreSharedKey over the legacy PreSharedKey when both are present and treats the redaction +// sentinel "**********" as an absent pre-shared key. func loginRequestMDMConflicts(msg *proto.LoginRequest, policy *mdm.Policy) []string { - if msg == nil || policy.IsEmpty() { + if msg == nil { return nil } - var conflicts []string - mark := func(key string) { conflicts = append(conflicts, key) } - if msg.ManagementUrl != "" && policy.HasKey(mdm.KeyManagementURL) { - if want, ok := policy.GetString(mdm.KeyManagementURL); !ok || want != msg.ManagementUrl { - mark(mdm.KeyManagementURL) - } + // Collapse the two PSK fields + the redaction sentinel down to a + // single "got" string the shared check can compare against the + // policy: OptionalPreSharedKey wins if set; PreSharedKey (deprecated) + // is the fallback; sentinel echo is treated as "field not set". + pskGot := "" + if msg.OptionalPreSharedKey != nil { + pskGot = *msg.OptionalPreSharedKey + } else if msg.PreSharedKey != "" { //nolint:staticcheck // SA1019: legacy proto field still accepted by Login + pskGot = msg.PreSharedKey //nolint:staticcheck // SA1019 + } + if pskGot == preSharedKeyRedactedSentinel { + pskGot = "" } - // PSK: PreSharedKey (deprecated) and OptionalPreSharedKey are both - // accepted by Login; either trips the gate if it diverges from the - // MDM-enforced PSK. - if policy.HasKey(mdm.KeyPreSharedKey) { - psk := "" - set := false - if msg.OptionalPreSharedKey != nil { - psk = *msg.OptionalPreSharedKey - set = true - } else if msg.PreSharedKey != "" { - psk = msg.PreSharedKey - set = true - } - if set && psk != "**********" { - if want, ok := policy.GetString(mdm.KeyPreSharedKey); !ok || want != psk { - mark(mdm.KeyPreSharedKey) - } - } - } - - checkBool := func(key string, p *bool) { - if p == nil || !policy.HasKey(key) { - return - } - if want, ok := policy.GetBool(key); !ok || want != *p { - mark(key) - } - } - checkBool(mdm.KeyRosenpassEnabled, msg.RosenpassEnabled) - checkBool(mdm.KeyRosenpassPermissive, msg.RosenpassPermissive) - checkBool(mdm.KeyDisableAutoConnect, msg.DisableAutoConnect) - checkBool(mdm.KeyAllowServerSSH, msg.ServerSSHAllowed) - checkBool(mdm.KeyDisableClientRoutes, msg.DisableClientRoutes) - checkBool(mdm.KeyDisableServerRoutes, msg.DisableServerRoutes) - checkBool(mdm.KeyBlockInbound, msg.BlockInbound) - - if msg.WireguardPort != nil && policy.HasKey(mdm.KeyWireguardPort) { - if want, ok := policy.GetInt(mdm.KeyWireguardPort); !ok || want != *msg.WireguardPort { - mark(mdm.KeyWireguardPort) - } - } - return conflicts + return resolveConflicts(policy, []conflictCheck{ + conflictString(mdm.KeyManagementURL, msg.ManagementUrl), + conflictString(mdm.KeyPreSharedKey, pskGot), + conflictBool(mdm.KeyRosenpassEnabled, msg.RosenpassEnabled), + conflictBool(mdm.KeyRosenpassPermissive, msg.RosenpassPermissive), + conflictBool(mdm.KeyDisableAutoConnect, msg.DisableAutoConnect), + conflictBool(mdm.KeyAllowServerSSH, msg.ServerSSHAllowed), + conflictBool(mdm.KeyDisableClientRoutes, msg.DisableClientRoutes), + conflictBool(mdm.KeyDisableServerRoutes, msg.DisableServerRoutes), + conflictBool(mdm.KeyBlockInbound, msg.BlockInbound), + conflictInt64(mdm.KeyWireguardPort, msg.WireguardPort), + }) } // rejectMDMManagedFieldConflicts returns a FailedPrecondition gRPC error @@ -355,7 +424,10 @@ func loginRequestMDMConflicts(msg *proto.LoginRequest, policy *mdm.Policy) []str // fields tries to change an MDM-enforced value to something else, and // nil otherwise. The whole request is rejected on any conflict; non- // conflicting fields in the same request are not applied either (no -// partial apply). +// rejectMDMManagedFieldConflicts returns a gRPC FailedPrecondition error when any MDM-managed fields conflict. +// If `conflicts` is empty this function returns nil. When conflicts exist it produces a FailedPrecondition status +// whose message lists the conflicting fields and attempts to attach a `proto.MDMManagedFieldsViolation` detail; +// if attaching details fails the base status error is returned. A warning is logged listing the rejected keys. func rejectMDMManagedFieldConflicts(policy *mdm.Policy, conflicts []string) error { if len(conflicts) == 0 { return nil diff --git a/client/server/server.go b/client/server/server.go index 6d6d58b37..37dbd4354 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -343,50 +343,62 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques return nil, err } + config, err := setConfigInputFromRequest(msg) + if err != nil { + return nil, err + } + + if _, err := profilemanager.UpdateConfig(config); err != nil { + log.Errorf("failed to update profile config: %v", err) + return nil, fmt.Errorf("failed to update profile config: %w", err) + } + + return &proto.SetConfigResponse{}, nil +} + +// setConfigInputFromRequest translates a SetConfigRequest into the +// profilemanager.ConfigInput that profilemanager.UpdateConfig consumes. +// Pure mapping with no business logic beyond presence-aware copying of +// optional fields and the "empty / clean" semantics for the two slice +// fields (DNS labels, NAT external IPs). Extracted from SetConfig to +// keep the handler's cognitive complexity below the SonarCube +// threshold; the body of this function is intentionally linear and +// branchy because each proto field is its own optional case. +func setConfigInputFromRequest(msg *proto.SetConfigRequest) (profilemanager.ConfigInput, error) { + var config profilemanager.ConfigInput + profState := profilemanager.ActiveProfileState{ Name: msg.ProfileName, Username: msg.Username, } - profPath, err := profState.FilePath() if err != nil { log.Errorf("failed to get active profile file path: %v", err) - return nil, fmt.Errorf("failed to get active profile file path: %w", err) + return config, fmt.Errorf("failed to get active profile file path: %w", err) } - - var config profilemanager.ConfigInput - config.ConfigPath = profPath if msg.ManagementUrl != "" { config.ManagementURL = msg.ManagementUrl } - if msg.AdminURL != "" { config.AdminURL = msg.AdminURL } - if msg.InterfaceName != nil { config.InterfaceName = msg.InterfaceName } - if msg.WireguardPort != nil { wgPort := int(*msg.WireguardPort) config.WireguardPort = &wgPort } - - if msg.OptionalPreSharedKey != nil { - if *msg.OptionalPreSharedKey != "" { - config.PreSharedKey = msg.OptionalPreSharedKey - } + if msg.OptionalPreSharedKey != nil && *msg.OptionalPreSharedKey != "" { + config.PreSharedKey = msg.OptionalPreSharedKey } if msg.CleanDNSLabels { config.DNSLabels = domain.List{} - } else if msg.DnsLabels != nil { - dnsLabels := domain.FromPunycodeList(msg.DnsLabels) - config.DNSLabels = dnsLabels + config.DNSLabels = domain.FromPunycodeList(msg.DnsLabels) } if msg.CleanNATExternalIPs { @@ -399,7 +411,6 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques if string(msg.CustomDNSAddress) == "empty" { config.CustomDNSAddress = []byte{} } - config.ExtraIFaceBlackList = msg.ExtraIFaceBlacklist if msg.DnsRouteInterval != nil { @@ -432,18 +443,11 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques ttl := int(*msg.SshJWTCacheTTL) config.SSHJWTCacheTTL = &ttl } - if msg.Mtu != nil { mtu := uint16(*msg.Mtu) config.MTU = &mtu } - - if _, err := profilemanager.UpdateConfig(config); err != nil { - log.Errorf("failed to update profile config: %v", err) - return nil, fmt.Errorf("failed to update profile config: %w", err) - } - - return &proto.SetConfigResponse{}, nil + return config, nil } // Login uses setup key to prepare configuration for the daemon. @@ -1737,6 +1741,14 @@ func (s *Server) checkProfilesDisabled() bool { return s.profilesDisabled } +// checkNetworksDisabled reports whether the networks/exit-node feature +// is disabled on this daemon instance. Resolved MDM-first: when the +// active policy declares mdm.KeyDisableNetworks the policy value wins +// (regardless of true/false), so an admin can re-enable the feature +// via MDM even on a host that was installed with --disable-networks. +// Falls back to the s.networksDisabled CLI flag when the policy is +// silent on the key. Mirrors checkProfilesDisabled and +// checkUpdateSettingsDisabled. func (s *Server) checkNetworksDisabled() bool { if s.config != nil { if v, ok := s.config.Policy().GetBool(mdm.KeyDisableNetworks); ok { diff --git a/client/ui/client_ui.go b/client/ui/client_ui.go index 95ad9e9de..84ef4e9e2 100644 --- a/client/ui/client_ui.go +++ b/client/ui/client_ui.go @@ -65,6 +65,12 @@ const ( mdmFieldSuffix = " (MDM)" ) +// main is the entry point for the UI tray/client binary. +// +// It parses CLI flags, initializes logging, creates the Fyne application and tray icons, +// and constructs the service client (which may open a requested UI window). If a window-mode +// flag is set the Fyne event loop runs and main returns; otherwise it ensures only one +// instance is running, sets up signal handling and fonts, and starts the system tray loop. func main() { flags := parseFlags() @@ -1705,7 +1711,7 @@ func (s *serviceClient) applyMDMLocks(managed []string) { // Entry's placeholder slot. The placeholder is the only signal the user // gets that a PSK is configured, because the entry's Text is forced to // empty to keep the password reveal toggle from leaking the -// "**********" sentinel. +// It returns an empty string if no pre-shared key is present; returns "MDM-managed" if the pre-shared key is enforced by MDM; otherwise returns "configured". func preSharedKeyPlaceholder(cfg *proto.GetConfigResponse) string { if cfg == nil || cfg.PreSharedKey == "" { return "" diff --git a/docs/netbird-macos.sh b/docs/netbird-macos.sh old mode 100755 new mode 100644 index c2493fa70..7d3f93b2f --- a/docs/netbird-macos.sh +++ b/docs/netbird-macos.sh @@ -75,15 +75,19 @@ readonly PLIST_DIR='/Library/Managed Preferences' readonly PLIST_PATH="$PLIST_DIR/io.netbird.client.plist" readonly LOG_TAG='netbird-mdm' +# log sends a message to the system logger with the configured tag and echoes the message to stdout prefixed by an ISO 8601 UTC timestamp and the tag. log() { /usr/bin/logger -t "$LOG_TAG" "$*" printf '%s [%s] %s\n' "$(date -u '+%Y-%m-%dT%H:%M:%SZ')" "$LOG_TAG" "$*" } +# is_set returns success if the provided value is non-empty and is not equal to the special NULL marker. is_set() { - [[ -n "$1" && "$1" != "$NULL" ]] + local value="$1" + [[ -n "$value" && "$value" != "$NULL" ]] } +# start_plist creates the temporary plist file at "$PLIST_PATH.tmp" containing the XML plist header and opening `` for the policy plist. start_plist() { cat > "$PLIST_PATH.tmp" <<'EOF' @@ -93,6 +97,7 @@ start_plist() { EOF } +# end_plist appends the closing `` and `` tags to the temporary plist file. end_plist() { cat >> "$PLIST_PATH.tmp" <<'EOF' @@ -100,6 +105,7 @@ end_plist() { EOF } +# emit_string appends a plist `/` entry for a given key and value to "$PLIST_PATH.tmp", XML-escaping `&`, `<`, and `>` and logging the assignment; if the key is "preSharedKey" the logged value is masked. emit_string() { local key="$1" value="$2" log_value="$2" # Escape XML entities in the value @@ -112,6 +118,8 @@ emit_string() { log "set $key = $log_value" } +# emit_bool writes a boolean plist entry for a given key into the temporary plist file. +# emit_bool accepts `true/True/TRUE/1/yes` as true and `false/False/FALSE/0/no` as false; on invalid input it logs an error and skips emitting the key. emit_bool() { local key="$1" value="$2" local xml_bool @@ -124,6 +132,7 @@ emit_bool() { log "set $key = $value" } +# emit_int validates that VALUE contains only decimal digits and, if valid, appends an `` plist entry for KEY to the temporary plist (`$PLIST_PATH.tmp`) and logs the assignment; on invalid input it logs a skip and does not emit the key. emit_int() { local key="$1" value="$2" if ! [[ "$value" =~ ^[0-9]+$ ]]; then @@ -134,6 +143,7 @@ emit_int() { log "set $key = $value" } +# main builds the NetBird MDM plist from configured policy variables, validates and installs it to /Library/Managed Preferences/io.netbird.client.plist (root:wheel, 644) and optionally triggers the NetBird daemon to reload. main() { log "applying NetBird MDM policy to $PLIST_PATH" /bin/mkdir -p "$PLIST_DIR" diff --git a/go.mod b/go.mod index e8df6c697..d96c6f7ca 100644 --- a/go.mod +++ b/go.mod @@ -131,6 +131,7 @@ require ( gorm.io/driver/sqlite v1.5.7 gorm.io/gorm v1.25.12 gvisor.dev/gvisor v0.0.0-20260219192049-0f2374377e89 + howett.net/plist v1.0.1 ) require ( @@ -328,7 +329,6 @@ require ( gopkg.in/square/go-jose.v2 v2.6.0 // indirect gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - howett.net/plist v1.0.1 // indirect rsc.io/qr v0.2.0 // indirect )