Make 'tsh scp' error messages more useful

lib/srv/regular/sshserver.go

@@ -2014,12 +2014,6 @@ func (s *Server) handleProxyJump(ctx context.Context, ccx *sshutils.ConnectionCo
 	}
 }
 
-// TODO: tsh scp will display neither the message sent in stderr or in
-// the reply; github.com/pkg/sftp ignores the SSH channel stderr, and
-// golang.org/x/crypto/ssh.channel.SendRequest ignores the message in
-// a channel reply. This is bad UX for users, as
-// 'ssh: subsystem request failed' will be the only error displayed when
-// access is denied.
 func (s *Server) replyError(ch ssh.Channel, req *ssh.Request, err error) {
 	s.Logger.WithError(err).Errorf("failure handling SSH %q request", req.Type)
 	// Terminate the error with a newline when writing to remote channel's
@@ -2050,7 +2044,6 @@ func (s *Server) parseSubsystemRequest(req *ssh.Request, ch ssh.Channel, ctx *sr
 		return newHomeDirSubsys(), nil
 	case r.Name == sftpSubsystem:
 		if err := ctx.CheckSFTPAllowed(s.reg); err != nil {
-			s.replyError(ch, req, err)
 			return nil, trace.Wrap(err)
 		}
 

lib/sshutils/sftp/sftp.go

@@ -255,7 +255,22 @@ func (c *Config) TransferFiles(ctx context.Context, sshClient *ssh.Client) error
 		s.Setenv(string(FileTransferRequestID), fileTransferRequestID)
 	}
 
+	pe, err := s.StderrPipe()
+	if err != nil {
+		return trace.Wrap(err)
+	}
 	if err := s.RequestSubsystem("sftp"); err != nil {
+		// If the subsystem request failed and a generic error is
+		// returned, return the session's stderr as the error if it's
+		// non-empty, as the session's stderr may have a more useful
+		// error message. String comparison is only used here because
+		// the error is not exported.
+		if strings.Contains(err.Error(), "ssh: subsystem request failed") {
+			var sb strings.Builder
+			if n, _ := io.Copy(&sb, pe); n > 0 {
+				return trace.Wrap(errors.New(sb.String()))
+			}
+		}
 		return trace.Wrap(err)
 	}
 	pw, err := s.StdinPipe()

tool/tsh/tsh.go

@@ -3241,7 +3241,6 @@ func onSCP(cf *CLIConf) error {
 	if err == nil || (err != nil && errors.Is(err, context.Canceled)) {
 		return nil
 	}
-	fmt.Fprintln(os.Stderr, utils.UserMessageFromError(err))
 
 	return trace.Wrap(err)
 }