diff --git a/client/ssh/server/command_execution.go b/client/ssh/server/command_execution.go index 2d2ceae24..0035a5f74 100644 --- a/client/ssh/server/command_execution.go +++ b/client/ssh/server/command_execution.go @@ -6,7 +6,6 @@ import ( "io" "os" "os/exec" - "runtime" "time" "github.com/gliderlabs/ssh" @@ -25,7 +24,7 @@ func (s *Server) handleCommand(logger *log.Entry, session ssh.Session, privilege logger.Infof("executing %s for %s from %s: %s", commandType, localUser.Username, session.RemoteAddr(), safeLogCommand(session.Command())) - execCmd, err := s.createCommandWithPrivileges(privilegeResult, session, hasPty) + execCmd, err := s.createCommand(privilegeResult, session, hasPty) if err != nil { logger.Errorf("%s creation failed: %v", commandType, err) @@ -44,38 +43,19 @@ func (s *Server) handleCommand(logger *log.Entry, session ssh.Session, privilege return } - var success bool - if hasPty { - success = s.handlePty(logger, session, privilegeResult, ptyReq, winCh) - } else { - success = s.executeCommand(logger, session, execCmd) + if s.executeCommand(logger, session, execCmd) { + logger.Debugf("%s execution completed", commandType) } - - if !success { - return - } - - logger.Debugf("%s execution completed", commandType) } -func (s *Server) createCommandWithPrivileges(privilegeResult PrivilegeCheckResult, session ssh.Session, hasPty bool) (*exec.Cmd, error) { +func (s *Server) createCommand(privilegeResult PrivilegeCheckResult, session ssh.Session, hasPty bool) (*exec.Cmd, error) { localUser := privilegeResult.User - var cmd *exec.Cmd - var err error - - // If we used fallback (unprivileged process), skip su and use direct execution - if privilegeResult.UsedFallback { - log.Debugf("using fallback - direct execution for current user") - cmd, err = s.createDirectCommand(session, localUser) - } else { - // Try su first for system integration (PAM/audit) when privileged - cmd, err = s.createSuCommand(session, localUser) - if err != nil { - // Always fall back to executor if su fails - log.Debugf("su command failed, falling back to executor: %v", err) - cmd, err = s.createExecutorCommand(session, localUser, hasPty) - } + // Try su first for system integration (PAM/audit) when privileged + cmd, err := s.createSuCommand(session, localUser, hasPty) + if err != nil || privilegeResult.UsedFallback { + log.Debugf("su command failed, falling back to executor: %v", err) + cmd, err = s.createExecutorCommand(session, localUser, hasPty) } if err != nil { @@ -86,20 +66,6 @@ func (s *Server) createCommandWithPrivileges(privilegeResult PrivilegeCheckResul return cmd, nil } -// getShellCommandArgs returns the shell command and arguments for executing a command string -func (s *Server) getShellCommandArgs(shell, cmdString string) []string { - if runtime.GOOS == "windows" { - if cmdString == "" { - return []string{shell, "-NoLogo"} - } - return []string{shell, "-Command", cmdString} - } - - if cmdString == "" { - return []string{shell} - } - return []string{shell, "-c", cmdString} -} // executeCommand executes the command and handles I/O and exit codes func (s *Server) executeCommand(logger *log.Entry, session ssh.Session, execCmd *exec.Cmd) bool { diff --git a/client/ssh/server/command_execution_unix.go b/client/ssh/server/command_execution_unix.go index 09ed8c71f..f78678743 100644 --- a/client/ssh/server/command_execution_unix.go +++ b/client/ssh/server/command_execution_unix.go @@ -19,7 +19,7 @@ import ( ) // createSuCommand creates a command using su -l -c for privilege switching -func (s *Server) createSuCommand(session ssh.Session, localUser *user.User) (*exec.Cmd, error) { +func (s *Server) createSuCommand(session ssh.Session, localUser *user.User, hasPty bool) (*exec.Cmd, error) { suPath, err := exec.LookPath("su") if err != nil { return nil, fmt.Errorf("su command not available: %w", err) @@ -30,7 +30,7 @@ func (s *Server) createSuCommand(session ssh.Session, localUser *user.User) (*ex return nil, fmt.Errorf("no command specified for su execution") } - // Use su -l -c to execute the command as the target user with login environment + // TODO: handle pty flag if available args := []string{"-l", localUser.Username, "-c", command} cmd := exec.CommandContext(session.Context(), suPath, args...) @@ -39,6 +39,14 @@ func (s *Server) createSuCommand(session ssh.Session, localUser *user.User) (*ex return cmd, nil } +// getShellCommandArgs returns the shell command and arguments for executing a command string +func (s *Server) getShellCommandArgs(shell, cmdString string) []string { + if cmdString == "" { + return []string{shell, "-l"} + } + return []string{shell, "-l", "-c", cmdString} +} + // prepareCommandEnv prepares environment variables for command execution on Unix func (s *Server) prepareCommandEnv(localUser *user.User, session ssh.Session) []string { env := prepareUserEnv(localUser, getUserShell(localUser.Uid)) @@ -94,7 +102,7 @@ func (s *Server) handlePty(logger *log.Entry, session ssh.Session, privilegeResu localUser := privilegeResult.User logger.Infof("executing Pty command for %s from %s: %s", localUser.Username, session.RemoteAddr(), safeLogCommand(cmd)) - execCmd, err := s.createPtyCommandWithPrivileges(cmd, privilegeResult, ptyReq, session) + execCmd, err := s.createPtyCommand(privilegeResult, ptyReq, session) if err != nil { logger.Errorf("Pty command creation failed: %v", err) errorMsg := "User switching failed - login command not available\r\n" diff --git a/client/ssh/server/command_execution_windows.go b/client/ssh/server/command_execution_windows.go index 759e908de..b0b76f22d 100644 --- a/client/ssh/server/command_execution_windows.go +++ b/client/ssh/server/command_execution_windows.go @@ -264,6 +264,14 @@ func (s *Server) handlePty(logger *log.Entry, session ssh.Session, privilegeResu return true } +// getShellCommandArgs returns the shell command and arguments for executing a command string +func (s *Server) getShellCommandArgs(shell, cmdString string) []string { + if cmdString == "" { + return []string{shell, "-NoLogo"} + } + return []string{shell, "-Command", cmdString} +} + func (s *Server) handlePtyWithUserSwitching(logger *log.Entry, session ssh.Session, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, _ <-chan ssh.Window, _ []string) { localUser := privilegeResult.User diff --git a/client/ssh/server/executor_unix.go b/client/ssh/server/executor_unix.go index 818b82caa..8adc824ef 100644 --- a/client/ssh/server/executor_unix.go +++ b/client/ssh/server/executor_unix.go @@ -99,8 +99,10 @@ func (pd *PrivilegeDropper) DropPrivileges(targetUID, targetGID uint32, suppleme originalUID := os.Geteuid() originalGID := os.Getegid() - if err := pd.setGroupsAndIDs(targetUID, targetGID, supplementaryGroups); err != nil { - return err + if originalUID != int(targetUID) || originalGID != int(targetGID) { + if err := pd.setGroupsAndIDs(targetUID, targetGID, supplementaryGroups); err != nil { + return fmt.Errorf("set groups and IDs: %w", err) + } } if err := pd.validatePrivilegeDropSuccess(targetUID, targetGID, originalUID, originalGID); err != nil { @@ -188,8 +190,7 @@ func (pd *PrivilegeDropper) ExecuteWithPrivilegeDrop(ctx context.Context, config // TODO: Implement Pty support for executor path if config.PTY { - log.Warnf("Pty requested but executor does not support Pty yet - continuing without Pty") - config.PTY = false // Disable Pty and continue + config.PTY = false } if err := pd.DropPrivileges(config.UID, config.GID, config.Groups); err != nil { diff --git a/client/ssh/server/executor_windows.go b/client/ssh/server/executor_windows.go index e680da42d..0a34d9ca9 100644 --- a/client/ssh/server/executor_windows.go +++ b/client/ssh/server/executor_windows.go @@ -522,6 +522,6 @@ func (pd *PrivilegeDropper) createProcessWithToken(ctx context.Context, sourceTo } // createSuCommand creates a command using su -l -c for privilege switching (Windows stub) -func (s *Server) createSuCommand(ssh.Session, *user.User) (*exec.Cmd, error) { +func (s *Server) createSuCommand(ssh.Session, *user.User, bool) (*exec.Cmd, error) { return nil, fmt.Errorf("su command not available on Windows") } diff --git a/client/ssh/server/server_test.go b/client/ssh/server/server_test.go index 171a50aac..461fb758f 100644 --- a/client/ssh/server/server_test.go +++ b/client/ssh/server/server_test.go @@ -475,8 +475,9 @@ func TestSSHServer_WindowsShellHandling(t *testing.T) { // Test Unix shell behavior args := server.getShellCommandArgs("/bin/sh", "echo test") assert.Equal(t, "/bin/sh", args[0]) - assert.Equal(t, "-c", args[1]) - assert.Equal(t, "echo test", args[2]) + assert.Equal(t, "-l", args[1]) + assert.Equal(t, "-c", args[2]) + assert.Equal(t, "echo test", args[3]) } } diff --git a/client/ssh/server/userswitching_unix.go b/client/ssh/server/userswitching_unix.go index 969cb6578..51e521fcf 100644 --- a/client/ssh/server/userswitching_unix.go +++ b/client/ssh/server/userswitching_unix.go @@ -59,41 +59,9 @@ func isFullyNumeric(username string) bool { return true } -// createSecurePtyUserSwitchCommand creates a Pty command with proper user switching -// For privileged processes, uses login command. For non-privileged, falls back to shell. -func (s *Server) createPtyUserSwitchCommand(_ []string, localUser *user.User, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) { - if !isCurrentProcessPrivileged() { - // Non-privileged process: fallback to shell with login flag - return s.createNonPrivilegedPtyCommand(localUser, ptyReq, session) - } - - // Privileged process: use login command for proper user switching - return s.createPrivilegedPtyLoginCommand(localUser, ptyReq, session) -} - -// createNonPrivilegedPtyCommand creates a Pty command for non-privileged processes -func (s *Server) createNonPrivilegedPtyCommand(localUser *user.User, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) { - shell := getUserShell(localUser.Uid) - args := []string{shell, "-l"} - - execCmd := exec.CommandContext(session.Context(), args[0], args[1:]...) - execCmd.Dir = localUser.HomeDir - execCmd.Env = s.preparePtyEnv(localUser, ptyReq, session) - - return execCmd, nil -} - -// createPrivilegedPtyLoginCommand creates a Pty command using login for privileged processes -func (s *Server) createPrivilegedPtyLoginCommand(localUser *user.User, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) { - rawCmd := session.RawCommand() - - // If there's a command to execute, use su -l -c instead of login - if rawCmd != "" { - return s.createPrivilegedPtySuCommand(localUser, ptyReq, session, rawCmd) - } - - // For interactive sessions (no command), use login - loginPath, args, err := s.getRootLoginCmd(localUser.Username, session.RemoteAddr()) +// createPtyLoginCommand creates a Pty command using login for privileged processes +func (s *Server) createPtyLoginCommand(localUser *user.User, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) { + loginPath, args, err := s.getLoginCmd(localUser.Username, session.RemoteAddr()) if err != nil { return nil, fmt.Errorf("get login command: %w", err) } @@ -121,8 +89,8 @@ func (s *Server) createPrivilegedPtySuCommand(localUser *user.User, ptyReq ssh.P return execCmd, nil } -// getRootLoginCmd returns the login command and args for privileged Pty user switching -func (s *Server) getRootLoginCmd(username string, remoteAddr net.Addr) (string, []string, error) { +// getLoginCmd returns the login command and args for privileged Pty user switching +func (s *Server) getLoginCmd(username string, remoteAddr net.Addr) (string, []string, error) { loginPath, err := exec.LookPath("login") if err != nil { return "", nil, fmt.Errorf("login command not available: %w", err) @@ -244,23 +212,29 @@ func enableUserSwitching() error { return nil } -// createPtyCommandWithPrivileges creates the exec.Cmd for Pty execution respecting privilege check results -func (s *Server) createPtyCommandWithPrivileges(cmd []string, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) { +// createPtyCommand creates the exec.Cmd for Pty execution respecting privilege check results +func (s *Server) createPtyCommand(privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, session ssh.Session) (*exec.Cmd, error) { localUser := privilegeResult.User - if privilegeResult.RequiresUserSwitching { - return s.createPtyUserSwitchCommand(cmd, localUser, ptyReq, session) + if privilegeResult.UsedFallback { + return s.createDirectPtyCommand(session, localUser, ptyReq), nil } - // No user switching needed - create direct Pty command - shell := getUserShell(localUser.Uid) - rawCmd := session.RawCommand() - args := s.getShellCommandArgs(shell, rawCmd) - execCmd := exec.CommandContext(session.Context(), args[0], args[1:]...) + return s.createPtyLoginCommand(localUser, ptyReq, session) +} - execCmd.Dir = localUser.HomeDir - execCmd.Env = s.preparePtyEnv(localUser, ptyReq, session) - return execCmd, nil +// createDirectPtyCommand creates a direct Pty command without privilege dropping +func (s *Server) createDirectPtyCommand(session ssh.Session, localUser *user.User, ptyReq ssh.Pty) *exec.Cmd { + log.Debugf("creating direct Pty command for user %s (no user switching needed)", localUser.Username) + + shell := getUserShell(localUser.Uid) + args := s.getShellCommandArgs(shell, session.RawCommand()) + + cmd := exec.CommandContext(session.Context(), args[0], args[1:]...) + cmd.Dir = localUser.HomeDir + cmd.Env = s.preparePtyEnv(localUser, ptyReq, session) + + return cmd } // preparePtyEnv prepares environment variables for Pty execution diff --git a/client/ssh/server/userswitching_windows.go b/client/ssh/server/userswitching_windows.go index f5dddf2b7..8a65549da 100644 --- a/client/ssh/server/userswitching_windows.go +++ b/client/ssh/server/userswitching_windows.go @@ -79,9 +79,17 @@ func (s *Server) createExecutorCommand(session ssh.Session, localUser *user.User return s.createUserSwitchCommand(localUser, session, hasPty) } -// createDirectCommand is not supported on Windows - always use user switching with token creation +// createDirectCommand creates a command that runs without privilege dropping func (s *Server) createDirectCommand(session ssh.Session, localUser *user.User) (*exec.Cmd, error) { - return nil, fmt.Errorf("direct command execution not supported on Windows - use user switching with token creation") + log.Debugf("creating direct command for user %s (no user switching needed)", localUser.Username) + + shell := getUserShell(localUser.Uid) + args := s.getShellCommandArgs(shell, session.RawCommand()) + + cmd := exec.CommandContext(session.Context(), args[0], args[1:]...) + cmd.Dir = localUser.HomeDir + + return cmd, nil } // createUserSwitchCommand creates a command with Windows user switching