diff --git a/client/cmd/ssh_exec_unix.go b/client/cmd/ssh_exec_unix.go index 2412f072c5e..42f2ec8aefc 100644 --- a/client/cmd/ssh_exec_unix.go +++ b/client/cmd/ssh_exec_unix.go @@ -35,7 +35,7 @@ func init() { sshExecCmd.Flags().UintSliceVar(&sshExecGroups, "groups", nil, "Supplementary group IDs (can be repeated)") sshExecCmd.Flags().StringVar(&sshExecWorkingDir, "working-dir", "", "Working directory") sshExecCmd.Flags().StringVar(&sshExecShell, "shell", "/bin/sh", "Shell to execute") - sshExecCmd.Flags().BoolVar(&sshExecPTY, "pty", false, "Request PTY (will fail as executor doesn't support PTY)") + sshExecCmd.Flags().BoolVar(&sshExecPTY, "pty", false, "Request PTY") sshExecCmd.Flags().StringVar(&sshExecCommand, "cmd", "", "Command to execute") if err := sshExecCmd.MarkFlagRequired("uid"); err != nil { diff --git a/client/ssh/server/command_execution_js.go b/client/ssh/server/command_execution_js.go index 3aeaa135cda..1eafae5c1fc 100644 --- a/client/ssh/server/command_execution_js.go +++ b/client/ssh/server/command_execution_js.go @@ -42,11 +42,6 @@ func (s *Server) detectSuPtySupport(context.Context) bool { return false } -// detectUtilLinuxLogin always returns false on JS/WASM -func (s *Server) detectUtilLinuxLogin(context.Context) bool { - return false -} - // executeCommandWithPty is not supported on JS/WASM func (s *Server) executeCommandWithPty(logger *log.Entry, session ssh.Session, execCmd *exec.Cmd, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, winCh <-chan ssh.Window) bool { logger.Errorf("PTY command execution not supported on JS/WASM") diff --git a/client/ssh/server/command_execution_unix.go b/client/ssh/server/command_execution_unix.go index 279b8934103..9b1cc8f50ff 100644 --- a/client/ssh/server/command_execution_unix.go +++ b/client/ssh/server/command_execution_unix.go @@ -11,7 +11,6 @@ import ( "os/exec" "os/user" "path/filepath" - "runtime" "strings" "sync" "syscall" @@ -77,29 +76,6 @@ func (s *Server) detectSuPtySupport(ctx context.Context) bool { return supported } -// detectUtilLinuxLogin checks if login is from util-linux (vs shadow-utils). -// util-linux login uses vhangup() which requires setsid wrapper to avoid killing parent. -// See https://bugs.debian.org/1078023 for details. -func (s *Server) detectUtilLinuxLogin(ctx context.Context) bool { - if runtime.GOOS != "linux" { - return false - } - - ctx, cancel := context.WithTimeout(ctx, 500*time.Millisecond) - defer cancel() - - cmd := exec.CommandContext(ctx, "login", "--version") - output, err := cmd.CombinedOutput() - if err != nil { - log.Debugf("login --version failed (likely shadow-utils): %v", err) - return false - } - - isUtilLinux := strings.Contains(string(output), "util-linux") - log.Debugf("util-linux login detected: %v", isUtilLinux) - return isUtilLinux -} - // createSuCommand creates a command using su - for privilege switching. func (s *Server) createSuCommand(logger *log.Entry, session ssh.Session, localUser *user.User, hasPty bool) (*exec.Cmd, error) { if err := validateUsername(localUser.Username); err != nil { diff --git a/client/ssh/server/command_execution_windows.go b/client/ssh/server/command_execution_windows.go index e1ba777f606..a6d954f284b 100644 --- a/client/ssh/server/command_execution_windows.go +++ b/client/ssh/server/command_execution_windows.go @@ -370,11 +370,6 @@ func (s *Server) detectSuPtySupport(context.Context) bool { return false } -// detectUtilLinuxLogin always returns false on Windows -func (s *Server) detectUtilLinuxLogin(context.Context) bool { - return false -} - // executeCommandWithPty executes a command with PTY allocation on Windows using ConPty func (s *Server) executeCommandWithPty(logger *log.Entry, session ssh.Session, _ *exec.Cmd, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, _ <-chan ssh.Window) bool { localUser := privilegeResult.User diff --git a/client/ssh/server/executor_unix.go b/client/ssh/server/executor_unix.go index ee0b0ff781d..c5d57cd81c0 100644 --- a/client/ssh/server/executor_unix.go +++ b/client/ssh/server/executor_unix.go @@ -14,6 +14,7 @@ import ( "syscall" log "github.com/sirupsen/logrus" + "golang.org/x/sys/unix" ) // Exit codes for executor process communication @@ -213,11 +214,6 @@ func (pd *PrivilegeDropper) validateCurrentPrivileges(targetUID, targetGID uint3 func (pd *PrivilegeDropper) ExecuteWithPrivilegeDrop(ctx context.Context, config ExecutorConfig) { log.Tracef("dropping privileges to UID=%d, GID=%d, groups=%v", config.UID, config.GID, config.Groups) - // TODO: Implement Pty support for executor path - if config.PTY { - config.PTY = false - } - if err := pd.DropPrivileges(config.UID, config.GID, config.Groups); err != nil { _, _ = fmt.Fprintf(os.Stderr, "privilege drop failed: %v\n", err) os.Exit(ExitCodePrivilegeDropFail) @@ -229,6 +225,28 @@ func (pd *PrivilegeDropper) ExecuteWithPrivilegeDrop(ctx context.Context, config } } + if config.PTY { + if err := pd.prepareControllingTerminal(); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "PTY setup failed: %v\n", err) + os.Exit(ExitCodeShellExecFail) + } + + argv := []string{"-" + filepath.Base(config.Shell)} + if config.Command == "" { + log.Tracef("executing login shell via exec: %s", config.Shell) + } else { + argv = append(argv, "-c", config.Command) + cmdParts := strings.Fields(config.Command) + safeCmd := safeLogCommand(cmdParts) + log.Tracef("executing %s -c %s via exec", config.Shell, safeCmd) + } + + if err := syscall.Exec(config.Shell, argv, os.Environ()); err != nil { + log.Debugf("exec failed: %v", err) + os.Exit(ExitCodeShellExecFail) + } + } + var execCmd *exec.Cmd if config.Command == "" { execCmd = exec.CommandContext(ctx, config.Shell) @@ -263,6 +281,43 @@ func (pd *PrivilegeDropper) ExecuteWithPrivilegeDrop(ctx context.Context, config os.Exit(ExitCodeSuccess) } +func (pd *PrivilegeDropper) prepareControllingTerminal() error { + if _, err := syscall.Setsid(); err != nil { + if errors.Is(err, syscall.EPERM) { + // EPERM can mean we're already a session leader or that a new session was not created. + // Only treat EPERM as non-fatal if we're already the session leader. + sid, gerr := unix.Getsid(0) + if gerr != nil { + return fmt.Errorf("setsid EPERM and getsid failed: %w", gerr) + } + if sid != os.Getpid() { + return fmt.Errorf("setsid: %w", err) + } + // Already a session leader; continue. + } else { + return fmt.Errorf("setsid: %w", err) + } + } + + stdinFD := int(os.Stdin.Fd()) + err := unix.IoctlSetInt(stdinFD, unix.TIOCSCTTY, 0) + if err == nil { + return nil + } + + if errors.Is(err, syscall.EPERM) { + pd.log().Debugf("TIOCSCTTY failed with EPERM; possible causes include existing controlling terminal, not being a session leader, or container/seccomp restrictions; continuing: %v", err) + return nil + } + + if errors.Is(err, syscall.ENOTTY) || errors.Is(err, syscall.EINVAL) { + pd.log().Debugf("stdin is not a PTY controlling terminal candidate, continuing: %v", err) + return nil + } + + return fmt.Errorf("set controlling terminal: %w", err) +} + // validatePrivileges validates that privilege dropping to the target UID/GID is allowed func (pd *PrivilegeDropper) validatePrivileges(uid, gid uint32) error { currentUID := uint32(os.Geteuid()) diff --git a/client/ssh/server/executor_unix_test.go b/client/ssh/server/executor_unix_test.go index 0c5108f57fa..ce52d716d24 100644 --- a/client/ssh/server/executor_unix_test.go +++ b/client/ssh/server/executor_unix_test.go @@ -75,11 +75,15 @@ func TestPrivilegeDropper_ValidatePrivileges(t *testing.T) { func TestPrivilegeDropper_CreateExecutorCommand(t *testing.T) { pd := NewPrivilegeDropper() + currentUID := uint32(os.Geteuid()) + currentGID := uint32(os.Getegid()) + uidStr := strconv.FormatUint(uint64(currentUID), 10) + gidStr := strconv.FormatUint(uint64(currentGID), 10) config := ExecutorConfig{ - UID: 1000, - GID: 1000, - Groups: []uint32{1000, 1001}, + UID: currentUID, + GID: currentGID, + Groups: []uint32{currentGID}, WorkingDir: "/home/testuser", Shell: "/bin/bash", Command: "ls -la", @@ -93,12 +97,11 @@ func TestPrivilegeDropper_CreateExecutorCommand(t *testing.T) { assert.Contains(t, cmd.Args, "ssh") assert.Contains(t, cmd.Args, "exec") assert.Contains(t, cmd.Args, "--uid") - assert.Contains(t, cmd.Args, "1000") + assert.Contains(t, cmd.Args, uidStr) assert.Contains(t, cmd.Args, "--gid") - assert.Contains(t, cmd.Args, "1000") + assert.Contains(t, cmd.Args, gidStr) assert.Contains(t, cmd.Args, "--groups") - assert.Contains(t, cmd.Args, "1000") - assert.Contains(t, cmd.Args, "1001") + assert.Contains(t, cmd.Args, gidStr) assert.Contains(t, cmd.Args, "--working-dir") assert.Contains(t, cmd.Args, "/home/testuser") assert.Contains(t, cmd.Args, "--shell") @@ -109,11 +112,13 @@ func TestPrivilegeDropper_CreateExecutorCommand(t *testing.T) { func TestPrivilegeDropper_CreateExecutorCommandInteractive(t *testing.T) { pd := NewPrivilegeDropper() + currentUID := uint32(os.Geteuid()) + currentGID := uint32(os.Getegid()) config := ExecutorConfig{ - UID: 1000, - GID: 1000, - Groups: []uint32{1000}, + UID: currentUID, + GID: currentGID, + Groups: []uint32{currentGID}, WorkingDir: "/home/testuser", Shell: "/bin/bash", Command: "", @@ -128,6 +133,29 @@ func TestPrivilegeDropper_CreateExecutorCommandInteractive(t *testing.T) { assert.NotContains(t, cmd.Args, "--interactive") } +func TestPrivilegeDropper_CreateExecutorCommandWithPTY(t *testing.T) { + pd := NewPrivilegeDropper() + currentUID := uint32(os.Geteuid()) + currentGID := uint32(os.Getegid()) + + config := ExecutorConfig{ + UID: currentUID, + GID: currentGID, + Groups: []uint32{currentGID}, + WorkingDir: "/home/testuser", + Shell: "/bin/bash", + Command: "", + PTY: true, + } + + cmd, err := pd.CreateExecutorCommand(context.Background(), config) + require.NoError(t, err) + require.NotNil(t, cmd) + + assert.Contains(t, cmd.Args, "--pty") + assert.NotContains(t, cmd.Args, "--cmd") +} + // TestPrivilegeDropper_ActualPrivilegeDrop tests actual privilege dropping // This test requires root privileges and will be skipped if not running as root func TestPrivilegeDropper_ActualPrivilegeDrop(t *testing.T) { diff --git a/client/ssh/server/server.go b/client/ssh/server/server.go index 1ddb60f8e46..dc4637fff0c 100644 --- a/client/ssh/server/server.go +++ b/client/ssh/server/server.go @@ -170,8 +170,7 @@ type Server struct { authorizer *sshauth.Authorizer - suSupportsPty bool - loginIsUtilLinux bool + suSupportsPty bool } type JWTConfig struct { @@ -227,7 +226,6 @@ func (s *Server) Start(ctx context.Context, addr netip.AddrPort) error { } s.suSupportsPty = s.detectSuPtySupport(ctx) - s.loginIsUtilLinux = s.detectUtilLinuxLogin(ctx) ln, addrDesc, err := s.createListener(ctx, addr) if err != nil { diff --git a/client/ssh/server/userswitching_unix.go b/client/ssh/server/userswitching_unix.go index d80b77042a3..1bb53d42bb8 100644 --- a/client/ssh/server/userswitching_unix.go +++ b/client/ssh/server/userswitching_unix.go @@ -96,8 +96,8 @@ func (s *Server) getLoginCmd(username string, remoteAddr net.Addr) (string, []st } } -// getLinuxLoginCmd returns the login command for Linux systems. -// Handles differences between util-linux and shadow-utils login implementations. +// getLinuxLoginCmd returns the login command and arguments for Linux systems. +// On Arch Linux without /etc/pam.d/remote the -h flag is omitted to avoid PAM failures. func (s *Server) getLinuxLoginCmd(loginPath, username, remoteIP string) (string, []string) { // Special handling for Arch Linux without /etc/pam.d/remote var loginArgs []string @@ -107,23 +107,12 @@ func (s *Server) getLinuxLoginCmd(loginPath, username, remoteIP string) (string, loginArgs = []string{"-f", username, "-h", remoteIP, "-p"} } - // util-linux login requires setsid -c to create a new session and set the - // controlling terminal. Without this, vhangup() kills the parent process. + // util-linux login uses vhangup() which requires a new session and controlling + // terminal. This is handled by pty.StartWithSize() which sets SysProcAttr.Setsid + // and SysProcAttr.Setctty on the command. // See https://bugs.debian.org/1078023 for details. - // TODO: handle this via the executor using syscall.Setsid() + TIOCSCTTY + syscall.Exec() - // to avoid external setsid dependency. - if !s.loginIsUtilLinux { - return loginPath, loginArgs - } - - setsidPath, err := exec.LookPath("setsid") - if err != nil { - log.Warnf("setsid not available but util-linux login detected, login may fail: %v", err) - return loginPath, loginArgs - } - args := append([]string{"-w", "-c", loginPath}, loginArgs...) - return setsidPath, args + return loginPath, loginArgs } // fileExists checks if a file exists