Merge pull request #167 from deads2k/inject-big

bug 1981498: add vulnerable legacy injector to allow for upgrade clusters to use ...

pkg/controller/api/api.go

@@ -20,6 +20,10 @@ const (
 	InjectCABundleAnnotationName      = "service.beta.openshift.io/inject-cabundle"
 	AlphaInjectCABundleAnnotationName = "service.alpha.openshift.io/inject-cabundle"
 	InjectionDataKey                  = "service-ca.crt"
+
+	// VulnerableLegacyInjectCABundleAnnotationName is only honored on configmaps intended for bound token injection in
+	// migrated/upgraded clusters that have not switched to a tighter verification bundle.
+	VulnerableLegacyInjectCABundleAnnotationName = "service.alpha.openshift.io/inject-vulnerable-legacy-cabundle"
 )
 
 // Annotations on service

pkg/controller/cabundleinjector/configmap.go

@@ -3,6 +3,7 @@ package cabundleinjector
 import (
 	"context"
 
+	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kcoreclient "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -17,6 +18,8 @@ type configMapCABundleInjector struct {
 	client   kcoreclient.ConfigMapsGetter
 	lister   listers.ConfigMapLister
 	caBundle string
+
+	filterFn func(configMap *corev1.ConfigMap) bool
 }
 
 func newConfigMapInjectorConfig(config *caBundleInjectorConfig) controllerConfig {
@@ -40,6 +43,46 @@ func newConfigMapInjectorConfig(config *caBundleInjectorConfig) controllerConfig
 	}
 }
 
