From be837df9c4f14d6743c2695b70199cba0b508e85 Mon Sep 17 00:00:00 2001 From: Joel Date: Thu, 31 Mar 2022 16:47:55 +0200 Subject: [PATCH 1/5] Write error and return on failed websocket upgrade (#11606) --- lib/web/terminal.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/web/terminal.go b/lib/web/terminal.go index 2b33a7add49cb..2e35cf3981fba 100644 --- a/lib/web/terminal.go +++ b/lib/web/terminal.go @@ -200,7 +200,10 @@ func (t *TerminalHandler) Serve(w http.ResponseWriter, r *http.Request) { ws, err := upgrader.Upgrade(w, r, nil) if err != nil { - t.log.Errorf("Error upgrading to websocket: %v", err) + errMsg := "Error upgrading to websocket" + t.log.Errorf("%v: %v", errMsg, err) + http.Error(w, errMsg, http.StatusInternalServerError) + return } t.handler(ws, r) From f917754e6f827f1e411443f99f459c8412bcc5fa Mon Sep 17 00:00:00 2001 From: Joel Date: Sun, 3 Apr 2022 14:48:41 +0200 Subject: [PATCH 2/5] Broadcast controls keys if session is moderated (#11661) --- lib/kube/proxy/sess.go | 10 +++------- lib/srv/sess.go | 5 +++++ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/kube/proxy/sess.go b/lib/kube/proxy/sess.go index a60753e934420..3ebe173ac8617 100644 --- a/lib/kube/proxy/sess.go +++ b/lib/kube/proxy/sess.go @@ -329,13 +329,6 @@ func newSession(ctx authContext, forwarder *Forwarder, req *http.Request, params io := srv.NewTermManager() - if tty { - err = io.BroadcastMessage(fmt.Sprintf("Creating session with ID: %v...", id.String())) - if err != nil { - return nil, trace.Wrap(err) - } - } - s := &session{ ctx: ctx, forwarder: forwarder, @@ -361,6 +354,9 @@ func newSession(ctx authContext, forwarder *Forwarder, req *http.Request, params displayParticipantRequirements: utils.AsBool(q.Get("displayParticipantRequirements")), } + s.BroadcastMessage("Creating session with ID: %v...", id.String()) + s.BroadcastMessage(srv.SessionControlsInfoBroadcast) + go func() { if _, open := <-s.io.TerminateNotifier(); open { err := s.Close() diff --git a/lib/srv/sess.go b/lib/srv/sess.go index 28860ac7e2f55..289b3ce97ba52 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -51,6 +51,10 @@ import ( const PresenceVerifyInterval = time.Second * 15 const PresenceMaxDifference = time.Minute +// SessionControlsInfoBroadcast is sent in tandem with session creation +// to inform any joining users about the session controls. +const SessionControlsInfoBroadcast = "Controls\r\n - CTRL-C: Leave the session\r\n - t: Forcefully terminate the session (moderators only)" + var serverSessions = prometheus.NewGauge( prometheus.GaugeOpts{ Name: teleport.MetricServerInteractiveSessions, @@ -1005,6 +1009,7 @@ func (s *session) startInteractive(ch ssh.Channel, ctx *ServerContext) error { s.io.AddReader("reader", inReader) s.io.AddWriter("session-recorder", utils.WriteCloserWithContext(ctx.srv.Context(), s.recorder)) s.BroadcastMessage("Creating session with ID: %v...", s.id) + s.BroadcastMessage(SessionControlsInfoBroadcast) if err := s.term.Run(); err != nil { ctx.Errorf("Unable to run shell command: %v.", err) From 7267262be381447e5a040b1ab4532dc05be9f2b3 Mon Sep 17 00:00:00 2001 From: Joel Date: Thu, 7 Apr 2022 16:05:28 +0200 Subject: [PATCH 3/5] Clarify RBAC rule application (#11672) --- docs/pages/access-controls/guides/moderated-sessions.mdx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/pages/access-controls/guides/moderated-sessions.mdx b/docs/pages/access-controls/guides/moderated-sessions.mdx index fee04919ef893..45472acda82e8 100644 --- a/docs/pages/access-controls/guides/moderated-sessions.mdx +++ b/docs/pages/access-controls/guides/moderated-sessions.mdx @@ -67,6 +67,10 @@ spec: count: 2 ``` +#### Combining Policies + +The authorizer applies require policies within a role together with an OR operator and the policies from each role with an AND operator. In practice, this means that for every role with at least one require policy, one of its policies must be met before a session can be started. + ### `join_sessions` #### Options From c68d004330cb7e6acb7b73c74a275216194200fb Mon Sep 17 00:00:00 2001 From: Joel Date: Mon, 4 Apr 2022 16:06:35 +0200 Subject: [PATCH 4/5] Use a buffered channel for the terminate notifier (#11687) --- lib/srv/termmanager.go | 2 +- lib/srv/termmanager_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/srv/termmanager.go b/lib/srv/termmanager.go index 33dea82fc944f..bba19b6013094 100644 --- a/lib/srv/termmanager.go +++ b/lib/srv/termmanager.go @@ -68,7 +68,7 @@ func NewTermManager() *TermManager { closed: false, readStateUpdate: sync.NewCond(&sync.Mutex{}), incoming: make(chan []byte, 100), - terminateNotifier: make(chan struct{}), + terminateNotifier: make(chan struct{}, 1), } } diff --git a/lib/srv/termmanager_test.go b/lib/srv/termmanager_test.go index d1cdb1c2e2b10..b1bc322135bf8 100644 --- a/lib/srv/termmanager_test.go +++ b/lib/srv/termmanager_test.go @@ -41,7 +41,7 @@ func TestCTRLCCapture(t *testing.T) { m := NewTermManager() r, w := io.Pipe() m.AddReader("foo", r) - go w.Write([]byte("\x03")) + w.Write([]byte("\x03")) select { case <-m.TerminateNotifier(): From 20017535c614c43ecd45f7c3bdad4ce5c333b004 Mon Sep 17 00:00:00 2001 From: Joel Date: Wed, 6 Apr 2022 17:31:24 +0200 Subject: [PATCH 5/5] Restrict moderated sessions users from accessing V8 kube cluster agents (#11691) --- integration/integration_test.go | 114 ++++++++++++++++++++++++++++++++ lib/auth/auth_with_roles.go | 24 +++++++ lib/auth/grpcserver.go | 3 +- lib/auth/session_access.go | 7 +- 4 files changed, 144 insertions(+), 4 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index 1fe08ec18544c..2ce43bebf68a8 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -206,6 +206,7 @@ func TestIntegrations(t *testing.T) { t.Run("UUIDBasedProxy", suite.bind(testUUIDBasedProxy)) t.Run("WindowChange", suite.bind(testWindowChange)) t.Run("SSHTracker", suite.bind(testSSHTracker)) + t.Run("TestKubeAgentFiltering", suite.bind(testKubeAgentFiltering)) } // testAuditOn creates a live session, records a bunch of data through it @@ -5950,3 +5951,116 @@ outer: t.FailNow() } + +// TestKubeAgentFiltering tests that kube-agent filtering for pre-v8 agents and +// moderated sessions users works as expected. +func testKubeAgentFiltering(t *testing.T, suite *integrationTestSuite) { + ctx := context.Background() + + type testCase struct { + name string + server types.Server + role types.Role + user types.User + wantsLen int + } + + v8Agent, err := types.NewServer("kube-h", types.KindKubeService, types.ServerSpecV2{ + Version: "8.0.0", + KubernetesClusters: []*types.KubernetesCluster{{Name: "foo"}}, + }) + require.NoError(t, err) + + v9Agent, err := types.NewServer("kube-h", types.KindKubeService, types.ServerSpecV2{ + Version: "9.0.0", + KubernetesClusters: []*types.KubernetesCluster{{Name: "foo"}}, + }) + require.NoError(t, err) + + plainRole, err := types.NewRole("plain", types.RoleSpecV5{}) + require.NoError(t, err) + + moderatedRole, err := types.NewRole("moderated", types.RoleSpecV5{ + Allow: types.RoleConditions{ + RequireSessionJoin: []*types.SessionRequirePolicy{ + { + Name: "bar", + Kinds: []string{string(types.KubernetesSessionKind)}, + }, + }, + }, + }) + require.NoError(t, err) + + plainUser, err := types.NewUser("bob") + require.NoError(t, err) + plainUser.SetRoles([]string{plainRole.GetName()}) + + moderatedUser, err := types.NewUser("alice") + require.NoError(t, err) + moderatedUser.SetRoles([]string{moderatedRole.GetName()}) + + testCases := []testCase{ + { + name: "unrestricted user, v8 agent", + server: v8Agent, + role: plainRole, + user: plainUser, + wantsLen: 1, + }, + { + name: "restricted user, v8 agent", + server: v8Agent, + role: moderatedRole, + user: moderatedUser, + wantsLen: 0, + }, + { + name: "unrestricted user, v9 agent", + server: v9Agent, + role: plainRole, + user: plainUser, + wantsLen: 1, + }, + { + name: "restricted user, v9 agent", + server: v9Agent, + role: moderatedRole, + user: moderatedUser, + wantsLen: 1, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + teleport := suite.newTeleport(t, nil, true) + defer teleport.StopAll() + + adminSite := teleport.Process.GetAuthServer() + _, err := adminSite.UpsertKubeServiceV2(ctx, testCase.server) + require.NoError(t, err) + err = adminSite.UpsertRole(ctx, testCase.role) + require.NoError(t, err) + err = adminSite.CreateUser(ctx, testCase.user) + require.NoError(t, err) + + cl, err := teleport.NewClient(ClientConfig{ + Login: testCase.user.GetName(), + Cluster: Site, + Host: Host, + Port: teleport.GetPortSSHInt(), + }) + require.NoError(t, err) + + proxy, err := cl.ConnectToProxy(ctx) + require.NoError(t, err) + + userSite, err := proxy.ConnectToCluster(ctx, Site, false) + require.NoError(t, err) + + services, err := userSite.GetKubeServices(ctx) + require.NoError(t, err) + require.Len(t, services, testCase.wantsLen) + }) + } +} diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index c3bdb11ab38a8..993a641be0f42 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -21,6 +21,7 @@ import ( "net/url" "time" + "github.com/coreos/go-semver/semver" "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/proto" @@ -3581,6 +3582,29 @@ func (a *ServerWithRoles) filterKubeServices(server types.Server) error { // connect to them. mfaParams := services.AccessMFAParams{Verified: true} + // Filter out agents that don't have support for moderated sessions access + // checking if the user has any roles that require it. + if hasLocalUserRole(a.context.Checker) { + roles := a.context.Checker.(LocalUserRoleSet) + agentVersion, versionErr := semver.NewVersion(server.GetTeleportVersion()) + + hasK8SRequirePolicy := func() bool { + for _, role := range roles.RoleSet { + for _, policy := range role.GetSessionRequirePolicies() { + if ContainsSessionKind(policy.Kinds, types.KubernetesSessionKind) { + return true + } + } + } + + return false + } + + if hasK8SRequirePolicy() && (versionErr != nil || agentVersion.LessThan(*MinSupportedModeratedSessionsVersion)) { + return trace.AccessDenied("cannot use moderated sessions with pre-v9 kubernetes agents") + } + } + filtered := make([]*types.KubernetesCluster, 0, len(server.GetKubernetesClusters())) for _, kube := range server.GetKubernetesClusters() { k8sV3, err := types.NewKubernetesClusterV3FromLegacyCluster(server.GetNamespace(), kube) diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index 7945b98980f93..3e88c69c6b4cb 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -1708,8 +1708,7 @@ func downgradeRole(ctx context.Context, role *types.RoleV5) (*types.RoleV5, erro } } - minSupportedVersionForV5Roles := semver.New(utils.VersionBeforeAlpha("9.0.0")) - if clientVersion == nil || clientVersion.LessThan(*minSupportedVersionForV5Roles) { + if clientVersion == nil || clientVersion.LessThan(*MinSupportedModeratedSessionsVersion) { log.Debugf(`Client version "%s" is unknown or less than 9.0.0, converting role to v4`, clientVersionString) downgraded, err := services.DowngradeRoleToV4(role) if err != nil { diff --git a/lib/auth/session_access.go b/lib/auth/session_access.go index 82df61ac6bd20..7e854e57680df 100644 --- a/lib/auth/session_access.go +++ b/lib/auth/session_access.go @@ -21,6 +21,7 @@ import ( "regexp" "strings" + "github.com/coreos/go-semver/semver" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/utils" @@ -28,6 +29,8 @@ import ( "github.com/vulcand/predicate" ) +var MinSupportedModeratedSessionsVersion = semver.New(utils.VersionBeforeAlpha("9.0.0")) + // SessionAccessEvaluator takes a set of policies // and uses rules to evaluate them to determine when a session may start // and if a user can join a session. @@ -70,7 +73,7 @@ func getAllowPolicies(participant SessionAccessContext) []*types.SessionJoinPoli return policies } -func containsKind(s []string, e types.SessionKind) bool { +func ContainsSessionKind(s []string, e types.SessionKind) bool { for _, a := range s { if types.SessionKind(a) == e { return true @@ -162,7 +165,7 @@ func (e *SessionAccessEvaluator) matchesJoin(allow *types.SessionJoinPolicy) boo } func (e *SessionAccessEvaluator) matchesKind(allow []string) bool { - if containsKind(allow, e.kind) || containsKind(allow, "*") { + if ContainsSessionKind(allow, e.kind) || ContainsSessionKind(allow, "*") { return true }