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 committed Jun 27, 2023
1 parent c5282cf commit 587c3c5
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 49 deletions.
98 changes: 68 additions & 30 deletions lib/kube/proxy/forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,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.kubeCluster, 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.kubeCluster,
KubernetesUsers: utils.StringsSliceFromSet(c.kubeUsers),
KubernetesGroups: utils.StringsSliceFromSet(c.kubeGroups),
KubernetesUsers: kubeUsers,
KubernetesGroups: kubeGroups,
KubernetesLabels: c.kubeClusterLabels,
}
}
Expand Down Expand Up @@ -1185,7 +1195,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 @@ -1208,7 +1218,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 @@ -1230,7 +1240,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 @@ -1544,6 +1554,44 @@ func setupImpersonationHeaders(log logrus.FieldLogger, ctx authContext, headers
if ctx.teleportCluster.isRemote {
return nil
}

impersonateUser, impersonateGroups, err := computeImpersonatedPrincipals(ctx.kubeUsers, ctx.kubeGroups, headers)
if err != nil {
return trace.Wrap(err)
}

headers.Set(ImpersonateUserHeader, impersonateUser)

// Make sure to overwrite the exiting headers, instead of appending to
// them.
headers.Del(ImpersonateGroupHeader)
for _, group := range impersonateGroups {
headers.Add(ImpersonateGroupHeader, group)
}

return nil
}

// copyImpersonationHeaders copies the impersonation headers from the source
// request to the destination request.
func copyImpersonationHeaders(dst, src http.Header) {
dst.Del(ImpersonateUserHeader)
dst.Del(ImpersonateGroupHeader)

for _, v := range src.Values(ImpersonateUserHeader) {
dst.Add(ImpersonateUserHeader, v)
}

for _, v := range src.Values(ImpersonateGroupHeader) {
dst.Add(ImpersonateGroupHeader, v)
}
}

