diff --git a/constants.go b/constants.go index ba0d807abf104..af059bc1ffe54 100644 --- a/constants.go +++ b/constants.go @@ -536,6 +536,10 @@ const ( // HomeDirNotFound is returned when a the "teleport checkhomedir" command cannot // find the user's home directory. HomeDirNotFound = 254 + // HomeDirNotAccessible is returned when a the "teleport checkhomedir" command has + // found the user's home directory, but the user does NOT have permissions to + // access it. + HomeDirNotAccessible = 253 ) // MaxEnvironmentFileLines is the maximum number of lines in a environment file. diff --git a/lib/srv/exec_linux_test.go b/lib/srv/exec_linux_test.go index c9738d7305612..47eeeea3e8459 100644 --- a/lib/srv/exec_linux_test.go +++ b/lib/srv/exec_linux_test.go @@ -26,6 +26,7 @@ import ( "os" "os/exec" "os/user" + "path/filepath" "strconv" "syscall" "testing" @@ -33,20 +34,48 @@ import ( "github.com/gravitational/trace" "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/teleport/lib/utils/host" ) func TestOSCommandPrep(t *testing.T) { + utils.RequireRoot(t) + srv := newMockServer(t) scx := newExecServerContext(t, srv) - usr, err := user.Current() + // 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)) + + username := "test-os-command-prep" + scx.Identity.Login = username + _, err := host.UserAdd(username, nil, host.UserOpts{ + Home: tempHome, + }) + 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) + }) + + usr, err := user.Lookup(username) require.NoError(t, err) + uid, err := strconv.Atoi(usr.Uid) + require.NoError(t, err) + + require.NoError(t, os.Chown(tempHome, uid, -1)) expectedEnv := []string{ "LANG=en_US.UTF-8", - getDefaultEnvPath(strconv.Itoa(os.Geteuid()), defaultLoginDefsPath), + getDefaultEnvPath(usr.Uid, defaultLoginDefsPath), fmt.Sprintf("HOME=%s", usr.HomeDir), - fmt.Sprintf("USER=%s", usr.Username), + fmt.Sprintf("USER=%s", username), "SHELL=/bin/sh", "SSH_CLIENT=10.0.0.5 4817 3022", "SSH_CONNECTION=10.0.0.5 4817 127.0.0.1 3022", @@ -99,12 +128,9 @@ func TestOSCommandPrep(t *testing.T) { require.Equal(t, []string{"/bin/sh", "-c", "top"}, cmd.Args) require.Equal(t, syscall.SIGKILL, cmd.SysProcAttr.Pdeathsig) - if os.Geteuid() != 0 { - t.Skip("skipping portion of test which must run as root") - } - // Missing home directory - HOME should still be set to the given // home dir, but the command should set it's CWD to root instead. + changeHomeDir(t, username, "/wrong/place") usr.HomeDir = "/wrong/place" root := string(os.PathSeparator) expectedEnv[2] = "HOME=/wrong/place" diff --git a/lib/srv/reexec.go b/lib/srv/reexec.go index 2812ee8649ba7..1bb1865c0cfe7 100644 --- a/lib/srv/reexec.go +++ b/lib/srv/reexec.go @@ -534,6 +534,8 @@ func (o *osWrapper) startNewParker(ctx context.Context, credential *syscall.Cred return nil } +const rootDirectory = "/" + func RunNetworking() (errw io.Writer, code int, err error) { // SIGQUIT is used by teleport to initiate graceful shutdown, waiting for // existing exec sessions to close before ending the process. For this to @@ -619,10 +621,13 @@ func RunNetworking() (errw io.Writer, code int, err error) { } // Create a minimal default environment for the user. - homeDir := localUser.HomeDir - if !utils.IsDir(homeDir) { - homeDir = "/" + workingDir := rootDirectory + + hasAccess, err := CheckHomeDir(localUser) + if hasAccess && err == nil { + workingDir = localUser.HomeDir } + os.Setenv("HOME", localUser.HomeDir) os.Setenv("USER", c.Login) @@ -638,8 +643,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") + 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. @@ -872,16 +877,21 @@ func getConnFile(conn net.Conn) (*os.File, error) { } } -// runCheckHomeDir check's if the active user's $HOME dir exists. +// runCheckHomeDir checks if the active user's $HOME dir exists and is accessible. func runCheckHomeDir() (errw io.Writer, code int, err error) { - home, err := os.UserHomeDir() - if err != nil { - return io.Discard, teleport.HomeDirNotFound, nil - } - if !utils.IsDir(home) { - return io.Discard, teleport.HomeDirNotFound, nil + code = teleport.RemoteCommandSuccess + if err := hasAccessibleHomeDir(); err != nil { + switch { + case trace.IsNotFound(err), trace.IsBadParameter(err): + code = teleport.HomeDirNotFound + case trace.IsAccessDenied(err): + code = teleport.HomeDirNotAccessible + default: + code = teleport.RemoteCommandFailure + } } - return io.Discard, teleport.RemoteCommandSuccess, nil + + return io.Discard, code, nil } // runPark does nothing, forever. @@ -1055,18 +1065,20 @@ func buildCommand(c *ExecCommand, localUser *user.User, tty *os.File, pamEnviron // Set the command's cwd to the user's $HOME, or "/" if // they don't have an existing home dir. // TODO (atburke): Generalize this to support Windows. - exists, err := CheckHomeDir(localUser) + hasAccess, err := CheckHomeDir(localUser) if err != nil { return nil, trace.Wrap(err) - } else if exists { + } + + if hasAccess { cmd.Dir = localUser.HomeDir - } else if !exists { + } else { // Write failure to find home dir to stdout, same as OpenSSH. - msg := fmt.Sprintf("Could not set shell's cwd to home directory %q, defaulting to %q\n", localUser.HomeDir, string(os.PathSeparator)) + msg := fmt.Sprintf("Could not set shell's cwd to home directory %q, defaulting to %q\n", localUser.HomeDir, rootDirectory) if _, err := cmd.Stdout.Write([]byte(msg)); err != nil { return nil, trace.Wrap(err) } - cmd.Dir = string(os.PathSeparator) + cmd.Dir = rootDirectory } // Only set process credentials if the UID/GID of the requesting user are @@ -1200,16 +1212,73 @@ func copyCommand(ctx *ServerContext, cmdmsg *ExecCommand) { } } -// CheckHomeDir checks if the user's home dir exists +func coerceHomeDirError(usr *user.User, err error) error { + if os.IsNotExist(err) { + return trace.NotFound("home directory %q not found for user %q", usr.HomeDir, usr.Name) + } + + if os.IsPermission(err) { + return trace.AccessDenied("%q does not have permission to access %q", usr.Name, usr.HomeDir) + } + + return err +} + +// hasAccessibleHomeDir checks if the current user has access to an existing home directory. +func hasAccessibleHomeDir() error { + // this should usually be fetching a cached value + currentUser, err := user.Current() + if err != nil { + return trace.Wrap(err) + } + + fi, err := os.Stat(currentUser.HomeDir) + if err != nil { + return trace.Wrap(coerceHomeDirError(currentUser, err)) + } + + if !fi.IsDir() { + return trace.BadParameter("%q is not a directory", currentUser.HomeDir) + } + + cwd, err := os.Getwd() + 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)) + } + + return nil +} + +// CheckHomeDir checks if the user's home directory exists and is accessible to the user. Only catastrophic +// errors will be returned, which means a missing, inaccessible, or otherwise invalid home directory will result +// in a return of (false, nil) func CheckHomeDir(localUser *user.User) (bool, error) { - if fi, err := os.Stat(localUser.HomeDir); err == nil { - return fi.IsDir(), nil + currentUser, err := user.Current() + if err != nil { + return false, trace.Wrap(err) + } + + // don't spawn a subcommand if already running as the user in question + if currentUser.Uid == localUser.Uid { + if err := hasAccessibleHomeDir(); err != nil { + if trace.IsNotFound(err) || trace.IsAccessDenied(err) || trace.IsBadParameter(err) { + return false, nil + } + + return false, trace.Wrap(err) + } + + return true, nil } - // In some environments, the user's home directory exists but isn't visible to - // root, e.g. /home is mounted to an nfs export with root_squash enabled. - // In case we are in that scenario, re-exec teleport as the user to check - // if the home dir actually does exist. executable, err := os.Executable() if err != nil { return false, trace.Wrap(err) @@ -1225,6 +1294,7 @@ func CheckHomeDir(localUser *user.User) (bool, error) { Path: executable, Args: []string{executable, teleport.CheckHomeDirSubCommand}, Env: []string{"HOME=" + localUser.HomeDir}, + Dir: rootDirectory, SysProcAttr: &syscall.SysProcAttr{ Setsid: true, Credential: credential, @@ -1235,11 +1305,13 @@ func CheckHomeDir(localUser *user.User) (bool, error) { reexecCommandOSTweaks(cmd) if err := cmd.Run(); err != nil { - if cmd.ProcessState.ExitCode() == teleport.HomeDirNotFound { - return false, nil + if cmd.ProcessState.ExitCode() == teleport.RemoteCommandFailure { + return false, trace.Wrap(err) } - return false, trace.Wrap(err) + + return false, nil } + return true, nil } diff --git a/lib/srv/reexec_test.go b/lib/srv/reexec_test.go index 55dd248d0adc3..a9d6934c7b233 100644 --- a/lib/srv/reexec_test.go +++ b/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" @@ -410,3 +411,72 @@ func testX11Forward(ctx context.Context, t *testing.T, proc *networking.Process, require.NoError(t, err) require.Equal(t, fakeXauthEntry, readXauthEntry) } + +func TestRootCheckHomeDir(t *testing.T) { + utils.RequireRoot(t) + + tmp := t.TempDir() + require.NoError(t, os.Chmod(filepath.Dir(tmp), 0777)) + require.NoError(t, os.Chmod(tmp, 0777)) + + home := filepath.Join(tmp, "home") + noAccess := filepath.Join(tmp, "no_access") + file := filepath.Join(tmp, "file") + notFound := filepath.Join(tmp, "not_found") + + require.NoError(t, os.Mkdir(home, 0700)) + require.NoError(t, os.Mkdir(noAccess, 0700)) + _, err := os.Create(file) + require.NoError(t, err) + + login := utils.GenerateLocalUsername(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) + }) + + testUser, err := user.Lookup(login) + require.NoError(t, err) + + uid, err := strconv.Atoi(testUser.Uid) + require.NoError(t, err) + + gid, err := strconv.Atoi(testUser.Gid) + require.NoError(t, err) + + require.NoError(t, os.Chown(home, uid, gid)) + require.NoError(t, os.Chown(file, uid, gid)) + + hasAccess, err := CheckHomeDir(testUser) + require.NoError(t, err) + require.True(t, hasAccess) + + changeHomeDir(t, login, file) + hasAccess, err = CheckHomeDir(testUser) + require.NoError(t, err) + require.False(t, hasAccess) + + changeHomeDir(t, login, notFound) + hasAccess, err = CheckHomeDir(testUser) + require.NoError(t, err) + require.False(t, hasAccess) + + changeHomeDir(t, login, noAccess) + hasAccess, err = CheckHomeDir(testUser) + require.NoError(t, err) + require.False(t, hasAccess) +} + +func changeHomeDir(t *testing.T, username, home string) { + usermodBin, err := exec.LookPath("usermod") + assert.NoError(t, err, "usermod binary must be present") + + cmd := exec.Command(usermodBin, "--home", home, username) + _, err = cmd.CombinedOutput() + assert.NoError(t, err, "changing home should not error") + assert.Equal(t, 0, cmd.ProcessState.ExitCode(), "changing home should exit 0") +} diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index af50422167966..5719a56060509 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -1266,7 +1266,7 @@ func (s *Server) HandleNewConn(ctx context.Context, ccx *sshutils.ConnectionCont // Create host user. created, userCloser, err := s.termHandlers.SessionRegistry.UpsertHostUser(identityContext) if err != nil { - log.Infof("error while creating host users: %s", err) + log.Warnf("error while creating host users: %s", err) } // Indicate that the user was created by Teleport.