From 758f2ce44f82d68a1a67765823179c3f743e199d Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 12 Nov 2019 12:44:53 -0500 Subject: [PATCH] allow individual ca bundles to be empty in union --- .../pkg/server/dynamiccertificates/tlsconfig.go | 9 ++++----- .../pkg/server/dynamiccertificates/tlsconfig_test.go | 6 ------ .../pkg/server/dynamiccertificates/union_content.go | 4 +++- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig.go index 0c428ad4aa7..78e094a5d81 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig.go @@ -103,10 +103,9 @@ func (c *DynamicServingCertificateController) newTLSContent() (*dynamicCertifica if c.clientCA != nil { currClientCABundle := c.clientCA.CurrentCABundleContent() - // don't remove all content. The value was configured at one time, so continue using that. - if len(currClientCABundle) == 0 { - return nil, fmt.Errorf("not loading an empty client ca bundle from %q", c.clientCA.Name()) - } + // we allow removing all client ca bundles because the server is still secure when this happens. it just means + // that there isn't a hint to clients about which client-cert to used. this happens when there is no client-ca + // yet known for authentication, which can happen in aggregated apiservers and some kube-apiserver deployment modes. newContent.clientCA = caBundleContent{caBundle: currClientCABundle} } @@ -152,7 +151,7 @@ func (c *DynamicServingCertificateController) syncCerts() error { newClientCAPool := x509.NewCertPool() newClientCAs, err := cert.ParseCertsPEM(newContent.clientCA.caBundle) if err != nil { - return fmt.Errorf("unable to load client CA file: %v", err) + return fmt.Errorf("unable to load client CA file %q: %v", string(newContent.clientCA.caBundle), err) } for i, cert := range newClientCAs { klog.V(2).Infof("loaded client CA [%d/%q]: %s", i, c.clientCA.Name(), GetHumanCertDetail(cert)) diff --git a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig_test.go b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig_test.go index 5989321d081..6503d197c9d 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/tlsconfig_test.go @@ -99,12 +99,6 @@ func TestNewStaticCertKeyContent(t *testing.T) { sniCerts: []sniCertKeyContent{{certKeyContent: certKeyContent{cert: serverCert, key: serverKey}, sniNames: []string{"foo"}}}, }, }, - { - name: "missingCA", - clientCA: &staticCAContent{name: "test-ca", caBundle: &caBundleAndVerifier{caBundle: []byte("")}}, - expected: nil, - expectedErr: `not loading an empty client ca bundle from "test-ca"`, - }, { name: "nil", expected: &dynamicCertificateContent{clientCA: caBundleContent{}, servingCert: certKeyContent{}}, 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 4a37ee00f97..89e19ea5a3a 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 @@ -48,7 +48,9 @@ func (c unionCAContent) Name() string { func (c unionCAContent) CurrentCABundleContent() []byte { caBundles := [][]byte{} for _, curr := range c { - caBundles = append(caBundles, curr.CurrentCABundleContent()) + if currCABytes := curr.CurrentCABundleContent(); len(currCABytes) > 0 { + caBundles = append(caBundles, []byte(strings.TrimSpace(string(currCABytes)))) + } } return bytes.Join(caBundles, []byte("\n"))