From 4ad02a91a4c1d62b150449443fd1e7fecda4acab Mon Sep 17 00:00:00 2001 From: Pieter Hollander Date: Wed, 18 Feb 2026 14:26:28 +0100 Subject: [PATCH 1/5] ssh: implement PTY support in executor and remove setsid wrapper Implement PTY support in the executor privilege-dropping path using setsid(2) + TIOCSCTTY to establish a controlling terminal natively, replacing the external setsid wrapper that was previously needed for util-linux login's vhangup() behavior. - Add prepareControllingTerminal() with setsid/TIOCSCTTY and graceful error handling for containers and non-PTY scenarios - Use syscall.Exec() in the PTY path so the shell directly replaces the executor process - Remove setsid -c wrapper from getLinuxLoginCmd; pty.StartWithSize() sets Setsid and Setctty on the child process - Remove detectUtilLinuxLogin() and loginIsUtilLinux field (now dead code) - Fix executor tests to use current UID/GID instead of hardcoded 1000 - Add TestPrivilegeDropper_CreateExecutorCommandWithPTY --- client/cmd/ssh_exec_unix.go | 2 +- client/ssh/server/command_execution_js.go | 5 -- client/ssh/server/command_execution_unix.go | 24 --------- .../ssh/server/command_execution_windows.go | 5 -- client/ssh/server/executor_unix.go | 54 +++++++++++++++++-- client/ssh/server/executor_unix_test.go | 48 +++++++++++++---- client/ssh/server/server.go | 4 +- client/ssh/server/userswitching_unix.go | 19 ++----- 8 files changed, 93 insertions(+), 68 deletions(-) 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..6546b3c3552 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,30 @@ 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) + } + + os.Exit(ExitCodeSuccess) + } + var execCmd *exec.Cmd if config.Command == "" { execCmd = exec.CommandContext(ctx, config.Shell) @@ -263,6 +283,30 @@ func (pd *PrivilegeDropper) ExecuteWithPrivilegeDrop(ctx context.Context, config os.Exit(ExitCodeSuccess) } +func (pd *PrivilegeDropper) prepareControllingTerminal() error { + if _, err := syscall.Setsid(); err != nil && !errors.Is(err, syscall.EPERM) { + 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()) 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..bc658350068 100644 --- a/client/ssh/server/userswitching_unix.go +++ b/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 From 0f3419b2e181edbd130cf30ac15fbd41154381ce Mon Sep 17 00:00:00 2001 From: Pieter Hollander Date: Wed, 18 Feb 2026 14:55:30 +0100 Subject: [PATCH 2/5] ssh: remove unreachable exit after exec --- client/ssh/server/executor_unix.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/ssh/server/executor_unix.go b/client/ssh/server/executor_unix.go index 6546b3c3552..3da123016f0 100644 --- a/client/ssh/server/executor_unix.go +++ b/client/ssh/server/executor_unix.go @@ -245,8 +245,6 @@ func (pd *PrivilegeDropper) ExecuteWithPrivilegeDrop(ctx context.Context, config log.Debugf("exec failed: %v", err) os.Exit(ExitCodeShellExecFail) } - - os.Exit(ExitCodeSuccess) } var execCmd *exec.Cmd From dff7ee6c072cc902da09a0a0174e30ac7f739113 Mon Sep 17 00:00:00 2001 From: Pieter <1392118+pieterhollander@users.noreply.github.com> Date: Wed, 18 Feb 2026 15:17:30 +0100 Subject: [PATCH 3/5] Update client/ssh/server/executor_unix.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- client/ssh/server/executor_unix.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/client/ssh/server/executor_unix.go b/client/ssh/server/executor_unix.go index 3da123016f0..edebc315131 100644 --- a/client/ssh/server/executor_unix.go +++ b/client/ssh/server/executor_unix.go @@ -282,8 +282,21 @@ func (pd *PrivilegeDropper) ExecuteWithPrivilegeDrop(ctx context.Context, config } func (pd *PrivilegeDropper) prepareControllingTerminal() error { - if _, err := syscall.Setsid(); err != nil && !errors.Is(err, syscall.EPERM) { - return fmt.Errorf("setsid: %w", err) + 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()) From 52748b6acd4d6d2798727294b06834aa25caf9c2 Mon Sep 17 00:00:00 2001 From: Pieter <1392118+pieterhollander@users.noreply.github.com> Date: Wed, 18 Feb 2026 15:20:48 +0100 Subject: [PATCH 4/5] Update client/ssh/server/executor_unix.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- client/ssh/server/executor_unix.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/ssh/server/executor_unix.go b/client/ssh/server/executor_unix.go index edebc315131..c5d57cd81c0 100644 --- a/client/ssh/server/executor_unix.go +++ b/client/ssh/server/executor_unix.go @@ -306,7 +306,7 @@ func (pd *PrivilegeDropper) prepareControllingTerminal() error { } if errors.Is(err, syscall.EPERM) { - pd.log().Debugf("TIOCSCTTY denied (likely container seccomp policy), continuing: %v", err) + 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 } From 8e7f9ea88a7d01c9e47fb13b69d314bf36986b02 Mon Sep 17 00:00:00 2001 From: Pieter Hollander Date: Wed, 18 Feb 2026 15:24:26 +0100 Subject: [PATCH 5/5] ssh: update getLinuxLoginCmd doc comment to reflect current behavior --- client/ssh/server/userswitching_unix.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/ssh/server/userswitching_unix.go b/client/ssh/server/userswitching_unix.go index bc658350068..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