Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow a verifyoptionsfunc to indicate that no certpool is available #84864

Merged
merged 2 commits into from Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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)
}
Expand Up @@ -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
}
}

Expand Down
Expand Up @@ -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 {
Expand All @@ -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:] {
Expand Down Expand Up @@ -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:] {
Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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.
Expand Down
Expand Up @@ -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 {
Expand Down
Expand Up @@ -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 {
liggitt marked this conversation as resolved.
Show resolved Hide resolved
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())
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion test/integration/scheduler/extender_test.go
Expand Up @@ -27,7 +27,7 @@ import (
"testing"
"time"

"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down
4 changes: 4 additions & 0 deletions test/integration/scheduler/taint_test.go
Expand Up @@ -85,6 +85,8 @@ func TestTaintNodeByCondition(t *testing.T) {
defer algorithmprovider.ApplyFeatureGates()()

context = initTestScheduler(t, context, false, nil)
defer cleanupTest(t, context)

cs := context.clientSet
informers := context.informerFactory
nsName := context.ns.Name
Expand Down Expand Up @@ -655,6 +657,8 @@ func TestTaintBasedEvictions(t *testing.T) {
for i, test := range tests {
t.Run(test.name, func(t *testing.T) {
context := initTestMaster(t, "taint-based-evictions", admission)
defer cleanupTest(t, context)

// Build clientset and informers for controllers.
externalClientset := kubernetes.NewForConfigOrDie(&restclient.Config{
QPS: -1,
Expand Down