From 14917dbc2248950125c7b5ce8d97b1e550327ca6 Mon Sep 17 00:00:00 2001 From: "Theodor S. Midtlien" Date: Sat, 25 Jul 2026 17:59:33 +0200 Subject: [PATCH] Refactor owner to be daemon-wide --- client/cmd/owner.go | 31 ++-- client/cmd/service.go | 7 + client/cmd/service_controller.go | 3 + client/cmd/service_params.go | 51 +++++++ client/cmd/service_params_test.go | 8 +- client/internal/ipcauth/interceptor.go | 2 +- client/internal/ipcauth/policy.go | 19 +-- client/server/ownership.go | 194 +++++++++++++++++++------ client/server/ownership_test.go | 85 +++++++++++ client/server/server.go | 11 +- 10 files changed, 338 insertions(+), 73 deletions(-) diff --git a/client/cmd/owner.go b/client/cmd/owner.go index 1ac146cd9..b786cfed5 100644 --- a/client/cmd/owner.go +++ b/client/cmd/owner.go @@ -13,18 +13,19 @@ import ( var ownerCmd = &cobra.Command{ Use: "owner", - Short: "Manage who may control the active NetBird profile", - Long: `Manage the owners of the active profile's daemon control channel. + Short: "Manage who may control the NetBird daemon", + Long: `Manage the daemon-wide owners. -Ownership is enforced per profile: an isolated profile can only be controlled by -its owner principals (plus root/administrator). A new profile is automatically -owned by its creator; an unowned profile is claimed by the first caller.`, +Owners are enforced on the daemon and stored in the service parameters. All owners +may control the daemon and use the shared default profile (plus root/administrator), +every other profile stays isolated to the user that created it. An unowned daemon +is claimed by the first caller (trust-on-first-use).`, } var ownerAddCmd = &cobra.Command{ Use: "add ", - Short: "Add an owner principal to the active profile", - Long: `Add an owner principal to the active profile. Principals are typed: + Short: "Add a daemon owner principal", + Long: `Add a daemon-wide owner principal. Principals are typed: uid:1000 a Unix user ID gid:1000 a Unix group ID group:netbird-admins a Unix group name (resolved via NSS/getent) @@ -37,7 +38,7 @@ Requires root/administrator or an existing owner.`, if _, err := c.AddOwner(ctx, &proto.AddOwnerRequest{Principal: args[0]}); err != nil { return err } - cmd.Printf("Added owner %q to the active profile\n", args[0]) + cmd.Printf("Added daemon owner %q\n", args[0]) return nil }) }, @@ -45,8 +46,8 @@ Requires root/administrator or an existing owner.`, var ownerResetCmd = &cobra.Command{ Use: "reset", - Short: "Clear the active profile's owner list (root/administrator only)", - Long: `Clear the active profile's owner list, returning it to the unowned + Short: "Clear the daemon owner list (root/administrator only)", + Long: `Clear the daemon-wide owner list, returning the daemon to the unowned state. The next caller then claims ownership (trust-on-first-use). Requires root/administrator.`, RunE: func(cmd *cobra.Command, args []string) error { @@ -54,7 +55,7 @@ root/administrator.`, if _, err := c.ResetOwner(ctx, &proto.ResetOwnerRequest{}); err != nil { return err } - cmd.Println("Owner list cleared; the next caller will claim ownership") + cmd.Println("Daemon owner list cleared, the next caller will claim ownership") return nil }) }, @@ -62,13 +63,13 @@ root/administrator.`, var ownerShareCmd = &cobra.Command{ Use: "share", - Short: "Mark the active profile shared (any local user may control it)", + Short: "Mark the daemon shared (any local user may control it)", RunE: func(cmd *cobra.Command, args []string) error { return withDaemon(cmd, func(ctx context.Context, c proto.DaemonServiceClient) error { if _, err := c.ShareProfile(ctx, &proto.ShareProfileRequest{Shared: true}); err != nil { return err } - cmd.Println("Active profile is now shared with all local users") + cmd.Println("Daemon is now shared with all local users") return nil }) }, @@ -76,13 +77,13 @@ var ownerShareCmd = &cobra.Command{ var ownerUnshareCmd = &cobra.Command{ Use: "unshare", - Short: "Stop sharing the active profile (restrict to its owners)", + Short: "Stop sharing the daemon (restrict to its owners)", RunE: func(cmd *cobra.Command, args []string) error { return withDaemon(cmd, func(ctx context.Context, c proto.DaemonServiceClient) error { if _, err := c.ShareProfile(ctx, &proto.ShareProfileRequest{Shared: false}); err != nil { return err } - cmd.Println("Active profile is no longer shared") + cmd.Println("Daemon is no longer shared") return nil }) }, diff --git a/client/cmd/service.go b/client/cmd/service.go index cb79c43dc..37540882c 100644 --- a/client/cmd/service.go +++ b/client/cmd/service.go @@ -30,6 +30,12 @@ var ( serviceEnvVars []string jsonSocket string enableJSONSocket bool + // owners seeds the daemon-wide owner set at install time (--owner). At runtime + // the daemon reads and writes owners in service.json directly. + owners []string + // daemonShared carries the persisted daemon shared flag across + // install/reconfigure round-trips (set at runtime via `netbird owner share`). + daemonShared bool ) type program struct { @@ -55,6 +61,7 @@ func init() { serviceCmd.PersistentFlags().BoolVar(&networksDisabled, "disable-networks", false, "Disables network selection. If enabled, the client will not allow listing, selecting, or deselecting networks. To persist, use: netbird service install --disable-networks") serviceCmd.PersistentFlags().BoolVar(&enableJSONSocket, "enable-json-socket", false, "Enables the HTTP/JSON API socket served by grpc-gateway. To persist, use: netbird service install --enable-json-socket") serviceCmd.PersistentFlags().StringVar(&jsonSocket, "json-socket", defaultJSONSocket, "HTTP/JSON API socket address [unix|tcp|npipe]://[path|host:port|name]. Requires --enable-json-socket to serve. To persist, use: netbird service install --enable-json-socket --json-socket") + serviceCmd.PersistentFlags().StringSliceVar(&owners, "owner", nil, "Principal(s) allowed to control the daemon and its default profile: uid:1000, gid:1000, group:netbird-admins (NSS), or sid:S-1-5-... (Windows). Repeatable. Other profiles stay isolated per user. To persist: netbird service install --owner uid:1000") rootCmd.PersistentFlags().StringVarP(&serviceName, "service", "s", defaultServiceName, "Netbird system service name") serviceEnvDesc := `Sets extra environment variables for the service. ` + diff --git a/client/cmd/service_controller.go b/client/cmd/service_controller.go index 93a69d29e..0fcd22d37 100644 --- a/client/cmd/service_controller.go +++ b/client/cmd/service_controller.go @@ -115,6 +115,9 @@ func (p *program) Start(svc service.Service) error { } serverInstance := server.New(p.ctx, util.FindFirstLogPath(logFiles), configPath, profilesDisabled, updateSettingsDisabled, captureEnabled, networksDisabled) + // Daemon-wide owners live in service.json (governs the default profile and + // daemon access), wire persistence before serving so owner add / TOFU work. + serverInstance.SetDaemonOwnerStore(daemonOwnerStore{}) if err := serverInstance.Start(); err != nil { log.Fatalf("failed to start daemon: %v", err) } diff --git a/client/cmd/service_params.go b/client/cmd/service_params.go index 1cb5bcc75..9d4b5b6be 100644 --- a/client/cmd/service_params.go +++ b/client/cmd/service_params.go @@ -33,6 +33,15 @@ type serviceParams struct { DisableNetworks bool `json:"disable_networks,omitempty"` EnableJSONSocket bool `json:"enable_json_socket,omitempty"` ServiceEnvVars map[string]string `json:"service_env_vars,omitempty"` + // Owners lists the principals allowed to control this profile over the local + // IPC, as typed strings: "uid:1000", "gid:1000", "group:netbird-admins" + // (Unix, NSS-resolved) or "sid:S-1-5-..." (Windows user or group SID). Empty + // with Shared=false means the profile is owned by nobody yet, until claimed + Owners []string `json:"owners,omitempty"` + + // Shared, when true, lets any authenticated local caller control this profile + // (opt-in). Takes precedence over Owners. + Shared bool `json:"shared,omitempty"` } // serviceParamsPath returns the path to the service params file. @@ -40,6 +49,38 @@ func serviceParamsPath() string { return filepath.Join(configs.StateDir, serviceParamsFile) } +// daemonOwnerStore persists the daemon-wide owner set in service.json. It +// implements server.DaemonOwnerStore so the daemon can read owners at startup and +// mutate them at runtime (owner add, reset, share, TOFU claim) without server +// importing cmd. Load-modify-write preserves the other service.json fields. +type daemonOwnerStore struct{} + +func (daemonOwnerStore) Load() ([]string, bool, error) { + params, err := loadServiceParams() + if err != nil { + return nil, false, err + } + if params == nil { + return nil, false, nil + } + return params.Owners, params.Shared, nil +} + +func (daemonOwnerStore) Save(owners []string, shared bool) error { + params, err := loadServiceParams() + if err != nil { + return err + } + if params == nil { + // No service.json yet (daemon started without `service install`). Seed it + // from the running daemon's current parameters so the file stays complete. + params = currentServiceParams() + } + params.Owners = owners + params.Shared = shared + return saveServiceParams(params) +} + // loadServiceParams reads saved service parameters from disk. // Returns nil with no error if the file does not exist. func loadServiceParams() (*serviceParams, error) { @@ -86,6 +127,8 @@ func currentServiceParams() *serviceParams { EnableCapture: captureEnabled, DisableNetworks: networksDisabled, EnableJSONSocket: enableJSONSocket, + Owners: owners, + Shared: daemonShared, } if len(serviceEnvVars) > 0 { @@ -169,6 +212,14 @@ func applyServiceParams(cmd *cobra.Command, params *serviceParams) { networksDisabled = params.DisableNetworks } + // Carry the daemon-wide owner set forward across install/reconfigure so a + // runtime owner add or TOFU claim in service.json is not clobbered. --owner + // overrides. + if !serviceCmd.PersistentFlags().Changed("owner") && len(params.Owners) > 0 { + owners = params.Owners + } + daemonShared = params.Shared + applyServiceEnvParams(cmd, params) } diff --git a/client/cmd/service_params_test.go b/client/cmd/service_params_test.go index 94f98a0ce..fbf7c06a2 100644 --- a/client/cmd/service_params_test.go +++ b/client/cmd/service_params_test.go @@ -431,9 +431,15 @@ func TestServiceParams_BuildArgsCoversAllFlags(t *testing.T) { installerFile, err := parser.ParseFile(fset, "service_installer.go", nil, 0) require.NoError(t, err) - // Fields that are handled outside of buildServiceArguments (env vars go through newSVCConfig). + // Fields that are handled outside of buildServiceArguments. + // - ServiceEnvVars flows through newSVCConfig() EnvVars, not CLI args. + // - Owners/Shared are daemon-wide ownership persisted in service.json and + // read+mutated by the daemon at runtime (owner add / TOFU claim); they are + // deliberately NOT baked into the run args so runtime changes are not lost. fieldsNotInArgs := map[string]bool{ "ServiceEnvVars": true, + "Owners": true, + "Shared": true, } buildFields := extractFuncGlobalRefs(t, installerFile, "buildServiceArguments") diff --git a/client/internal/ipcauth/interceptor.go b/client/internal/ipcauth/interceptor.go index e8a437b40..de61b3eb9 100644 --- a/client/internal/ipcauth/interceptor.go +++ b/client/internal/ipcauth/interceptor.go @@ -73,7 +73,7 @@ func (i *Interceptor) authorize(ctx context.Context, fullMethod string) error { } // These RPCs authorize themselves in the handler (target-profile check) or are - // connection-lifecycle actions any authenticated local user may perform; they + // connection-lifecycle actions any authenticated local user may perform. They // bypass the active-profile gate. Still audit the C/H ones on allow. if handlerAuthorizedMethods[fullMethod] { i.auditAllow(id, fullMethod) diff --git a/client/internal/ipcauth/policy.go b/client/internal/ipcauth/policy.go index 8e88650f2..86be2b14e 100644 --- a/client/internal/ipcauth/policy.go +++ b/client/internal/ipcauth/policy.go @@ -21,15 +21,15 @@ type ProfilePolicy interface { // handlerAuthorizedMethods bypass the active-profile gate. Peer identity is // still required to reach them. Two groups: // -// - Per-user / per-target-profile operations whose handler does its own -// authorization bound to the caller identity: AddProfile, ListProfiles, -// GetActiveProfile, RemoveProfile, RenameProfile, and SwitchProfile (gated -// on the TARGET profile's ownership via authorizeTargetProfile). -// - Connection-lifecycle operations on the single shared daemon connection -// that any authenticated local user may perform: Down. Gating these on the -// ACTIVE profile's owner would trap a user behind another user's profile — -// they could neither disconnect nor switch to their OWN profile. SwitchProfile -// is bounded by its target check; Down only tears the connection down. +// - Per-user or per-target-profile ops that self-authorize in the handler, +// bound to the caller identity: AddProfile, ListProfiles, GetActiveProfile, +// RemoveProfile, RenameProfile, and SwitchProfile (target ownership checked +// via authorizeTargetProfile). +// - Connection-lifecycle ops any authenticated local user may run on the shared +// daemon connection: Down and Status. Gating these on the active profile's +// owner would trap a user behind another user's profile, unable to disconnect +// or switch to their own. SwitchProfile is bounded by its target check, Down +// and Status only read or tear down the connection. var handlerAuthorizedMethods = map[string]bool{ servicePath + "AddProfile": true, servicePath + "ListProfiles": true, @@ -38,6 +38,7 @@ var handlerAuthorizedMethods = map[string]bool{ servicePath + "RenameProfile": true, servicePath + "SwitchProfile": true, servicePath + "Down": true, + servicePath + "Status": true, } // auditMethods are worth an audit log line. Denials are always logged. diff --git a/client/server/ownership.go b/client/server/ownership.go index 244b702bc..2fa551642 100644 --- a/client/server/ownership.go +++ b/client/server/ownership.go @@ -18,13 +18,63 @@ import ( // Verify that the daemon Server implements ipcauth.ProfilePolicy. var _ ipcauth.ProfilePolicy = (*Server)(nil) -// ActiveProfileOwnership returns the active profile's ownership policy. Reads -// the in-memory active config (kept current by the handlers), falling back to -// the on-disk active profile when the daemon hasn't loaded one yet. +// DaemonOwnerStore persists the daemon-wide owner set. The cmd layer implements +// it over service.json. The interface lives here to avoid an import cycle. A nil +// store means the daemon is unowned, so non-privileged callers are denied on the +// default profile. +type DaemonOwnerStore interface { + // Load returns the persisted daemon owner principals and shared flag. + Load() (owners []string, shared bool, err error) + // Save persists the daemon owner principals and shared flag. + Save(owners []string, shared bool) error +} + +// SetDaemonOwnerStore installs the owner persistence backend and loads the +// current owner set into memory. Called once by cmd before serving RPCs. +func (s *Server) SetDaemonOwnerStore(store DaemonOwnerStore) { + s.mutex.Lock() + defer s.mutex.Unlock() + + s.daemonOwnerStore = store + if store == nil { + return + } + owners, shared, err := store.Load() + if err != nil { + log.Warnf("ownership: cannot load daemon owners, treating as unowned: %v", err) + return + } + s.owners = ipcauth.Ownership{Owners: owners, Shared: shared} + log.Infof("daemon owners loaded: %d principal(s), shared=%t", len(owners), shared) +} + +// activeIsDefaultLocked reports whether the active profile is the shared default. +// The default is owned daemon-wide, every other profile by its own per-profile +// owner. Caller must hold s.mutex. +func (s *Server) activeIsDefaultLocked() (bool, error) { + active, err := s.profileManager.GetActiveProfileState() + if err != nil { + return false, fmt.Errorf("get active profile: %w", err) + } + return active.ID == profilemanager.DefaultProfileName, nil +} + +// ActiveProfileOwnership returns the ownership the interceptor gates the active +// profile against. The default profile uses the daemon-wide owner set, every +// other profile uses its own collision-free owner (isolated per user). func (s *Server) ActiveProfileOwnership() ipcauth.Ownership { s.mutex.Lock() defer s.mutex.Unlock() + isDefault, err := s.activeIsDefaultLocked() + if err != nil { + log.Warnf("ownership: cannot determine active profile, treating as unowned: %v", err) + return ipcauth.Ownership{} + } + if isDefault { + return s.owners + } + cfg := s.config if cfg == nil { loaded, err := s.loadActiveProfileConfigLocked() @@ -38,12 +88,22 @@ func (s *Server) ActiveProfileOwnership() ipcauth.Ownership { } // ClaimActiveProfileOwnerIfUnowned atomically claims the active profile for id -// when it has no owners and is not shared (trust-on-first-use). Returns whether -// id is now an owner. Concurrent first-callers are serialized by s.mutex. +// when it has no owners and is not shared (trust-on-first-use). The default +// profile claims daemon-wide ownership via the owner store, every other profile +// claims its own per-profile owner. Returns whether id is now an owner. Concurrent +// first-callers are serialized by s.mutex. func (s *Server) ClaimActiveProfileOwnerIfUnowned(id ipcauth.Identity) (bool, error) { s.mutex.Lock() defer s.mutex.Unlock() + isDefault, err := s.activeIsDefaultLocked() + if err != nil { + return false, fmt.Errorf("determine active profile: %w", err) + } + if isDefault { + return s.claimDaemonOwnerLocked(id) + } + cfg := s.config if cfg == nil { loaded, err := s.loadActiveProfileConfigLocked() @@ -67,6 +127,45 @@ func (s *Server) ClaimActiveProfileOwnerIfUnowned(id ipcauth.Identity) (bool, er return true, nil } +// claimDaemonOwnerLocked claims daemon-wide ownership for id when the daemon is +// unowned and unshared, persisting via the owner store. Caller must hold s.mutex. +func (s *Server) claimDaemonOwnerLocked(id ipcauth.Identity) (bool, error) { + if s.daemonOwnerStore == nil { + return false, nil // no store, cannot claim, fail closed + } + if len(s.owners.Owners) > 0 || s.owners.Shared { + return false, nil // already owned or shared + } + principal := ipcauth.OwnerPrincipalForIdentity(id) + if err := s.daemonOwnerStore.Save([]string{principal}, false); err != nil { + return false, fmt.Errorf("persist daemon owner claim: %w", err) + } + s.owners = ipcauth.Ownership{Owners: []string{principal}} + log.Infof("daemon ownership claimed by %s (trust-on-first-use)", id) + return true, nil +} + +// addDaemonOwnerLocked adds id's principal to the daemon owner set and persists. +// Idempotent, and a no-op for privileged callers. Caller must hold s.mutex. +func (s *Server) addDaemonOwnerLocked(id ipcauth.Identity) error { + if id.IsPrivileged() { + return nil + } + if s.daemonOwnerStore == nil { + return fmt.Errorf("daemon owner store unavailable") + } + principal := ipcauth.OwnerPrincipalForIdentity(id) + if slices.Contains(s.owners.Owners, principal) { + return nil + } + next := append(slices.Clone(s.owners.Owners), principal) + if err := s.daemonOwnerStore.Save(next, s.owners.Shared); err != nil { + return err + } + s.owners.Owners = next + return nil +} + // activeProfileConfigPathLocked resolves the active profile's config file path. func (s *Server) activeProfileConfigPathLocked() (string, error) { activeProf, err := s.profileManager.GetActiveProfileState() @@ -98,22 +197,21 @@ func (s *Server) persistActiveProfileConfigLocked(cfg *profilemanager.Config) er return util.WriteJson(context.Background(), path, cfg) } -// activeConfigLocked returns the in-memory active config, loading it from disk -// if the daemon hasn't cached one. Caller must hold s.mutex. -func (s *Server) activeConfigLocked() (*profilemanager.Config, error) { - if s.config != nil { - return s.config, nil - } - return s.loadActiveProfileConfigLocked() -} - -// claimForCallerLocked adds the caller's principal to cfg (if absent) and -// persists. No-op for privileged callers (they need no ownership entry). Caller -// must hold s.mutex. +// claimForCallerLocked adds the caller's principal to the active profile's owner +// set (if absent) and persists. No-op for privileged callers. For the default +// profile it adds to the daemon-wide owners, for any other profile it adds to +// that profile's per-profile owners. Caller must hold s.mutex. func (s *Server) claimForCallerLocked(id ipcauth.Identity, cfg *profilemanager.Config) error { if id.IsPrivileged() { return nil } + isDefault, err := s.activeIsDefaultLocked() + if err != nil { + return err + } + if isDefault { + return s.addDaemonOwnerLocked(id) + } principal := ipcauth.OwnerPrincipalForIdentity(id) if slices.Contains(cfg.Owners, principal) { return nil @@ -148,6 +246,16 @@ func (s *Server) authorizeTargetProfile(ctx context.Context, target *profilemana return nil } + // The default profile is governed by the daemon-wide owners (all owners may + // use it), not a per-profile owner. Authorize against s.owners and never stamp. + if target.ID == profilemanager.DefaultProfileName { + if ipcauth.Authorize(s.owners, id, s.groupResolver) { + return nil + } + return gstatus.Errorf(codes.PermissionDenied, + "not authorized to use the default profile (caller %s is not a daemon owner)", id) + } + path, err := target.FilePath() if err != nil { return fmt.Errorf("resolve target profile path: %w", err) @@ -181,8 +289,8 @@ func (s *Server) authorizeTargetProfile(ctx context.Context, target *profilemana return nil } -// AddOwner adds a principal to the active profile's owner list. The interceptor -// has already confirmed the caller is an owner or privileged, the handler just +// AddOwner adds a principal to the daemon-wide owner set. The interceptor has +// already confirmed the caller is an owner or privileged, the handler just // validates and persists. func (s *Server) AddOwner(_ context.Context, msg *proto.AddOwnerRequest) (*proto.AddOwnerResponse, error) { principal := msg.GetPrincipal() @@ -193,25 +301,23 @@ func (s *Server) AddOwner(_ context.Context, msg *proto.AddOwnerRequest) (*proto s.mutex.Lock() defer s.mutex.Unlock() - cfg, err := s.activeConfigLocked() - if err != nil { - return nil, fmt.Errorf("load active profile config: %w", err) + if s.daemonOwnerStore == nil { + return nil, gstatus.Error(codes.Unavailable, "daemon owner store unavailable") } - if slices.Contains(cfg.Owners, principal) { + if slices.Contains(s.owners.Owners, principal) { return &proto.AddOwnerResponse{}, nil } - cfg.Owners = append(cfg.Owners, principal) - if err := s.persistActiveProfileConfigLocked(cfg); err != nil { - cfg.Owners = cfg.Owners[:len(cfg.Owners)-1] + next := append(slices.Clone(s.owners.Owners), principal) + if err := s.daemonOwnerStore.Save(next, s.owners.Shared); err != nil { return nil, fmt.Errorf("persist owner: %w", err) } - s.config = cfg - log.Infof("added owner %q to the active profile", principal) + s.owners.Owners = next + log.Infof("added daemon owner %q", principal) return &proto.AddOwnerResponse{}, nil } -// ResetOwner clears the active profile's owner list (and shared flag), returning -// it to the unowned state so the next caller re-claims via trust-on-first-use. +// ResetOwner clears the daemon-wide owner set (and shared flag), returning the +// daemon to the unowned state so the next caller re-claims via trust-on-first-use. // Privileged-only, so co-owners cannot evict each other. func (s *Server) ResetOwner(ctx context.Context, _ *proto.ResetOwnerRequest) (*proto.ResetOwnerResponse, error) { id, ok := ipcauth.IdentityFromContext(ctx) @@ -222,35 +328,31 @@ func (s *Server) ResetOwner(ctx context.Context, _ *proto.ResetOwnerRequest) (*p s.mutex.Lock() defer s.mutex.Unlock() - cfg, err := s.activeConfigLocked() - if err != nil { - return nil, fmt.Errorf("load active profile config: %w", err) + if s.daemonOwnerStore == nil { + return nil, gstatus.Error(codes.Unavailable, "daemon owner store unavailable") } - cfg.Owners = nil - cfg.Shared = false - if err := s.persistActiveProfileConfigLocked(cfg); err != nil { + if err := s.daemonOwnerStore.Save(nil, false); err != nil { return nil, fmt.Errorf("persist owner reset: %w", err) } - s.config = cfg - log.Infof("active profile owner list reset; next caller will re-claim (trust-on-first-use)") + s.owners = ipcauth.Ownership{} + log.Infof("daemon owner list reset, next caller will re-claim (trust-on-first-use)") return &proto.ResetOwnerResponse{}, nil } -// ShareProfile marks the active profile shared or unshared. The interceptor has +// ShareProfile marks the daemon shared or unshared. When shared, any authenticated +// local caller may control the daemon and its default profile. The interceptor has // already confirmed the caller is an owner or privileged. func (s *Server) ShareProfile(_ context.Context, msg *proto.ShareProfileRequest) (*proto.ShareProfileResponse, error) { s.mutex.Lock() defer s.mutex.Unlock() - cfg, err := s.activeConfigLocked() - if err != nil { - return nil, fmt.Errorf("load active profile config: %w", err) + if s.daemonOwnerStore == nil { + return nil, gstatus.Error(codes.Unavailable, "daemon owner store unavailable") } - cfg.Shared = msg.GetShared() - if err := s.persistActiveProfileConfigLocked(cfg); err != nil { + if err := s.daemonOwnerStore.Save(s.owners.Owners, msg.GetShared()); err != nil { return nil, fmt.Errorf("persist shared flag: %w", err) } - s.config = cfg - log.Infof("active profile shared flag set to %t", msg.GetShared()) + s.owners.Shared = msg.GetShared() + log.Infof("daemon shared flag set to %t", msg.GetShared()) return &proto.ShareProfileResponse{}, nil } diff --git a/client/server/ownership_test.go b/client/server/ownership_test.go index 4163f7b94..6f4e735c4 100644 --- a/client/server/ownership_test.go +++ b/client/server/ownership_test.go @@ -12,9 +12,94 @@ import ( "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/client/proto" "github.com/netbirdio/netbird/util" ) +// fakeOwnerStore is an in-memory server.DaemonOwnerStore for tests. +type fakeOwnerStore struct { + owners []string + shared bool +} + +func (f *fakeOwnerStore) Load() ([]string, bool, error) { return f.owners, f.shared, nil } +func (f *fakeOwnerStore) Save(o []string, s bool) error { f.owners, f.shared = o, s; return nil } + +// useTempProfileDirs points the profilemanager globals at a temp dir so +// GetActiveProfileState resolves to the default profile without touching /etc. +func useTempProfileDirs(t *testing.T) { + t.Helper() + tempDir := t.TempDir() + origDir := profilemanager.DefaultConfigPathDir + origActive := profilemanager.ActiveProfileStatePath + origDefault := profilemanager.DefaultConfigPath + profilemanager.ConfigDirOverride = tempDir + profilemanager.DefaultConfigPathDir = tempDir + profilemanager.ActiveProfileStatePath = filepath.Join(tempDir, "active_profile.json") + profilemanager.DefaultConfigPath = filepath.Join(tempDir, "default.json") + t.Cleanup(func() { + profilemanager.DefaultConfigPathDir = origDir + profilemanager.ActiveProfileStatePath = origActive + profilemanager.DefaultConfigPath = origDefault + profilemanager.ConfigDirOverride = "" + }) +} + +// TestDaemonOwnerPolicyDefaultProfile exercises the daemon-wide owner branch that +// governs the default profile: TOFU claim, add and reset, all via the store. +func TestDaemonOwnerPolicyDefaultProfile(t *testing.T) { + useTempProfileDirs(t) + + store := &fakeOwnerStore{} + s := &Server{profileManager: &profilemanager.ServiceManager{}, groupResolver: ipcauth.NewDefaultGroupResolver()} + s.SetDaemonOwnerStore(store) + + // Active profile is the default, daemon is unowned to start. + o := s.ActiveProfileOwnership() + assert.Empty(t, o.Owners) + assert.False(t, o.Shared) + + // Trust-on-first-use: the first caller claims daemon ownership, persisted. + claimed, err := s.ClaimActiveProfileOwnerIfUnowned(ipcauth.Identity{UID: 1000}) + require.NoError(t, err) + assert.True(t, claimed) + assert.Equal(t, []string{"uid:1000"}, store.owners) + assert.Equal(t, []string{"uid:1000"}, s.ActiveProfileOwnership().Owners) + + // A second, different caller does not re-claim an owned daemon. + claimed, err = s.ClaimActiveProfileOwnerIfUnowned(ipcauth.Identity{UID: 1001}) + require.NoError(t, err) + assert.False(t, claimed) + + // AddOwner appends a daemon-wide principal (persisted). + _, err = s.AddOwner(context.Background(), &proto.AddOwnerRequest{Principal: "uid:1001"}) + require.NoError(t, err) + assert.Equal(t, []string{"uid:1000", "uid:1001"}, store.owners) + + // ResetOwner (privileged) clears the daemon owner set. + _, err = s.ResetOwner(ctxWithIdentity(ipcauth.Identity{UID: 0}), &proto.ResetOwnerRequest{}) + require.NoError(t, err) + assert.Empty(t, store.owners) + assert.False(t, store.shared) +} + +// TestDaemonOwnerAllOwnersUseDefault verifies every daemon owner is authorized +// for the default profile, while a non-owner is denied. +func TestDaemonOwnerAllOwnersUseDefault(t *testing.T) { + s := &Server{groupResolver: ipcauth.NewDefaultGroupResolver()} + s.owners = ipcauth.Ownership{Owners: []string{"uid:1000", "uid:1001"}} + + deflt := &profilemanager.Profile{ID: profilemanager.ID(profilemanager.DefaultProfileName), Name: "default"} + + // Both owners may use the default profile. + require.NoError(t, s.authorizeTargetProfile(ctxWithIdentity(ipcauth.Identity{UID: 1000}), deflt, true)) + require.NoError(t, s.authorizeTargetProfile(ctxWithIdentity(ipcauth.Identity{UID: 1001}), deflt, true)) + + // A non-owner is denied the default profile. + err := s.authorizeTargetProfile(ctxWithIdentity(ipcauth.Identity{UID: 2000}), deflt, true) + assert.Equal(t, codes.PermissionDenied, gstatus.Code(err)) +} + // writeTargetProfile writes a profile JSON with the given ownership and returns // a Profile handle pointing at it (Path set, so FilePath() resolves directly). func writeTargetProfile(t *testing.T, dir, id string, owners []string, shared bool) *profilemanager.Profile { diff --git a/client/server/server.go b/client/server/server.go index e040c2337..091a7db4f 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -130,9 +130,18 @@ type Server struct { // groupResolver resolves a Unix caller's supplementary group membership // (NSS/getent) so gid:/group: owner principals authorize correctly. Nil on - // Windows (SID group membership travels in the identity itself); ipcauth + // Windows, where SID group membership travels in the identity itself. ipcauth // treats a nil resolver as "no group matching". groupResolver ipcauth.GroupResolver + + // owners is the in-memory daemon-wide owner set, loaded from daemonOwnerStore. + // It governs the default profile and daemon-wide access. Non-default profiles + // stay isolated per user via their own per-profile owner. Guarded by s.mutex. + owners ipcauth.Ownership + // daemonOwnerStore persists owners to service.json, injected by cmd via + // SetDaemonOwnerStore. Nil before injection or in tests, where the daemon is + // treated as unowned and non-privileged callers are denied on the default. + daemonOwnerStore DaemonOwnerStore } type oauthAuthFlow struct {