diff --git a/client/server/ownership.go b/client/server/ownership.go index 7791f1f4d..244b702bc 100644 --- a/client/server/ownership.go +++ b/client/server/ownership.go @@ -127,6 +127,60 @@ func (s *Server) claimForCallerLocked(id ipcauth.Identity, cfg *profilemanager.C return nil } +// authorizeTargetProfile authorizes a caller to operate on a specific target +// profile. It MUST be called after bindCallerUsername, which enforces the legacy +// per-username-directory guard. this layers the collision-free Owners field on +// top of it: +// +// - Privileged callers (root / elevated-admin) may operate on any profile. +// - If the target has Owners (or is Shared), they are authoritative. This +// disambiguates users whose sanitized usernames collide. +// - If the target is unowned (a legacy profile predating ownership), passing +// the username guard is sufficient and then the profile is claimed. +// +// Caller must hold s.mutex (it may persist an ownership claim). +func (s *Server) authorizeTargetProfile(ctx context.Context, target *profilemanager.Profile, claim bool) error { + id, ok := ipcauth.IdentityFromContext(ctx) + if !ok { + return gstatus.Error(codes.PermissionDenied, "caller identity could not be verified") + } + if id.IsPrivileged() { + return nil + } + + path, err := target.FilePath() + if err != nil { + return fmt.Errorf("resolve target profile path: %w", err) + } + cfg, err := profilemanager.GetConfig(path) + if err != nil { + return fmt.Errorf("load target profile config: %w", err) + } + + ownership := ipcauth.Ownership{Owners: cfg.Owners, Shared: cfg.Shared} + + // Owned or shared: the Owners field is authoritative (collision-free). + if len(ownership.Owners) > 0 || ownership.Shared { + if ipcauth.Authorize(ownership, id, s.groupResolver) { + return nil + } + return gstatus.Errorf(codes.PermissionDenied, + "not authorized to operate on profile %q (owned by another principal)", target.Name) + } + + // Unowned legacy profile: the username guard authorizes. Stamp the caller + // as owner so future access is collision-free. + if claim { + principal := ipcauth.OwnerPrincipalForIdentity(id) + cfg.Owners = []string{principal} + if err := util.WriteJson(context.Background(), path, cfg); err != nil { + return fmt.Errorf("persist profile ownership claim: %w", err) + } + log.Infof("profile %q (%s) claimed by %s on first access (trust-on-first-use)", target.Name, target.ID, id) + } + 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 // validates and persists. diff --git a/client/server/ownership_test.go b/client/server/ownership_test.go new file mode 100644 index 000000000..4163f7b94 --- /dev/null +++ b/client/server/ownership_test.go @@ -0,0 +1,88 @@ +package server + +import ( + "context" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/internal/ipcauth" + "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/util" +) + +// 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 { + t.Helper() + path := filepath.Join(dir, id+".json") + cfg := &profilemanager.Config{Owners: owners, Shared: shared} + require.NoError(t, util.WriteJson(context.Background(), path, cfg)) + return &profilemanager.Profile{ID: profilemanager.ID(id), Name: id, Path: path} +} + +func readOwners(t *testing.T, path string) ([]string, bool) { + t.Helper() + cfg, err := profilemanager.GetConfig(path) + require.NoError(t, err) + return cfg.Owners, cfg.Shared +} + +func TestAuthorizeTargetProfile(t *testing.T) { + s := &Server{groupResolver: ipcauth.NewDefaultGroupResolver()} + owner := ipcauth.Identity{UID: 1000} + other := ipcauth.Identity{UID: 1001} + root := ipcauth.Identity{UID: 0} + + t.Run("no identity denies", func(t *testing.T) { + p := writeTargetProfile(t, t.TempDir(), "p", []string{"uid:1000"}, false) + err := s.authorizeTargetProfile(context.Background(), p, true) + assert.Equal(t, codes.PermissionDenied, gstatus.Code(err)) + }) + + t.Run("privileged allowed on another's profile", func(t *testing.T) { + p := writeTargetProfile(t, t.TempDir(), "p", []string{"uid:1000"}, false) + assert.NoError(t, s.authorizeTargetProfile(ctxWithIdentity(root), p, true)) + }) + + t.Run("owner allowed", func(t *testing.T) { + p := writeTargetProfile(t, t.TempDir(), "p", []string{"uid:1000"}, false) + assert.NoError(t, s.authorizeTargetProfile(ctxWithIdentity(owner), p, true)) + }) + + t.Run("non-owner denied", func(t *testing.T) { + p := writeTargetProfile(t, t.TempDir(), "p", []string{"uid:1000"}, false) + err := s.authorizeTargetProfile(ctxWithIdentity(other), p, true) + assert.Equal(t, codes.PermissionDenied, gstatus.Code(err)) + }) + + t.Run("shared allows any caller", func(t *testing.T) { + p := writeTargetProfile(t, t.TempDir(), "p", nil, true) + assert.NoError(t, s.authorizeTargetProfile(ctxWithIdentity(other), p, true)) + }) + + t.Run("unowned claim stamps owner", func(t *testing.T) { + p := writeTargetProfile(t, t.TempDir(), "p", nil, false) + require.NoError(t, s.authorizeTargetProfile(ctxWithIdentity(other), p, true)) + + owners, shared := readOwners(t, p.Path) + assert.Equal(t, []string{"uid:1001"}, owners) + assert.False(t, shared) + + // A different caller is now locked out of the claimed profile. + err := s.authorizeTargetProfile(ctxWithIdentity(owner), p, true) + assert.Equal(t, codes.PermissionDenied, gstatus.Code(err)) + }) + + t.Run("unowned without claim leaves profile unowned", func(t *testing.T) { + p := writeTargetProfile(t, t.TempDir(), "p", nil, false) + require.NoError(t, s.authorizeTargetProfile(ctxWithIdentity(other), p, false)) + + owners, _ := readOwners(t, p.Path) + assert.Empty(t, owners) + }) +} diff --git a/client/server/profile_authz.go b/client/server/profile_authz.go index b1a3876ae..a53b3ee4b 100644 --- a/client/server/profile_authz.go +++ b/client/server/profile_authz.go @@ -13,14 +13,12 @@ import ( ) // bindCallerUsername enforces that a non-privileged caller may only operate on -// its OWN user's profiles. Profiles live in per-username directories, but the -// username is a client-supplied gRPC field the server historically never -// checked; binding it to the caller's kernel identity closes the "pass another -// user's username" hole. Privileged callers (root / elevated-admin) may manage -// any user's profiles. +// its OWN user's profiles. This binds the client-supplied gRPC field to the +// caller's kernel identity. +// Privileged callers (root / elevated-admin) may manage any user's profiles. func (s *Server) bindCallerUsername(ctx context.Context, requested string) error { if requested == "" { - return nil // handlers validate emptiness themselves; nothing to bind + return nil } id, ok := ipcauth.IdentityFromContext(ctx) diff --git a/client/server/server.go b/client/server/server.go index ebe96c2c2..e040c2337 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -127,6 +127,12 @@ type Server struct { updateManager *updater.Manager jwtCache *jwtCache + + // 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 + // treats a nil resolver as "no group matching". + groupResolver ipcauth.GroupResolver } type oauthAuthFlow struct { @@ -151,6 +157,7 @@ func New(ctx context.Context, logFile string, configFile string, profilesDisable jwtCache: newJWTCache(), extendAuthSessionFlow: auth.NewPendingFlow(), probeThrottle: newProbeThrottle(probeThreshold), + groupResolver: ipcauth.NewDefaultGroupResolver(), } agent := &serverAgent{s} s.sleepHandler = sleephandler.New(agent) @@ -1080,6 +1087,19 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi if err := s.bindCallerUsername(callerCtx, targetUsername); err != nil { return nil, err } + // Authorize against the target profile's owners, claiming an unowned + // legacy target for the caller. + resolveUsername := targetUsername + if *msg.ProfileName == profilemanager.DefaultProfileName { + resolveUsername = "" + } + resolvedTarget, err := s.resolveProfileHandle(*msg.ProfileName, resolveUsername) + if err != nil { + return nil, err + } + if err := s.authorizeTargetProfile(callerCtx, resolvedTarget, true); err != nil { + return nil, err + } if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { log.Errorf("failed to switch profile: %v", err) return nil, err @@ -2080,6 +2100,10 @@ func (s *Server) RenameProfile(ctx context.Context, msg *proto.RenameProfileRequ return nil, err } + if err := s.authorizeTargetProfile(ctx, resolved, true); err != nil { + return nil, err + } + err = s.profileManager.RenameProfile(resolved.ID, msg.Username, msg.NewProfileName) if err != nil { log.Errorf("failed to rename profile: %v", err) @@ -2113,6 +2137,11 @@ func (s *Server) RemoveProfile(ctx context.Context, msg *proto.RemoveProfileRequ return nil, err } + // claim=false: don't stamp ownership on a profile we're about to delete. + if err := s.authorizeTargetProfile(ctx, resolved, false); err != nil { + return nil, err + } + if err := s.logoutFromProfile(ctx, resolved); err != nil { log.Warnf("failed to logout from profile %s before removal: %v", resolved.ID, err) }