+// newVulnerableLegacyConfigMapInjectorConfig injects a configmap that contains more certificates than are required to
+// verify service serving certificates.
+func newVulnerableLegacyConfigMapInjectorConfig(config *caBundleInjectorConfig) controllerConfig {
+	informer := config.kubeInformers.Core().V1().ConfigMaps()
+
+	syncer := &configMapCABundleInjector{
+		client:   config.kubeClient.CoreV1(),
+		lister:   informer.Lister(),
+		caBundle: string(config.legacyVulnerableCABundle),
+
+		// only set content for the one configmap that we are required to for backward compatibility.  This limits the
+		// future potential for abuse.
+		filterFn: func(configMap *corev1.ConfigMap) bool {
+			// if either of the preferred annotations are present, the legacy injector needs to stand down and allow
+			// the preferred injector to take over.  This avoids dueling updates.
+			if _, ok := configMap.Annotations[api.InjectCABundleAnnotationName]; ok {
+				return false
+			}
+			if _, ok := configMap.Annotations[api.AlphaInjectCABundleAnnotationName]; ok {
+				return false
+			}
+
+			if configMap.Name == "openshift-service-ca.crt" {
+				return true
+			}
+			return false
+		},
+	}
+
+	return controllerConfig{
+		name:     "LegacyVulnerableConfigMapCABundleInjector",
+		sync:     syncer.Sync,
+		informer: informer.Informer(),
+		annotationsChecker: annotationsChecker(
+			api.VulnerableLegacyInjectCABundleAnnotationName,
+		),
+		namespaced: true,
+	}
+}
+
 func (bi *configMapCABundleInjector) Sync(ctx context.Context, syncCtx factory.SyncContext) error {
 	namespace, name := namespacedObjectFromQueueKey(syncCtx.QueueKey())
 
@@ -50,6 +93,10 @@ func (bi *configMapCABundleInjector) Sync(ctx context.Context, syncCtx factory.S
 		return err
 	}
 
+	if bi.filterFn != nil && !bi.filterFn(configMap) {
+		return nil
+	}
+
 	// skip updating when the CA bundle is already there
 	if data, ok := configMap.Data[api.InjectionDataKey]; ok &&
 		data == bi.caBundle && len(configMap.Data) == 1 {

pkg/controller/cabundleinjector/starter.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"os"
 	"strings"
 	"time"
 
@@ -25,6 +26,13 @@ type caBundleInjectorConfig struct {
 	caBundle      []byte
 	kubeClient    *kubernetes.Clientset
 	kubeInformers kubeinformers.SharedInformerFactory
+
+	// legacyVulnerableCABundle is a CA bundle which included more certificates than are needed for verifying service
+	// serving certificates.  This was addressed in new installs of 4.8, but migrated clusters continue to have the old
+	// content inside of their bound tokens for the service-ca.crt.
+	// This CA bundle should only be used for specifically named configmaps which explicitly indicate their desire.
+	// This makes it impossible for customers to use and being to rely upon.
+	legacyVulnerableCABundle []byte
 }
 
 type startInformersFunc func(stopChan <-chan struct{})
@@ -44,21 +52,36 @@ func StartCABundleInjector(ctx context.Context, controllerContext *controllercmd
 	// TODO(marun) Detect and respond to changes in this path rather than
 	// depending on the operator for redeployment
 	caBundleFile := "/var/run/configmaps/signing-cabundle/ca-bundle.crt"
-
 	caBundleContent, err := ioutil.ReadFile(caBundleFile)
 	if err != nil {
 		return err
 	}
 
+	// this construction matches what the old kube controller manager did. It added the entire ca.crt to the service-ca.crt.
+	vulnerableLegacyCABundleContent, err := ioutil.ReadFile(caBundleFile)
+	if err != nil {
+		return err
+	}
+	saTokenCAFile := "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
+	saTokenCABundleContent, err := ioutil.ReadFile(saTokenCAFile)
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	if len(saTokenCABundleContent) > 0 {
+		vulnerableLegacyCABundleContent = append(vulnerableLegacyCABundleContent, saTokenCABundleContent...)
+		vulnerableLegacyCABundleContent = append(vulnerableLegacyCABundleContent, []byte("\n")...)
+	}
+
 	client := kubernetes.NewForConfigOrDie(controllerContext.ProtoKubeConfig)
 	defaultResync := 20 * time.Minute
 	informers := kubeinformers.NewSharedInformerFactory(client, defaultResync)
 	injectorConfig := &caBundleInjectorConfig{
-		config:        controllerContext.ProtoKubeConfig,
-		defaultResync: defaultResync,
-		caBundle:      caBundleContent,
-		kubeClient:    client,
-		kubeInformers: informers,
+		config:                   controllerContext.ProtoKubeConfig,
+		defaultResync:            defaultResync,
+		caBundle:                 caBundleContent,
+		legacyVulnerableCABundle: vulnerableLegacyCABundleContent,
+		kubeClient:               client,
+		kubeInformers:            informers,
 	}
 
 	stopChan := ctx.Done()
@@ -69,7 +92,9 @@ func StartCABundleInjector(ctx context.Context, controllerContext *controllercmd
 		newCRDInjectorConfig,
 		newMutatingWebhookInjectorConfig,
 		newValidatingWebhookInjectorConfig,
+		newVulnerableLegacyConfigMapInjectorConfig, // this has to be kept for cluster migrated from before 4.7
 	}
+
 	injectionControllers := []factory.Controller{}
 	for _, configConstructor := range configConstructors {
 		ctlConfig := configConstructor(injectorConfig)

test/e2e/e2e_test.go

@@ -308,6 +308,27 @@ func checkConfigMapCABundleInjectionData(client *kubernetes.Clientset, configMap
 	return nil
 }
 
+func pollForConfigMapCAInjection(client *kubernetes.Clientset, configMapName, namespace string) error {
+	return wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) {
+		cm, err := client.CoreV1().ConfigMaps(namespace).Get(context.TODO(), configMapName, metav1.GetOptions{})
+		if err != nil && errors.IsNotFound(err) {
+			return false, nil
+		}
+		if err != nil {
+			return false, err
+		}
+
+		if len(cm.Data) != 1 {
+			return false, nil
+		}
+		_, ok := cm.Data[api.InjectionDataKey]
+		if !ok {
+			return false, nil
+		}
+		return true, nil
+	})
+}
+
 func pollForServiceServingSecretWithReturn(client *kubernetes.Clientset, secretName, namespace string) (*v1.Secret, error) {
 	var secret *v1.Secret
 	err := wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) {
@@ -1317,6 +1338,90 @@ func TestE2E(t *testing.T) {
 		}
 	})
 
+	// test vulnerable-legacy ca bundle injection configmap
+	t.Run("vulnerable-legacy-ca-bundle-injection-configmap", func(t *testing.T) {
+		ns, cleanup, err := createTestNamespace(t, adminClient, "test-"+randSeq(5))
+		if err != nil {
+			t.Fatalf("could not create test namespace: %v", err)
+		}
+		defer cleanup()
+
+		// names other than the one we need are never published to
+		neverPublished := &v1.ConfigMap{
+			TypeMeta: metav1.TypeMeta{},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:        "test-configmap-" + randSeq(5),
+				Annotations: map[string]string{api.VulnerableLegacyInjectCABundleAnnotationName: "true"},
+			},
+		}
+		_, err = adminClient.CoreV1().ConfigMaps(ns.Name).Create(context.TODO(), neverPublished, metav1.CreateOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+		// with this name, content should never be published.  We wait ten seconds
+		err = pollForConfigMapCAInjection(adminClient, neverPublished.Name, ns.Name)
+		if err != wait.ErrWaitTimeout {
+			t.Fatal(err)
+		}
+
+		publishedConfigMap := &v1.ConfigMap{
+			TypeMeta: metav1.TypeMeta{},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:        "openshift-service-ca.crt",
+				Annotations: map[string]string{api.VulnerableLegacyInjectCABundleAnnotationName: "true"},
+			},
+		}
+		publishedConfigMap, err = adminClient.CoreV1().ConfigMaps(ns.Name).Create(context.TODO(), publishedConfigMap, metav1.CreateOptions{})
+		// tolerate "already exists" to handle the case where we're running the e2e on a cluster that already has this
+		// configmap present and injected.
+		if err != nil && !errors.IsAlreadyExists(err) {
+			t.Fatal(err)
+		}
+		publishedConfigMap, err = adminClient.CoreV1().ConfigMaps(ns.Name).Get(context.TODO(), "openshift-service-ca.crt", metav1.GetOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// this one should be injected
+		err = pollForConfigMapCAInjection(adminClient, publishedConfigMap.Name, ns.Name)
+		if err != nil {
+			t.Fatal(err)
+		}
+		originalContent := publishedConfigMap.Data[api.InjectionDataKey]
+
+		_, hasNewStyleAnnotation := publishedConfigMap.Annotations[api.InjectCABundleAnnotationName]
+		if hasNewStyleAnnotation {
+			// add old injection to be sure only new is honored
+			publishedConfigMap.Annotations[api.VulnerableLegacyInjectCABundleAnnotationName] = "true"
+			publishedConfigMap, err = adminClient.CoreV1().ConfigMaps(ns.Name).Update(context.TODO(), publishedConfigMap, metav1.UpdateOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+		} else {
+			// hand-off to new injector
+			publishedConfigMap.Annotations[api.InjectCABundleAnnotationName] = "true"
+			publishedConfigMap, err = adminClient.CoreV1().ConfigMaps(ns.Name).Update(context.TODO(), publishedConfigMap, metav1.UpdateOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
+
+		// the content should now change pretty quick.  We sleep because it's easier than writing a new poll and I'm pressed for time
+		time.Sleep(5 * time.Second)
+		publishedConfigMap, err = adminClient.CoreV1().ConfigMaps(ns.Name).Get(context.TODO(), publishedConfigMap.Name, metav1.GetOptions{})
+
+		// if we changed the injection, we should see different content
+		if hasNewStyleAnnotation {
+			if publishedConfigMap.Data[api.InjectionDataKey] != originalContent {
+				t.Fatal("Content switch and it should not have.  The better ca bundle should win.")
+			}
+		} else {
+			if publishedConfigMap.Data[api.InjectionDataKey] == originalContent {
+				t.Fatal("Content did not update like it was supposed to.  The better ca bundle should win.")
+			}
+		}
+	})
+
 	t.Run("metrics", func(t *testing.T) {
 		promClient, err := newPrometheusClientForConfig(adminConfig)
 		if err != nil {