[client] ssh: fix #5345 by implementing PTY support in executor and removing setsid wrapper

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 {

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

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 {

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

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 denied (likely container seccomp policy), 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())

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

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 {

client/ssh/server/userswitching_unix.go

@@ -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