Skip to content

Commit

Permalink
Fix audit log report of kubernetes_users and kubernetes_groups (#…
Browse files Browse the repository at this point in the history 
…28204)

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
  • Loading branch information
@tigrato
tigrato committed Jun 27, 2023
1 parent 4d58d34 commit 83aaf96
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 14 deletions.
30 changes: 20 additions & 10 deletions lib/kube/proxy/forwarder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,11 +402,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,
}
}
Expand Down Expand Up @@ -835,7 +845,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)
Expand Down Expand Up @@ -1411,7 +1421,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,
Expand All @@ -1434,7 +1444,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,
}

Expand All @@ -1456,7 +1466,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(),
Expand Down Expand Up @@ -1771,7 +1781,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)
}
Expand Down Expand Up @@ -1807,7 +1817,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 {
Expand Down Expand Up @@ -2686,7 +2696,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 {
Expand Down
89 changes: 89 additions & 0 deletions lib/kube/proxy/forwarder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (

"github.com/gravitational/teleport"
"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"
Expand Down Expand Up @@ -1791,3 +1792,91 @@ func Test_copyImpersonationHeaders(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)
})
}
}
8 changes: 4 additions & 4 deletions lib/kube/proxy/sess.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,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(),
Expand Down Expand Up @@ -651,7 +651,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,
}

Expand Down Expand Up @@ -765,7 +765,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,
}

Expand Down Expand Up @@ -813,7 +813,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(),
Expand Down

0 comments on commit 83aaf96

Please sign in to comment.