removing child process log and addressing feedback

lib/srv/exec_linux_test.go

@@ -45,6 +45,9 @@ func TestOSCommandPrep(t *testing.T) {
 	srv := newMockServer(t)
 	scx := newExecServerContext(t, srv)
 
+	// because CheckHomeDir now inspects access to the home directory as the actual user after a rexec,
+	// we need to setup a real, non-root user with a valid home directory in order for this test to
+	// exercise the correct paths
 	tempHome := t.TempDir()
 	require.NoError(t, os.Chmod(filepath.Dir(tempHome), 0777))
 
@@ -55,6 +58,8 @@ func TestOSCommandPrep(t *testing.T) {
 	})
 	require.NoError(t, err)
 	t.Cleanup(func() {
+		// change homedir back so user deletion doesn't fail
+		changeHomeDir(t, username, tempHome)
 		_, err := host.UserDel(username)
 		require.NoError(t, err)
 	})
@@ -134,9 +139,6 @@ func TestOSCommandPrep(t *testing.T) {
 
 	require.Equal(t, root, cmd.Dir)
 	require.Equal(t, expectedEnv, cmd.Env)
-
-	// change homedir back so user deletion doesn't fail
-	changeHomeDir(t, username, tempHome)
 }
 
 func TestConfigureCommand(t *testing.T) {

lib/srv/reexec.go

@@ -25,7 +25,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"log/slog"
 	"net"
 	"os"
 	"os/exec"
@@ -546,7 +545,6 @@ func RunNetworking() (errw io.Writer, code int, err error) {
 	// Use stderr so that it's not forwarded to the remote client.
 	errorWriter := os.Stderr
 
-	log := slog.New(slog.NewJSONHandler(errorWriter, nil))
 	// Parent sends the command payload in the third file descriptor.
 	cmdfd := os.NewFile(CommandFile, fdName(CommandFile))
 	if cmdfd == nil {
@@ -621,14 +619,11 @@ func RunNetworking() (errw io.Writer, code int, err error) {
 	}
 
 	// Create a minimal default environment for the user.
-	homeDir := string(os.PathSeparator)
+	workingDir := string(os.PathSeparator)
 	hasAccess, err := CheckHomeDir(localUser)
-	if err != nil {
-		log.ErrorContext(context.Background(), "user does not have access to home dir", "error", err, "home", localUser.HomeDir, "username", c.Login)
-	}
 
 	if hasAccess && err == nil {
-		homeDir = localUser.HomeDir
+		workingDir = localUser.HomeDir
 	}
 
 	os.Setenv("HOME", localUser.HomeDir)
@@ -646,8 +641,8 @@ func RunNetworking() (errw io.Writer, code int, err error) {
 	}
 
 	// Ensure that the working directory is one that the local user has access to.
-	if err := os.Chdir(homeDir); err != nil {
-		return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err, "failed to set working directory for networking process: %s", homeDir)
+	if err := os.Chdir(workingDir); err != nil {
+		return errorWriter, teleport.RemoteCommandFailure, trace.Wrap(err, "failed to set working directory for networking process: %s", workingDir)
 	}
 
 	// Build request listener from first extra file that was passed to command.
@@ -1248,17 +1243,15 @@ func hasAccessibleHomeDir() error {
 	if err != nil {
 		return trace.Wrap(err)
 	}
+	// make sure we return to the original working directory
+	defer os.Chdir(cwd)
 
 	// attemping to cd into the target directory is the easiest, cross-platform way to test
 	// whether or not the current user has access
 	if err := os.Chdir(currentUser.HomeDir); err != nil {
 		return trace.Wrap(coerceHomeDirError(currentUser, err))
 	}
 
-	if err := os.Chdir(cwd); err != nil {
-		return trace.Errorf("unable to return to original working directory")
-	}
-
 	return nil
 }
 
@@ -1273,7 +1266,7 @@ func CheckHomeDir(localUser *user.User) (bool, error) {
 	if currentUser.Uid == localUser.Uid {
 		if err := hasAccessibleHomeDir(); err != nil {
 			if trace.IsNotFound(err) || trace.IsAccessDenied(err) || trace.IsBadParameter(err) {
-				return false, err
+				return false, nil
 			}
 
 			return false, trace.Wrap(err)

lib/srv/reexec_test.go

@@ -36,6 +36,7 @@ import (
 	"syscall"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ssh/agent"
 
@@ -432,6 +433,8 @@ func TestRootCheckHomeDir(t *testing.T) {
 	_, err = host.UserAdd(login, nil, host.UserOpts{Home: home})
 	require.NoError(t, err)
 	t.Cleanup(func() {
+		// change back to accessible home so deletion works
+		changeHomeDir(t, login, home)
 		_, err := host.UserDel(login)
 		require.NoError(t, err)
 	})
@@ -466,17 +469,14 @@ func TestRootCheckHomeDir(t *testing.T) {
 	hasAccess, err = CheckHomeDir(testUser)
 	require.NoError(t, err)
 	require.False(t, hasAccess)
-
-	// change back to accessible home so deletion works
-	changeHomeDir(t, login, home)
 }
 
 func changeHomeDir(t *testing.T, username, home string) {
 	usermodBin, err := exec.LookPath("usermod")
-	require.NoError(t, err, "usermod binary must be present")
+	assert.NoError(t, err, "usermod binary must be present")
 
 	cmd := exec.Command(usermodBin, "--home", home, username)
 	_, err = cmd.CombinedOutput()
-	require.NoError(t, err, "changing home should not error")
-	require.Equal(t, 0, cmd.ProcessState.ExitCode(), "changing home should exit 0")
+	assert.NoError(t, err, "changing home should not error")
+	assert.Equal(t, 0, cmd.ProcessState.ExitCode(), "changing home should exit 0")
 }