diff --git a/client/internal/profilemanager/profilemanager.go b/client/internal/profilemanager/profilemanager.go index 6813a0d6d..b93663828 100644 --- a/client/internal/profilemanager/profilemanager.go +++ b/client/internal/profilemanager/profilemanager.go @@ -32,6 +32,10 @@ type Profile struct { Path string IsActive bool Owners []ipcauth.Principal + // LegacyUserDir is the sanitized username of the per-username directory the + // profile was found in, empty for the profiles.v1 directory and for the + // default profile. It is the only legacy evidence of profile ownership. + LegacyUserDir string } // AccessibleBy reports whether a kernel-attested caller may address this @@ -43,9 +47,11 @@ func (p *Profile) AccessibleBy(id ipcauth.Identity) bool { if ipcauth.IsPrivilegedCaller(id) { return true } - // TODO: decide on unowned behavior if len(p.Owners) == 0 { - return false + // A profile in a per-username directory belonged to a use, unowned fails + // closed. The account named by the directory reclaims it on their next + // lookup or until claimed by a privileged caller. + return p.LegacyUserDir == "" } return p.Owners[0].Matches(id) } diff --git a/client/internal/profilemanager/service.go b/client/internal/profilemanager/service.go index 9371967b7..b2337bced 100644 --- a/client/internal/profilemanager/service.go +++ b/client/internal/profilemanager/service.go @@ -10,11 +10,14 @@ import ( "path/filepath" "runtime" "sort" + "strconv" "strings" + "sync" "syscall" log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/client/internal/getent" "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/util" ) @@ -56,11 +59,11 @@ type profileMeta struct { Name string } -// nolint:unused type ownerMeta struct { Owners []string } +// ownersFieldName is the key on disk. func (e *ErrAmbiguousHandle) Error() string { switch e.Kind { case AmbiguityKindIDPrefix: @@ -599,6 +602,8 @@ func (s *ServiceManager) loadAllProfilesForIdentity(userID ipcauth.Identity) ([] return nil, err } + s.claimLegacyProfiles(allProfiles, userID) + accessible := make([]Profile, 0, len(allProfiles)) for _, p := range allProfiles { if p.AccessibleBy(userID) { @@ -609,6 +614,106 @@ func (s *ServiceManager) loadAllProfilesForIdentity(userID ipcauth.Identity) ([] return accessible, nil } +var ( + legacyDirMu sync.Mutex + legacyDirCache = map[string]string{} +) + +// claimLegacyProfiles stamps the caller on every unowned profile in the +// directory their own user name produced before the ownership model. +// +// Ownership lives in the file now, so the directory name is only a leftover. +// Flattening is a separate step we are doing in the future. Moving it would +// pull the state file out from under an engine that captured its path at +// connect time. +func (s *ServiceManager) claimLegacyProfiles(profiles []Profile, id ipcauth.Identity) { + // A privileged caller reaches every profile already and an internal load + // has no caller, so neither should leave an owner behind. + if !id.Known() || ipcauth.IsPrivilegedCaller(id) { + return + } + + if !hasUnownedLegacyProfile(profiles) { + return + } + + dir, ok := legacyDirForIdentity(id) + if !ok { + return + } + + principal := ipcauth.OwnerPrincipalForIdentity(id) + parsed, ok := ipcauth.ParsePrincipal(principal) + if !ok { + log.Warnf("not claiming legacy profiles, %q is not a usable owner", principal) + return + } + + for i := range profiles { + p := &profiles[i] + if len(p.Owners) > 0 || p.LegacyUserDir == "" || p.LegacyUserDir != dir { + continue + } + + if err := StampOwner(p.Path, id); err != nil { + log.Warnf("could not claim legacy profile %s for %s: %v", p.Path, principal, err) + continue + } + + p.Owners = []ipcauth.Principal{parsed} + log.Infof("claimed legacy profile %s for %s, its directory is named after that account", p.Path, principal) + } +} + +func hasUnownedLegacyProfile(profiles []Profile) bool { + for i := range profiles { + if profiles[i].LegacyUserDir != "" && len(profiles[i].Owners) == 0 { + return true + } + } + return false +} + +// legacyDirForIdentity is a variable so a test can supply an account name +// without depending on the host's user database. +var legacyDirForIdentity = resolveLegacyDir + +// resolveLegacyDir returns the per-username directory the old layout would have +// created for a caller, and whether there is one. +// +// Successes are cached for the process, failures are not, so a directory +// service that is briefly unreachable does not lock its users out until the +// daemon restarts. +func resolveLegacyDir(id ipcauth.Identity) (string, bool) { + key := ipcauth.OwnerPrincipalForIdentity(id) + + legacyDirMu.Lock() + cached, hit := legacyDirCache[key] + legacyDirMu.Unlock() + if hit { + return cached, cached != "" + } + + lookup := strconv.FormatUint(uint64(id.UID), 10) + if id.IsWindows() { + lookup = id.SID + } + + u, err := getent.LookupUserID(lookup) + if err != nil { + log.Warnf("cannot resolve %s to an account name, its legacy profiles stay unowned: %v", key, err) + return "", false + } + + dir := sanitizeProfileName(u.Username) + + legacyDirMu.Lock() + legacyDirCache[key] = dir + legacyDirMu.Unlock() + + return dir, dir != "" +} + func (s *ServiceManager) loadAllProfiles() ([]Profile, error) { _, activeIsDefault := s.activeProfileID() defaultName := readProfileName(DefaultConfigPath) @@ -643,16 +748,23 @@ func (s *ServiceManager) loadAllProfiles() ([]Profile, error) { // from before the ID-keyed layout. The first is not necessarily under // DefaultConfigPathDir: a ServiceManager can be pointed at a directory of // its own, which is what the mobile bindings do. - dirs := []string{s.profilesDirPath()} + dirs := []profileDir{{path: s.profilesDirPath()}} configPathDir, err := os.ReadDir(DefaultConfigPathDir) if err != nil && !errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("read profile directory: %w", err) } for _, entry := range configPathDir { - if entry.IsDir() { - dirs = append(dirs, filepath.Join(DefaultConfigPathDir, entry.Name())) + if !entry.IsDir() || entry.Name() == DefaultProfilePathDir { + continue } + // Every other subdirectory is named after the account that created the + // profiles in it. The dot in profiles.v1 is what keeps them apart, + // since sanitizeProfileName drops dots. + dirs = append(dirs, profileDir{ + path: filepath.Join(DefaultConfigPathDir, entry.Name()), + legacyUser: entry.Name(), + }) } var fileProfiles []Profile @@ -660,10 +772,10 @@ func (s *ServiceManager) loadAllProfiles() ([]Profile, error) { for _, dir := range dirs { // The profiles directory is usually one of the subdirectories above, // so without this a profile would be listed twice. - if scanned[dir] { + if scanned[dir.path] { continue } - scanned[dir] = true + scanned[dir.path] = true dirProfiles, err := s.getProfilesFromDirectory(dir) if err != nil { @@ -683,7 +795,15 @@ func (s *ServiceManager) loadAllProfiles() ([]Profile, error) { return profiles, nil } -func (s *ServiceManager) getProfilesFromDirectory(configDir string) ([]Profile, error) { +// profileDir is one directory the loader scans. legacyUser is the sanitized +// username it is named after, empty for the directory profiles go to now. +type profileDir struct { + path string + legacyUser string +} + +func (s *ServiceManager) getProfilesFromDirectory(dir profileDir) ([]Profile, error) { + configDir := dir.path activeID, _ := s.activeProfileID() entries, err := os.ReadDir(configDir) if err != nil { @@ -725,11 +845,12 @@ func (s *ServiceManager) getProfilesFromDirectory(configDir string) ([]Profile, continue } fileProfiles = append(fileProfiles, Profile{ - ID: stem, - Name: name, - Path: path, - IsActive: stem == ID(activeID), - Owners: owners, + ID: stem, + Name: name, + Path: path, + IsActive: stem == ID(activeID), + Owners: owners, + LegacyUserDir: dir.legacyUser, }) } return fileProfiles, nil @@ -777,12 +898,14 @@ func readProfileOwners(path string) ([]ipcauth.Principal, error) { return []ipcauth.Principal{principal}, nil } -// nolint: unused,unusedfunc +// StampOwner records owner as a profile's owner, replacing whoever is recorded +// now. func StampOwner(path string, owner ipcauth.Identity) error { data, err := os.ReadFile(path) if err != nil { return err } + var cfg Config if err := json.Unmarshal(data, &cfg); err != nil { return err @@ -790,7 +913,7 @@ func StampOwner(path string, owner ipcauth.Identity) error { cfg.Owners = []string{ipcauth.OwnerPrincipalForIdentity(owner)} if err := util.WriteJson(context.Background(), path, cfg); err != nil { - return fmt.Errorf("failed to write profile owner: %w", err) + return fmt.Errorf("write profile owner: %w", err) } return nil } diff --git a/client/internal/profilemanager/service_test.go b/client/internal/profilemanager/service_test.go index 4d6b5f28d..d5b8f5fd5 100644 --- a/client/internal/profilemanager/service_test.go +++ b/client/internal/profilemanager/service_test.go @@ -2,10 +2,12 @@ package profilemanager import ( "context" + "encoding/json" "errors" "os" "os/user" "path/filepath" + "runtime" "strconv" "strings" "testing" @@ -289,7 +291,7 @@ func TestListProfiles_PrivilegedResolvesUnfiltered(t *testing.T) { }) } -func TestListProfiles_UnownedIsPrivilegedOnly(t *testing.T) { +func TestListProfiles_UnownedOutsideALegacyDirIsOpen(t *testing.T) { withTestSM(t, func(sm *ServiceManager, _ ipcauth.Identity) { unowned, err := sm.AddProfile("unowned", nil) require.NoError(t, err) @@ -297,16 +299,257 @@ func TestListProfiles_UnownedIsPrivilegedOnly(t *testing.T) { alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) got, err := sm.ListProfiles(alice) require.NoError(t, err) - assert.NotContains(t, profileIDs(got), unowned.ID.String(), - "an unowned profile is not addressable until it is claimed") + assert.Contains(t, profileIDs(got), unowned.ID.String(), + "a profile that never had an owner stays usable until someone claims it") + assert.Contains(t, profileIDs(got), defaultProfileName, + "a fresh install has to be usable before anything is claimed") + + nobody, err := sm.ListProfiles(ipcauth.Identity{}) + require.NoError(t, err) + assert.Empty(t, profileIDs(nobody), "an unattested caller reaches nothing") + }) +} + +// withLegacyLayout wires up the globals the way withTestSM does, without the +// identity, since these tests supply their own. +func withLegacyLayout(t *testing.T, fn func(sm *ServiceManager, configDir string)) { + t.Helper() + withTempConfigDir(t, func(configDir string) { + withPatchedGlobals(t, configDir, func() { + sm := &ServiceManager{} + require.NoError(t, sm.CreateDefaultProfile()) + fn(sm, configDir) + }) + }) +} + +// writeLegacyProfile drops a profile into a per-username directory, the layout +// every profile used before the ID-keyed one. It writes no name, since a legacy +// profile took its display name from the filename. +func writeLegacyProfile(t *testing.T, configDir, dirName, id string, fields map[string]any) string { + t.Helper() + dir := filepath.Join(configDir, dirName) + require.NoError(t, os.MkdirAll(dir, 0700)) + + doc := map[string]any{} + for k, v := range fields { + doc[k] = v + } + path := filepath.Join(dir, id+".json") + require.NoError(t, util.WriteJson(context.Background(), path, doc)) + return path +} + +// stubLegacyDir replaces the account lookup, so the claim path can be exercised +// without an account existing on the machine running the test. +func stubLegacyDir(t *testing.T, dir string) { + t.Helper() + orig := legacyDirForIdentity + legacyDirForIdentity = func(ipcauth.Identity) (string, bool) { return dir, dir != "" } + t.Cleanup(func() { legacyDirForIdentity = orig }) +} + +func readOwners(t *testing.T, path string) []string { + t.Helper() + data, err := os.ReadFile(path) + require.NoError(t, err) + var meta ownerMeta + require.NoError(t, json.Unmarshal(data, &meta)) + return meta.Owners +} + +func TestListProfiles_UnownedLegacyProfileIsPrivilegedOnly(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, configDir string) { + path := writeLegacyProfile(t, configDir, "alice", "work", nil) + stubLegacyDir(t, "bob") + + bob := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ListProfiles(bob) + require.NoError(t, err) + assert.NotContains(t, profileIDs(got), "work", + "a profile sitting in someone else's directory is not free to take") + assert.Empty(t, readOwners(t, path), "and it is not claimed on the way past") root := ipcauth.KnownForTest(ipcauth.Identity{UID: 0}) got, err = sm.ListProfiles(root) require.NoError(t, err) - assert.Contains(t, profileIDs(got), unowned.ID.String()) + assert.Contains(t, profileIDs(got), "work", + "root still reaches it, which is how it gets reassigned") }) } +func TestListProfiles_ClaimsLegacyProfileForItsOwnAccount(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, configDir string) { + path := writeLegacyProfile(t, configDir, "alice", "work", nil) + stubLegacyDir(t, "alice") + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ListProfiles(alice) + require.NoError(t, err) + assert.Contains(t, profileIDs(got), "work", + "the claim lands before the listing is filtered, so the gap closes in one call") + assert.Equal(t, []string{"uid:4242"}, readOwners(t, path)) + + // The claim is on disk now, so it is the owner check and not the + // directory name that keeps the next caller out. + stubLegacyDir(t, "alice") + other := ipcauth.KnownForTest(ipcauth.Identity{UID: 5252}) + got, err = sm.ListProfiles(other) + require.NoError(t, err) + assert.NotContains(t, profileIDs(got), "work") + assert.Equal(t, []string{"uid:4242"}, readOwners(t, path), + "a second caller does not overwrite a stamped owner") + }) +} + +func TestClaimLegacyProfile_LeavesTheProfileWhereItIs(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, configDir string) { + path := writeLegacyProfile(t, configDir, "alice", "work", nil) + for _, suffix := range []string{".state.json", prefsFileSuffix} { + require.NoError(t, os.WriteFile(filepath.Join(configDir, "alice", "work"+suffix), []byte("{}"), 0600)) + } + stubLegacyDir(t, "alice") + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ListProfiles(alice) + require.NoError(t, err) + + claimed := ownedProfile(t, got, "uid:4242") + assert.Equal(t, ID("work"), claimed.ID, "claiming does not re-key the profile") + assert.Equal(t, path, claimed.Path) + assert.Equal(t, "alice", claimed.LegacyUserDir, + "the directory is a leftover now, not something the claim rewrites") + + // The engine captures its state file path at connect time and writes + // back to whatever it captured, so nothing here may move that file. + assert.FileExists(t, filepath.Join(configDir, "alice", "work.state.json")) + assert.FileExists(t, filepath.Join(configDir, "alice", "work"+prefsFileSuffix)) + assert.NoFileExists(t, filepath.Join(configDir, DefaultProfilePathDir, "work.json")) + }) +} + +func TestClaimLegacyProfile_NamesakeInAnotherDirectoryIsUntouched(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, configDir string) { + writeLegacyProfile(t, configDir, "alice", "work", nil) + bobs := writeLegacyProfile(t, configDir, "bob", "work", nil) + stubLegacyDir(t, "alice") + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ListProfiles(alice) + require.NoError(t, err) + + claimed := ownedProfile(t, got, "uid:4242") + assert.Equal(t, filepath.Join(configDir, "alice", "work.json"), claimed.Path, + "a legacy ID is a display name two accounts can hold, so the path is what tells them apart") + assert.Empty(t, readOwners(t, bobs), "bob's namesake is not claimed") + }) +} + +func ownedProfile(t *testing.T, profiles []Profile, principal string) Profile { + t.Helper() + var found []Profile + for _, p := range profiles { + if len(p.Owners) == 1 && p.Owners[0].String() == principal { + found = append(found, p) + } + } + require.Len(t, found, 1, "exactly one profile is owned by %s", principal) + return found[0] +} + +func TestClaimLegacyProfile_SkipsOneAlreadyOwnedInTheSameDirectory(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, configDir string) { + // One unowned profile keeps the claim running over the directory, so + // the owned one beside it is reached and has to be left alone. + taken := writeLegacyProfile(t, configDir, "alice", "taken", map[string]any{ + "Owners": []string{"uid:9999"}, + }) + free := writeLegacyProfile(t, configDir, "alice", "free", nil) + stubLegacyDir(t, "alice") + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + _, err := sm.ListProfiles(alice) + require.NoError(t, err) + + assert.Equal(t, []string{"uid:9999"}, readOwners(t, taken), + "a profile that already has an owner is not restamped") + assert.Equal(t, []string{"uid:4242"}, readOwners(t, free)) + }) +} + +func TestListProfiles_PrivilegedCallerDoesNotClaim(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, configDir string) { + path := writeLegacyProfile(t, configDir, "root", "work", nil) + stubLegacyDir(t, "root") + + root := ipcauth.KnownForTest(ipcauth.Identity{UID: 0}) + got, err := sm.ListProfiles(root) + require.NoError(t, err) + assert.Contains(t, profileIDs(got), "work") + assert.Empty(t, readOwners(t, path), + "root reaches every profile anyway, so a listing must not stamp one") + }) +} + +func TestStampOwner(t *testing.T) { + withLegacyLayout(t, func(_ *ServiceManager, configDir string) { + path := writeLegacyProfile(t, configDir, "alice", "work", map[string]any{ + "DisableAutoConnect": true, + }) + + require.NoError(t, StampOwner(path, ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}))) + assert.Equal(t, []string{"uid:4242"}, readOwners(t, path)) + + var fields map[string]any + data, err := os.ReadFile(path) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(data, &fields)) + assert.Equal(t, true, fields["DisableAutoConnect"], + "the rest of the config survives the stamp") + + require.NoError(t, StampOwner(path, ipcauth.KnownForTest(ipcauth.Identity{UID: 5252}))) + assert.Equal(t, []string{"uid:5252"}, readOwners(t, path), + "stamping replaces whoever is recorded, the caller decides whether to") + }) +} + +func TestClaimLegacyProfile_LeavesAnOwnerItCannotParse(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, configDir string) { + // A principal kind this build does not know, which is what a client + // newer than this one leaves behind after a downgrade. The loader + // drops such a profile rather than reporting it as unowned, which is + // what keeps the claim from treating it as free to take. + path := writeLegacyProfile(t, configDir, "alice", "work", map[string]any{ + "Owners": []string{"group:devs"}, + }) + stubLegacyDir(t, "alice") + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ListProfiles(alice) + require.NoError(t, err) + + assert.NotContains(t, profileIDs(got), "work") + assert.Equal(t, []string{"group:devs"}, readOwners(t, path), + "the profile is not taken over, it waits for an explicit claim") + }) +} + +func TestResolveLegacyDir_SanitizesTheSameStringTheOldLayoutDid(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the windows path resolves a SID rather than a numeric uid") + } + + u, err := user.Current() + require.NoError(t, err) + uid, err := strconv.ParseUint(u.Uid, 10, 32) + require.NoError(t, err) + + got, ok := resolveLegacyDir(ipcauth.KnownForTest(ipcauth.Identity{UID: uint32(uid)})) + require.True(t, ok) + assert.Equal(t, sanitizeProfileName(u.Username), got, + "the directory has to come from the account name, sanitized the way the old layout did") +} + func TestListProfiles_UnreadableOwnersAreSkipped(t *testing.T) { withTestSM(t, func(sm *ServiceManager, _ ipcauth.Identity) { configDir, err := sm.getConfigDir()