takes recursive ownership of home directories when they already exist

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 {

lib/utils/fs_unix_test.go → 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 <http://www.gnu.org/licenses/>.
  */
 
-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

lib/srv/usermgmt.go

@@ -671,7 +671,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)

lib/srv/usermgmt_linux.go

@@ -22,6 +22,7 @@ import (
 	"bufio"
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"os/exec"
 	"os/user"
@@ -237,18 +238,55 @@ 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)
 	}
 
 	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))
 }

lib/utils/fs.go

@@ -373,35 +373,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 {