block SFTP connections when moderated sessions are enforced

lib/srv/ctx.go

@@ -71,7 +71,10 @@ var (
 	)
 )
 
-var ErrNodeFileCopyingNotPermitted = trace.AccessDenied("node does not allow file copying via SCP or SFTP")
+var (
+	ErrNodeFileCopyingNotPermitted  = trace.AccessDenied("node does not allow file copying via SCP or SFTP")
+	errCannotStartUnattendedSession = trace.AccessDenied("lacking privileges to start unattended session")
+)
 
 func init() {
 	prometheus.MustRegister(serverTX)
@@ -644,6 +647,28 @@ func (c *ServerContext) CheckFileCopyingAllowed() error {
 	return nil
 }
 
+// CheckSFTPAllowed returns an error if remote file operations via SCP
+// or SFTP are not allowed by the user's role or the node's config, or
+// if the user is not allowed to start unattended sessions.
+func (c *ServerContext) CheckSFTPAllowed() error {
+	if err := c.CheckFileCopyingAllowed(); err != nil {
+		return trace.Wrap(err)
+	}
+
+	// ensure moderated session policies allow starting an unattended session
+	policySets := c.Identity.AccessChecker.SessionPolicySets()
+	checker := auth.NewSessionAccessEvaluator(policySets, types.SSHSessionKind, c.Identity.TeleportUser)
+	canStart, _, err := checker.FulfilledFor(nil)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	if !canStart {
+		return errCannotStartUnattendedSession
+	}
+
+	return nil
+}
+
 // OpenXServerListener opens a new XServer unix listener.
 func (c *ServerContext) OpenXServerListener(x11Req x11.ForwardRequestPayload, displayOffset, maxDisplays int) error {
 	l, display, err := x11.OpenNewXServerListener(displayOffset, maxDisplays, x11Req.ScreenNumber)

lib/srv/ctx_test.go

@@ -24,7 +24,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestCheckFileCopyingAllowed(t *testing.T) {
+func TestCheckSFTPAllowed(t *testing.T) {
 	srv := newMockServer(t)
 	ctx := newTestServerContext(t, srv, nil)
 
@@ -107,6 +107,29 @@ func TestCheckFileCopyingAllowed(t *testing.T) {
 			},
 			expectedErr: errRoleFileCopyingNotPermitted,
 		},
+		{
+			name:                 "moderated sessions enforced",
+			nodeAllowFileCopying: true,
+			roles: []types.Role{
+				&types.RoleV5{
+					Kind: types.KindNode,
+					Spec: types.RoleSpecV5{
+						Allow: types.RoleConditions{
+							RequireSessionJoin: []*types.SessionRequirePolicy{
+								{
+									Name:   "test",
+									Filter: `contains(user.roles, "auditor")`,
+									Kinds:  []string{string(types.SSHSessionKind)},
+									Modes:  []string{string(types.SessionModeratorMode)},
+									Count:  3,
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErr: errCannotStartUnattendedSession,
+		},
 	}
 
 	for _, tt := range tests {
@@ -123,7 +146,7 @@ func TestCheckFileCopyingAllowed(t *testing.T) {
 				roles,
 			)
 
-			err := ctx.CheckFileCopyingAllowed()
+			err := ctx.CheckSFTPAllowed()
 			if tt.expectedErr == nil {
 				require.NoError(t, err)
 			} else {

lib/srv/regular/sshserver.go

@@ -2055,6 +2055,12 @@ 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.Error(err)
 	// Terminate the error with a newline when writing to remote channel's
@@ -2084,10 +2090,8 @@ func (s *Server) parseSubsystemRequest(req *ssh.Request, ch ssh.Channel, ctx *sr
 	case r.Name == teleport.GetHomeDirSubsystem:
 		return newHomeDirSubsys(), nil
 	case r.Name == sftpSubsystem:
-		if err := ctx.CheckFileCopyingAllowed(); err != nil {
-			// Add an extra newline here to separate this error message
-			// from a potential OpenSSH error message
-			writeStderr(ch, err.Error()+"\n")
+		if err := ctx.CheckSFTPAllowed(); err != nil {
+			s.replyError(ch, req, err)
 			return nil, trace.Wrap(err)
 		}
 

lib/srv/sess.go

@@ -285,7 +285,7 @@ func (s *SessionRegistry) OpenExecSession(ctx context.Context, channel ssh.Chann
 	}
 
 	if !canStart {
-		return trace.AccessDenied("lacking privileges to start unattended session")
+		return errCannotStartUnattendedSession
 	}
 
 	// Start a non-interactive session (TTY attached). Close the session if an error