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>
This commit is contained in:
Monis Khan 2023-03-20 13:11:38 -04:00
parent 15894cfc85
commit e9866d2794
No known key found for this signature in database
11 changed files with 335 additions and 22 deletions

View File

@ -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)

View File

@ -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)

View File

@ -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
}

View File

@ -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)
}
})
}
}

View File

@ -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)
})

View File

@ -19,14 +19,19 @@ package filters
import (
"context"
"errors"
"github.com/stretchr/testify/assert"
"k8s.io/apiserver/pkg/authentication/authenticator"
"k8s.io/apiserver/pkg/authentication/user"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"k8s.io/apiserver/pkg/authentication/authenticator"
"k8s.io/apiserver/pkg/authentication/authenticatorfactory"
"k8s.io/apiserver/pkg/authentication/request/headerrequest"
"k8s.io/apiserver/pkg/authentication/user"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
)
func TestAuthenticateRequestWithAud(t *testing.T) {
@ -93,6 +98,7 @@ func TestAuthenticateRequestWithAud(t *testing.T) {
}
}),
tc.apiAuds,
nil,
)
auth.ServeHTTP(httptest.NewRecorder(), &http.Request{Header: map[string][]string{"Authorization": {"Something"}}})
if tc.expectSuccess {
@ -164,6 +170,7 @@ func TestAuthenticateMetrics(t *testing.T) {
http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
}),
tc.apiAuds,
nil,
func(ctx context.Context, resp *authenticator.Response, ok bool, err error, apiAudiences authenticator.Audiences, authStart time.Time, authFinish time.Time) {
called = 1
if tc.expectOk != ok {
@ -214,6 +221,7 @@ func TestAuthenticateRequest(t *testing.T) {
t.Errorf("unexpected call to failed")
}),
nil,
nil,
)
auth.ServeHTTP(httptest.NewRecorder(), &http.Request{Header: map[string][]string{"Authorization": {"Something"}}})
@ -234,6 +242,7 @@ func TestAuthenticateRequestFailed(t *testing.T) {
close(failed)
}),
nil,
nil,
)
auth.ServeHTTP(httptest.NewRecorder(), &http.Request{})
@ -254,9 +263,205 @@ func TestAuthenticateRequestError(t *testing.T) {
close(failed)
}),
nil,
nil,
)
auth.ServeHTTP(httptest.NewRecorder(), &http.Request{})
<-failed
}
func TestAuthenticateRequestClearHeaders(t *testing.T) {
testcases := map[string]struct {
nameHeaders []string
groupHeaders []string
extraPrefixHeaders []string
requestHeaders http.Header
finalHeaders http.Header
}{
"user match": {
nameHeaders: []string{"X-Remote-User"},
requestHeaders: http.Header{"X-Remote-User": {"Bob"}},
},
"user first match": {
nameHeaders: []string{
"X-Remote-User",
"A-Second-X-Remote-User",
"Another-X-Remote-User",
},
requestHeaders: http.Header{
"X-Remote-User": {"", "First header, second value"},
"A-Second-X-Remote-User": {"Second header, first value", "Second header, second value"},
"Another-X-Remote-User": {"Third header, first value"}},
},
"user case-insensitive": {
nameHeaders: []string{"x-REMOTE-user"}, // configured headers can be case-insensitive
requestHeaders: http.Header{"X-Remote-User": {"Bob"}}, // the parsed headers are normalized by the http package
},
"groups none": {
nameHeaders: []string{"X-Remote-User"},
groupHeaders: []string{"X-Remote-Group"},
requestHeaders: http.Header{
"X-Remote-User": {"Bob"},
},
},
"groups all matches": {
nameHeaders: []string{"X-Remote-User"},
groupHeaders: []string{"X-Remote-Group-1", "X-Remote-Group-2"},
requestHeaders: http.Header{
"X-Remote-User": {"Bob"},
"X-Remote-Group-1": {"one-a", "one-b"},
"X-Remote-Group-2": {"two-a", "two-b"},
},
},
"groups case-insensitive": {
nameHeaders: []string{"X-REMOTE-User"},
groupHeaders: []string{"X-REMOTE-Group"},
requestHeaders: http.Header{
"X-Remote-User": {"Bob"},
"X-Remote-Group": {"Users"},
},
},
"extra prefix matches case-insensitive": {
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-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"},
},
},
"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"},
},
},
"custom config but request contains standard headers": {
nameHeaders: []string{"foo"},
groupHeaders: []string{"bar"},
extraPrefixHeaders: []string{"baz"},
requestHeaders: http.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-Remote-Group-1": {"one-a", "one-b"},
"X-Remote-Group-2": {"two-a", "two-b"},
},
},
"custom config but request contains standard and custom headers": {
nameHeaders: []string{"one"},
groupHeaders: []string{"two"},
extraPrefixHeaders: []string{"three-"},
requestHeaders: http.Header{
"X-Remote-User": {"Bob"},
"X-Remote-Group-3": {"one-a", "one-b"},
"X-Remote-Group-4": {"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"},
"One": {"echo", "foxtrot"},
"Two": {"golf", "hotel"},
"Three-Four": {"india", "juliet"},
},
finalHeaders: http.Header{
"X-Remote-Group-3": {"one-a", "one-b"},
"X-Remote-Group-4": {"two-a", "two-b"},
},
},
"escaped extra keys": {
nameHeaders: []string{"X-Remote-User"},
groupHeaders: []string{"X-Remote-Group"},
extraPrefixHeaders: []string{"X-Remote-Extra-"},
requestHeaders: http.Header{
"X-Remote-User": {"Bob"},
"X-Remote-Group": {"one-a", "one-b"},
"X-Remote-Extra-Alpha": {"alphabetical"},
"X-Remote-Extra-Alph4num3r1c": {"alphanumeric"},
"X-Remote-Extra-Percent%20encoded": {"percent encoded"},
"X-Remote-Extra-Almost%zzpercent%xxencoded": {"not quite percent encoded"},
"X-Remote-Extra-Example.com%2fpercent%2520encoded": {"url with double percent encoding"},
"X-Remote-Extra-Example.com%2F%E4%BB%8A%E6%97%A5%E3%81%AF": {"url with unicode"},
"X-Remote-Extra-Abc123!#$+.-_*\\^`~|'": {"header key legal characters"},
},
},
}
for k, testcase := range testcases {
t.Run(k, func(t *testing.T) {
var handlerCalls, authnCalls int
auth := WithAuthentication(
http.HandlerFunc(func(_ http.ResponseWriter, req *http.Request) {
handlerCalls++
}),
authenticator.RequestFunc(func(req *http.Request) (*authenticator.Response, bool, error) {
authnCalls++
return &authenticator.Response{User: &user.DefaultInfo{Name: "panda"}}, true, nil
}),
http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
t.Errorf("unexpected call to handler")
}),
nil,
&authenticatorfactory.RequestHeaderConfig{
UsernameHeaders: headerrequest.StaticStringSlice(testcase.nameHeaders),
GroupHeaders: headerrequest.StaticStringSlice(testcase.groupHeaders),
ExtraHeaderPrefixes: headerrequest.StaticStringSlice(testcase.extraPrefixHeaders),
},
)
auth.ServeHTTP(httptest.NewRecorder(), &http.Request{Header: testcase.requestHeaders})
if handlerCalls != 1 || authnCalls != 1 {
t.Errorf("unexpected calls: handlerCalls=%d, authnCalls=%d", handlerCalls, authnCalls)
}
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)
}
})
}
}

