diff --git a/lib/srv/ctx.go b/lib/srv/ctx.go index 8a04ec4b9d864..e66f7409c417f 100644 --- a/lib/srv/ctx.go +++ b/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) diff --git a/lib/srv/ctx_test.go b/lib/srv/ctx_test.go index b627eaf4ff0e1..aeeedc71ffcac 100644 --- a/lib/srv/ctx_test.go +++ b/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 { diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index 722a64451e25e..c1087644447bd 100644 --- a/lib/srv/regular/sshserver.go +++ b/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) } diff --git a/lib/srv/sess.go b/lib/srv/sess.go index 06c1d3bba4f4b..5998aec106e1d 100644 --- a/lib/srv/sess.go +++ b/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