Merge pull request #83459 from deads2k/cert-reload-subject

dynamic delegated authn header reload

pkg/kubeapiserver/authenticator/config.go

@@ -93,16 +93,13 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, er
 	// front-proxy, BasicAuth methods, local first, then remote
 	// Add the front proxy authenticator if requested
 	if config.RequestHeaderConfig != nil {
-		requestHeaderAuthenticator, err := headerrequest.NewDynamicVerifyOptionsSecure(
+		requestHeaderAuthenticator := headerrequest.NewDynamicVerifyOptionsSecure(
 			config.RequestHeaderConfig.VerifyOptionFn,
 			config.RequestHeaderConfig.AllowedClientNames,
 			config.RequestHeaderConfig.UsernameHeaders,
 			config.RequestHeaderConfig.GroupHeaders,
 			config.RequestHeaderConfig.ExtraHeaderPrefixes,
 		)
-		if err != nil {
-			return nil, nil, err
-		}
 		authenticators = append(authenticators, authenticator.WrapAudienceAgnosticRequest(config.APIAudiences, requestHeaderAuthenticator))
 	}
 

pkg/kubeapiserver/options/BUILD

@@ -102,6 +102,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/request/headerrequest:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/server/options:go_default_library",
         "//vendor/github.com/google/go-cmp/cmp:go_default_library",
     ],

pkg/kubeapiserver/options/authentication_test.go

@@ -27,6 +27,7 @@ import (
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/authenticatorfactory"
+	"k8s.io/apiserver/pkg/authentication/request/headerrequest"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
 	kubeauthenticator "k8s.io/kubernetes/pkg/kubeapiserver/authenticator"
 )
@@ -161,11 +162,11 @@ func TestToAuthenticationConfig(t *testing.T) {
 		TokenFailureCacheTTL: 0,
 
 		RequestHeaderConfig: &authenticatorfactory.RequestHeaderConfig{
-			UsernameHeaders:     []string{"x-remote-user"},
+			UsernameHeaders:     headerrequest.StaticStringSlice{"x-remote-user"},
-			GroupHeaders:        []string{"x-remote-group"},
+			GroupHeaders:        headerrequest.StaticStringSlice{"x-remote-group"},
-			ExtraHeaderPrefixes: []string{"x-remote-extra-"},
+			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"},
 		},
 	}
 

staging/src/k8s.io/apiserver/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)
 	}
 

staging/src/k8s.io/apiserver/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
 }

staging/src/k8s.io/apiserver/pkg/authentication/request/headerrequest/BUILD

@@ -19,7 +19,6 @@ go_library(
     importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/authentication/request/headerrequest",
     importpath = "k8s.io/apiserver/pkg/authentication/request/headerrequest",
     deps = [
-        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/request/x509:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",

staging/src/k8s.io/apiserver/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,
+		nameHeaders:         nameHeaders,
-		groupHeaders:        trimmedGroupHeaders,
+		groupHeaders:        groupHeaders,
-		extraHeaderPrefixes: trimmedExtraHeaderPrefixes,
+		extraHeaderPrefixes: extraHeaderPrefixes,
-	}, nil
+	}
 }
 
 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)
+	trimmedNameHeaders, err := trimHeaders(nameHeaders...)
-}
+	if err != nil {
-
+		return nil, err
-// TODO make the string slices dynamic too.
+	}
-func NewDynamicVerifyOptionsSecure(verifyOptionFn x509request.VerifyOptionFunc, proxyClientNames []string, nameHeaders []string, groupHeaders []string, extraHeaderPrefixes []string) (authenticator.Request, error) {
+	trimmedGroupHeaders, err := trimHeaders(groupHeaders...)
-	headerAuthenticator, err := New(nameHeaders, groupHeaders, extraHeaderPrefixes)
+	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)
+	groups := allHeaderValues(req.Header, a.groupHeaders.Value())
-	extra := newExtra(req.Header, a.extraHeaderPrefixes)
+	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)
 		}
 	}

staging/src/k8s.io/apiserver/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
+}

staging/src/k8s.io/apiserver/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) {
+	for _, allowedCommonName := range a.allowedCommonNames.Value() {
-		return nil
+		if allowedCommonName == subject.CommonName {
+			return nil
+		}
 	}
 	return fmt.Errorf("x509: subject with cn=%s is not in the allowed list", subject.CommonName)
 }

staging/src/k8s.io/apiserver/pkg/server/options/BUILD

@@ -49,6 +49,7 @@ go_library(
         "//staging/src/k8s.io/apiserver/pkg/audit:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/audit/policy:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/request/headerrequest:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/request/x509:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authorization/authorizerfactory:go_default_library",
@@ -114,6 +115,7 @@ go_test(
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/apis/audit/v1:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/request/headerrequest:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/features:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/server:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage/storagebackend:go_default_library",

staging/src/k8s.io/apiserver/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,
+		UsernameHeaders:     headerrequest.StaticStringSlice(s.UsernameHeaders),
-		GroupHeaders:        s.GroupHeaders,
+		GroupHeaders:        headerrequest.StaticStringSlice(s.GroupHeaders),
-		ExtraHeaderPrefixes: s.ExtraHeaderPrefixes,
+		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
 }
 

staging/src/k8s.io/apiserver/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"},
+				UsernameHeaders:     headerrequest.StaticStringSlice{"x-remote-user"},
-				GroupHeaders:        []string{"x-remote-group"},
+				GroupHeaders:        headerrequest.StaticStringSlice{"x-remote-group"},
-				ExtraHeaderPrefixes: []string{"x-remote-extra-"},
+				ExtraHeaderPrefixes: headerrequest.StaticStringSlice{"x-remote-extra-"},
-				AllowedNames:        []string{"kube-aggregator"},
+				AllowedNames:        headerrequest.StaticStringSlice{"kube-aggregator"},
 			},
 		},
 		{
 			name: "test when ClientCAFile is not nil",
 			testOptions: &RequestHeaderAuthenticationOptions{
 				ClientCAFile:        "testdata/root.pem",
-				UsernameHeaders:     []string{"x-remote-user"},
+				UsernameHeaders:     headerrequest.StaticStringSlice{"x-remote-user"},
-				GroupHeaders:        []string{"x-remote-group"},
+				GroupHeaders:        headerrequest.StaticStringSlice{"x-remote-group"},
-				ExtraHeaderPrefixes: []string{"x-remote-extra-"},
+				ExtraHeaderPrefixes: headerrequest.StaticStringSlice{"x-remote-extra-"},
-				AllowedNames:        []string{"kube-aggregator"},
+				AllowedNames:        headerrequest.StaticStringSlice{"kube-aggregator"},
 			},
 			expectConfig: &authenticatorfactory.RequestHeaderConfig{
-				UsernameHeaders:     []string{"x-remote-user"},
+				UsernameHeaders:     headerrequest.StaticStringSlice{"x-remote-user"},
-				GroupHeaders:        []string{"x-remote-group"},
+				GroupHeaders:        headerrequest.StaticStringSlice{"x-remote-group"},
-				ExtraHeaderPrefixes: []string{"x-remote-extra-"},
+				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"},
 			},
 		},
 	}