From b200f47e6df937a7076dc638770e346b210b78d3 Mon Sep 17 00:00:00 2001 From: riccardom Date: Thu, 4 Jun 2026 15:59:08 +0200 Subject: [PATCH] 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 (?) --- client/server/mdm.go | 99 +++++++++++++++++++++++++++++++++++++++++ client/server/server.go | 16 +++++++ 2 files changed, 115 insertions(+) diff --git a/client/server/mdm.go b/client/server/mdm.go index 5263b1655..da0deb4f2 100644 --- a/client/server/mdm.go +++ b/client/server/mdm.go @@ -207,6 +207,105 @@ func mdmManagedFieldConflicts(msg *proto.SetConfigRequest, policy *mdm.Policy) [ return conflicts } +// loginRequestHasConfigOverrides reports whether the LoginRequest +// carries ANY field that would mutate persisted daemon configuration +// (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. +func loginRequestHasConfigOverrides(msg *proto.LoginRequest) bool { + if msg == nil { + return false + } + return msg.ManagementUrl != "" || + msg.AdminURL != "" || + msg.PreSharedKey != "" || + msg.OptionalPreSharedKey != nil || + len(msg.CustomDNSAddress) > 0 || + len(msg.NatExternalIPs) > 0 || msg.CleanNATExternalIPs || + msg.RosenpassEnabled != nil || + msg.InterfaceName != nil || + msg.WireguardPort != nil || + msg.DisableAutoConnect != nil || + msg.ServerSSHAllowed != nil || + msg.RosenpassPermissive != nil || + len(msg.ExtraIFaceBlacklist) > 0 || + msg.NetworkMonitor != nil || + msg.DnsRouteInterval != nil || + msg.DisableClientRoutes != nil || + msg.DisableServerRoutes != nil || + msg.DisableDns != nil || + msg.DisableFirewall != nil || + msg.BlockLanAccess != nil || + msg.DisableNotifications != nil || + len(msg.DnsLabels) > 0 || msg.CleanDNSLabels || + msg.LazyConnectionEnabled != nil || + msg.BlockInbound != nil +} + +// 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. +func loginRequestMDMConflicts(msg *proto.LoginRequest, policy *mdm.Policy) []string { + if msg == nil || policy.IsEmpty() { + 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: 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 +} + // rejectMDMManagedFieldConflicts returns a FailedPrecondition gRPC error // with an MDMManagedFieldsViolation detail when any of the requested // fields tries to change an MDM-enforced value to something else, and diff --git a/client/server/server.go b/client/server/server.go index e042e47aa..a4a2e30d0 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -440,6 +440,22 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques // Login uses setup key to prepare configuration for the daemon. func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*proto.LoginResponse, error) { + // Config-override gates. LoginRequest carries the same surface as + // SetConfigRequest (managementUrl, PSK, ssh/rosenpass/port toggles, + // ...), so the same protections must apply. Without these the CLI + // command `netbird up --management-url=X` (which falls through to + // Login when SetConfig is rejected — see cmd/up.go) would silently + // bypass `--disable-update-settings` and any MDM policy. + if loginRequestHasConfigOverrides(msg) { + if s.checkUpdateSettingsDisabled() { + return nil, gstatus.Errorf(codes.Unavailable, errUpdateSettingsDisabled) + } + policy := loadMDMPolicy() + if err := rejectMDMManagedFieldConflicts(policy, loginRequestMDMConflicts(msg, policy)); err != nil { + return nil, err + } + } + s.mutex.Lock() if s.actCancel != nil { s.actCancel()