fix file copy check not happening for agentless SSH connections (#25012)

constants.go

@@ -770,6 +770,9 @@ const (
 	// GetHomeDirSubsystem is an SSH subsystem request that Teleport
 	// uses to get the home directory of a remote user.
 	GetHomeDirSubsystem = "gethomedir"
+
+	// SFTPSubsystem is the SFTP SSH subsystem.
+	SFTPSubsystem = "sftp"
 )
 
 // A principal name for use in SSH certificates.

lib/srv/exec.go

@@ -94,7 +94,7 @@ func NewExecRequest(ctx *ServerContext, command string) (Exec, error) {
 		}, nil
 	}
 
-	// If this is a unregistered OpenSSH node or proxy recoding mode is
+	// If this is a registered OpenSSH node or proxy recoding mode is
 	// enabled, execute the command on a remote host. This is used by
 	// in-memory forwarding nodes.
 	if ctx.ServerSubKind == types.SubKindOpenSSHNode || services.IsRecordAtProxy(ctx.SessionRecordingConfig.GetMode()) {
@@ -231,22 +231,17 @@ func (e *localExec) String() string {
 }
 
 func (e *localExec) transformSecureCopy() error {
-	// split up command by space to grab the first word. if we don't have anything
-	// it's an interactive shell the user requested and not scp, return
-	args := strings.Split(e.GetCommand(), " ")
-	if len(args) == 0 {
-		return nil
-	}
-
-	// see the user is not requesting scp, return
-	_, f := filepath.Split(args[0])
-	if f != teleport.SCP {
-		return nil
-	}
-
-	if err := e.Ctx.CheckFileCopyingAllowed(); err != nil {
+	isSCPCmd, err := checkSCPAllowed(e.Ctx, e.GetCommand())
+	if err != nil {
 		return trace.Wrap(err)
 	}
+	if !isSCPCmd {
+		return nil
+	}
+	_, scpArgs, ok := strings.Cut(e.GetCommand(), " ")
+	if !ok {
+		return nil
+	}
 
 	// for scp requests update the command to execute to launch teleport with
 	// scp parameters just like openssh does.
@@ -258,11 +253,30 @@ func (e *localExec) transformSecureCopy() error {
 		teleportBin,
 		e.Ctx.ServerConn.RemoteAddr().String(),
 		e.Ctx.ServerConn.LocalAddr().String(),
-		strings.Join(args[1:], " "))
+		scpArgs,
+	)
 
 	return nil
 }
 
+// checkSCPAllowed will return false if the command is not a SCP command,
+// and if it is it will return true and potentially an error if file
+// copying is not allowed.
+func checkSCPAllowed(scx *ServerContext, command string) (bool, error) {
+	// split up command by space to grab the first word. if we don't have anything
+	// it's an interactive shell the user requested and not scp, return
+	args := strings.Split(command, " ")
+	if len(args) == 0 {
+		return false, nil
+	}
+	// see the user is not requesting scp, return
+	if _, f := filepath.Split(args[0]); f != teleport.SCP {
+		return false, nil
+	}
+
+	return true, trace.Wrap(scx.CheckFileCopyingAllowed())
+}
+
 // waitForContinue will wait 10 seconds for the continue signal, if not
 // received, it will stop waiting and exit.
 func waitForContinue(contfd *os.File) error {
@@ -315,6 +329,10 @@ func (e *remoteExec) SetCommand(command string) {
 // Start launches the given command returns (nil, nil) if successful.
 // ExecResult is only used to communicate an error while launching.
 func (e *remoteExec) Start(ctx context.Context, ch ssh.Channel) (*ExecResult, error) {
+	if _, err := checkSCPAllowed(e.ctx, e.GetCommand()); err != nil {
+		return nil, trace.Wrap(err)
+	}
+
 	// hook up stdout/err the channel so the user can interact with the command
 	e.session.Stdout = ch
 	e.session.Stderr = ch.Stderr()
@@ -543,7 +561,7 @@ func parseSecureCopy(path string) (string, string, bool, error) {
 		action = events.SCPActionUpload
 	}
 
-	// Exract the name of the Teleport executable on disk.
+	// Extract the name of the Teleport executable on disk.
 	teleportPath, err := os.Executable()
 	if err != nil {
 		return "", "", false, trace.Wrap(err)

lib/srv/forward/sshserver.go

@@ -957,9 +957,13 @@ func (s *Server) handleSessionChannel(ctx context.Context, nch ssh.NewChannel) {
 
 	scx.RemoteClient = s.remoteClient
 	scx.ChannelType = teleport.ChanSession
+	// Allow file copying at the server level as controlling node-wide
+	// file copying isn't supported for OpenSSH nodes. Users not allowed
+	// to copy files will still get checked and denied properly.
+	scx.SetAllowFileCopying(true)
 	defer scx.Close()
 
-	// Create a "session" channel on the remote host.  Note that we
+	// Create a "session" channel on the remote host. Note that we
 	// create the remote session channel before accepting the local
 	// channel request; this allows us to propagate the rejection
 	// reason/message in the event the channel is rejected.
@@ -1282,6 +1286,13 @@ func (s *Server) handleSubsystem(ctx context.Context, ch ssh.Channel, req *ssh.R
 		return trace.Wrap(err)
 	}
 
+	// if SFTP was requested, check that
+	if subsystem.subsytemName == teleport.SFTPSubsystem {
+		if err := serverContext.CheckSFTPAllowed(s.sessionRegistry); err != nil {
+			return trace.Wrap(err)
+		}
+	}
+
 	// start the requested subsystem, if it fails to start return result right away
 	err = subsystem.Start(ctx, ch)
 	if err != nil {

lib/srv/regular/sshserver.go

@@ -67,8 +67,6 @@ import (
 	"github.com/gravitational/teleport/lib/utils"
 )
 
-const sftpSubsystem = "sftp"
-
 var log = logrus.WithFields(logrus.Fields{
 	trace.Component: teleport.ComponentNode,
 })
@@ -2054,7 +2052,7 @@ func (s *Server) parseSubsystemRequest(req *ssh.Request, ch ssh.Channel, ctx *sr
 	// DELETE IN 15.0.0 (deprecated, tsh will not be using this anymore)
 	case r.Name == teleport.GetHomeDirSubsystem:
 		return newHomeDirSubsys(), nil
-	case r.Name == sftpSubsystem:
+	case r.Name == teleport.SFTPSubsystem:
 		if err := ctx.CheckSFTPAllowed(s.reg); err != nil {
 			return nil, trace.Wrap(err)
 		}