support empty requestheader-allowed-names

see kubevirt#1295 Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
mhenriks · Jul 14, 2020 · 2d17cb1 · 2d17cb1
1 parent dd0b5f6
commit 2d17cb1
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 12 deletions.
diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go
@@ -197,15 +197,6 @@ func (app *cdiAPIApp) getKeysAndCerts() error {
 func (app *cdiAPIApp) getTLSConfig() (*tls.Config, error) {
 	authConfig := app.authConfigWatcher.GetAuthConfig()
 
-	validName := func(name string) bool {
-		for _, n := range authConfig.AllowedNames {
-			if n == name {
-				return true
-			}
-		}
-		return false
-	}
-
 	cert, err := app.certWarcher.GetCertificate(nil)
 	if err != nil {
 		return nil, err
@@ -220,7 +211,7 @@ func (app *cdiAPIApp) getTLSConfig() (*tls.Config, error) {
 				return nil
 			}
 			for i := range verifiedChains {
-				if validName(verifiedChains[i][0].Subject.CommonName) {
+				if authConfig.ValidateName(verifiedChains[i][0].Subject.CommonName) {
 					return nil
 				}
 			}

diff --git a/pkg/apiserver/auth-config.go b/pkg/apiserver/auth-config.go
@@ -64,6 +64,19 @@ type authConfigWatcher struct {
 	mutex  sync.RWMutex
 }
 
+// ValidateName checks if name is allowed
+func (ac *AuthConfig) ValidateName(name string) bool {
+	klog.V(3).Infof("Validating CN: %s", name)
+	for _, n := range ac.AllowedNames {
+		if n == name {
+			return true
+		}
+	}
+	// no allowed names means anyone is allowed
+	// https://kubernetes.io/docs/tasks/extend-kubernetes/configure-aggregation-layer/#kubernetes-apiserver-client-authentication
+	return len(ac.AllowedNames) == 0
+}
+
 // NewAuthConfigWatcher crates a new authConfigWatcher
 func NewAuthConfigWatcher(client kubernetes.Interface, stopCh <-chan struct{}) AuthConfigWatcher {
 	informerFactory := informers.NewFilteredSharedInformerFactory(client,

diff --git a/pkg/apiserver/auth-config_test.go b/pkg/apiserver/auth-config_test.go
@@ -21,13 +21,15 @@ package apiserver
 
 import (
 	"crypto/tls"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
 	"net/http"
 	"net/http/httptest"
 	"reflect"
 	"time"
 
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -65,6 +67,12 @@ func getAPIServerConfigMap() *corev1.ConfigMap {
 	}
 }
 
+func getAPIServerConfigMapNoAllowedNames() *corev1.ConfigMap {
+	cm := getAPIServerConfigMap()
+	cm.Data["requestheader-allowed-names"] = "[]"
+	return cm
+}
+
 func verifyAuthConfig(cm *corev1.ConfigMap, authConfig *AuthConfig) {
 	if !reflect.DeepEqual([]byte(cm.Data["client-ca-file"]), authConfig.ClientCABytes) {
 		Fail("client-ca-file not stored correctly")
@@ -184,4 +192,20 @@ var _ = Describe("Auth config tests", func() {
 			Fail("Client cert pools do not match")
 		}
 	})
+
+	DescribeTable("Validate client CN", func(f func() *corev1.ConfigMap, name string, allowed bool) {
+		ch := make(chan struct{})
+		kubeobjects := []runtime.Object{}
+		kubeobjects = append(kubeobjects, f())
+
+		client := k8sfake.NewSimpleClientset(kubeobjects...)
+		authConfigWatcher := NewAuthConfigWatcher(client, ch)
+
+		result := authConfigWatcher.GetAuthConfig().ValidateName(name)
+		Expect(result).To(Equal(allowed))
+	},
+		Entry("with allowed names", getAPIServerConfigMap, "front-proxy-client", true),
+		Entry("without allowed names", getAPIServerConfigMapNoAllowedNames, "front-proxy-client", true),
+		Entry("with allowed names", getAPIServerConfigMap, "foobar", false),
+	)
 })