From bac025b6d1f0c5cc544990e06e2b1e4e88d4c794 Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Fri, 23 Jun 2023 16:25:35 +0100 Subject: [PATCH] Fix audit log report of `kubernetes_users` and `kubernetes_groups` Prior to this commit, if the user specified a filter for a user - `--as` - or group - `--as-group` -, Teleport incorrectly reported the used groups and users for the request. It reported all users and groups the user is allowed to impersonate instead of the ones actually used for the request. This commit fixes that behavior by applying a filter based on the users and groups received from Impersonation headers. Fixes #28166 --- lib/kube/proxy/forwarder.go | 30 +++++++---- lib/kube/proxy/forwarder_test.go | 89 ++++++++++++++++++++++++++++++++ lib/kube/proxy/sess.go | 8 +-- 3 files changed, 113 insertions(+), 14 deletions(-) diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index 9f313d0ffc508..fc686434d9496 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -458,11 +458,21 @@ func (c *authContext) key() string { return fmt.Sprintf("%v:%v:%v:%v:%v:%v:%v", c.teleportCluster.name, c.User.GetName(), c.kubeUsers, c.kubeGroups, c.kubeClusterName, c.certExpires.Unix(), c.Identity.GetIdentity().ActiveRequests) } -func (c *authContext) eventClusterMeta() apievents.KubernetesClusterMetadata { +func (c *authContext) eventClusterMeta(req *http.Request) apievents.KubernetesClusterMetadata { + var kubeUsers, kubeGroups []string + + if impersonateUser, impersonateGroups, err := computeImpersonatedPrincipals(c.kubeUsers, c.kubeGroups, req.Header); err == nil { + kubeUsers = []string{impersonateUser} + kubeGroups = impersonateGroups + } else { + kubeUsers = utils.StringsSliceFromSet(c.kubeUsers) + kubeGroups = utils.StringsSliceFromSet(c.kubeGroups) + } + return apievents.KubernetesClusterMetadata{ KubernetesCluster: c.kubeClusterName, - KubernetesUsers: utils.StringsSliceFromSet(c.kubeUsers), - KubernetesGroups: utils.StringsSliceFromSet(c.kubeGroups), + KubernetesUsers: kubeUsers, + KubernetesGroups: kubeGroups, KubernetesLabels: c.kubeClusterLabels, } } @@ -998,7 +1008,7 @@ func (f *Forwarder) emitAuditEvent(ctx *authContext, req *http.Request, sess *cl RequestPath: req.URL.Path, Verb: req.Method, ResponseCode: int32(status), - KubernetesClusterMetadata: ctx.eventClusterMeta(), + KubernetesClusterMetadata: ctx.eventClusterMeta(req), } r.populateEvent(event) @@ -1583,7 +1593,7 @@ func (f *Forwarder) execNonInteractive(ctx *authContext, w http.ResponseWriter, SessionMetadata: sessionMetadata, UserMetadata: ctx.eventUserMeta(), ConnectionMetadata: connectionMetdata, - KubernetesClusterMetadata: ctx.eventClusterMeta(), + KubernetesClusterMetadata: ctx.eventClusterMeta(req), KubernetesPodMetadata: eventPodMeta, InitialCommand: request.cmd, @@ -1606,7 +1616,7 @@ func (f *Forwarder) execNonInteractive(ctx *authContext, w http.ResponseWriter, CommandMetadata: apievents.CommandMetadata{ Command: strings.Join(request.cmd, " "), }, - KubernetesClusterMetadata: ctx.eventClusterMeta(), + KubernetesClusterMetadata: ctx.eventClusterMeta(req), KubernetesPodMetadata: eventPodMeta, } @@ -1628,7 +1638,7 @@ func (f *Forwarder) execNonInteractive(ctx *authContext, w http.ResponseWriter, Interactive: false, StartTime: sessionStart, EndTime: f.cfg.Clock.Now().UTC(), - KubernetesClusterMetadata: ctx.eventClusterMeta(), + KubernetesClusterMetadata: ctx.eventClusterMeta(req), KubernetesPodMetadata: eventPodMeta, InitialCommand: request.cmd, SessionRecording: ctx.recordingConfig.GetMode(), @@ -1966,7 +1976,7 @@ func setupImpersonationHeaders(log logrus.FieldLogger, ctx authContext, headers return nil } - impersonateUser, impersonateGroups, err := computeImpersonatedPrincipals(log, ctx.kubeUsers, ctx.kubeGroups, headers) + impersonateUser, impersonateGroups, err := computeImpersonatedPrincipals(ctx.kubeUsers, ctx.kubeGroups, headers) if err != nil { return trace.Wrap(err) } @@ -2002,7 +2012,7 @@ func copyImpersonationHeaders(dst, src http.Header) { // received in the `Impersonate-User` and `Impersonate-Groups` headers and the // allowed values. If the user didn't specify any user and groups to impersonate, // Teleport will use every group the user is allowed to impersonate. -func computeImpersonatedPrincipals(log logrus.FieldLogger, kubeUsers, kubeGroups map[string]struct{}, headers http.Header) (string, []string, error) { +func computeImpersonatedPrincipals(kubeUsers, kubeGroups map[string]struct{}, headers http.Header) (string, []string, error) { var impersonateUser string var impersonateGroups []string for header, values := range headers { @@ -2895,7 +2905,7 @@ func (f *Forwarder) handleDeleteCollectionReq(req *http.Request, authCtx *authCo allowedKubeUsers, allowedKubeGroups = fillDefaultKubePrincipalDetails(allowedKubeUsers, allowedKubeGroups, authCtx.User.GetName()) impersonatedUsers, impersonatedGroups, err := computeImpersonatedPrincipals( - f.log, utils.StringsSet(allowedKubeUsers), utils.StringsSet(allowedKubeGroups), + utils.StringsSet(allowedKubeUsers), utils.StringsSet(allowedKubeGroups), req.Header, ) if err != nil { diff --git a/lib/kube/proxy/forwarder_test.go b/lib/kube/proxy/forwarder_test.go index 24e82991236c3..86ee775221c22 100644 --- a/lib/kube/proxy/forwarder_test.go +++ b/lib/kube/proxy/forwarder_test.go @@ -50,6 +50,7 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" + apievents "github.com/gravitational/teleport/api/types/events" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/testauthority" "github.com/gravitational/teleport/lib/authz" @@ -1908,3 +1909,91 @@ func TestKubernetesLicenseEnforcement(t *testing.T) { }) } } + +func Test_authContext_eventClusterMeta(t *testing.T) { + t.Parallel() + kubeClusterLabels := map[string]string{ + "label": "value", + } + type args struct { + req *http.Request + ctx *authContext + } + tests := []struct { + name string + args args + want apievents.KubernetesClusterMetadata + }{ + { + name: "no headers in request", + args: args{ + req: &http.Request{ + Header: http.Header{}, + }, + ctx: &authContext{ + kubeClusterName: "clusterName", + kubeClusterLabels: kubeClusterLabels, + kubeGroups: map[string]struct{}{"kube-group-a": {}, "kube-group-b": {}}, + kubeUsers: map[string]struct{}{"kube-user-a": {}}, + }, + }, + want: apievents.KubernetesClusterMetadata{ + KubernetesCluster: "clusterName", + KubernetesLabels: kubeClusterLabels, + KubernetesGroups: []string{"kube-group-a", "kube-group-b"}, + KubernetesUsers: []string{"kube-user-a"}, + }, + }, + { + name: "with filter headers in request", + args: args{ + req: &http.Request{ + Header: http.Header{ + ImpersonateUserHeader: []string{"kube-user-a"}, + ImpersonateGroupHeader: []string{"kube-group-b", "kube-group-c"}, + }, + }, + ctx: &authContext{ + kubeClusterName: "clusterName", + kubeClusterLabels: kubeClusterLabels, + kubeGroups: map[string]struct{}{"kube-group-a": {}, "kube-group-b": {}, "kube-group-c": {}}, + kubeUsers: map[string]struct{}{"kube-user-a": {}, "kube-user-b": {}}, + }, + }, + want: apievents.KubernetesClusterMetadata{ + KubernetesCluster: "clusterName", + KubernetesLabels: kubeClusterLabels, + KubernetesGroups: []string{"kube-group-b", "kube-group-c"}, + KubernetesUsers: []string{"kube-user-a"}, + }, + }, + { + name: "without filter headers in request and multi groups", + args: args{ + req: &http.Request{ + Header: http.Header{}, + }, + ctx: &authContext{ + kubeClusterName: "clusterName", + kubeClusterLabels: kubeClusterLabels, + kubeGroups: map[string]struct{}{"kube-group-a": {}, "kube-group-b": {}, "kube-group-c": {}}, + kubeUsers: map[string]struct{}{"kube-user-a": {}, "kube-user-b": {}}, + }, + }, + want: apievents.KubernetesClusterMetadata{ + KubernetesCluster: "clusterName", + KubernetesLabels: kubeClusterLabels, + KubernetesGroups: []string{"kube-group-a", "kube-group-b", "kube-group-c"}, + KubernetesUsers: []string{"kube-user-a", "kube-user-b"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.args.ctx.eventClusterMeta(tt.args.req) + sort.Strings(got.KubernetesGroups) + sort.Strings(got.KubernetesGroups) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/lib/kube/proxy/sess.go b/lib/kube/proxy/sess.go index d9bb5272db28c..a668244ef9593 100644 --- a/lib/kube/proxy/sess.go +++ b/lib/kube/proxy/sess.go @@ -559,7 +559,7 @@ func (s *session) launch() error { Protocol: events.EventProtocolKube, }, TerminalSize: termParams.Serialize(), - KubernetesClusterMetadata: s.ctx.eventClusterMeta(), + KubernetesClusterMetadata: s.ctx.eventClusterMeta(s.req), KubernetesPodMetadata: eventPodMeta, InitialCommand: q["command"], SessionRecording: s.ctx.recordingConfig.GetMode(), @@ -658,7 +658,7 @@ func (s *session) lockedSetupLaunch(request *remoteCommandRequest, q url.Values, }, UserMetadata: s.ctx.eventUserMeta(), TerminalSize: params.Serialize(), - KubernetesClusterMetadata: s.ctx.eventClusterMeta(), + KubernetesClusterMetadata: s.ctx.eventClusterMeta(s.req), KubernetesPodMetadata: eventPodMeta, } @@ -772,7 +772,7 @@ func (s *session) lockedSetupLaunch(request *remoteCommandRequest, q url.Values, CommandMetadata: apievents.CommandMetadata{ Command: strings.Join(request.cmd, " "), }, - KubernetesClusterMetadata: s.ctx.eventClusterMeta(), + KubernetesClusterMetadata: s.ctx.eventClusterMeta(s.req), KubernetesPodMetadata: eventPodMeta, } @@ -820,7 +820,7 @@ func (s *session) lockedSetupLaunch(request *remoteCommandRequest, q url.Values, Participants: s.allParticipants(), StartTime: sessionStart, EndTime: s.forwarder.cfg.Clock.Now().UTC(), - KubernetesClusterMetadata: s.ctx.eventClusterMeta(), + KubernetesClusterMetadata: s.ctx.eventClusterMeta(s.req), KubernetesPodMetadata: eventPodMeta, InitialCommand: request.cmd, SessionRecording: s.ctx.recordingConfig.GetMode(),