Allow all headers besides impersonation-* through impersonation proxy

This commit is contained in:
Margo Crawford 2021-03-02 14:56:54 -08:00
parent 60f92d5fe2
commit 4c68050706
2 changed files with 38 additions and 42 deletions

View File

@ -10,6 +10,7 @@ import (
"net/http"
"net/http/httputil"
"net/url"
"regexp"
"strings"
"time"
@ -27,16 +28,8 @@ import (
"go.pinniped.dev/internal/kubeclient"
)
// allowedHeaders are the set of HTTP headers that are allowed to be forwarded through the impersonation proxy.
//nolint: gochecknoglobals
var allowedHeaders = []string{
"Accept",
"Accept-Encoding",
"User-Agent",
"Connection",
"Upgrade",
"Content-Type",
}
// nolint: gochecknoglobals
var impersonateHeaderRegex = regexp.MustCompile("Impersonate-.*")
type proxy struct {
cache *authncache.Cache
@ -157,17 +150,9 @@ type httpError struct {
func (e *httpError) Error() string { return e.message }
func ensureNoImpersonationHeaders(r *http.Request) error {
if _, ok := r.Header[transport.ImpersonateUserHeader]; ok {
return fmt.Errorf("%q header already exists", transport.ImpersonateUserHeader)
}
if _, ok := r.Header[transport.ImpersonateGroupHeader]; ok {
return fmt.Errorf("%q header already exists", transport.ImpersonateGroupHeader)
}
for header := range r.Header {
if strings.HasPrefix(header, transport.ImpersonateUserExtraHeaderPrefix) {
return fmt.Errorf("%q header already exists", transport.ImpersonateUserExtraHeaderPrefix)
for key := range r.Header {
if impersonateHeaderRegex.MatchString(key) {
return fmt.Errorf("%q header already exists", key)
}
}
@ -209,11 +194,12 @@ func getProxyHeaders(userInfo user.Info, requestHeaders http.Header) http.Header
//nolint:bodyclose // We return a nil http.Response above, so there is nothing to close.
_, _ = transport.NewImpersonatingRoundTripper(impersonateConfig, impersonateHeaderSpy).RoundTrip(fakeReq)
// Copy over the allowed header values from the original request to the new request.
for _, header := range allowedHeaders {
values := requestHeaders.Values(header)
for i := range values {
newHeaders.Add(header, values[i])
// Copy over all headers except the Authorization header from the original request to the new request.
for key := range requestHeaders {
if key != "Authorization" {
for _, val := range requestHeaders.Values(key) {
newHeaders.Add(key, val)
}
}
}

View File

@ -132,7 +132,16 @@ func TestImpersonator(t *testing.T) {
request: newRequest(map[string][]string{"Impersonate-Extra-something": {"something"}}),
wantHTTPBody: "impersonation header already exists\n",
wantHTTPStatus: http.StatusBadRequest,
wantLogs: []string{"\"msg\"=\"impersonation header already exists\" \"error\"=\"\\\"Impersonate-Extra-\\\" header already exists\" \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
wantLogs: []string{"\"msg\"=\"impersonation header already exists\" \"error\"=\"\\\"Impersonate-Extra-something\\\" header already exists\" \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
},
{
name: "Impersonate-* header already in request",
request: newRequest(map[string][]string{
"Impersonate-Something": {"some-newfangled-impersonate-header"},
}),
wantHTTPBody: "impersonation header already exists\n",
wantHTTPStatus: http.StatusBadRequest,
wantLogs: []string{"\"msg\"=\"impersonation header already exists\" \"error\"=\"\\\"Impersonate-Something\\\" header already exists\" \"method\"=\"GET\" \"url\"=\"http://pinniped.dev/blah\""},
},
{
name: "missing authorization header",
@ -198,15 +207,15 @@ func TestImpersonator(t *testing.T) {
{
name: "token validates",
request: newRequest(map[string][]string{
"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, defaultAPIGroup)},
"User-Agent": {"test-user-agent"},
"Accept": {"some-accepted-format"},
"Accept-Encoding": {"some-accepted-encoding"},
"Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy`
"Upgrade": {"some-upgrade"},
"Content-Type": {"some-type"},
"Content-Length": {"some-length"},
"Malicious-Header": {"test-header-value-1"}, // this header should not be forwarded to the Kube API server
"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, defaultAPIGroup)},
"User-Agent": {"test-user-agent"},
"Accept": {"some-accepted-format"},
"Accept-Encoding": {"some-accepted-encoding"},
"Connection": {"Upgrade"}, // the value "Upgrade" is handled in a special way by `httputil.NewSingleHostReverseProxy`
"Upgrade": {"some-upgrade"},
"Content-Type": {"some-type"},
"Content-Length": {"some-length"},
"Other-Header": {"test-header-value-1"}, // this header will be passed through
}),
expectMockToken: func(t *testing.T, recorder *mocktokenauthenticator.MockTokenMockRecorder) {
userInfo := user.DefaultInfo{
@ -230,6 +239,7 @@ func TestImpersonator(t *testing.T) {
"Connection": {"Upgrade"},
"Upgrade": {"some-upgrade"},
"Content-Type": {"some-type"},
"Other-Header": {"test-header-value-1"},
},
wantHTTPBody: "successful proxied response",
wantHTTPStatus: http.StatusOK,
@ -238,9 +248,8 @@ func TestImpersonator(t *testing.T) {
{
name: "token validates and the kube API request returns an error",
request: newRequest(map[string][]string{
"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, defaultAPIGroup)},
"Malicious-Header": {"test-header-value-1"},
"User-Agent": {"test-user-agent"},
"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, defaultAPIGroup)},
"User-Agent": {"test-user-agent"},
}),
expectMockToken: func(t *testing.T, recorder *mocktokenauthenticator.MockTokenMockRecorder) {
userInfo := user.DefaultInfo{
@ -269,9 +278,9 @@ func TestImpersonator(t *testing.T) {
name: "token validates with custom api group",
apiGroupOverride: customAPIGroup,
request: newRequest(map[string][]string{
"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, customAPIGroup)},
"Malicious-Header": {"test-header-value-1"},
"User-Agent": {"test-user-agent"},
"Authorization": {"Bearer " + impersonationtoken.Make(t, "test-token", &goodAuthenticator, customAPIGroup)},
"Other-Header": {"test-header-value-1"},
"User-Agent": {"test-user-agent"},
}),
expectMockToken: func(t *testing.T, recorder *mocktokenauthenticator.MockTokenMockRecorder) {
userInfo := user.DefaultInfo{
@ -291,6 +300,7 @@ func TestImpersonator(t *testing.T) {
"Impersonate-Group": {"test-group-1", "test-group-2"},
"Impersonate-User": {"test-user"},
"User-Agent": {"test-user-agent"},
"Other-Header": {"test-header-value-1"},
},
wantHTTPBody: "successful proxied response",
wantHTTPStatus: http.StatusOK,