security: Prevent access to SSH nodes using SessionJoinPrincipal

lib/srv/forward/sshserver.go

@@ -643,7 +643,6 @@ func (s *Server) newRemoteClient(ctx context.Context, systemLogin string) (*trac
 		return nil, trace.Wrap(err)
 	}
 	return client, nil
-
 }
 
 // signersWithSHA1Fallback returns the signers provided by signersCb and appends
@@ -785,6 +784,16 @@ func (s *Server) handleChannel(ctx context.Context, nch ssh.NewChannel) {
 
 	// Channels of type "direct-tcpip" handles request for port forwarding.
 	case teleport.ChanDirectTCPIP:
+		// On forward server in direct-tcpip" channels from SessionJoinPrincipal
+		//  should be rejected, otherwise it's possible to use the
+		// "-teleport-internal-join" user to bypass RBAC.
+		if s.identityContext.Login == teleport.SSHSessionJoinPrincipal {
+			s.log.Error("Connection rejected, direct-tcpip with SessionJoinPrincipal in forward node must be blocked")
+			if err := nch.Reject(ssh.Prohibited, fmt.Sprintf("attempted %v channel open in join-only mode", channelType)); err != nil {
+				s.log.Warnf("Failed to reject channel: %v", err)
+			}
+			return
+		}
 		req, err := sshutils.ParseDirectTCPIPReq(nch.ExtraData())
 		if err != nil {
 			s.log.Errorf("Failed to parse request data: %v, err: %v", string(nch.ExtraData()), err)

lib/srv/forward/sshserver_test.go

@@ -19,13 +19,22 @@
 package forward
 
 import (
+	"context"
 	"crypto/rand"
+	"errors"
+	"os/user"
+	"sync/atomic"
 	"testing"
 
+	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ssh"
 
+	"github.com/gravitational/teleport"
 	apisshutils "github.com/gravitational/teleport/api/utils/sshutils"
+	"github.com/gravitational/teleport/lib/srv"
+	"github.com/gravitational/teleport/lib/sshutils"
+	"github.com/gravitational/teleport/lib/utils"
 )
 
 func TestSignersWithSHA1Fallback(t *testing.T) {
@@ -104,3 +113,86 @@ func TestSignersWithSHA1Fallback(t *testing.T) {
 		})
 	}
 }
+
+type newChannelMock struct {
+	channelType string
+	accepted    atomic.Bool
+	rejected    atomic.Bool
+}
+
+func (n *newChannelMock) Accept() (ssh.Channel, <-chan *ssh.Request, error) {
+	n.accepted.Store(true)
+	return nil, nil, errors.New("mock channel")
+}
+
+func (n *newChannelMock) Reject(reason ssh.RejectionReason, message string) error {
+	n.rejected.Store(true)
+	return nil
+}
+
+func (n *newChannelMock) ChannelType() string {
+	return n.channelType
+}
+
+func (n *newChannelMock) ExtraData() []byte {
+	return ssh.Marshal(sshutils.DirectTCPIPReq{
+		Host:     "localhost",
+		Port:     0,
+		Orig:     "localhost",
+		OrigPort: 0,
+	})
+}
+
+// TestDirectTCPIP ensures that ssh client using SessionJoinPrincipal as Login
+// cannot connect using "direct-tcpip" on forward mode.
+//
+// Forward requires a lot of depependencies and we don't have top level tests
+// yet here. If we add it in future, test should be rework to use public methods
+// instead of internals.
+func TestDirectTCPIP(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+
+	cases := []struct {
+		name           string
+		login          string
+		expectAccepted bool
+		expectRejected bool
+	}{
+		{
+			name:           "join principal rejected",
+			login:          teleport.SSHSessionJoinPrincipal,
+			expectAccepted: false,
+			expectRejected: true,
+		},
+		{
+			name: "user allowed",
+			login: func() string {
+				u, err := user.Current()
+				require.NoError(t, err)
+				return u.Username
+			}(),
+			expectAccepted: true,
+			// expectRejected is set to true because we are using mock channel
+			// which return errors on accept.
+			expectRejected: true,
+		},
+	}
+
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			s := Server{
+				log:             utils.NewLoggerForTests().WithField(trace.Component, "test"),
+				identityContext: srv.IdentityContext{Login: tt.login},
+			}
+
+			nch := &newChannelMock{channelType: teleport.ChanDirectTCPIP}
+			s.handleChannel(ctx, nch)
+			require.Equal(t, tt.expectRejected, nch.rejected.Load())
+			require.Equal(t, tt.expectAccepted, nch.accepted.Load())
+		})
+	}
+}

