Clear front proxy headers after authentication is complete

This matches the logic we have for the Authorization header as well
as the impersonation headers.

Signed-off-by: Monis Khan <mok@microsoft.com>

cmd/kube-scheduler/app/server.go

@@ -250,7 +250,7 @@ func buildHandlerChain(handler http.Handler, authn authenticator.Request, authz
 	failedHandler := genericapifilters.Unauthorized(scheme.Codecs)
 
 	handler = genericapifilters.WithAuthorization(handler, authz, scheme.Codecs)
-	handler = genericapifilters.WithAuthentication(handler, authn, failedHandler, nil)
+	handler = genericapifilters.WithAuthentication(handler, authn, failedHandler, nil, nil)
 	handler = genericapifilters.WithRequestInfo(handler, requestInfoResolver)
 	handler = genericapifilters.WithCacheControl(handler)
 	handler = genericfilters.WithHTTPLogging(handler)

pkg/kubeapiserver/options/authentication.go

@@ -237,6 +237,10 @@ func (o *BuiltInAuthenticationOptions) Validate() []error {
 		}
 	}
 
+	if o.RequestHeader != nil {
+		allErrors = append(allErrors, o.RequestHeader.Validate()...)
+	}
+
 	return allErrors
 }
 
@@ -472,6 +476,7 @@ func (o *BuiltInAuthenticationOptions) ApplyTo(authInfo *genericapiserver.Authen
 		}
 	}
 
