Prevent Kube proxy from set the default Kube impersonation headers (#32839)

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 <tiago.silva@goteleport.com>
This commit is contained in:
Tiago Silva
2023-10-02 14:26:09 +00:00
committed by GitHub
parent 030b0ea1e3
commit 955331b73e
5 changed files with 69 additions and 33 deletions
+13 -11
View File
@@ -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,
+51 -17
View File
@@ -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)
if err == nil {
// Sort header values to get predictable ordering.
for _, vals := range tt.inHeaders {
sort.Strings(vals)
tt := tt
t.Run(tt.desc, func(t *testing.T) {
var kubeCreds kubeCreds
if !tt.isProxy {
kubeCreds = &staticKubeCreds{}
}
require.Empty(t, cmp.Diff(tt.inHeaders, tt.wantHeaders))
}
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)
}
require.Empty(t, cmp.Diff(tt.inHeaders, tt.wantHeaders))
}
},
)
}
}
+1 -1
View File
@@ -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)
}
+3 -3
View File
@@ -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)
}
+1 -1
View File
@@ -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)