From 703545a3db21d388aa9e4379bb70457f4872547c Mon Sep 17 00:00:00 2001 From: David Eads Date: Thu, 3 Oct 2019 12:56:42 -0400 Subject: [PATCH] add the ability for dynamic header names in delegated authentication Kubernetes-commit: 58256346693717fd12f121f0cf74fe1e003edb0f --- .../authenticatorfactory/delegating.go | 5 +- .../authenticatorfactory/requestheader.go | 9 +- .../request/headerrequest/requestheader.go | 88 ++++++++++++++----- .../request/x509/verify_options.go | 22 +++++ pkg/authentication/request/x509/x509.go | 14 +-- pkg/server/options/authentication.go | 41 ++++++++- pkg/server/options/authentication_test.go | 25 +++--- 7 files changed, 152 insertions(+), 52 deletions(-) diff --git a/pkg/authentication/authenticatorfactory/delegating.go b/pkg/authentication/authenticatorfactory/delegating.go index 2b8f118b8..ffd55de59 100644 --- a/pkg/authentication/authenticatorfactory/delegating.go +++ b/pkg/authentication/authenticatorfactory/delegating.go @@ -63,16 +63,13 @@ func (c DelegatingAuthenticatorConfig) New() (authenticator.Request, *spec.Secur // front-proxy first, then remote // Add the front proxy authenticator if requested if c.RequestHeaderConfig != nil { - requestHeaderAuthenticator, err := headerrequest.NewDynamicVerifyOptionsSecure( + requestHeaderAuthenticator := headerrequest.NewDynamicVerifyOptionsSecure( c.RequestHeaderConfig.VerifyOptionFn, c.RequestHeaderConfig.AllowedClientNames, c.RequestHeaderConfig.UsernameHeaders, c.RequestHeaderConfig.GroupHeaders, c.RequestHeaderConfig.ExtraHeaderPrefixes, ) - if err != nil { - return nil, nil, err - } authenticators = append(authenticators, requestHeaderAuthenticator) } diff --git a/pkg/authentication/authenticatorfactory/requestheader.go b/pkg/authentication/authenticatorfactory/requestheader.go index b5c667499..7108232fd 100644 --- a/pkg/authentication/authenticatorfactory/requestheader.go +++ b/pkg/authentication/authenticatorfactory/requestheader.go @@ -17,20 +17,21 @@ limitations under the License. package authenticatorfactory import ( + "k8s.io/apiserver/pkg/authentication/request/headerrequest" x509request "k8s.io/apiserver/pkg/authentication/request/x509" ) type RequestHeaderConfig struct { // UsernameHeaders are the headers to check (in order, case-insensitively) for an identity. The first header with a value wins. - UsernameHeaders []string + UsernameHeaders headerrequest.StringSliceProvider // GroupHeaders are the headers to check (case-insensitively) for a group names. All values will be used. - GroupHeaders []string + GroupHeaders headerrequest.StringSliceProvider // ExtraHeaderPrefixes are the head prefixes to check (case-insentively) for filling in // the user.Info.Extra. All values of all matching headers will be added. - ExtraHeaderPrefixes []string + ExtraHeaderPrefixes headerrequest.StringSliceProvider // VerifyOptionFn are the options for verifying incoming connections using mTLS. Generally this points to CA bundle file which is used verify the identity of the front proxy. // It may produce different options at will. VerifyOptionFn x509request.VerifyOptionFunc // AllowedClientNames is a list of common names that may be presented by the authenticating front proxy. Empty means: accept any. - AllowedClientNames []string + AllowedClientNames headerrequest.StringSliceProvider } diff --git a/pkg/authentication/request/headerrequest/requestheader.go b/pkg/authentication/request/headerrequest/requestheader.go index d5e59c631..abf509a97 100644 --- a/pkg/authentication/request/headerrequest/requestheader.go +++ b/pkg/authentication/request/headerrequest/requestheader.go @@ -24,26 +24,47 @@ import ( "net/url" "strings" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/authentication/authenticator" x509request "k8s.io/apiserver/pkg/authentication/request/x509" "k8s.io/apiserver/pkg/authentication/user" utilcert "k8s.io/client-go/util/cert" ) +// StringSliceProvider is a way to get a string slice value. It is heavily used for authentication headers among other places. +type StringSliceProvider interface { + // Value returns the current string slice. Callers should never mutate the returned value. + Value() []string +} + +// StringSliceProviderFunc is a function that matches the StringSliceProvider interface +type StringSliceProviderFunc func() []string + +// Value returns the current string slice. Callers should never mutate the returned value. +func (d StringSliceProviderFunc) Value() []string { + return d() +} + +// StaticStringSlice a StringSliceProvider that returns a fixed value +type StaticStringSlice []string + +// Value returns the current string slice. Callers should never mutate the returned value. +func (s StaticStringSlice) Value() []string { + return s +} + type requestHeaderAuthRequestHandler struct { // nameHeaders are the headers to check (in order, case-insensitively) for an identity. The first header with a value wins. - nameHeaders []string + nameHeaders StringSliceProvider // groupHeaders are the headers to check (case-insensitively) for group membership. All values of all headers will be added. - groupHeaders []string + groupHeaders StringSliceProvider // extraHeaderPrefixes are the head prefixes to check (case-insensitively) for filling in // the user.Info.Extra. All values of all matching headers will be added. - extraHeaderPrefixes []string + extraHeaderPrefixes StringSliceProvider } -func New(nameHeaders []string, groupHeaders []string, extraHeaderPrefixes []string) (authenticator.Request, error) { +func New(nameHeaders, groupHeaders, extraHeaderPrefixes []string) (authenticator.Request, error) { trimmedNameHeaders, err := trimHeaders(nameHeaders...) if err != nil { return nil, err @@ -57,11 +78,19 @@ func New(nameHeaders []string, groupHeaders []string, extraHeaderPrefixes []stri return nil, err } + return NewDynamic( + StaticStringSlice(trimmedNameHeaders), + StaticStringSlice(trimmedGroupHeaders), + StaticStringSlice(trimmedExtraHeaderPrefixes), + ), nil +} + +func NewDynamic(nameHeaders, groupHeaders, extraHeaderPrefixes StringSliceProvider) authenticator.Request { return &requestHeaderAuthRequestHandler{ - nameHeaders: trimmedNameHeaders, - groupHeaders: trimmedGroupHeaders, - extraHeaderPrefixes: trimmedExtraHeaderPrefixes, - }, nil + nameHeaders: nameHeaders, + groupHeaders: groupHeaders, + extraHeaderPrefixes: extraHeaderPrefixes, + } } func trimHeaders(headerNames ...string) ([]string, error) { @@ -97,36 +126,51 @@ func NewSecure(clientCA string, proxyClientNames []string, nameHeaders []string, opts.Roots.AddCert(cert) } - return NewDynamicVerifyOptionsSecure(x509request.StaticVerifierFn(opts), proxyClientNames, nameHeaders, groupHeaders, extraHeaderPrefixes) -} - -// TODO make the string slices dynamic too. -func NewDynamicVerifyOptionsSecure(verifyOptionFn x509request.VerifyOptionFunc, proxyClientNames []string, nameHeaders []string, groupHeaders []string, extraHeaderPrefixes []string) (authenticator.Request, error) { - headerAuthenticator, err := New(nameHeaders, groupHeaders, extraHeaderPrefixes) + trimmedNameHeaders, err := trimHeaders(nameHeaders...) + if err != nil { + return nil, err + } + trimmedGroupHeaders, err := trimHeaders(groupHeaders...) + if err != nil { + return nil, err + } + trimmedExtraHeaderPrefixes, err := trimHeaders(extraHeaderPrefixes...) if err != nil { return nil, err } - return x509request.NewDynamicCAVerifier(verifyOptionFn, headerAuthenticator, sets.NewString(proxyClientNames...)), nil + return NewDynamicVerifyOptionsSecure( + x509request.StaticVerifierFn(opts), + StaticStringSlice(proxyClientNames), + StaticStringSlice(trimmedNameHeaders), + StaticStringSlice(trimmedGroupHeaders), + StaticStringSlice(trimmedExtraHeaderPrefixes), + ), nil +} + +func NewDynamicVerifyOptionsSecure(verifyOptionFn x509request.VerifyOptionFunc, proxyClientNames, nameHeaders, groupHeaders, extraHeaderPrefixes StringSliceProvider) authenticator.Request { + headerAuthenticator := NewDynamic(nameHeaders, groupHeaders, extraHeaderPrefixes) + + return x509request.NewDynamicCAVerifier(verifyOptionFn, headerAuthenticator, proxyClientNames) } func (a *requestHeaderAuthRequestHandler) AuthenticateRequest(req *http.Request) (*authenticator.Response, bool, error) { - name := headerValue(req.Header, a.nameHeaders) + name := headerValue(req.Header, a.nameHeaders.Value()) if len(name) == 0 { return nil, false, nil } - groups := allHeaderValues(req.Header, a.groupHeaders) - extra := newExtra(req.Header, a.extraHeaderPrefixes) + groups := allHeaderValues(req.Header, a.groupHeaders.Value()) + extra := newExtra(req.Header, a.extraHeaderPrefixes.Value()) // clear headers used for authentication - for _, headerName := range a.nameHeaders { + for _, headerName := range a.nameHeaders.Value() { req.Header.Del(headerName) } - for _, headerName := range a.groupHeaders { + for _, headerName := range a.groupHeaders.Value() { req.Header.Del(headerName) } for k := range extra { - for _, prefix := range a.extraHeaderPrefixes { + for _, prefix := range a.extraHeaderPrefixes.Value() { req.Header.Del(prefix + k) } } diff --git a/pkg/authentication/request/x509/verify_options.go b/pkg/authentication/request/x509/verify_options.go index 56e1e71f9..78992580a 100644 --- a/pkg/authentication/request/x509/verify_options.go +++ b/pkg/authentication/request/x509/verify_options.go @@ -47,3 +47,25 @@ func NewStaticVerifierFromFile(clientCA string) (VerifyOptionFunc, error) { return StaticVerifierFn(opts), nil } + +// StringSliceProvider is a way to get a string slice value. It is heavily used for authentication headers among other places. +type StringSliceProvider interface { + // Value returns the current string slice. Callers should never mutate the returned value. + Value() []string +} + +// StringSliceProviderFunc is a function that matches the StringSliceProvider interface +type StringSliceProviderFunc func() []string + +// Value returns the current string slice. Callers should never mutate the returned value. +func (d StringSliceProviderFunc) Value() []string { + return d() +} + +// StaticStringSlice a StringSliceProvider that returns a fixed value +type StaticStringSlice []string + +// Value returns the current string slice. Callers should never mutate the returned value. +func (s StaticStringSlice) Value() []string { + return s +} diff --git a/pkg/authentication/request/x509/x509.go b/pkg/authentication/request/x509/x509.go index 17f853e9f..77a4711dc 100644 --- a/pkg/authentication/request/x509/x509.go +++ b/pkg/authentication/request/x509/x509.go @@ -148,17 +148,17 @@ type Verifier struct { // allowedCommonNames contains the common names which a verified certificate is allowed to have. // If empty, all verified certificates are allowed. - allowedCommonNames sets.String + allowedCommonNames StringSliceProvider } // NewVerifier create a request.Authenticator by verifying a client cert on the request, then delegating to the wrapped auth func NewVerifier(opts x509.VerifyOptions, auth authenticator.Request, allowedCommonNames sets.String) authenticator.Request { - return NewDynamicCAVerifier(StaticVerifierFn(opts), auth, allowedCommonNames) + return NewDynamicCAVerifier(StaticVerifierFn(opts), auth, StaticStringSlice(allowedCommonNames.List())) } // NewDynamicCAVerifier create a request.Authenticator by verifying a client cert on the request, then delegating to the wrapped auth // TODO make the allowedCommonNames dynamic -func NewDynamicCAVerifier(verifyOptionsFn VerifyOptionFunc, auth authenticator.Request, allowedCommonNames sets.String) authenticator.Request { +func NewDynamicCAVerifier(verifyOptionsFn VerifyOptionFunc, auth authenticator.Request, allowedCommonNames StringSliceProvider) authenticator.Request { return &Verifier{verifyOptionsFn, auth, allowedCommonNames} } @@ -188,12 +188,14 @@ func (a *Verifier) AuthenticateRequest(req *http.Request) (*authenticator.Respon func (a *Verifier) verifySubject(subject pkix.Name) error { // No CN restrictions - if len(a.allowedCommonNames) == 0 { + if len(a.allowedCommonNames.Value()) == 0 { return nil } // Enforce CN restrictions - if a.allowedCommonNames.Has(subject.CommonName) { - return nil + for _, allowedCommonName := range a.allowedCommonNames.Value() { + if allowedCommonName == subject.CommonName { + return nil + } } return fmt.Errorf("x509: subject with cn=%s is not in the allowed list", subject.CommonName) } diff --git a/pkg/server/options/authentication.go b/pkg/server/options/authentication.go index 1147a7196..25654cf11 100644 --- a/pkg/server/options/authentication.go +++ b/pkg/server/options/authentication.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "strings" "time" "github.com/spf13/pflag" @@ -28,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/authentication/authenticatorfactory" + "k8s.io/apiserver/pkg/authentication/request/headerrequest" "k8s.io/apiserver/pkg/authentication/request/x509" "k8s.io/apiserver/pkg/server" "k8s.io/client-go/kubernetes" @@ -49,6 +51,35 @@ type RequestHeaderAuthenticationOptions struct { AllowedNames []string } +func (s *RequestHeaderAuthenticationOptions) Validate() []error { + allErrors := []error{} + + if err := checkForWhiteSpaceOnly("requestheader-username-headers", s.UsernameHeaders...); err != nil { + allErrors = append(allErrors, err) + } + if err := checkForWhiteSpaceOnly("requestheader-group-headers", s.GroupHeaders...); err != nil { + allErrors = append(allErrors, err) + } + if err := checkForWhiteSpaceOnly("requestheader-extra-headers-prefix", s.ExtraHeaderPrefixes...); err != nil { + allErrors = append(allErrors, err) + } + if err := checkForWhiteSpaceOnly("requestheader-allowed-names", s.AllowedNames...); err != nil { + allErrors = append(allErrors, err) + } + + return allErrors +} + +func checkForWhiteSpaceOnly(flag string, headerNames ...string) error { + for _, headerName := range headerNames { + if len(strings.TrimSpace(headerName)) == 0 { + return fmt.Errorf("empty value in %q", flag) + } + } + + return nil +} + func (s *RequestHeaderAuthenticationOptions) AddFlags(fs *pflag.FlagSet) { if s == nil { return @@ -87,11 +118,11 @@ func (s *RequestHeaderAuthenticationOptions) ToAuthenticationRequestHeaderConfig } return &authenticatorfactory.RequestHeaderConfig{ - UsernameHeaders: s.UsernameHeaders, - GroupHeaders: s.GroupHeaders, - ExtraHeaderPrefixes: s.ExtraHeaderPrefixes, + UsernameHeaders: headerrequest.StaticStringSlice(s.UsernameHeaders), + GroupHeaders: headerrequest.StaticStringSlice(s.GroupHeaders), + ExtraHeaderPrefixes: headerrequest.StaticStringSlice(s.ExtraHeaderPrefixes), VerifyOptionFn: verifyFn, - AllowedClientNames: s.AllowedNames, + AllowedClientNames: headerrequest.StaticStringSlice(s.AllowedNames), }, nil } @@ -167,6 +198,8 @@ func NewDelegatingAuthenticationOptions() *DelegatingAuthenticationOptions { func (s *DelegatingAuthenticationOptions) Validate() []error { allErrors := []error{} + allErrors = append(allErrors, s.RequestHeader.Validate()...) + return allErrors } diff --git a/pkg/server/options/authentication_test.go b/pkg/server/options/authentication_test.go index f43bc6a90..22b5fb41d 100644 --- a/pkg/server/options/authentication_test.go +++ b/pkg/server/options/authentication_test.go @@ -24,6 +24,7 @@ import ( "testing" "k8s.io/apiserver/pkg/authentication/authenticatorfactory" + "k8s.io/apiserver/pkg/authentication/request/headerrequest" "k8s.io/apiserver/pkg/server" openapicommon "k8s.io/kube-openapi/pkg/common" ) @@ -37,27 +38,27 @@ func TestToAuthenticationRequestHeaderConfig(t *testing.T) { { name: "test when ClientCAFile is nil", testOptions: &RequestHeaderAuthenticationOptions{ - UsernameHeaders: []string{"x-remote-user"}, - GroupHeaders: []string{"x-remote-group"}, - ExtraHeaderPrefixes: []string{"x-remote-extra-"}, - AllowedNames: []string{"kube-aggregator"}, + UsernameHeaders: headerrequest.StaticStringSlice{"x-remote-user"}, + GroupHeaders: headerrequest.StaticStringSlice{"x-remote-group"}, + ExtraHeaderPrefixes: headerrequest.StaticStringSlice{"x-remote-extra-"}, + AllowedNames: headerrequest.StaticStringSlice{"kube-aggregator"}, }, }, { name: "test when ClientCAFile is not nil", testOptions: &RequestHeaderAuthenticationOptions{ ClientCAFile: "testdata/root.pem", - UsernameHeaders: []string{"x-remote-user"}, - GroupHeaders: []string{"x-remote-group"}, - ExtraHeaderPrefixes: []string{"x-remote-extra-"}, - AllowedNames: []string{"kube-aggregator"}, + UsernameHeaders: headerrequest.StaticStringSlice{"x-remote-user"}, + GroupHeaders: headerrequest.StaticStringSlice{"x-remote-group"}, + ExtraHeaderPrefixes: headerrequest.StaticStringSlice{"x-remote-extra-"}, + AllowedNames: headerrequest.StaticStringSlice{"kube-aggregator"}, }, expectConfig: &authenticatorfactory.RequestHeaderConfig{ - UsernameHeaders: []string{"x-remote-user"}, - GroupHeaders: []string{"x-remote-group"}, - ExtraHeaderPrefixes: []string{"x-remote-extra-"}, + UsernameHeaders: headerrequest.StaticStringSlice{"x-remote-user"}, + GroupHeaders: headerrequest.StaticStringSlice{"x-remote-group"}, + ExtraHeaderPrefixes: headerrequest.StaticStringSlice{"x-remote-extra-"}, VerifyOptionFn: nil, // this is nil because you can't compare functions - AllowedClientNames: []string{"kube-aggregator"}, + AllowedClientNames: headerrequest.StaticStringSlice{"kube-aggregator"}, }, }, }