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) (#28339)

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 c5282cf commit 618fa6b
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 56 deletions.
105 changes: 68 additions & 37 deletions lib/kube/proxy/forwarder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"net"
"net/http"
"path/filepath"
"runtime/debug"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -367,11 +366,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 +1194,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 +1217,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 +1239,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 +1553,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 +1600,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 "", nil, 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 "", nil, 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 @@ -1567,24 +1614,21 @@ func setupImpersonationHeaders(log logrus.FieldLogger, ctx authContext, headers
continue
}
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 "", nil, 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 "", nil, 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 "", nil, trace.AccessDenied("%v, unsupported impersonation header %q", ImpersonationRequestDeniedMessage, header)
}
}

impersonateGroups = apiutils.Deduplicate(impersonateGroups)

// By default, if no kubernetes_users is set (which will be a majority),
// user will impersonate themselves, which is the backwards-compatible behavior.
//
Expand All @@ -1602,41 +1646,28 @@ 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 "", nil, 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 "", nil, 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 +1722,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
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/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
View file
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
View file
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 618fa6b

Please sign in to comment.