lib/srv/regular/sshserver.go

@@ -66,11 +66,9 @@ import (
 
 const sftpSubsystem = "sftp"
 
-var (
-	log = logrus.WithFields(logrus.Fields{
-		trace.Component: teleport.ComponentNode,
-	})
-)
+var log = logrus.WithFields(logrus.Fields{
+	trace.Component: teleport.ComponentNode,
+})
 
 // Server implements SSH server that uses configuration backend and
 // certificate-based authentication
@@ -314,7 +312,6 @@ func (s *Server) close() {
 	if s.users != nil {
 		s.users.Shutdown()
 	}
-
 }
 
 // Close closes listening socket and stops accepting connections
@@ -1205,6 +1202,16 @@ func (s *Server) HandleNewChan(ctx context.Context, ccx *sshutils.ConnectionCont
 		}()
 	// Channels of type "direct-tcpip" handles request for port forwarding.
 	case teleport.ChanDirectTCPIP:
+		// On regular server in "normal" mode "direct-tcpip" channels from
+		// SessionJoinPrincipal should be rejected, otherwise it's possible
+		// to use the "-teleport-internal-join" user to bypass RBAC.
+		if identityContext.Login == teleport.SSHSessionJoinPrincipal {
+			s.Logger.Error("Connection rejected, direct-tcpip with SessionJoinPrincipal in regular node must be blocked")
+			rejectChannel(
+				nch, ssh.Prohibited,
+				fmt.Sprintf("attempted %v channel open in join-only mode", channelType))
+			return
+		}
 		req, err := sshutils.ParseDirectTCPIPReq(nch.ExtraData())
 		if err != nil {
 			s.Logger.Errorf("Failed to parse request data: %v, err: %v.", string(nch.ExtraData()), err)
@@ -1572,7 +1579,6 @@ func (s *Server) dispatch(ctx context.Context, ch ssh.Channel, req *ssh.Request,
 			return trace.AccessDenied("attempted %v request in join-only mode", req.Type)
 		}
 	}
-
 	switch req.Type {
 	case tracessh.TracingRequest:
 		return nil

lib/srv/regular/sshserver_test.go

@@ -651,6 +651,23 @@ func TestDirectTCPIP(t *testing.T) {
 	body, err := io.ReadAll(resp.Body)
 	require.NoError(t, err)
 	require.Equal(t, []byte("hello, world\n"), body)
+
+	t.Run("SessionJoinPrincipal cannot use direct-tcpip", func(t *testing.T) {
+		// Ensure that ssh client using SessionJoinPrincipal as Login, cannot
+		// connect using "direct-tcpip".
+		ctx := context.Background()
+		cliUsingSessionJoin := f.newSSHClient(ctx, t, &user.User{Username: teleport.SSHSessionJoinPrincipal})
+		httpClientUsingSessionJoin := http.Client{
+			Transport: &http.Transport{
+				Dial: func(network string, addr string) (net.Conn, error) {
+					return cliUsingSessionJoin.DialContext(ctx, "tcp", u.Host)
+				},
+			},
+		}
+		//nolint:bodyclose // We expect an error here, no need to close.
+		_, err := httpClientUsingSessionJoin.Get(ts.URL)
+		require.ErrorContains(t, err, "ssh: rejected: administratively prohibited (attempted direct-tcpip channel open in join-only mode")
+	})
 }
 
 func TestAdvertiseAddr(t *testing.T) {
@@ -1065,7 +1082,7 @@ func x11EchoSession(ctx context.Context, t *testing.T, clt *tracessh.Client) x11
 	require.NoError(t, err)
 
 	// Allow non-root user to write to the temp file
-	err = tmpFile.Chmod(fs.FileMode(0777))
+	err = tmpFile.Chmod(fs.FileMode(0o777))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		os.Remove(tmpFile.Name())