From a35396c4a50159757f13a8006c8b33d5f16ce26b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Mon, 20 Jun 2022 10:42:37 +0200 Subject: [PATCH 01/18] Only return active trackers in the webapi sessions API --- lib/web/apiserver.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 0cef0bf6470ac..26b9e0b1ad634 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -2130,7 +2130,7 @@ func (h *Handler) siteSessionsGet(w http.ResponseWriter, r *http.Request, p http sessions := make([]session.Session, 0, len(trackers)) for _, tracker := range trackers { - if tracker.GetSessionKind() == types.SSHSessionKind { + if tracker.GetSessionKind() == types.SSHSessionKind && tracker.GetState() == types.SessionState_SessionStateRunning { sessions = append(sessions, trackerToLegacySession(tracker, p.ByName("site"))) } } @@ -2163,7 +2163,7 @@ func (h *Handler) siteSessionGet(w http.ResponseWriter, r *http.Request, p httpr return nil, trace.Wrap(err) } - if tracker.GetSessionKind() != types.SSHSessionKind { + if tracker.GetSessionKind() != types.SSHSessionKind || tracker.GetState() != types.SessionState_SessionStateRunning { return nil, trace.NotFound("session %v not found", sessionID) } From 00e63d7705620b62e3e0510ba88db2a17bd91857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Mon, 20 Jun 2022 10:53:28 +0200 Subject: [PATCH 02/18] Always allow session owners to join their own session --- lib/auth/auth_with_roles.go | 2 +- lib/auth/session_access.go | 9 ++++++++- lib/auth/session_access_test.go | 26 ++++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 9bc50876f4858..1579addfe7403 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -272,7 +272,7 @@ func (a *ServerWithRoles) CreateSessionTracker(ctx context.Context, tracker type } func (a *ServerWithRoles) filterSessionTracker(ctx context.Context, joinerRoles []types.Role, tracker types.SessionTracker) bool { - evaluator := NewSessionAccessEvaluator(tracker.GetHostPolicySets(), tracker.GetSessionKind()) + evaluator := NewSessionAccessEvaluator(tracker.GetHostPolicySets(), tracker.GetSessionKind(), tracker.GetHostUser()) modes := evaluator.CanJoin(SessionAccessContext{Roles: joinerRoles}) if len(modes) == 0 { diff --git a/lib/auth/session_access.go b/lib/auth/session_access.go index 8ad4e2348e5c3..eb42f718d8ef1 100644 --- a/lib/auth/session_access.go +++ b/lib/auth/session_access.go @@ -43,14 +43,16 @@ type SessionAccessEvaluator struct { kind types.SessionKind policySets []*types.SessionTrackerPolicySet isModerated bool + owner string } // NewSessionAccessEvaluator creates a new session access evaluator for a given session kind // and a set of roles attached to the host user. -func NewSessionAccessEvaluator(policySets []*types.SessionTrackerPolicySet, kind types.SessionKind) SessionAccessEvaluator { +func NewSessionAccessEvaluator(policySets []*types.SessionTrackerPolicySet, kind types.SessionKind, owner string) SessionAccessEvaluator { e := SessionAccessEvaluator{ kind: kind, policySets: policySets, + owner: owner, } for _, policySet := range policySets { @@ -189,6 +191,11 @@ func (e *SessionAccessEvaluator) CanJoin(user SessionAccessContext) []types.Sess return preAccessControlsModes(e.kind) } + // Session owners can always join their own sessions. + if user.Username == e.owner { + return []types.SessionParticipantMode{types.SessionPeerMode, types.SessionModeratorMode, types.SessionObserverMode} + } + var modes []types.SessionParticipantMode // Loop over every allow policy attached the participant and check it's applicability. diff --git a/lib/auth/session_access_test.go b/lib/auth/session_access_test.go index c2794c058609c..8cdb8ff88e6ef 100644 --- a/lib/auth/session_access_test.go +++ b/lib/auth/session_access_test.go @@ -28,6 +28,7 @@ type startTestCase struct { host []types.Role sessionKinds []types.SessionKind participants []SessionAccessContext + owner string expected []bool } @@ -185,7 +186,7 @@ func TestSessionAccessStart(t *testing.T) { } for i, kind := range testCase.sessionKinds { - evaluator := NewSessionAccessEvaluator(policies, kind) + evaluator := NewSessionAccessEvaluator(policies, kind, testCase.owner) result, _, err := evaluator.FulfilledFor(testCase.participants) require.NoError(t, err) require.Equal(t, testCase.expected[i], result) @@ -199,6 +200,7 @@ type joinTestCase struct { host types.Role sessionKinds []types.SessionKind participant SessionAccessContext + owner string expected []bool } @@ -250,6 +252,25 @@ func successGlobJoinTestCase(t *testing.T) joinTestCase { } } +func successSameUserJoinTestCase(t *testing.T) joinTestCase { + hostRole, err := types.NewRole("host", types.RoleSpecV5{}) + require.NoError(t, err) + participantRole, err := types.NewRole("participant", types.RoleSpecV5{}) + require.NoError(t, err) + + return joinTestCase{ + name: "success", + host: hostRole, + sessionKinds: []types.SessionKind{types.SSHSessionKind}, + participant: SessionAccessContext{ + Username: "john", + Roles: []types.Role{participantRole}, + }, + owner: "john", + expected: []bool{true}, + } +} + func failRoleJoinTestCase(t *testing.T) joinTestCase { hostRole, err := types.NewRole("host", types.RoleSpecV5{}) require.NoError(t, err) @@ -314,6 +335,7 @@ func TestSessionAccessJoin(t *testing.T) { testCases := []joinTestCase{ successJoinTestCase(t), successGlobJoinTestCase(t), + successSameUserJoinTestCase(t), failRoleJoinTestCase(t), failKindJoinTestCase(t), versionDefaultJoinTestCase(t), @@ -323,7 +345,7 @@ func TestSessionAccessJoin(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { for i, kind := range testCase.sessionKinds { policy := testCase.host.GetSessionPolicySet() - evaluator := NewSessionAccessEvaluator([]*types.SessionTrackerPolicySet{&policy}, kind) + evaluator := NewSessionAccessEvaluator([]*types.SessionTrackerPolicySet{&policy}, kind, testCase.owner) result := evaluator.CanJoin(testCase.participant) require.Equal(t, testCase.expected[i], len(result) > 0) } From 7dde8c56fe4debf4fcf2098e46824e789da20cbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Mon, 20 Jun 2022 10:55:52 +0200 Subject: [PATCH 03/18] Show pending trackers --- lib/web/apiserver.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 26b9e0b1ad634..25d1994fa3852 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -2130,7 +2130,7 @@ func (h *Handler) siteSessionsGet(w http.ResponseWriter, r *http.Request, p http sessions := make([]session.Session, 0, len(trackers)) for _, tracker := range trackers { - if tracker.GetSessionKind() == types.SSHSessionKind && tracker.GetState() == types.SessionState_SessionStateRunning { + if tracker.GetSessionKind() == types.SSHSessionKind && tracker.GetState() != types.SessionState_SessionStateTerminated { sessions = append(sessions, trackerToLegacySession(tracker, p.ByName("site"))) } } @@ -2163,7 +2163,7 @@ func (h *Handler) siteSessionGet(w http.ResponseWriter, r *http.Request, p httpr return nil, trace.Wrap(err) } - if tracker.GetSessionKind() != types.SSHSessionKind || tracker.GetState() != types.SessionState_SessionStateRunning { + if tracker.GetSessionKind() != types.SSHSessionKind || tracker.GetState() == types.SessionState_SessionStateTerminated { return nil, trace.NotFound("session %v not found", sessionID) } From f76c34391aed6530102206a052fa8ef3fe3c7bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Mon, 20 Jun 2022 11:14:43 +0200 Subject: [PATCH 04/18] Fix build errors --- lib/kube/proxy/forwarder.go | 2 +- lib/kube/proxy/sess.go | 2 +- lib/srv/sess.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index ea0939c52502d..c3cd8ceabcf80 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -1011,7 +1011,7 @@ func (f *Forwarder) execNonInteractive(ctx *authContext, w http.ResponseWriter, policySets = append(policySets, &policySet) } - authorizer := auth.NewSessionAccessEvaluator(policySets, types.KubernetesSessionKind) + authorizer := auth.NewSessionAccessEvaluator(policySets, types.KubernetesSessionKind, ctx.User.String()) canStart, _, err := authorizer.FulfilledFor(nil) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/kube/proxy/sess.go b/lib/kube/proxy/sess.go index 0a405bada0f6a..5b4ec4632b690 100644 --- a/lib/kube/proxy/sess.go +++ b/lib/kube/proxy/sess.go @@ -319,7 +319,7 @@ func newSession(ctx authContext, forwarder *Forwarder, req *http.Request, params } q := req.URL.Query() - accessEvaluator := auth.NewSessionAccessEvaluator(policySets, types.KubernetesSessionKind) + accessEvaluator := auth.NewSessionAccessEvaluator(policySets, types.KubernetesSessionKind, ctx.User.String()) io := srv.NewTermManager() diff --git a/lib/srv/sess.go b/lib/srv/sess.go index 8747a08f72b1f..2ab4f85144cc0 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -570,7 +570,7 @@ func newSession(id rsession.ID, r *SessionRegistry, ctx *ServerContext) (*sessio stopC: make(chan struct{}), startTime: startTime, serverCtx: ctx.srv.Context(), - access: auth.NewSessionAccessEvaluator(policySets, types.SSHSessionKind), + access: auth.NewSessionAccessEvaluator(policySets, types.SSHSessionKind, ctx.Identity.TeleportUser), scx: ctx, presenceEnabled: ctx.Identity.Certificate.Extensions[teleport.CertExtensionMFAVerified] != "", io: NewTermManager(), From 416f48bed231659492b362e996b93ea267921412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Mon, 20 Jun 2022 11:41:16 +0200 Subject: [PATCH 05/18] rename test case --- lib/auth/session_access_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/auth/session_access_test.go b/lib/auth/session_access_test.go index 8cdb8ff88e6ef..02ebdd80097a1 100644 --- a/lib/auth/session_access_test.go +++ b/lib/auth/session_access_test.go @@ -259,7 +259,7 @@ func successSameUserJoinTestCase(t *testing.T) joinTestCase { require.NoError(t, err) return joinTestCase{ - name: "success", + name: "successSameUser", host: hostRole, sessionKinds: []types.SessionKind{types.SSHSessionKind}, participant: SessionAccessContext{ From 05932dbfc5ca6914a5f6062c5eeb9d821d128fca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Tue, 21 Jun 2022 21:35:55 +0200 Subject: [PATCH 06/18] Set correct username when joining, matters from CLI --- lib/srv/sess.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/srv/sess.go b/lib/srv/sess.go index 2ab4f85144cc0..bc180d9206f07 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -1637,7 +1637,8 @@ func (s *session) addParty(p *party, mode types.SessionParticipantMode) error { func (s *session) join(ch ssh.Channel, ctx *ServerContext, mode types.SessionParticipantMode) (*party, error) { if ctx.Identity.TeleportUser != s.initiator { accessContext := auth.SessionAccessContext{ - Roles: ctx.Identity.AccessChecker.Roles(), + Username: ctx.Identity.TeleportUser, + Roles: ctx.Identity.AccessChecker.Roles(), } modes := s.access.CanJoin(accessContext) From 205093e6bb5f49649d3c4ae21d377f65c29c356b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 15:12:59 +0200 Subject: [PATCH 07/18] Distinguish between starts and join + remove first duplicate user from tracker --- lib/srv/sess.go | 19 ++++++----------- lib/web/apiserver.go | 2 -- lib/web/apiserver_test.go | 6 +++++- lib/web/terminal.go | 45 +++++++++++++++++++++++++++++++-------- 4 files changed, 48 insertions(+), 24 deletions(-) diff --git a/lib/srv/sess.go b/lib/srv/sess.go index bc180d9206f07..21ff234045fb7 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -1762,18 +1762,13 @@ func (p *party) closeUnderSessionLock() { // on an interval until the session tracker is closed. func (s *session) trackSession(teleportUser string, policySet []*types.SessionTrackerPolicySet) error { trackerSpec := types.SessionTrackerSpecV1{ - SessionID: s.id.String(), - Kind: string(types.SSHSessionKind), - State: types.SessionState_SessionStatePending, - Hostname: s.registry.Srv.GetInfo().GetHostname(), - Address: s.scx.srv.ID(), - ClusterName: s.scx.ClusterName, - Login: s.login, - Participants: []types.Participant{{ - ID: teleportUser, - User: teleportUser, - LastActive: s.registry.clock.Now(), - }}, + SessionID: s.id.String(), + Kind: string(types.SSHSessionKind), + State: types.SessionState_SessionStatePending, + Hostname: s.registry.Srv.GetInfo().GetHostname(), + Address: s.scx.srv.ID(), + ClusterName: s.scx.ClusterName, + Login: s.login, HostUser: teleportUser, Reason: s.scx.env[teleport.EnvSSHSessionReason], HostPolicies: policySet, diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 25d1994fa3852..f639e328f6021 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -1970,8 +1970,6 @@ func (h *Handler) clusterNodesGet(w http.ResponseWriter, r *http.Request, p http // // {"server_id": "uuid", "login": "admin", "term": {"h": 120, "w": 100}, "sid": "123"} // -// Session id can be empty -// // Successful response is a websocket stream that allows read write to the server // func (h *Handler) siteNodeConnect( diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 0c581a1a2c487..5c229111b33cb 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -1478,7 +1478,7 @@ func TestActiveSessions(t *testing.T) { sess := sessResp.Sessions[0] require.Equal(t, sid, sess.ID) require.Equal(t, s.node.GetNamespace(), sess.Namespace) - require.NotNil(t, sess.Parties) + require.Len(t, sess.Parties, 1) require.Greater(t, sess.TerminalParams.H, 0) require.Greater(t, sess.TerminalParams.W, 0) require.Equal(t, pack.login, sess.Login) @@ -3312,6 +3312,10 @@ func (mock authProviderMock) GetSessionEvents(n string, s session.ID, c int, p b return []events.EventFields{}, nil } +func (mock authProviderMock) GetSessionTracker(ctx context.Context, sessionID string) (types.SessionTracker, error) { + return nil, trace.NotFound("foo") +} + func (s *WebSuite) makeTerminal(t *testing.T, pack *authPack, opts ...session.ID) (*websocket.Conn, error) { var sessionID session.ID if len(opts) == 0 { diff --git a/lib/web/terminal.go b/lib/web/terminal.go index de11ce877f338..919cae5e788f5 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -36,6 +36,7 @@ import ( "github.com/gravitational/teleport" authproto "github.com/gravitational/teleport/api/client/proto" + apidefaults "github.com/gravitational/teleport/api/defaults" tracessh "github.com/gravitational/teleport/api/observability/tracing/ssh" "github.com/gravitational/teleport/api/types" wanlib "github.com/gravitational/teleport/lib/auth/webauthn" @@ -84,6 +85,7 @@ type TerminalRequest struct { type AuthProvider interface { GetNodes(ctx context.Context, namespace string) ([]types.Server, error) GetSessionEvents(namespace string, sid session.ID, after int, includePrintEvents bool) ([]events.EventFields, error) + GetSessionTracker(ctx context.Context, sessionID string) (types.SessionTracker, error) } // NewTerminal creates a web-based terminal based on WebSockets and returns a @@ -116,6 +118,17 @@ func NewTerminal(ctx context.Context, req TerminalRequest, authProvider AuthProv return nil, trace.BadParameter("invalid server name %q: %v", req.Server, err) } + var join bool + _, err = authProvider.GetSessionTracker(ctx, string(req.SessionID)) + switch { + case trace.IsNotFound(err): + join = false + case err != nil: + return nil, trace.Wrap(err) + default: + join = true + } + return &TerminalHandler{ log: logrus.WithFields(logrus.Fields{ trace.Component: teleport.ComponentWebsocket, @@ -129,6 +142,7 @@ func NewTerminal(ctx context.Context, req TerminalRequest, authProvider AuthProv encoder: unicode.UTF8.NewEncoder(), decoder: unicode.UTF8.NewDecoder(), wsLock: &sync.Mutex{}, + join: join, }, nil } @@ -178,6 +192,9 @@ type TerminalHandler struct { closeOnce sync.Once wsLock *sync.Mutex + + // join is set if we're joining an existing session + join bool } // Serve builds a connect to the remote node and then pumps back two types of @@ -260,7 +277,7 @@ func (t *TerminalHandler) handler(ws *websocket.Conn, r *http.Request) { // Create a Teleport client, if not able to, show the reason to the user in // the terminal. - tc, err := t.makeClient(ws, r) + tc, err := t.makeClient(ws, r, t.join) if err != nil { t.log.WithError(err).Infof("Failed creating a client for session %v.", t.params.SessionID) writeErr := t.writeError(err, ws) @@ -282,7 +299,7 @@ func (t *TerminalHandler) handler(ws *websocket.Conn, r *http.Request) { go t.startPingLoop(ws) // Pump raw terminal in/out and audit events into the websocket. - go t.streamTerminal(ws, tc) + go t.streamTerminal(ws, tc, t.join) go t.streamEvents(ws, tc) // Block until the terminal session is complete. @@ -291,7 +308,7 @@ func (t *TerminalHandler) handler(ws *websocket.Conn, r *http.Request) { } // makeClient builds a *client.TeleportClient for the connection. -func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request) (*client.TeleportClient, error) { +func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request, join bool) (*client.TeleportClient, error) { clientConfig, err := makeTeleportClientConfig(r.Context(), t.ctx) if err != nil { return nil, trace.Wrap(err) @@ -301,8 +318,14 @@ func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request) (*clie // communicate over the websocket. stream := t.asTerminalStream(ws) + if join { + clientConfig.HostLogin = teleport.SSHSessionJoinPrincipal + } else { + clientConfig.HostLogin = t.params.Login + } + + clientConfig.Env = map[string]string{sshutils.SessionEnvVar: string(t.params.SessionID)} clientConfig.ForwardAgent = client.ForwardAgentLocal - clientConfig.HostLogin = t.params.Login clientConfig.Namespace = t.params.Namespace clientConfig.Stdout = stream clientConfig.Stderr = stream @@ -313,7 +336,6 @@ func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request) (*clie } clientConfig.Host = t.hostName clientConfig.HostPort = t.hostPort - clientConfig.Env = map[string]string{sshutils.SessionEnvVar: string(t.params.SessionID)} clientConfig.ClientAddr = r.RemoteAddr if len(t.params.InteractiveCommand) > 0 { @@ -425,12 +447,17 @@ func promptMFAChallenge( // streamTerminal opens a SSH connection to the remote host and streams // events back to the web client. -func (t *TerminalHandler) streamTerminal(ws *websocket.Conn, tc *client.TeleportClient) { +func (t *TerminalHandler) streamTerminal(ws *websocket.Conn, tc *client.TeleportClient, join bool) { defer t.terminalCancel() + var err error - // Establish SSH connection to the server. This function will block until - // either an error occurs or it completes successfully. - err := tc.SSH(t.terminalContext, t.params.InteractiveCommand, false) + if join { + err = tc.Join(t.terminalContext, types.SessionPeerMode, apidefaults.Namespace, t.params.SessionID, tc.Stdin) + } else { + // Establish SSH connection to the server. This function will block until + // either an error occurs or it completes successfully. + err = tc.SSH(t.terminalContext, t.params.InteractiveCommand, false) + } // TODO IN: 5.0 // From 99d291e8be0dfa2591f396c4fa099f1cb540eb12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 15:57:43 +0200 Subject: [PATCH 08/18] dedup users --- api/types/session_tracker.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/api/types/session_tracker.go b/api/types/session_tracker.go index b4736d5e56519..790ef314970c3 100644 --- a/api/types/session_tracker.go +++ b/api/types/session_tracker.go @@ -291,7 +291,16 @@ func (s *SessionTrackerV1) GetParticipants() []Participant { // AddParticipant adds a participant to the session tracker. func (s *SessionTrackerV1) AddParticipant(participant Participant) { - s.Spec.Participants = append(s.Spec.Participants, participant) + contains := false + for _, p := range s.Spec.Participants { + if p.User == participant.User { + contains = true + } + } + + if !contains { + s.Spec.Participants = append(s.Spec.Participants, participant) + } } // RemoveParticipant removes a participant from the session tracker. From a3347920d0dd6d6435293c52005bc6ba205ccd92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 16:18:20 +0200 Subject: [PATCH 09/18] fix --- api/types/session_tracker.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/api/types/session_tracker.go b/api/types/session_tracker.go index 790ef314970c3..b4736d5e56519 100644 --- a/api/types/session_tracker.go +++ b/api/types/session_tracker.go @@ -291,16 +291,7 @@ func (s *SessionTrackerV1) GetParticipants() []Participant { // AddParticipant adds a participant to the session tracker. func (s *SessionTrackerV1) AddParticipant(participant Participant) { - contains := false - for _, p := range s.Spec.Participants { - if p.User == participant.User { - contains = true - } - } - - if !contains { - s.Spec.Participants = append(s.Spec.Participants, participant) - } + s.Spec.Participants = append(s.Spec.Participants, participant) } // RemoveParticipant removes a participant from the session tracker. From f0ef967787f4d16d473222fd486e055519a5d873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 18:15:27 +0200 Subject: [PATCH 10/18] dedup participants before conversion --- lib/web/apiserver.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index f639e328f6021..8951ed15edbef 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -2079,7 +2079,13 @@ func trackerToLegacySession(tracker types.SessionTracker, clusterName string) se participants := tracker.GetParticipants() parties := make([]session.Party, 0, len(participants)) + found := map[string]struct{}{} for _, participant := range participants { + if _, ok := found[participant.User]; ok { + continue + } + + found[participant.User] = struct{}{} parties = append(parties, session.Party{ ID: session.ID(participant.ID), User: participant.User, From 6898e9458cd5a6474616f9bc58cd0913bc1e0861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 18:26:01 +0200 Subject: [PATCH 11/18] oop --- lib/web/terminal.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/web/terminal.go b/lib/web/terminal.go index 919cae5e788f5..9def841a43b5c 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -277,7 +277,7 @@ func (t *TerminalHandler) handler(ws *websocket.Conn, r *http.Request) { // Create a Teleport client, if not able to, show the reason to the user in // the terminal. - tc, err := t.makeClient(ws, r, t.join) + tc, err := t.makeClient(ws, r) if err != nil { t.log.WithError(err).Infof("Failed creating a client for session %v.", t.params.SessionID) writeErr := t.writeError(err, ws) @@ -299,7 +299,7 @@ func (t *TerminalHandler) handler(ws *websocket.Conn, r *http.Request) { go t.startPingLoop(ws) // Pump raw terminal in/out and audit events into the websocket. - go t.streamTerminal(ws, tc, t.join) + go t.streamTerminal(ws, tc) go t.streamEvents(ws, tc) // Block until the terminal session is complete. @@ -308,7 +308,7 @@ func (t *TerminalHandler) handler(ws *websocket.Conn, r *http.Request) { } // makeClient builds a *client.TeleportClient for the connection. -func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request, join bool) (*client.TeleportClient, error) { +func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request) (*client.TeleportClient, error) { clientConfig, err := makeTeleportClientConfig(r.Context(), t.ctx) if err != nil { return nil, trace.Wrap(err) @@ -318,7 +318,7 @@ func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request, join b // communicate over the websocket. stream := t.asTerminalStream(ws) - if join { + if t.join { clientConfig.HostLogin = teleport.SSHSessionJoinPrincipal } else { clientConfig.HostLogin = t.params.Login @@ -447,11 +447,11 @@ func promptMFAChallenge( // streamTerminal opens a SSH connection to the remote host and streams // events back to the web client. -func (t *TerminalHandler) streamTerminal(ws *websocket.Conn, tc *client.TeleportClient, join bool) { +func (t *TerminalHandler) streamTerminal(ws *websocket.Conn, tc *client.TeleportClient) { defer t.terminalCancel() var err error - if join { + if t.join { err = tc.Join(t.terminalContext, types.SessionPeerMode, apidefaults.Namespace, t.params.SessionID, tc.Stdin) } else { // Establish SSH connection to the server. This function will block until From 7066f8dec2990f4a372bd21a28af27efce94cdaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 18:50:23 +0200 Subject: [PATCH 12/18] test --- lib/web/terminal.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/web/terminal.go b/lib/web/terminal.go index 9def841a43b5c..de409e4fc3b8a 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -36,7 +36,6 @@ import ( "github.com/gravitational/teleport" authproto "github.com/gravitational/teleport/api/client/proto" - apidefaults "github.com/gravitational/teleport/api/defaults" tracessh "github.com/gravitational/teleport/api/observability/tracing/ssh" "github.com/gravitational/teleport/api/types" wanlib "github.com/gravitational/teleport/lib/auth/webauthn" @@ -324,7 +323,6 @@ func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request) (*clie clientConfig.HostLogin = t.params.Login } - clientConfig.Env = map[string]string{sshutils.SessionEnvVar: string(t.params.SessionID)} clientConfig.ForwardAgent = client.ForwardAgentLocal clientConfig.Namespace = t.params.Namespace clientConfig.Stdout = stream @@ -336,6 +334,7 @@ func (t *TerminalHandler) makeClient(ws *websocket.Conn, r *http.Request) (*clie } clientConfig.Host = t.hostName clientConfig.HostPort = t.hostPort + clientConfig.Env = map[string]string{sshutils.SessionEnvVar: string(t.params.SessionID)} clientConfig.ClientAddr = r.RemoteAddr if len(t.params.InteractiveCommand) > 0 { @@ -451,13 +450,10 @@ func (t *TerminalHandler) streamTerminal(ws *websocket.Conn, tc *client.Teleport defer t.terminalCancel() var err error - if t.join { - err = tc.Join(t.terminalContext, types.SessionPeerMode, apidefaults.Namespace, t.params.SessionID, tc.Stdin) - } else { - // Establish SSH connection to the server. This function will block until - // either an error occurs or it completes successfully. - err = tc.SSH(t.terminalContext, t.params.InteractiveCommand, false) - } + // Establish SSH connection to the server. This function will block until + // either an error occurs or it completes successfully. + err = tc.SSH(t.terminalContext, t.params.InteractiveCommand, false) + panic(trace.DebugReport(err)) // TODO IN: 5.0 // From 14ddc8dde40184521d985249e2e5de46c61a3adf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 18:53:08 +0200 Subject: [PATCH 13/18] specify owner during tracker filtering --- lib/auth/auth_with_roles.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 1579addfe7403..c174e3d92e81f 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -273,7 +273,7 @@ func (a *ServerWithRoles) CreateSessionTracker(ctx context.Context, tracker type func (a *ServerWithRoles) filterSessionTracker(ctx context.Context, joinerRoles []types.Role, tracker types.SessionTracker) bool { evaluator := NewSessionAccessEvaluator(tracker.GetHostPolicySets(), tracker.GetSessionKind(), tracker.GetHostUser()) - modes := evaluator.CanJoin(SessionAccessContext{Roles: joinerRoles}) + modes := evaluator.CanJoin(SessionAccessContext{Username: a.context.User.String(), Roles: joinerRoles}) if len(modes) == 0 { return false From c4333a3f2730e5a6ab6ff12ef646f84d7e5a1885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 19:34:56 +0200 Subject: [PATCH 14/18] fix weblogins --- lib/auth/auth.go | 2 +- lib/services/role.go | 10 +++++++++- lib/srv/forward/sshserver.go | 9 +++++++++ lib/srv/regular/sshserver.go | 16 ++++++++++++++++ lib/srv/sess.go | 4 ++-- lib/web/terminal.go | 1 - 6 files changed, 37 insertions(+), 5 deletions(-) diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 451db895d04cd..37e6d9b3d3f3c 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -1089,7 +1089,7 @@ func (a *Server) generateUserCert(req certRequest) (*proto.Certs, error) { // Add the special join-only principal used for joining sessions. // All users have access to this and join RBAC rules are checked after the connection is established. - allowedLogins = append(allowedLogins, "-teleport-internal-join") + allowedLogins = append(allowedLogins, teleport.SSHSessionJoinPrincipal) requestedResourcesStr, err := types.ResourceIDsToString(req.checker.GetAllowedResourceIDs()) if err != nil { diff --git a/lib/services/role.go b/lib/services/role.go index d64b9803b079c..251d25b9a1951 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -122,7 +122,7 @@ func NewImplicitRole() types.Role { // // Used in tests only. func RoleForUser(u types.User) types.Role { - role, _ := types.NewRoleV3(RoleNameForUser(u.GetName()), types.RoleSpecV5{ + role, _ := types.NewRole(RoleNameForUser(u.GetName()), types.RoleSpecV5{ Options: types.RoleOptions{ CertificateFormat: constants.CertificateFormatStandard, MaxSessionTTL: types.NewDuration(defaults.MaxCertDuration), @@ -150,6 +150,14 @@ func RoleForUser(u types.User) types.Role { types.NewRule(types.KindLock, RW()), types.NewRule(types.KindToken, RW()), }, + JoinSessions: []*types.SessionJoinPolicy{ + { + Name: "foo", + Roles: []string{"*"}, + Kinds: []string{string(types.SSHSessionKind)}, + Modes: []string{string(types.SessionPeerMode)}, + }, + }, }, }) return role diff --git a/lib/srv/forward/sshserver.go b/lib/srv/forward/sshserver.go index cd124ccbfc70b..9e53913b9d2ae 100644 --- a/lib/srv/forward/sshserver.go +++ b/lib/srv/forward/sshserver.go @@ -928,6 +928,15 @@ func (s *Server) dispatch(ctx context.Context, ch ssh.Channel, req *ssh.Request, // SSH will send them anyway but it seems fine to silently drop them. case sshutils.SubsystemRequest: return s.handleSubsystem(ctx, ch, req, scx) + case sshutils.AgentForwardRequest: + // to maintain interoperability with OpenSSH, agent forwarding requests + // should never fail, all errors should be logged and we should continue + // processing requests. + err := s.handleAgentForward(ch, req, scx) + if err != nil { + s.log.Debug(err) + } + return nil default: return trace.AccessDenied("attempted %v request in join-only mode", req.Type) } diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index dc8059baf9351..9fed71e35d208 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -1594,6 +1594,22 @@ func (s *Server) dispatch(ctx context.Context, ch ssh.Channel, req *ssh.Request, // SSH will send them anyway but it seems fine to silently drop them. case sshutils.SubsystemRequest: return s.handleSubsystem(ctx, ch, req, serverContext) + case sshutils.AgentForwardRequest: + // This happens when SSH client has agent forwarding enabled, in this case + // client sends a special request, in return SSH server opens new channel + // that uses SSH protocol for agent drafted here: + // https://tools.ietf.org/html/draft-ietf-secsh-agent-02 + // the open ssh proto spec that we implement is here: + // http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.agent + + // to maintain interoperability with OpenSSH, agent forwarding requests + // should never fail, all errors should be logged and we should continue + // processing requests. + err := s.handleAgentForwardNode(req, serverContext) + if err != nil { + log.Warn(err) + } + return nil default: return trace.AccessDenied("attempted %v request in join-only mode", req.Type) } diff --git a/lib/srv/sess.go b/lib/srv/sess.go index 21ff234045fb7..5b166672ec038 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -544,7 +544,7 @@ func newSession(id rsession.ID, r *SessionRegistry, ctx *ServerContext) (*sessio if err != nil { return nil, trace.Wrap(err) } - if existing.Login != rsess.Login { + if existing.Login != rsess.Login && rsess.Login != teleport.SSHSessionJoinPrincipal { return nil, trace.AccessDenied( "can't switch users from %v to %v for session %v", rsess.Login, existing.Login, id) @@ -1544,7 +1544,7 @@ func (s *session) addParty(p *party, mode types.SessionParticipantMode) error { s.mu.Lock() defer s.mu.Unlock() - if s.login != p.login { + if s.login != p.login && p.login != teleport.SSHSessionJoinPrincipal { return trace.AccessDenied( "can't switch users from %v to %v for session %v", s.login, p.login, s.id) diff --git a/lib/web/terminal.go b/lib/web/terminal.go index de409e4fc3b8a..c72cff268e66c 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -453,7 +453,6 @@ func (t *TerminalHandler) streamTerminal(ws *websocket.Conn, tc *client.Teleport // Establish SSH connection to the server. This function will block until // either an error occurs or it completes successfully. err = tc.SSH(t.terminalContext, t.params.InteractiveCommand, false) - panic(trace.DebugReport(err)) // TODO IN: 5.0 // From 0bfd5a2195a846f8ec2a1cee41176250fa33f299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 19:36:18 +0200 Subject: [PATCH 15/18] remove dedup --- lib/web/apiserver.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 8951ed15edbef..f639e328f6021 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -2079,13 +2079,7 @@ func trackerToLegacySession(tracker types.SessionTracker, clusterName string) se participants := tracker.GetParticipants() parties := make([]session.Party, 0, len(participants)) - found := map[string]struct{}{} for _, participant := range participants { - if _, ok := found[participant.User]; ok { - continue - } - - found[participant.User] = struct{}{} parties = append(parties, session.Party{ ID: session.ID(participant.ID), User: participant.User, From 33ec80f76c1472826a42ea725f878c9e729137db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 19:42:46 +0200 Subject: [PATCH 16/18] revert --- lib/web/apiserver_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 5c229111b33cb..9c7bd44822ba1 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -1478,7 +1478,7 @@ func TestActiveSessions(t *testing.T) { sess := sessResp.Sessions[0] require.Equal(t, sid, sess.ID) require.Equal(t, s.node.GetNamespace(), sess.Namespace) - require.Len(t, sess.Parties, 1) + require.NotNil(t, sess.Parties) require.Greater(t, sess.TerminalParams.H, 0) require.Greater(t, sess.TerminalParams.W, 0) require.Equal(t, pack.login, sess.Login) From 232cbb1c104fc58d6b0ad4e5485c6f599da68d6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 19:44:42 +0200 Subject: [PATCH 17/18] revert useless change --- lib/web/terminal.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/web/terminal.go b/lib/web/terminal.go index c72cff268e66c..90319812dff67 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -448,11 +448,10 @@ func promptMFAChallenge( // events back to the web client. func (t *TerminalHandler) streamTerminal(ws *websocket.Conn, tc *client.TeleportClient) { defer t.terminalCancel() - var err error // Establish SSH connection to the server. This function will block until // either an error occurs or it completes successfully. - err = tc.SSH(t.terminalContext, t.params.InteractiveCommand, false) + err := tc.SSH(t.terminalContext, t.params.InteractiveCommand, false) // TODO IN: 5.0 // From 34ad43cc860d350874e22e34aa2d426345ac48ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Wejdenst=C3=A5l?= Date: Wed, 22 Jun 2022 20:41:02 +0200 Subject: [PATCH 18/18] push --- lib/auth/auth_with_roles.go | 2 +- lib/kube/proxy/forwarder.go | 2 +- lib/kube/proxy/sess.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index c174e3d92e81f..b0a6905f0442e 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -273,7 +273,7 @@ func (a *ServerWithRoles) CreateSessionTracker(ctx context.Context, tracker type func (a *ServerWithRoles) filterSessionTracker(ctx context.Context, joinerRoles []types.Role, tracker types.SessionTracker) bool { evaluator := NewSessionAccessEvaluator(tracker.GetHostPolicySets(), tracker.GetSessionKind(), tracker.GetHostUser()) - modes := evaluator.CanJoin(SessionAccessContext{Username: a.context.User.String(), Roles: joinerRoles}) + modes := evaluator.CanJoin(SessionAccessContext{Username: a.context.User.GetName(), Roles: joinerRoles}) if len(modes) == 0 { return false diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index c3cd8ceabcf80..1aa3bb60d8f42 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -1011,7 +1011,7 @@ func (f *Forwarder) execNonInteractive(ctx *authContext, w http.ResponseWriter, policySets = append(policySets, &policySet) } - authorizer := auth.NewSessionAccessEvaluator(policySets, types.KubernetesSessionKind, ctx.User.String()) + authorizer := auth.NewSessionAccessEvaluator(policySets, types.KubernetesSessionKind, ctx.User.GetName()) canStart, _, err := authorizer.FulfilledFor(nil) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/kube/proxy/sess.go b/lib/kube/proxy/sess.go index 5b4ec4632b690..ebb38f7effc4c 100644 --- a/lib/kube/proxy/sess.go +++ b/lib/kube/proxy/sess.go @@ -319,7 +319,7 @@ func newSession(ctx authContext, forwarder *Forwarder, req *http.Request, params } q := req.URL.Query() - accessEvaluator := auth.NewSessionAccessEvaluator(policySets, types.KubernetesSessionKind, ctx.User.String()) + accessEvaluator := auth.NewSessionAccessEvaluator(policySets, types.KubernetesSessionKind, ctx.User.GetName()) io := srv.NewTermManager()