// computeImpersonatedPrincipals computes the intersection between the information
// 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(kubeUsers, kubeGroups map[string]struct{}, headers http.Header) (string, []string, error) {
var impersonateUser string
var impersonateGroups []string
for header, values := range headers {
Expand All @@ -1553,10 +1601,10 @@ func setupImpersonationHeaders(log logrus.FieldLogger, ctx authContext, headers
switch header {
case ImpersonateUserHeader:
if impersonateUser != "" {
return trace.AccessDenied("%v, user already specified to %q", ImpersonationRequestDeniedMessage, impersonateUser)
return impersonateUser, impersonateGroups, trace.AccessDenied("%v, user already specified to %q", ImpersonationRequestDeniedMessage, impersonateUser)
}
if len(values) == 0 || len(values) > 1 {
return trace.AccessDenied("%v, invalid user header %q", ImpersonationRequestDeniedMessage, values)
return impersonateUser, impersonateGroups, trace.AccessDenied("%v, invalid user header %q", ImpersonationRequestDeniedMessage, values)
}
// when Kubernetes go-client sends impersonated groups it also sends the impersonated user.
// The issue arrises when the impersonated user was not defined and the user want to just impersonate
Expand All @@ -1568,18 +1616,18 @@ func setupImpersonationHeaders(log logrus.FieldLogger, ctx authContext, headers
}
impersonateUser = values[0]

if _, ok := ctx.kubeUsers[impersonateUser]; !ok {
return trace.AccessDenied("%v, user header %q is not allowed in roles", ImpersonationRequestDeniedMessage, impersonateUser)
if _, ok := kubeUsers[impersonateUser]; !ok {
return impersonateUser, impersonateGroups, trace.AccessDenied("%v, user header %q is not allowed in roles", ImpersonationRequestDeniedMessage, impersonateUser)
}
case ImpersonateGroupHeader:
for _, group := range values {
if _, ok := ctx.kubeGroups[group]; !ok {
return trace.AccessDenied("%v, group header %q value is not allowed in roles", ImpersonationRequestDeniedMessage, group)
if _, ok := kubeGroups[group]; !ok {
return impersonateUser, impersonateGroups, trace.AccessDenied("%v, group header %q value is not allowed in roles", ImpersonationRequestDeniedMessage, group)
}
impersonateGroups = append(impersonateGroups, group)
}
default:
return trace.AccessDenied("%v, unsupported impersonation header %q", ImpersonationRequestDeniedMessage, header)
return impersonateUser, impersonateGroups, trace.AccessDenied("%v, unsupported impersonation header %q", ImpersonationRequestDeniedMessage, header)
}
}

Expand All @@ -1602,41 +1650,31 @@ func setupImpersonationHeaders(log logrus.FieldLogger, ctx authContext, headers
// link the user identity with the IAM role, for example `IAM#{{external.email}}`
//
if impersonateUser == "" {
switch len(ctx.kubeUsers) {
switch len(kubeUsers) {
// this is currently not possible as kube users have at least one
// user (user name), but in case if someone breaks it, catch here
case 0:
debug.PrintStack()
return trace.AccessDenied("assumed at least one user to be present")
return impersonateUser, impersonateGroups, trace.AccessDenied("assumed at least one user to be present")
// if there is deterministic choice, make it to improve user experience
case 1:
for user := range ctx.kubeUsers {
for user := range kubeUsers {
impersonateUser = user
break
}
default:
return trace.AccessDenied(
return impersonateUser, impersonateGroups, trace.AccessDenied(
"please select a user to impersonate, refusing to select a user due to several kubernetes_users set up for this user")
}
}

if len(impersonateGroups) == 0 {
for group := range ctx.kubeGroups {
for group := range kubeGroups {
impersonateGroups = append(impersonateGroups, group)
}
}

if !ctx.teleportCluster.isRemote {
headers.Set(ImpersonateUserHeader, impersonateUser)

// Make sure to overwrite the exiting headers, instead of appending to
// them.
headers.Del(ImpersonateGroupHeader)
for _, group := range impersonateGroups {
headers.Add(ImpersonateGroupHeader, group)
}
}
return nil
return impersonateUser, impersonateGroups, nil
}

// catchAll forwards all HTTP requests to the target k8s API server
Expand Down Expand Up @@ -1691,7 +1729,7 @@ func (f *Forwarder) catchAll(ctx *authContext, w http.ResponseWriter, req *http.
RequestPath: req.URL.Path,
Verb: req.Method,
ResponseCode: int32(rw.Status()),
KubernetesClusterMetadata: ctx.eventClusterMeta(),
KubernetesClusterMetadata: ctx.eventClusterMeta(req),
}
r := parseResourcePath(req.URL.Path)
if r.skipEvent {
Expand Down
89 changes: 89 additions & 0 deletions lib/kube/proxy/forwarder_test.go
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/backend/memory"
Expand Down Expand Up @@ -1737,3 +1738,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{
kubeCluster: "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{
kubeCluster: "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{
kubeCluster: "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)
})
}
}
15 changes: 0 additions & 15 deletions lib/kube/proxy/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,21 +152,6 @@ func (s *SpdyRoundTripper) dial(url *url.URL) (net.Conn, error) {
return conn, nil
}

// copyImpersonationHeaders copies the impersonation headers from the source
// request to the destination request.
func copyImpersonationHeaders(dst, src http.Header) {
dst.Del(ImpersonateUserHeader)
dst.Del(ImpersonateGroupHeader)

for _, v := range src.Values(ImpersonateUserHeader) {
dst.Add(ImpersonateUserHeader, v)
}

for _, v := range src.Values(ImpersonateGroupHeader) {
dst.Add(ImpersonateGroupHeader, v)
}
}

// RoundTrip executes the Request and upgrades it. After a successful upgrade,
// clients may call SpdyRoundTripper.Connection() to retrieve the upgraded
// connection.
Expand Down
8 changes: 4 additions & 4 deletions lib/kube/proxy/sess.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,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 @@ -659,7 +659,7 @@ func (s *session) lockedSetupLaunch(request *remoteCommandRequest, q url.Values,
Impersonator: s.ctx.Identity.GetIdentity().Impersonator,
},
TerminalSize: params.Serialize(),
KubernetesClusterMetadata: s.ctx.eventClusterMeta(),
KubernetesClusterMetadata: s.ctx.eventClusterMeta(s.req),
KubernetesPodMetadata: eventPodMeta,
}

Expand Down Expand Up @@ -773,7 +773,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 @@ -821,7 +821,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 587c3c5

Please sign in to comment.