adding failure case logging for home directory ownership changes

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",

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

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,

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))
 }

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")
 }