From e48b99207c318dfb1b82c0f355ed564bf5227be6 Mon Sep 17 00:00:00 2001 From: Tiago Silva Date: Mon, 2 Oct 2023 11:39:20 +0100 Subject: [PATCH] Prevent Kube proxy from set the default Kube impersonation headers This PR removes the ability of the Kubernetes Proxy to set the default values of Kubernetes Impersonation headers. This task is delegated to the kubernetes_service or kubernetes legacy proxy that handles the request. This is required because under certain contiditons it might have requests that failed because the values were improperly defined. Signed-off-by: Tiago Silva --- lib/kube/proxy/forwarder.go | 24 ++++---- lib/kube/proxy/forwarder_test.go | 66 ++++++++++++++++----- lib/kube/proxy/resource_deletecollection.go | 2 +- lib/kube/proxy/roundtrip.go | 6 +- lib/kube/proxy/sess.go | 2 +- 5 files changed, 68 insertions(+), 32 deletions(-) diff --git a/lib/kube/proxy/forwarder.go b/lib/kube/proxy/forwarder.go index 3665505fffaef..bbba5c91d8f54 100644 --- a/lib/kube/proxy/forwarder.go +++ b/lib/kube/proxy/forwarder.go @@ -1539,7 +1539,7 @@ func (f *Forwarder) execNonInteractive(ctx *authContext, w http.ResponseWriter, } }() - executor, err := f.getExecutor(*ctx, sess, req) + executor, err := f.getExecutor(sess, req) if err != nil { execEvent.Code = events.ExecFailureCode execEvent.Error, execEvent.ExitCode = exitCode(err) @@ -1712,7 +1712,7 @@ func (f *Forwarder) exec(authCtx *authContext, w http.ResponseWriter, req *http. func (f *Forwarder) remoteExec(ctx *authContext, w http.ResponseWriter, req *http.Request, p httprouter.Params, sess *clusterSession, request remoteCommandRequest, proxy *remoteCommandProxy) (resp any, err error) { defer proxy.Close() - executor, err := f.getExecutor(*ctx, sess, req) + executor, err := f.getExecutor(sess, req) if err != nil { f.log.WithError(err).Warning("Failed creating executor.") return nil, trace.Wrap(err) @@ -1777,7 +1777,7 @@ func (f *Forwarder) portForward(authCtx *authContext, w http.ResponseWriter, req return nil, trace.Wrap(err) } - dialer, err := f.getSPDYDialer(*authCtx, sess, req) + dialer, err := f.getSPDYDialer(sess, req) if err != nil { return nil, trace.Wrap(err) } @@ -1856,7 +1856,7 @@ const ( func (f *Forwarder) setupForwardingHeaders(sess *clusterSession, req *http.Request, withImpersonationHeaders bool) error { if withImpersonationHeaders { - if err := setupImpersonationHeaders(f.log, sess.authContext, req.Header); err != nil { + if err := setupImpersonationHeaders(f.log, sess, req.Header); err != nil { return trace.Wrap(err) } } @@ -1882,12 +1882,14 @@ func (f *Forwarder) setupForwardingHeaders(sess *clusterSession, req *http.Reque } // setupImpersonationHeaders sets up Impersonate-User and Impersonate-Group headers -func setupImpersonationHeaders(log logrus.FieldLogger, ctx authContext, headers http.Header) error { - if ctx.teleportCluster.isRemote { +func setupImpersonationHeaders(log logrus.FieldLogger, sess *clusterSession, headers http.Header) error { + // If the request is remote or this instance is a proxy, + // do not set up impersonation headers. + if sess.teleportCluster.isRemote || sess.kubeAPICreds == nil { return nil } - impersonateUser, impersonateGroups, err := computeImpersonatedPrincipals(ctx.kubeUsers, ctx.kubeGroups, headers) + impersonateUser, impersonateGroups, err := computeImpersonatedPrincipals(sess.kubeUsers, sess.kubeGroups, headers) if err != nil { return trace.Wrap(err) } @@ -2075,7 +2077,7 @@ func (f *Forwarder) catchAll(authCtx *authContext, w http.ResponseWriter, req *h } } -func (f *Forwarder) getExecutor(ctx authContext, sess *clusterSession, req *http.Request) (remotecommand.Executor, error) { +func (f *Forwarder) getExecutor(sess *clusterSession, req *http.Request) (remotecommand.Executor, error) { tlsConfig, useImpersonation, err := f.getTLSConfig(sess) if err != nil { return nil, trace.Wrap(err) @@ -2083,7 +2085,7 @@ func (f *Forwarder) getExecutor(ctx authContext, sess *clusterSession, req *http upgradeRoundTripper := NewSpdyRoundTripperWithDialer(roundTripperConfig{ ctx: req.Context(), - authCtx: ctx, + sess: sess, dialWithContext: sess.DialWithContext(), tlsConfig: tlsConfig, pingPeriod: f.cfg.ConnPingPeriod, @@ -2108,7 +2110,7 @@ func (f *Forwarder) getExecutor(ctx authContext, sess *clusterSession, req *http // to SPDY protocol. // SPDY is a deprecated protocol, but it is still used by kubectl to manage data streams. // The dialer uses an HTTP1.1 connection to upgrade to SPDY. -func (f *Forwarder) getSPDYDialer(ctx authContext, sess *clusterSession, req *http.Request) (httpstream.Dialer, error) { +func (f *Forwarder) getSPDYDialer(sess *clusterSession, req *http.Request) (httpstream.Dialer, error) { tlsConfig, useImpersonation, err := f.getTLSConfig(sess) if err != nil { return nil, trace.Wrap(err) @@ -2116,7 +2118,7 @@ func (f *Forwarder) getSPDYDialer(ctx authContext, sess *clusterSession, req *ht upgradeRoundTripper := NewSpdyRoundTripperWithDialer(roundTripperConfig{ ctx: req.Context(), - authCtx: ctx, + sess: sess, dialWithContext: sess.DialWithContext(), tlsConfig: tlsConfig, pingPeriod: f.cfg.ConnPingPeriod, diff --git a/lib/kube/proxy/forwarder_test.go b/lib/kube/proxy/forwarder_test.go index 7c2a94c53f107..7b30fc81431b8 100644 --- a/lib/kube/proxy/forwarder_test.go +++ b/lib/kube/proxy/forwarder_test.go @@ -815,6 +815,7 @@ func TestSetupImpersonationHeaders(t *testing.T) { kubeUsers []string kubeGroups []string remoteCluster bool + isProxy bool inHeaders http.Header wantHeaders http.Header errAssertion require.ErrorAssertionFunc @@ -935,26 +936,59 @@ func TestSetupImpersonationHeaders(t *testing.T) { }, errAssertion: require.NoError, }, + { + desc: "no existing impersonation headers, proxy service", + kubeUsers: []string{"kube-user-a"}, + kubeGroups: []string{"kube-group-a", "kube-group-b"}, + isProxy: true, + inHeaders: http.Header{}, + wantHeaders: http.Header{}, + errAssertion: require.NoError, + }, + { + desc: "existing impersonation headers, proxy service", + kubeUsers: []string{"kube-user-a"}, + kubeGroups: []string{"kube-group-a", "kube-group-b"}, + isProxy: true, + inHeaders: http.Header{ + ImpersonateGroupHeader: []string{"kube-group-a"}, + }, + wantHeaders: http.Header{ + ImpersonateGroupHeader: []string{"kube-group-a"}, + }, + errAssertion: require.NoError, + }, } for _, tt := range tests { - err := setupImpersonationHeaders( - logrus.NewEntry(logrus.New()), - authContext{ - kubeUsers: utils.StringsSet(tt.kubeUsers), - kubeGroups: utils.StringsSet(tt.kubeGroups), - teleportCluster: teleportClusterClient{isRemote: tt.remoteCluster}, - }, - tt.inHeaders, - ) - tt.errAssertion(t, err) + tt := tt + t.Run(tt.desc, func(t *testing.T) { + var kubeCreds kubeCreds + if !tt.isProxy { + kubeCreds = &staticKubeCreds{} + } + err := setupImpersonationHeaders( + logrus.NewEntry(logrus.New()), + &clusterSession{ + kubeAPICreds: kubeCreds, + authContext: authContext{ + kubeUsers: utils.StringsSet(tt.kubeUsers), + kubeGroups: utils.StringsSet(tt.kubeGroups), + teleportCluster: teleportClusterClient{isRemote: tt.remoteCluster}, + }, + }, + tt.inHeaders, + ) + tt.errAssertion(t, err) - if err == nil { - // Sort header values to get predictable ordering. - for _, vals := range tt.inHeaders { - sort.Strings(vals) + if err == nil { + // Sort header values to get predictable ordering. + for _, vals := range tt.inHeaders { + sort.Strings(vals) + } + require.Empty(t, cmp.Diff(tt.inHeaders, tt.wantHeaders)) } - require.Empty(t, cmp.Diff(tt.inHeaders, tt.wantHeaders)) - } + }, + ) } } diff --git a/lib/kube/proxy/resource_deletecollection.go b/lib/kube/proxy/resource_deletecollection.go index e3ef02d7bb9dc..567ef44eec43d 100644 --- a/lib/kube/proxy/resource_deletecollection.go +++ b/lib/kube/proxy/resource_deletecollection.go @@ -497,7 +497,7 @@ func (f *Forwarder) handleDeleteCustomResourceCollection(w http.ResponseWriter, kubeUsers, kubeGroups := fillDefaultKubePrincipalDetails(allowedKubeGroups, allowedKubeUsers, sess.User.GetName()) sess.kubeUsers = utils.StringsSet(kubeUsers) sess.kubeGroups = utils.StringsSet(kubeGroups) - if err := setupImpersonationHeaders(f.log, sess.authContext, req.Header); err != nil { + if err := setupImpersonationHeaders(f.log, sess, req.Header); err != nil { return 0, trace.Wrap(err) } diff --git a/lib/kube/proxy/roundtrip.go b/lib/kube/proxy/roundtrip.go index b7b460a7fbb14..f5507170ffecc 100644 --- a/lib/kube/proxy/roundtrip.go +++ b/lib/kube/proxy/roundtrip.go @@ -69,8 +69,8 @@ var ( type roundTripperConfig struct { // ctx is a context for this round tripper ctx context.Context - // authCtx is the auth context to use for this round tripper - authCtx authContext + // sess is the cluster session + sess *clusterSession // dialWithContext is the function used connect to remote address dialWithContext dialContextFunc // tlsConfig holds the TLS configuration settings to use when connecting @@ -207,7 +207,7 @@ func (s *SpdyRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) copyImpersonationHeaders(header, s.originalHeaders) header.Set(httpstream.HeaderConnection, httpstream.HeaderUpgrade) header.Set(httpstream.HeaderUpgrade, streamspdy.HeaderSpdy31) - if err := setupImpersonationHeaders(log.StandardLogger(), s.authCtx, header); err != nil { + if err := setupImpersonationHeaders(log.StandardLogger(), s.sess, header); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/kube/proxy/sess.go b/lib/kube/proxy/sess.go index 490878a12838b..fc72ece14e2b9 100644 --- a/lib/kube/proxy/sess.go +++ b/lib/kube/proxy/sess.go @@ -597,7 +597,7 @@ func (s *session) launch() error { var executor remotecommand.Executor - executor, err = s.forwarder.getExecutor(s.ctx, s.sess, s.req) + executor, err = s.forwarder.getExecutor(s.sess, s.req) if err != nil { s.log.WithError(err).Warning("Failed creating executor.") return trace.Wrap(err)