diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/requestheader.go b/staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/requestheader.go index eb39252cbe3..b5aa1c524a7 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/requestheader.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/requestheader.go @@ -44,5 +44,5 @@ type CAContentProvider interface { // CurrentCABundleContent provides ca bundle byte content CurrentCABundleContent() []byte // VerifyOptions provides VerifyOptions for authenticators - VerifyOptions() x509.VerifyOptions + VerifyOptions() (x509.VerifyOptions, bool) } diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/verify_options.go b/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/verify_options.go index 78992580acc..462eb4cc95f 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/verify_options.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/verify_options.go @@ -25,8 +25,8 @@ import ( // StaticVerifierFn is a VerifyOptionFunc that always returns the same value. This allows verify options that cannot change. func StaticVerifierFn(opts x509.VerifyOptions) VerifyOptionFunc { - return func() x509.VerifyOptions { - return opts + return func() (x509.VerifyOptions, bool) { + return opts, true } } diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go b/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go index 77a4711dca1..da3ef45d26c 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go @@ -83,8 +83,10 @@ func (f UserConversionFunc) User(chain []*x509.Certificate) (*authenticator.Resp } // VerifyOptionFunc is function which provides a shallow copy of the VerifyOptions to the authenticator. This allows -// for cases where the options (particularly the CAs) can change. -type VerifyOptionFunc func() x509.VerifyOptions +// for cases where the options (particularly the CAs) can change. If the bool is false, then the returned VerifyOptions +// are ignored and the authenticator will express "no opinion". This allows a clear signal for cases where a CertPool +// is eventually expected, but not currently present. +type VerifyOptionFunc func() (x509.VerifyOptions, bool) // Authenticator implements request.Authenticator by extracting user info from verified client certificates type Authenticator struct { @@ -111,7 +113,11 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (*authenticator.R } // Use intermediates, if provided - optsCopy := a.verifyOptionsFn() + optsCopy, ok := a.verifyOptionsFn() + // if there are intentionally no verify options, then we cannot authenticate this request + if !ok { + return nil, false, nil + } if optsCopy.Intermediates == nil && len(req.TLS.PeerCertificates) > 1 { optsCopy.Intermediates = x509.NewCertPool() for _, intermediate := range req.TLS.PeerCertificates[1:] { @@ -169,7 +175,11 @@ func (a *Verifier) AuthenticateRequest(req *http.Request) (*authenticator.Respon } // Use intermediates, if provided - optsCopy := a.verifyOptionsFn() + optsCopy, ok := a.verifyOptionsFn() + // if there are intentionally no verify options, then we cannot authenticate this request + if !ok { + return nil, false, nil + } if optsCopy.Intermediates == nil && len(req.TLS.PeerCertificates) > 1 { optsCopy.Intermediates = x509.NewCertPool() for _, intermediate := range req.TLS.PeerCertificates[1:] { diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/client_ca.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/client_ca.go index 6a881aa4166..4348fa38784 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/client_ca.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/client_ca.go @@ -29,7 +29,7 @@ type CAContentProvider interface { // the value. By the time you get here, you should always be returning a value that won't fail. CurrentCABundleContent() []byte // VerifyOptions provides VerifyOptions for authenticators - VerifyOptions() x509.VerifyOptions + VerifyOptions() (x509.VerifyOptions, bool) } // dynamicCertificateContent holds the content that overrides the baseTLSConfig diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/dynamic_cafile_content.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/dynamic_cafile_content.go index bb0ebd5537d..09b5e616a43 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/dynamic_cafile_content.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/dynamic_cafile_content.go @@ -215,8 +215,13 @@ func (c *DynamicFileCAContent) CurrentCABundleContent() (cabundle []byte) { } // VerifyOptions provides verifyoptions compatible with authenticators -func (c *DynamicFileCAContent) VerifyOptions() x509.VerifyOptions { - return c.caBundle.Load().(*caBundleAndVerifier).verifyOptions +func (c *DynamicFileCAContent) VerifyOptions() (x509.VerifyOptions, bool) { + uncastObj := c.caBundle.Load() + if uncastObj == nil { + return x509.VerifyOptions{}, false + } + + return uncastObj.(*caBundleAndVerifier).verifyOptions, true } // newVerifyOptions creates a new verification func from a file. It reads the content and then fails. diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/static_content.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/static_content.go index dffee8e51b9..239610df647 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/static_content.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/static_content.go @@ -66,8 +66,8 @@ func (c *staticCAContent) CurrentCABundleContent() (cabundle []byte) { return c.caBundle.caBundle } -func (c *staticCAContent) VerifyOptions() x509.VerifyOptions { - return c.caBundle.verifyOptions +func (c *staticCAContent) VerifyOptions() (x509.VerifyOptions, bool) { + return c.caBundle.verifyOptions, true } type staticCertKeyContent struct { diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/union_content.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/union_content.go index ef63219cfa5..4a37ee00f97 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/union_content.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/union_content.go @@ -55,7 +55,12 @@ func (c unionCAContent) CurrentCABundleContent() []byte { } // CurrentCABundleContent provides ca bundle byte content -func (c unionCAContent) VerifyOptions() x509.VerifyOptions { +func (c unionCAContent) VerifyOptions() (x509.VerifyOptions, bool) { + currCABundle := c.CurrentCABundleContent() + if len(currCABundle) == 0 { + return x509.VerifyOptions{}, false + } + // TODO make more efficient. This isn't actually used in any of our mainline paths. It's called to build the TLSConfig // TODO on file changes, but the actual authentication runs against the individual items, not the union. ret, err := newCABundleAndVerifier(c.Name(), c.CurrentCABundleContent()) @@ -64,7 +69,7 @@ func (c unionCAContent) VerifyOptions() x509.VerifyOptions { panic(err) } - return ret.verifyOptions + return ret.verifyOptions, true } // AddListener adds a listener to be notified when the CA content changes.