View File

@ -146,6 +146,7 @@ func TestMetrics(t *testing.T) {
close(done)
}),
tt.apiAudience,
nil,
)
auth.ServeHTTP(httptest.NewRecorder(), &http.Request{})

View File

@ -345,6 +345,8 @@ type AuthenticationInfo struct {
APIAudiences authenticator.Audiences
// Authenticator determines which subject is making the request
Authenticator authenticator.Request
RequestHeaderConfig *authenticatorfactory.RequestHeaderConfig
}
type AuthorizationInfo struct {
@ -920,7 +922,7 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler {
failedHandler = filterlatency.TrackCompleted(failedHandler)
handler = filterlatency.TrackCompleted(handler)
handler = genericapifilters.WithAuthentication(handler, c.Authentication.Authenticator, failedHandler, c.Authentication.APIAudiences)
handler = genericapifilters.WithAuthentication(handler, c.Authentication.Authenticator, failedHandler, c.Authentication.APIAudiences, c.Authentication.RequestHeaderConfig)
handler = filterlatency.TrackStarted(handler, c.TracerProvider, "authentication")
handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true")

View File

@ -76,6 +76,16 @@ func (s *RequestHeaderAuthenticationOptions) Validate() []error {
allErrors = append(allErrors, err)
}
if len(s.UsernameHeaders) > 0 && !caseInsensitiveHas(s.UsernameHeaders, "X-Remote-User") {
klog.Warningf("--requestheader-username-headers is set without specifying the standard X-Remote-User header - API aggregation will not work")
}
if len(s.GroupHeaders) > 0 && !caseInsensitiveHas(s.GroupHeaders, "X-Remote-Group") {
klog.Warningf("--requestheader-group-headers is set without specifying the standard X-Remote-Group header - API aggregation will not work")
}
if len(s.ExtraHeaderPrefixes) > 0 && !caseInsensitiveHas(s.ExtraHeaderPrefixes, "X-Remote-Extra-") {
klog.Warningf("--requestheader-extra-headers-prefix is set without specifying the standard X-Remote-Extra- header prefix - API aggregation will not work")
}
return allErrors
}
@ -89,6 +99,15 @@ func checkForWhiteSpaceOnly(flag string, headerNames ...string) error {
return nil
}
func caseInsensitiveHas(headers []string, header string) bool {
for _, h := range headers {
if strings.EqualFold(h, header) {
return true
}
}
return false
}
func (s *RequestHeaderAuthenticationOptions) AddFlags(fs *pflag.FlagSet) {
if s == nil {
return
@ -357,6 +376,7 @@ func (s *DelegatingAuthenticationOptions) ApplyTo(authenticationInfo *server.Aut
}
if requestHeaderConfig != nil {
cfg.RequestHeaderConfig = requestHeaderConfig
authenticationInfo.RequestHeaderConfig = requestHeaderConfig
if err = authenticationInfo.ApplyClientCert(cfg.RequestHeaderConfig.CAContentProvider, servingInfo); err != nil {
return fmt.Errorf("unable to load request-header-client-ca-file: %v", err)
}

View File

@ -44,7 +44,7 @@ func BuildHandlerChain(apiHandler http.Handler, authorizationInfo *apiserver.Aut
handler = genericapifilters.WithAuthorization(apiHandler, authorizationInfo.Authorizer, scheme.Codecs)
}
if authenticationInfo != nil {
handler = genericapifilters.WithAuthentication(handler, authenticationInfo.Authenticator, failedHandler, nil)
handler = genericapifilters.WithAuthentication(handler, authenticationInfo.Authenticator, failedHandler, nil, nil)
}
handler = genericapifilters.WithRequestInfo(handler, requestInfoResolver)
handler = genericapifilters.WithCacheControl(handler)

View File

@ -238,6 +238,7 @@ func TestPodLogsKubeletClientCertReload(t *testing.T) {
w.WriteHeader(http.StatusUnauthorized)
}),
nil,
nil,
),
)
fakeKubeletServer.TLS = &tls.Config{ClientAuth: tls.RequestClientCert}