+	authInfo.RequestHeaderConfig = authenticatorConfig.RequestHeaderConfig
 	authInfo.APIAudiences = o.APIAudiences
 	if o.ServiceAccounts != nil && len(o.ServiceAccounts.Issuers) != 0 && len(o.APIAudiences) == 0 {
 		authInfo.APIAudiences = authenticator.Audiences(o.ServiceAccounts.Issuers)

staging/src/k8s.io/apiserver/pkg/authentication/request/headerrequest/requestheader.go

@@ -163,17 +163,7 @@ func (a *requestHeaderAuthRequestHandler) AuthenticateRequest(req *http.Request)
 	extra := newExtra(req.Header, a.extraHeaderPrefixes.Value())
 
 	// clear headers used for authentication
-	for _, headerName := range a.nameHeaders.Value() {
-		req.Header.Del(headerName)
-	}
-	for _, headerName := range a.groupHeaders.Value() {
-		req.Header.Del(headerName)
-	}
-	for k := range extra {
-		for _, prefix := range a.extraHeaderPrefixes.Value() {
-			req.Header.Del(prefix + k)
-		}
-	}
+	ClearAuthenticationHeaders(req.Header, a.nameHeaders, a.groupHeaders, a.extraHeaderPrefixes)
 
 	return &authenticator.Response{
 		User: &user.DefaultInfo{
@@ -184,6 +174,26 @@ func (a *requestHeaderAuthRequestHandler) AuthenticateRequest(req *http.Request)
 	}, true, nil
 }
 
+func ClearAuthenticationHeaders(h http.Header, nameHeaders, groupHeaders, extraHeaderPrefixes StringSliceProvider) {
+	for _, headerName := range nameHeaders.Value() {
+		h.Del(headerName)
+	}
+	for _, headerName := range groupHeaders.Value() {
+		h.Del(headerName)
+	}
+	for _, prefix := range extraHeaderPrefixes.Value() {
+		for k := range h {
+			if hasPrefixIgnoreCase(k, prefix) {
+				delete(h, k) // we have the raw key so avoid relying on canonicalization
+			}
+		}
+	}
+}
+
+func hasPrefixIgnoreCase(s, prefix string) bool {
+	return len(s) >= len(prefix) && strings.EqualFold(s[:len(prefix)], prefix)
+}
+
 func headerValue(h http.Header, headerNames []string) string {
 	for _, headerName := range headerNames {
 		headerValue := h.Get(headerName)
@@ -226,7 +236,7 @@ func newExtra(h http.Header, headerPrefixes []string) map[string][]string {
 	// we have to iterate over prefixes first in order to have proper ordering inside the value slices
 	for _, prefix := range headerPrefixes {
 		for headerName, vv := range h {
-			if !strings.HasPrefix(strings.ToLower(headerName), strings.ToLower(prefix)) {
+			if !hasPrefixIgnoreCase(headerName, prefix) {
 				continue
 			}
 

staging/src/k8s.io/apiserver/pkg/authentication/request/headerrequest/requestheader_test.go

@@ -21,6 +21,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+
 	"k8s.io/apiserver/pkg/authentication/user"
 )
 
@@ -30,6 +32,7 @@ func TestRequestHeader(t *testing.T) {
 		groupHeaders       []string
 		extraPrefixHeaders []string
 		requestHeaders     http.Header
+		finalHeaders       http.Header
 
 		expectedUser user.Info
 		expectedOk   bool
@@ -153,6 +156,39 @@ func TestRequestHeader(t *testing.T) {
 			expectedOk: true,
 		},
 
+		"extra prefix matches case-insensitive with unrelated headers": {
+			nameHeaders:        []string{"X-Remote-User"},
+			groupHeaders:       []string{"X-Remote-Group-1", "X-Remote-Group-2"},
+			extraPrefixHeaders: []string{"X-Remote-Extra-1-", "X-Remote-Extra-2-"},
+			requestHeaders: http.Header{
+				"X-Group-Remote":        {"snorlax"}, // unrelated header
+				"X-Group-Bear":          {"panda"},   // another unrelated header
+				"X-Remote-User":         {"Bob"},
+				"X-Remote-Group-1":      {"one-a", "one-b"},
+				"X-Remote-Group-2":      {"two-a", "two-b"},
+				"X-Remote-extra-1-key1": {"alfa", "bravo"},
+				"X-Remote-Extra-1-Key2": {"charlie", "delta"},
+				"X-Remote-Extra-1-":     {"india", "juliet"},
+				"X-Remote-extra-2-":     {"kilo", "lima"},
+				"X-Remote-extra-2-Key1": {"echo", "foxtrot"},
+				"X-Remote-Extra-2-key2": {"golf", "hotel"},
+			},
+			finalHeaders: http.Header{
+				"X-Group-Remote": {"snorlax"},
+				"X-Group-Bear":   {"panda"},
+			},
+			expectedUser: &user.DefaultInfo{
+				Name:   "Bob",
+				Groups: []string{"one-a", "one-b", "two-a", "two-b"},
+				Extra: map[string][]string{
+					"key1": {"alfa", "bravo", "echo", "foxtrot"},
+					"key2": {"charlie", "delta", "golf", "hotel"},
+					"":     {"india", "juliet", "kilo", "lima"},
+				},
+			},
+			expectedOk: true,
+		},
+
 		"escaped extra keys": {
 			nameHeaders:        []string{"X-Remote-User"},
 			groupHeaders:       []string{"X-Remote-Group"},
@@ -203,6 +239,14 @@ func TestRequestHeader(t *testing.T) {
 			if e, a := testcase.expectedUser, resp.User; !reflect.DeepEqual(e, a) {
 				t.Errorf("%v: expected %#v, got %#v", k, e, a)
 			}
+
+			want := testcase.finalHeaders
+			if want == nil && testcase.requestHeaders != nil {
+				want = http.Header{}
+			}
+			if diff := cmp.Diff(want, testcase.requestHeaders); len(diff) > 0 {
+				t.Errorf("unexpected final headers (-want +got):\n%s", diff)
+			}
 		})
 	}
 }

staging/src/k8s.io/apiserver/pkg/endpoints/filters/authentication.go

@@ -27,6 +27,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
+	"k8s.io/apiserver/pkg/authentication/authenticatorfactory"
+	"k8s.io/apiserver/pkg/authentication/request/headerrequest"
 	"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/klog/v2"
@@ -38,15 +40,20 @@ type recordMetrics func(context.Context, *authenticator.Response, bool, error, a
 // stores any such user found onto the provided context for the request. If authentication fails or returns an error
 // the failed handler is used. On success, "Authorization" header is removed from the request and handler
 // is invoked to serve the request.
-func WithAuthentication(handler http.Handler, auth authenticator.Request, failed http.Handler, apiAuds authenticator.Audiences) http.Handler {
-	return withAuthentication(handler, auth, failed, apiAuds, recordAuthMetrics)
+func WithAuthentication(handler http.Handler, auth authenticator.Request, failed http.Handler, apiAuds authenticator.Audiences, requestHeaderConfig *authenticatorfactory.RequestHeaderConfig) http.Handler {
+	return withAuthentication(handler, auth, failed, apiAuds, requestHeaderConfig, recordAuthMetrics)
 }
 
-func withAuthentication(handler http.Handler, auth authenticator.Request, failed http.Handler, apiAuds authenticator.Audiences, metrics recordMetrics) http.Handler {
+func withAuthentication(handler http.Handler, auth authenticator.Request, failed http.Handler, apiAuds authenticator.Audiences, requestHeaderConfig *authenticatorfactory.RequestHeaderConfig, metrics recordMetrics) http.Handler {
 	if auth == nil {
 		klog.Warning("Authentication is disabled")
 		return handler
 	}
+	standardRequestHeaderConfig := &authenticatorfactory.RequestHeaderConfig{
+		UsernameHeaders:     headerrequest.StaticStringSlice{"X-Remote-User"},
+		GroupHeaders:        headerrequest.StaticStringSlice{"X-Remote-Group"},
+		ExtraHeaderPrefixes: headerrequest.StaticStringSlice{"X-Remote-Extra-"},
+	}
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		authenticationStart := time.Now()
 
@@ -76,6 +83,24 @@ func withAuthentication(handler http.Handler, auth authenticator.Request, failed
 		// authorization header is not required anymore in case of a successful authentication.
 		req.Header.Del("Authorization")
 
+		// delete standard front proxy headers
+		headerrequest.ClearAuthenticationHeaders(
+			req.Header,
+			standardRequestHeaderConfig.UsernameHeaders,
+			standardRequestHeaderConfig.GroupHeaders,
+			standardRequestHeaderConfig.ExtraHeaderPrefixes,
+		)
+
+		// also delete any custom front proxy headers
+		if requestHeaderConfig != nil {
+			headerrequest.ClearAuthenticationHeaders(
+				req.Header,
+				requestHeaderConfig.UsernameHeaders,
+				requestHeaderConfig.GroupHeaders,
+				requestHeaderConfig.ExtraHeaderPrefixes,
+			)
+		}
+
 		req = req.WithContext(genericapirequest.WithUser(req.Context(), resp.User))
 		handler.ServeHTTP(w, req)
 	})