From df8153d4b1892a63e261b874b77d1062038a796d Mon Sep 17 00:00:00 2001 From: Erik Tate Date: Wed, 2 Oct 2024 13:44:46 -0400 Subject: [PATCH 1/3] takes recursive ownership of home directories when they already exist --- integration/hostuser_test.go | 40 +++++++++++++++++ .../hostusers_linux_test.go} | 12 ++--- lib/srv/usermgmt.go | 2 +- lib/srv/usermgmt_linux.go | 44 ++++++++++++++++--- lib/utils/fs.go | 29 ------------ 5 files changed, 86 insertions(+), 41 deletions(-) rename lib/{utils/fs_unix_test.go => srv/hostusers_linux_test.go} (93%) diff --git a/integration/hostuser_test.go b/integration/hostuser_test.go index 23f385d045c4..0a070e3eb5cd 100644 --- a/integration/hostuser_test.go +++ b/integration/hostuser_test.go @@ -32,6 +32,7 @@ import ( "path/filepath" "slices" "strings" + "syscall" "testing" "time" @@ -203,6 +204,45 @@ func TestRootHostUsersBackend(t *testing.T) { require.ErrorIs(t, err, os.ErrExist) require.NoFileExists(t, "/tmp/ignoreme") }) + + t.Run("Test CreateHomeDirectory recursively takes ownership of existing directory", func(t *testing.T) { + otheruser := "other-user" + t.Cleanup(func() { cleanupUsersAndGroups([]string{testuser, otheruser}, nil) }) + err := usersbk.CreateUser(testuser, nil, host.UserOpts{}) + require.NoError(t, err) + + err = usersbk.CreateUser(otheruser, nil, host.UserOpts{}) + require.NoError(t, err) + + tuser, err := usersbk.Lookup(testuser) + require.NoError(t, err) + + ouser, err := usersbk.Lookup(otheruser) + require.NoError(t, err) + + require.NoError(t, os.WriteFile("/etc/skel/testfile", []byte("test\n"), 0o700)) + + testHome := filepath.Join("/home", testuser) + err = usersbk.CreateHomeDirectory(testHome, ouser.Uid, ouser.Gid) + t.Cleanup(func() { + os.RemoveAll(testHome) + }) + require.NoError(t, err) + + info, err := os.Stat(testHome) + require.NoError(t, err) + initialOwnerUID := info.Sys().(*syscall.Stat_t).Uid + + err = usersbk.CreateHomeDirectory(testHome, tuser.Uid, tuser.Gid) + require.ErrorIs(t, err, os.ErrExist) + + info, err = os.Stat(testHome) + require.NoError(t, err) + finalOwnerUID := info.Sys().(*syscall.Stat_t).Uid + + require.NotEqual(t, initialOwnerUID, finalOwnerUID) + require.Equal(t, tuser.Uid, fmt.Sprintf("%d", finalOwnerUID)) + }) } func getUserGroups(t *testing.T, u *user.User) []string { diff --git a/lib/utils/fs_unix_test.go b/lib/srv/hostusers_linux_test.go similarity index 93% rename from lib/utils/fs_unix_test.go rename to lib/srv/hostusers_linux_test.go index 403e2efbd9b0..a822cd770c26 100644 --- a/lib/utils/fs_unix_test.go +++ b/lib/srv/hostusers_linux_test.go @@ -1,5 +1,5 @@ -//go:build !windows -// +build !windows +//go:build linux +// +build linux /* * Teleport @@ -19,7 +19,7 @@ * along with this program. If not, see . */ -package utils +package srv import ( "os" @@ -93,14 +93,14 @@ func TestRecursiveChown(t *testing.T) { t.Run("notFoundError", func(t *testing.T) { t.Parallel() - require.Error(t, RecursiveChown("/invalid/path/to/nowhere", 1000, 1000)) + require.Error(t, recursiveChown("/invalid/path/to/nowhere", 1000, 1000)) }) t.Run("simpleChown", func(t *testing.T) { t.Parallel() _, _, newUid, newGid, _ := setupRecursiveChownUser(t) dir1Path, dir1FilePath, _, _ := setupRecursiveChownFiles(t) - require.NoError(t, RecursiveChown(dir1Path, newUid, newGid)) + require.NoError(t, recursiveChown(dir1Path, newUid, newGid)) // validate ownership matches expected ids verifyOwnership(t, dir1Path, newUid, newGid) verifyOwnership(t, dir1FilePath, newUid, newGid) @@ -114,7 +114,7 @@ func TestRecursiveChown(t *testing.T) { } _, dir1FilePath, dir2Path, dir2SymlinkToFile := setupRecursiveChownFiles(t) - require.NoError(t, RecursiveChown(dir2Path, newUid, newGid)) + require.NoError(t, recursiveChown(dir2Path, newUid, newGid)) // Validate symlink has changed verifyOwnership(t, dir2SymlinkToFile, newUid, newGid) // Validate pointed file has not changed diff --git a/lib/srv/usermgmt.go b/lib/srv/usermgmt.go index 23397f8ef11d..8a782870bbc5 100644 --- a/lib/srv/usermgmt.go +++ b/lib/srv/usermgmt.go @@ -588,7 +588,7 @@ func (u *HostUserManagement) UserCleanup() { err := u.DeleteAllUsers() switch { case trace.IsNotFound(err): - u.log.DebugContext(u.ctx, "Error during temporary user cleanup, stopping cleanup job", "error", err) + u.log.Debug("Error during temporary user cleanup, stopping cleanup job", "error", err) return case err != nil: u.log.ErrorContext(u.ctx, "Error during temporary user cleanup", "error", err) diff --git a/lib/srv/usermgmt_linux.go b/lib/srv/usermgmt_linux.go index 3626a599beb7..194404678b56 100644 --- a/lib/srv/usermgmt_linux.go +++ b/lib/srv/usermgmt_linux.go @@ -22,6 +22,7 @@ import ( "bufio" "errors" "fmt" + "io/fs" "os" "os/exec" "os/user" @@ -237,11 +238,42 @@ func readDefaultSkel() (string, error) { return skel, trace.Wrap(err) } +func recursiveChown(dir string, uid, gid int) error { + // First, walk the directory to gather a list of files and directories to update before we open up to modifications + var pathsToUpdate []string + err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return trace.Wrap(err) + } + pathsToUpdate = append(pathsToUpdate, path) + return nil + }) + if err != nil { + return trace.Wrap(err) + } + + pathsToUpdate = append(pathsToUpdate, dir) + // filepath.WalkDir is documented to walk the paths in lexical order, iterating + // in the reverse order ensures that files are always Lchowned before their parent directory + for i := len(pathsToUpdate) - 1; i >= 0; i-- { + path := pathsToUpdate[i] + if err := os.Lchown(path, uid, gid); err != nil { + if errors.Is(err, os.ErrNotExist) { + // Unexpected condition where file was removed after discovery. + continue + } + return trace.Wrap(err) + } + } + return nil +} + func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS string) error { uid, err := strconv.Atoi(uidS) if err != nil { return trace.Wrap(err) } + gid, err := strconv.Atoi(gidS) if err != nil { return trace.Wrap(err) @@ -249,6 +281,12 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS err = os.Mkdir(userHome, 0o700) if err != nil { + if os.IsExist(err) { + if chownErr := recursiveChown(userHome, uid, gid); chownErr != nil { + return trace.Wrap(chownErr) + } + } + return trace.Wrap(err) } @@ -277,9 +315,5 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS } } - if err := utils.RecursiveChown(userHome, uid, gid); err != nil { - return trace.Wrap(err) - } - - return nil + return trace.Wrap(recursiveChown(userHome, uid, gid)) } diff --git a/lib/utils/fs.go b/lib/utils/fs.go index dbfd1b390d7b..b12c08a111f8 100644 --- a/lib/utils/fs.go +++ b/lib/utils/fs.go @@ -393,35 +393,6 @@ func RemoveFileIfExist(filePath string) error { return nil } -func RecursiveChown(dir string, uid, gid int) error { - // First, walk the directory to gather a list of files and directories to update before we open up to modifications - var pathsToUpdate []string - err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { - if err != nil { - return trace.Wrap(err) - } - pathsToUpdate = append(pathsToUpdate, path) - return nil - }) - if err != nil { - return trace.Wrap(err) - } - - // filepath.WalkDir is documented to walk the paths in lexical order, iterating - // in the reverse order ensures that files are always Lchowned before their parent directory - for i := len(pathsToUpdate) - 1; i >= 0; i-- { - path := pathsToUpdate[i] - if err := os.Lchown(path, uid, gid); err != nil { - if errors.Is(err, os.ErrNotExist) { - // Unexpected condition where file was removed after discovery. - continue - } - return trace.Wrap(err) - } - } - return nil -} - func CopyFile(src, dest string, perm os.FileMode) error { srcFile, err := os.Open(src) if err != nil { From 98709b924a930c80b4860b1c21effd56b1c83d3c Mon Sep 17 00:00:00 2001 From: Erik Tate Date: Fri, 4 Oct 2024 11:41:41 -0400 Subject: [PATCH 2/3] adding some safety around recursively chowning files owned by existing users --- integration/hostuser_test.go | 46 ++++++++++++++---- lib/srv/usermgmt_linux.go | 93 +++++++++++++++++++++++++++++------- 2 files changed, 114 insertions(+), 25 deletions(-) diff --git a/integration/hostuser_test.go b/integration/hostuser_test.go index 0a070e3eb5cd..284854bebf49 100644 --- a/integration/hostuser_test.go +++ b/integration/hostuser_test.go @@ -206,7 +206,14 @@ func TestRootHostUsersBackend(t *testing.T) { }) t.Run("Test CreateHomeDirectory recursively takes ownership of existing directory", func(t *testing.T) { + getOwnerUID := func(path string) uint32 { + info, err := os.Stat(path) + require.NoError(t, err) + return info.Sys().(*syscall.Stat_t).Uid + } + otheruser := "other-user" + t.Cleanup(func() { cleanupUsersAndGroups([]string{testuser, otheruser}, nil) }) err := usersbk.CreateUser(testuser, nil, host.UserOpts{}) require.NoError(t, err) @@ -229,19 +236,42 @@ func TestRootHostUsersBackend(t *testing.T) { }) require.NoError(t, err) - info, err := os.Stat(testHome) - require.NoError(t, err) - initialOwnerUID := info.Sys().(*syscall.Stat_t).Uid + // add a file owned by root that _shouldn't_ be owned at the end + require.NoError(t, os.WriteFile(filepath.Join(testHome, "rootfile"), []byte("test\n"), 0o700)) + + // grab initial owners + initialRootFileOwner := getOwnerUID(filepath.Join(testHome, "rootfile")) + initialHomeOwner := getOwnerUID(testHome) + initialTestFileOwner := getOwnerUID(filepath.Join(testHome, "testfile")) + // don't take ownership when the user still exists err = usersbk.CreateHomeDirectory(testHome, tuser.Uid, tuser.Gid) require.ErrorIs(t, err, os.ErrExist) - info, err = os.Stat(testHome) - require.NoError(t, err) - finalOwnerUID := info.Sys().(*syscall.Stat_t).Uid + finalHomeOwner := getOwnerUID(testHome) + finalTestFileOwner := getOwnerUID(filepath.Join(testHome, "testfile")) + finalRootFileOwner := getOwnerUID(filepath.Join(testHome, "rootfile")) + + require.Equal(t, initialRootFileOwner, finalRootFileOwner) // "rootfile" ownership unchanged + require.Equal(t, initialHomeOwner, finalHomeOwner) // initial owner still owns directory + require.Equal(t, initialTestFileOwner, finalTestFileOwner) // initial owner still owns directory contents + require.NotEqual(t, tuser.Uid, fmt.Sprintf("%d", finalHomeOwner)) + require.NotEqual(t, tuser.Uid, fmt.Sprintf("%d", finalTestFileOwner)) + + // take ownership of directory and contained files if the original user no longer exists + cleanupUsersAndGroups([]string{otheruser}, nil) + err = usersbk.CreateHomeDirectory(testHome, tuser.Uid, tuser.Gid) + require.ErrorIs(t, err, os.ErrExist) + + finalHomeOwner = getOwnerUID(testHome) + finalTestFileOwner = getOwnerUID(filepath.Join(testHome, "testfile")) + finalRootFileOwner = getOwnerUID(filepath.Join(testHome, "rootfile")) - require.NotEqual(t, initialOwnerUID, finalOwnerUID) - require.Equal(t, tuser.Uid, fmt.Sprintf("%d", finalOwnerUID)) + require.Equal(t, initialRootFileOwner, finalRootFileOwner) // root is still the owner + require.NotEqual(t, initialHomeOwner, finalHomeOwner) // owner is no longer initial user + require.NotEqual(t, initialTestFileOwner, finalTestFileOwner) // owner is no longer initial user + require.Equal(t, tuser.Uid, fmt.Sprintf("%d", finalHomeOwner)) // ensure new owner is test-user + require.Equal(t, tuser.Uid, fmt.Sprintf("%d", finalTestFileOwner)) // ensure new owner is test-user }) } diff --git a/lib/srv/usermgmt_linux.go b/lib/srv/usermgmt_linux.go index 194404678b56..25e0584d710b 100644 --- a/lib/srv/usermgmt_linux.go +++ b/lib/srv/usermgmt_linux.go @@ -29,6 +29,7 @@ import ( "path/filepath" "strconv" "strings" + "syscall" "github.com/gravitational/trace" log "github.com/sirupsen/logrus" @@ -238,33 +239,91 @@ func readDefaultSkel() (string, error) { return skel, trace.Wrap(err) } -func recursiveChown(dir string, uid, gid int) error { - // First, walk the directory to gather a list of files and directories to update before we open up to modifications - var pathsToUpdate []string - err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { - if err != nil { - return trace.Wrap(err) +func canTakeOwnership(observedUIDs map[uint32]bool, uid int, fi fs.FileInfo) (bool, error) { + stat := fi.Sys().(*syscall.Stat_t) + if stat == nil { + return false, nil + } + + exists, seen := observedUIDs[stat.Uid] + if seen { + return !exists, nil + } + + _, err := user.LookupId(strconv.FormatUint(uint64(stat.Uid), 10)) + exists = true + if err != nil { + if !errors.Is(err, user.UnknownUserIdError(stat.Uid)) { + return false, trace.Wrap(err) } - pathsToUpdate = append(pathsToUpdate, path) - return nil - }) + + exists = false + } + observedUIDs[stat.Uid] = exists + + return !exists, nil +} + +// recursiveChown changes ownership of a directory and its contents. If called in safe mode, the contained files' ownership will +// be left unchanged if the original owner still exists. +func recursiveChown(dir string, uid, gid int, safe bool) error { + observedUIDs := map[uint32]bool{ + 0: true, // root should always exist, so we can preload that UID + } + + dirFI, err := os.Lstat(dir) if err != nil { return trace.Wrap(err) } - pathsToUpdate = append(pathsToUpdate, dir) - // filepath.WalkDir is documented to walk the paths in lexical order, iterating - // in the reverse order ensures that files are always Lchowned before their parent directory - for i := len(pathsToUpdate) - 1; i >= 0; i-- { - path := pathsToUpdate[i] + // we can short circuit in safe mode if the directory itself can't be taken over + if safe { + canTakeDir, err := canTakeOwnership(observedUIDs, uid, dirFI) + if err != nil { + return trace.WrapWithMessage(err, "taking ownership of %q", dir) + } + + if !canTakeDir { + return trace.WrapWithMessage(os.ErrExist, "can not safely take ownership of %q when owning user still exists", dir) + } + } + + err = filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return trace.Wrap(err) + } + + if safe { + fi, err := d.Info() + if err != nil { + return nil + } + + takeOwnership, err := canTakeOwnership(observedUIDs, uid, fi) + if err != nil { + return nil + } + + if !takeOwnership { + return nil + } + } + if err := os.Lchown(path, uid, gid); err != nil { if errors.Is(err, os.ErrNotExist) { // Unexpected condition where file was removed after discovery. - continue + return nil } return trace.Wrap(err) } + + return nil + }) + + if err != nil { + return trace.Wrap(err) } + return nil } @@ -282,7 +341,7 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS err = os.Mkdir(userHome, 0o700) if err != nil { if os.IsExist(err) { - if chownErr := recursiveChown(userHome, uid, gid); chownErr != nil { + if chownErr := recursiveChown(userHome, uid, gid, true); chownErr != nil { return trace.Wrap(chownErr) } } @@ -315,5 +374,5 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS } } - return trace.Wrap(recursiveChown(userHome, uid, gid)) + return trace.Wrap(recursiveChown(userHome, uid, gid, false)) } From 53c3e8e06a03d4368b1affd43d9e86125c124288 Mon Sep 17 00:00:00 2001 From: Erik Tate Date: Fri, 4 Oct 2024 13:40:30 -0400 Subject: [PATCH 3/3] adding failure case logging for home directory ownership changes --- integration/hostuser_test.go | 3 ++- lib/srv/hostusers_linux_test.go | 8 +++++--- lib/srv/usermgmt.go | 8 +++++--- lib/srv/usermgmt_linux.go | 24 +++++++++++++++++------- lib/srv/usermgmt_other.go | 4 +++- 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/integration/hostuser_test.go b/integration/hostuser_test.go index 284854bebf49..cbc0dce1bbd3 100644 --- a/integration/hostuser_test.go +++ b/integration/hostuser_test.go @@ -82,7 +82,8 @@ func getUserShells(path string) (map[string]string, error) { func TestRootHostUsersBackend(t *testing.T) { utils.RequireRoot(t) sudoersTestDir := t.TempDir() - usersbk := srv.HostUsersProvisioningBackend{} + usersbk, err := srv.NewHostUsersBackend(utils.NewSlogLoggerForTests()) + require.NoError(t, err) sudoersbk := srv.HostSudoersProvisioningBackend{ SudoersPath: sudoersTestDir, HostUUID: "hostuuid", diff --git a/lib/srv/hostusers_linux_test.go b/lib/srv/hostusers_linux_test.go index a822cd770c26..378ac50bb989 100644 --- a/lib/srv/hostusers_linux_test.go +++ b/lib/srv/hostusers_linux_test.go @@ -29,6 +29,7 @@ import ( "syscall" "testing" + "github.com/gravitational/teleport/lib/utils" "github.com/stretchr/testify/require" ) @@ -90,17 +91,18 @@ func verifyOwnership(t *testing.T, path string, uid, gid int) { } func TestRecursiveChown(t *testing.T) { + log := utils.NewSlogLoggerForTests() t.Run("notFoundError", func(t *testing.T) { t.Parallel() - require.Error(t, recursiveChown("/invalid/path/to/nowhere", 1000, 1000)) + require.Error(t, recursiveChown(log, "/invalid/path/to/nowhere", 1000, 1000, false)) }) t.Run("simpleChown", func(t *testing.T) { t.Parallel() _, _, newUid, newGid, _ := setupRecursiveChownUser(t) dir1Path, dir1FilePath, _, _ := setupRecursiveChownFiles(t) - require.NoError(t, recursiveChown(dir1Path, newUid, newGid)) + require.NoError(t, recursiveChown(log, dir1Path, newUid, newGid, false)) // validate ownership matches expected ids verifyOwnership(t, dir1Path, newUid, newGid) verifyOwnership(t, dir1FilePath, newUid, newGid) @@ -114,7 +116,7 @@ func TestRecursiveChown(t *testing.T) { } _, dir1FilePath, dir2Path, dir2SymlinkToFile := setupRecursiveChownFiles(t) - require.NoError(t, recursiveChown(dir2Path, newUid, newGid)) + require.NoError(t, recursiveChown(log, dir2Path, newUid, newGid, false)) // Validate symlink has changed verifyOwnership(t, dir2SymlinkToFile, newUid, newGid) // Validate pointed file has not changed diff --git a/lib/srv/usermgmt.go b/lib/srv/usermgmt.go index 8a782870bbc5..c1949e098bc5 100644 --- a/lib/srv/usermgmt.go +++ b/lib/srv/usermgmt.go @@ -44,8 +44,10 @@ import ( // NewHostUsers initialize a new HostUsers object func NewHostUsers(ctx context.Context, storage services.PresenceInternal, uuid string) HostUsers { + log := slog.With(teleport.ComponentKey, teleport.Component(teleport.ComponentHostUsers)) + //nolint:staticcheck // SA4023. False positive on macOS. - backend, err := newHostUsersBackend() + backend, err := NewHostUsersBackend(log) switch { case trace.IsNotImplemented(err), trace.IsNotFound(err): slog.DebugContext(ctx, "Skipping host user management", "error", err) @@ -56,7 +58,7 @@ func NewHostUsers(ctx context.Context, storage services.PresenceInternal, uuid s } cancelCtx, cancelFunc := context.WithCancel(ctx) return &HostUserManagement{ - log: slog.With(teleport.ComponentKey, teleport.ComponentHostUsers), + log: log, backend: backend, ctx: cancelCtx, cancel: cancelFunc, @@ -588,7 +590,7 @@ func (u *HostUserManagement) UserCleanup() { err := u.DeleteAllUsers() switch { case trace.IsNotFound(err): - u.log.Debug("Error during temporary user cleanup, stopping cleanup job", "error", err) + u.log.DebugContext(u.ctx, "Error during temporary user cleanup, stopping cleanup job", "error", err) return case err != nil: u.log.ErrorContext(u.ctx, "Error during temporary user cleanup", "error", err) diff --git a/lib/srv/usermgmt_linux.go b/lib/srv/usermgmt_linux.go index 25e0584d710b..84ffc698cdec 100644 --- a/lib/srv/usermgmt_linux.go +++ b/lib/srv/usermgmt_linux.go @@ -20,9 +20,11 @@ package srv import ( "bufio" + "context" "errors" "fmt" "io/fs" + "log/slog" "os" "os/exec" "os/user" @@ -40,6 +42,7 @@ import ( // HostUsersProvisioningBackend is used to implement HostUsersBackend type HostUsersProvisioningBackend struct { + log *slog.Logger } // HostSudoersProvisioningBackend is used to implement HostSudoersBackend @@ -50,8 +53,8 @@ type HostSudoersProvisioningBackend struct { SudoersPath string } -// newHostUsersBackend initializes a new OS specific HostUsersBackend -func newHostUsersBackend() (HostUsersBackend, error) { +// NewHostUsersBackend initializes a new OS specific HostUsersBackend +func NewHostUsersBackend(log *slog.Logger) (HostUsersBackend, error) { var missing []string for _, requiredBin := range []string{"usermod", "useradd", "getent", "groupadd", "visudo"} { if _, err := exec.LookPath(requiredBin); err != nil { @@ -62,7 +65,7 @@ func newHostUsersBackend() (HostUsersBackend, error) { return nil, trace.NotFound("missing required binaries: %s", strings.Join(missing, ",")) } - return &HostUsersProvisioningBackend{}, nil + return &HostUsersProvisioningBackend{log: log}, nil } // newHostUsersBackend initializes a new OS specific HostUsersBackend @@ -251,7 +254,8 @@ func canTakeOwnership(observedUIDs map[uint32]bool, uid int, fi fs.FileInfo) (bo } _, err := user.LookupId(strconv.FormatUint(uint64(stat.Uid), 10)) - exists = true + // only set exists to true if the current owner differs from the given uid + exists = stat.Uid != uint32(uid) if err != nil { if !errors.Is(err, user.UnknownUserIdError(stat.Uid)) { return false, trace.Wrap(err) @@ -266,7 +270,9 @@ func canTakeOwnership(observedUIDs map[uint32]bool, uid int, fi fs.FileInfo) (bo // recursiveChown changes ownership of a directory and its contents. If called in safe mode, the contained files' ownership will // be left unchanged if the original owner still exists. -func recursiveChown(dir string, uid, gid int, safe bool) error { +func recursiveChown(log *slog.Logger, dir string, uid, gid int, safe bool) error { + ctx := context.Background() + l := log.With("uid", uid, "gid", gid, "safe", safe) observedUIDs := map[uint32]bool{ 0: true, // root should always exist, so we can preload that UID } @@ -293,14 +299,17 @@ func recursiveChown(dir string, uid, gid int, safe bool) error { return trace.Wrap(err) } + l = log.With("path", path) if safe { fi, err := d.Info() if err != nil { + l.WarnContext(ctx, "Could not retrieve file info for safe file ownership change", "error", err) return nil } takeOwnership, err := canTakeOwnership(observedUIDs, uid, fi) if err != nil { + l.WarnContext(ctx, "Could not determine if file ownership change is safe", "error", err) return nil } @@ -312,6 +321,7 @@ func recursiveChown(dir string, uid, gid int, safe bool) error { if err := os.Lchown(path, uid, gid); err != nil { if errors.Is(err, os.ErrNotExist) { // Unexpected condition where file was removed after discovery. + l.WarnContext(ctx, "File was removed before ownership change", "error", err) return nil } return trace.Wrap(err) @@ -341,7 +351,7 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS err = os.Mkdir(userHome, 0o700) if err != nil { if os.IsExist(err) { - if chownErr := recursiveChown(userHome, uid, gid, true); chownErr != nil { + if chownErr := recursiveChown(u.log, userHome, uid, gid, true); chownErr != nil { return trace.Wrap(chownErr) } } @@ -374,5 +384,5 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS } } - return trace.Wrap(recursiveChown(userHome, uid, gid, false)) + return trace.Wrap(recursiveChown(u.log, userHome, uid, gid, false)) } diff --git a/lib/srv/usermgmt_other.go b/lib/srv/usermgmt_other.go index 6685b1c61b95..b8d4530a3082 100644 --- a/lib/srv/usermgmt_other.go +++ b/lib/srv/usermgmt_other.go @@ -22,11 +22,13 @@ package srv import ( + "log/slog" + "github.com/gravitational/trace" ) //nolint:staticcheck // intended to always return an error for non-linux builds -func newHostUsersBackend() (HostUsersBackend, error) { +func NewHostUsersBackend(log *slog.Logger) (HostUsersBackend, error) { return nil, trace.NotImplemented("Host user creation management is only supported on linux") }