Skip to content

Commit

Permalink
Fix Kube impersonation header overwrite when dealing with remote clus…
Browse files Browse the repository at this point in the history
…ters (#22240) (#22247)

* Fix Kube impersonation header overwrite when dealing with remote clusters

When forwarding exec requests to remote clusters I introduced an empty
header that failed the validation in `setupImpersonationHeaders`. This
results in the request being denied.

This problem only affects Teleport 11 and 10, but I am fixing the behaviour
on master as well.

Fixes #22226

* add tests
  • Loading branch information
tigrato committed Feb 24, 2023
1 parent f5baf44 commit c034a7a
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 3 deletions.
2 changes: 1 addition & 1 deletion lib/kube/proxy/forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1549,7 +1549,7 @@ func setupImpersonationHeaders(log log.FieldLogger, ctx authContext, headers htt

// Make sure to overwrite the exiting headers, instead of appending to
// them.
headers[ImpersonateGroupHeader] = nil
headers.Del(ImpersonateGroupHeader)
for _, group := range impersonateGroups {
headers.Add(ImpersonateGroupHeader, group)
}
Expand Down
48 changes: 48 additions & 0 deletions lib/kube/proxy/forwarder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,19 @@ func (s ForwarderSuite) TestSetupImpersonationHeaders(c *check.C) {
},
wantErr: true,
},
{
desc: "empty impersonated group header ignored",
kubeUsers: []string{"kube-user-a"},
kubeGroups: []string{},
inHeaders: http.Header{
"Host": []string{"example.com"},
},
wantHeaders: http.Header{
"Host": []string{"example.com"},
ImpersonateUserHeader: []string{"kube-user-a"},
},
wantErr: false,
},
}
for _, tt := range tests {
c.Log(tt.desc)
Expand Down Expand Up @@ -1483,3 +1496,38 @@ func TestForwarder_clientCreds_cache(t *testing.T) {
})
}
}

func Test_copyImpersonationHeaders(t *testing.T) {
tests := []struct {
name string
inHeaders http.Header
wantHeaders http.Header
}{
{
name: "copy impersonation headers",
inHeaders: http.Header{
"Host": []string{"example.com"},
ImpersonateUserHeader: []string{"user-a"},
ImpersonateGroupHeader: []string{"kube-group-b"},
},
wantHeaders: http.Header{
ImpersonateUserHeader: []string{"user-a"},
ImpersonateGroupHeader: []string{"kube-group-b"},
},
},
{
name: "don't introduce empty impersonation headers",
inHeaders: http.Header{
"Host": []string{"example.com"},
},
wantHeaders: http.Header{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dst := http.Header{}
copyImpersonationHeaders(dst, tt.inHeaders)
require.Equal(t, tt.wantHeaders, dst)
})
}
}
4 changes: 2 additions & 2 deletions lib/kube/proxy/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ func (s *SpdyRoundTripper) dial(url *url.URL) (net.Conn, error) {
// copyImpersonationHeaders copies the impersonation headers from the source
// request to the destination request.
func copyImpersonationHeaders(dst, src http.Header) {
dst[ImpersonateUserHeader] = nil
dst[ImpersonateGroupHeader] = nil
dst.Del(ImpersonateUserHeader)
dst.Del(ImpersonateGroupHeader)

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

0 comments on commit c034a7a

Please sign in to comment.