diff --git a/pkg/activator/certificate/cache.go b/pkg/activator/certificate/cache.go index 4ece6408c63c..967b0dad9a78 100644 --- a/pkg/activator/certificate/cache.go +++ b/pkg/activator/certificate/cache.go @@ -26,21 +26,27 @@ import ( "go.uber.org/zap" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" v1 "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/tools/cache" + "knative.dev/networking/pkg/apis/networking" + "knative.dev/pkg/reconciler" "knative.dev/networking/pkg/certificates" netcfg "knative.dev/networking/pkg/config" "knative.dev/pkg/controller" - secretinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret" + nsconfigmapinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/configmap" + nssecretinformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret" "knative.dev/pkg/logging" "knative.dev/pkg/system" ) // CertCache caches certificates and CA pool. type CertCache struct { - secretInformer v1.SecretInformer - logger *zap.SugaredLogger + secretInformer v1.SecretInformer + configmapInformer v1.ConfigMapInformer + logger *zap.SugaredLogger certificate *tls.Certificate TLSConf tls.Config @@ -50,22 +56,25 @@ type CertCache struct { // NewCertCache creates and starts the certificate cache that watches Activators certificate. func NewCertCache(ctx context.Context) (*CertCache, error) { - secretInformer := secretinformer.Get(ctx) + nsSecretInformer := nssecretinformer.Get(ctx) + nsConfigmapInformer := nsconfigmapinformer.Get(ctx) cr := &CertCache{ - secretInformer: secretInformer, - logger: logging.FromContext(ctx), + secretInformer: nsSecretInformer, + configmapInformer: nsConfigmapInformer, + logger: logging.FromContext(ctx), } secret, err := cr.secretInformer.Lister().Secrets(system.Namespace()).Get(netcfg.ServingRoutingCertName) if err != nil { - return nil, fmt.Errorf("failed to get activator certificate, secret %s/%s was not found: %w. Enabling system-internal-tls requires the secret to be present and populated with a valid certificate and CA", + return nil, fmt.Errorf("failed to get activator certificate, secret %s/%s was not found: %w. Enabling system-internal-tls requires the secret to be present and populated with a valid certificate", system.Namespace(), netcfg.ServingRoutingCertName, err) } - cr.updateCache(secret) + cr.updateCertificate(secret) + cr.updateTrustPool() - secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + nsSecretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), netcfg.ServingRoutingCertName), Handler: cache.ResourceEventHandlerFuncs{ UpdateFunc: cr.handleCertificateUpdate, @@ -73,45 +82,112 @@ func NewCertCache(ctx context.Context) (*CertCache, error) { }, }) + nsConfigmapInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: reconciler.ChainFilterFuncs( + reconciler.LabelExistsFilterFunc(networking.TrustBundleLabelKey), + ), + Handler: controller.HandleAll(func(obj interface{}) { + cr.updateTrustPool() + }), + }) + return cr, nil } func (cr *CertCache) handleCertificateAdd(added interface{}) { if secret, ok := added.(*corev1.Secret); ok { - cr.updateCache(secret) + cr.updateCertificate(secret) + cr.updateTrustPool() } } -func (cr *CertCache) updateCache(secret *corev1.Secret) { +func (cr *CertCache) handleCertificateUpdate(_, new interface{}) { + cr.handleCertificateAdd(new) + cr.updateTrustPool() +} + +func (cr *CertCache) updateCertificate(secret *corev1.Secret) { cr.certificatesMux.Lock() defer cr.certificatesMux.Unlock() cert, err := tls.X509KeyPair(secret.Data[certificates.CertName], secret.Data[certificates.PrivateKeyName]) if err != nil { - cr.logger.Warnw("failed to parse secret", zap.Error(err)) + cr.logger.Warnf("failed to parse certificate in secret %s/%s: %v", secret.Namespace, secret.Name, zap.Error(err)) return } cr.certificate = &cert +} +// CA can optionally be in `ca.crt` in the `routing-serving-certs` secret +// and/or configured using a trust-bundle via ConfigMap that has the defined label `knative-ca-trust-bundle`. +func (cr *CertCache) updateTrustPool() { pool := x509.NewCertPool() - block, _ := pem.Decode(secret.Data[certificates.CaCertName]) - ca, err := x509.ParseCertificate(block.Bytes) - if err != nil { - cr.logger.Warnw("failed to parse CA", zap.Error(err)) - return - } - pool.AddCert(ca) + + cr.addSecretCAIfPresent(pool) + cr.addTrustBundles(pool) + + // Use the trust pool in upstream TLS context + cr.certificatesMux.Lock() + defer cr.certificatesMux.Unlock() cr.TLSConf.RootCAs = pool cr.TLSConf.ServerName = certificates.LegacyFakeDnsName cr.TLSConf.MinVersion = tls.VersionTLS13 } -func (cr *CertCache) handleCertificateUpdate(_, new interface{}) { - cr.handleCertificateAdd(new) +func (cr *CertCache) addSecretCAIfPresent(pool *x509.CertPool) { + secret, err := cr.secretInformer.Lister().Secrets(system.Namespace()).Get(netcfg.ServingRoutingCertName) + if err != nil { + cr.logger.Warnf("Failed to get secret %s/%s: %v", system.Namespace(), netcfg.ServingRoutingCertName, zap.Error(err)) + return + } + if len(secret.Data[certificates.CaCertName]) > 0 { + block, _ := pem.Decode(secret.Data[certificates.CaCertName]) + ca, err := x509.ParseCertificate(block.Bytes) + if err != nil { + cr.logger.Warnf("CA from Secret %s/%s[%s] is invalid and will be ignored: %v", + system.Namespace(), netcfg.ServingRoutingCertName, certificates.CaCertName, err) + } else { + pool.AddCert(ca) + } + } +} + +func (cr *CertCache) addTrustBundles(pool *x509.CertPool) { + selector, err := getLabelSelector(networking.TrustBundleLabelKey) + if err != nil { + cr.logger.Error("Failed to get label selector", zap.Error(err)) + return + } + cms, err := cr.configmapInformer.Lister().ConfigMaps(system.Namespace()).List(selector) + if err != nil { + cr.logger.Warnf("Failed to get ConfigMaps %s/%s with label %s: %v", system.Namespace(), + netcfg.ServingRoutingCertName, networking.TrustBundleLabelKey, zap.Error(err)) + return + } + + for _, cm := range cms { + for _, bundle := range cm.Data { + ok := pool.AppendCertsFromPEM([]byte(bundle)) + if !ok { + cr.logger.Warnf("Failed to add CA bundle from ConfigMaps %s/%s as it contains invalid certificates. Bundle: %s", system.Namespace(), + cm.Name, bundle) + } + } + } } // GetCertificate returns the cached certificates. func (cr *CertCache) GetCertificate(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { return cr.certificate, nil } + +func getLabelSelector(label string) (labels.Selector, error) { + selector := labels.NewSelector() + req, err := labels.NewRequirement(label, selection.Exists, make([]string, 0)) + if err != nil { + return nil, err + } + selector = selector.Add(*req) + return selector, nil +} diff --git a/pkg/activator/certificate/cache_test.go b/pkg/activator/certificate/cache_test.go index 091800168f73..79ed5216cfe5 100644 --- a/pkg/activator/certificate/cache_test.go +++ b/pkg/activator/certificate/cache_test.go @@ -20,20 +20,21 @@ import ( "context" "crypto/tls" "crypto/x509" - "encoding/pem" "reflect" "testing" "time" - "go.uber.org/zap" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/cache" + "knative.dev/networking/pkg/apis/networking" + "knative.dev/pkg/reconciler" "knative.dev/networking/pkg/certificates" netcfg "knative.dev/networking/pkg/config" fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake" + fakeconfigmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/fake" fakesecretinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/secret/fake" "knative.dev/pkg/controller" "knative.dev/pkg/logging" @@ -41,31 +42,83 @@ import ( "knative.dev/pkg/system" ) -func fakeCertCache(ctx context.Context) *CertCache { - secretInformer := fakesecretinformer.Get(ctx) +func TestReconcile(t *testing.T) { - cr := &CertCache{ - secretInformer: secretInformer, - certificate: nil, - TLSConf: tls.Config{}, - logger: logging.FromContext(ctx), + tests := []struct { + name string + secret *corev1.Secret + configMap *corev1.ConfigMap + expectedPool *x509.CertPool + }{{ + name: "Valid CA only in secret", + secret: secret, + configMap: nil, + expectedPool: getPoolWithCerts(secret.Data[certificates.CaCertName]), + }, + { + name: "Valid CA only in configmap", + secret: func() *corev1.Secret { + s := secret.DeepCopy() + delete(s.Data, certificates.CaCertName) + return s + }(), + configMap: validConfigmap, + expectedPool: getPoolWithCerts(configmapCA), + }, + { + name: "Valid CA in both configmap and secret", + secret: secret, + configMap: validConfigmap, + expectedPool: getPoolWithCerts(secretCA, configmapCA), + }, + { + name: "Invalid CA in configmap", + secret: secret, + configMap: invalidConfigmap, + expectedPool: getPoolWithCerts(secretCA), + }, } - secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ - FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), netcfg.ServingRoutingCertName), - Handler: cache.ResourceEventHandlerFuncs{ - UpdateFunc: cr.handleCertificateUpdate, - AddFunc: cr.handleCertificateAdd, - }, - }) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Setup + ctx, cancel, informers := rtesting.SetupFakeContextWithCancel(t) + cr := fakeCertCache(ctx) + waitInformers, err := rtesting.RunAndSyncInformers(ctx, informers...) + if err != nil { + cancel() + t.Fatal("failed to start informers:", err) + } + t.Cleanup(func() { + cancel() + waitInformers() + }) - return cr -} + // Data preparation + if test.secret != nil { + fakekubeclient.Get(ctx).CoreV1().Secrets(system.Namespace()).Create(ctx, test.secret, metav1.CreateOptions{}) + fakesecretinformer.Get(ctx).Informer().GetIndexer().Add(test.secret) + } + if test.configMap != nil { + fakekubeclient.Get(ctx).CoreV1().ConfigMaps(system.Namespace()).Create(ctx, test.configMap, metav1.CreateOptions{}) + fakeconfigmapinformer.Get(ctx).Informer().GetIndexer().Add(test.configMap) + } -func TestReconcile(t *testing.T) { + // Verify + if err := wait.PollUntilContextTimeout(ctx, 10*time.Millisecond, 2*time.Second, true, func(context.Context) (bool, error) { + cr.certificatesMux.RLock() + defer cr.certificatesMux.RUnlock() + cert, _ := cr.GetCertificate(nil) + return cert != nil && cr.TLSConf.RootCAs.Equal(test.expectedPool), nil + }); err != nil { + t.Fatalf("Did not meet the expected state: %s, err: %v", test.name, err) + } + }) + } + + // Additional secret tests ctx, cancel, informers := rtesting.SetupFakeContextWithCancel(t) cr := fakeCertCache(ctx) - waitInformers, err := rtesting.RunAndSyncInformers(ctx, informers...) if err != nil { cancel() @@ -75,33 +128,9 @@ func TestReconcile(t *testing.T) { cancel() waitInformers() }) - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: netcfg.ServingRoutingCertName, - Namespace: system.Namespace(), - }, - Data: map[string][]byte{ - certificates.CaCertName: ca, - certificates.PrivateKeyName: tlsKey, - certificates.CertName: tlsCrt, - }, - } - fakekubeclient.Get(ctx).CoreV1().Secrets(system.Namespace()).Create(ctx, secret, metav1.CreateOptions{}) fakesecretinformer.Get(ctx).Informer().GetIndexer().Add(secret) - // Wait for the resources to be created and the handler is called. - if err := wait.PollUntilContextTimeout(ctx, 10*time.Millisecond, 2*time.Second, true, func(context.Context) (bool, error) { - // To access cert.Certificate, take a lock. - cr.certificatesMux.RLock() - defer cr.certificatesMux.RUnlock() - cert, _ := cr.GetCertificate(nil) - return cert != nil, nil - }); err != nil { - t.Fatal("timeout to get the secret:", err) - } - // Update cert and key but keep using old CA, then the error is expected. secret.Data[certificates.CertName] = newTLSCrt secret.Data[certificates.PrivateKeyName] = newTLSKey @@ -109,7 +138,6 @@ func TestReconcile(t *testing.T) { fakekubeclient.Get(ctx).CoreV1().Secrets(system.Namespace()).Update(ctx, secret, metav1.UpdateOptions{}) if err := wait.PollUntilContextTimeout(ctx, 10*time.Millisecond, 5*time.Second, true, func(context.Context) (bool, error) { - // To access cert.Certificate, take a lock. cr.certificatesMux.RLock() defer cr.certificatesMux.RUnlock() cert, err := cr.GetCertificate(nil) @@ -120,29 +148,58 @@ func TestReconcile(t *testing.T) { // Update CA, now the error is gone. secret.Data[certificates.CaCertName] = newCA - - pool := x509.NewCertPool() - block, _ := pem.Decode(secret.Data[certificates.CaCertName]) - ca, err := x509.ParseCertificate(block.Bytes) - if err != nil { - cr.logger.Warnw("Failed to parse CA: %v", zap.Error(err)) - return - } - - pool.AddCert(ca) - + expectedPool := getPoolWithCerts(secret.Data[certificates.CaCertName]) fakekubeclient.Get(ctx).CoreV1().Secrets(system.Namespace()).Update(ctx, secret, metav1.UpdateOptions{}) if err := wait.PollUntilContextTimeout(ctx, 10*time.Millisecond, 10*time.Second, true, func(context.Context) (bool, error) { - // To access cr.TLSConf.RootCAs, take a lock. cr.certificatesMux.RLock() defer cr.certificatesMux.RUnlock() - return err == nil && pool.Equal(cr.TLSConf.RootCAs), nil + return cr.TLSConf.RootCAs.Equal(expectedPool), nil }); err != nil { t.Fatalf("timeout to update the cert: %v", err) } } -var ca = []byte( +func fakeCertCache(ctx context.Context) *CertCache { + secretInformer := fakesecretinformer.Get(ctx) + configmapInformer := fakeconfigmapinformer.Get(ctx) + + cr := &CertCache{ + secretInformer: secretInformer, + configmapInformer: configmapInformer, + certificate: nil, + TLSConf: tls.Config{}, + logger: logging.FromContext(ctx), + } + + secretInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: controller.FilterWithNameAndNamespace(system.Namespace(), netcfg.ServingRoutingCertName), + Handler: cache.ResourceEventHandlerFuncs{ + UpdateFunc: cr.handleCertificateUpdate, + AddFunc: cr.handleCertificateAdd, + }, + }) + + configmapInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{ + FilterFunc: reconciler.ChainFilterFuncs( + reconciler.LabelExistsFilterFunc(networking.TrustBundleLabelKey), + ), + Handler: controller.HandleAll(func(obj interface{}) { + cr.updateTrustPool() + }), + }) + + return cr +} + +func getPoolWithCerts(certs ...[]byte) *x509.CertPool { + pool := x509.NewCertPool() + for _, c := range certs { + pool.AppendCertsFromPEM(c) + } + return pool +} + +var secretCA = []byte( `-----BEGIN CERTIFICATE----- MIIDRzCCAi+gAwIBAgIQQo5HxzWURsnoOnFP1U2vKjANBgkqhkiG9w0BAQsFADAu MRQwEgYDVQQKEwtrbmF0aXZlLmRldjEWMBQGA1UEAxMNY29udHJvbC1wbGFuZTAe @@ -164,6 +221,27 @@ stXnfHpI3C14v4t00tZ9JHVNIeyGapXmKpq8rys5bnDWoUv62I+W/s4Kx3jUx2pO yycz8WDCfsgD/kiJ3GiITxBH8oHotFZdjFnF -----END CERTIFICATE-----`) +var configmapCA = []byte( + `-----BEGIN CERTIFICATE----- +MIIDDTCCAfWgAwIBAgIQMQuip05h7NLQq2TB+j9ZmTANBgkqhkiG9w0BAQsFADAW +MRQwEgYDVQQDEwtrbmF0aXZlLmRldjAeFw0yMzExMjIwOTAwNDhaFw0yNDAyMjAw +OTAwNDhaMBYxFDASBgNVBAMTC2tuYXRpdmUuZGV2MIIBIjANBgkqhkiG9w0BAQEF +AAOCAQ8AMIIBCgKCAQEA3clC3CV7sy0TpUKNuTku6QmP9z8JUCbLCPCLACCUc1zG +FEokqOva6TakgvAntXLkB3TEsbdCJlNm6qFbbko6DBfX6rEggqZs40x3/T+KH66u +4PvMT3fzEtaMJDK/KQOBIvVHrKmPkvccUYK/qWY7rgBjVjjLVSJrCn4dKaEZ2JNr +Fd0KNnaaW/dP9/FvviLqVJvHnTMHH5qyRRr1kUGTrc8njRKwpHcnUdauiDoWRKxo +Zlyy+MhQfdbbyapX984WsDjCvrDXzkdGgbRNAf+erl6yUm6pHpQhyFFo/zndx6Uq +QXA7jYvM2M3qCnXmaFowidoLDsDyhwoxD7WT8zur/QIDAQABo1cwVTAOBgNVHQ8B +Af8EBAMCAgQwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDwYDVR0TAQH/BAUwAwEB/zAd +BgNVHQ4EFgQU7p4VuECNOcnrP9ulOjc4J37Q2VUwDQYJKoZIhvcNAQELBQADggEB +AAv26Vnk+ptQrppouF7yHV8fZbfnehpm07HIZkmnXO2vAP+MZJDNrHjy8JAVzXjt ++OlzqAL0cRQLsUptB0btoJuw23eq8RXgJo05OLOPQ2iGNbAATQh2kLwBWd/CMg+V +KJ4EIEpF4dmwOohsNR6xa/JoArIYH0D7gh2CwjrdGZr/tq1eMSL+uZcuX5OiE44A +2oXF9/jsqerOcH7QUMejSnB8N7X0LmUvH4jAesQgr7jo1JTOBs7GF6wb+U76NzFa +8ms2iAWhoplQ+EHR52wffWb0k6trXspq4O6v/J+nq9Ky3vC36so+G1ZFkMhCdTVJ +ZmrBsSMWeT2l07qeei2UFRU= +-----END CERTIFICATE-----`) + var tlsCrt = []byte( `-----BEGIN CERTIFICATE----- MIIDkzCCAnugAwIBAgIQe0pM5sKLGeu6+P0xpW+TJTANBgkqhkiG9w0BAQsFADAu @@ -288,3 +366,41 @@ uNLDuUL4EPGk084uoa/rwQxUDwWQ05aw81c/Q0ssPeyekgLNfet4HX4lzBDJWZEQ FUu9LuwG/tVRBIecvo/IcUuQ1/UObbRAXpp0Y8aO56UVeBvOb9bG2/wjRJmrgR+1 vCCFsBlglA== -----END CERTIFICATE-----`) + +var secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: netcfg.ServingRoutingCertName, + Namespace: system.Namespace(), + }, + Data: map[string][]byte{ + certificates.CaCertName: secretCA, + certificates.PrivateKeyName: tlsKey, + certificates.CertName: tlsCrt, + }, +} + +var validConfigmap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + Namespace: system.Namespace(), + Labels: map[string]string{ + networking.TrustBundleLabelKey: "true", + }, + }, + Data: map[string]string{ + certificates.CertName: string(configmapCA), + }, +} + +var invalidConfigmap = &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid", + Namespace: system.Namespace(), + Labels: map[string]string{ + networking.TrustBundleLabelKey: "true", + }, + }, + Data: map[string]string{ + certificates.CertName: "not a CA", + }, +} diff --git a/vendor/knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/configmap/configmap.go b/vendor/knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/configmap/configmap.go new file mode 100644 index 000000000000..fd724cad371a --- /dev/null +++ b/vendor/knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/configmap/configmap.go @@ -0,0 +1,50 @@ +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package configmap + +import ( + "context" + + v1 "k8s.io/client-go/informers/core/v1" + "knative.dev/pkg/controller" + "knative.dev/pkg/injection" + "knative.dev/pkg/injection/clients/namespacedkube/informers/factory" + "knative.dev/pkg/logging" +) + +func init() { + injection.Default.RegisterInformer(withInformer) +} + +// Key is used for associating the Informer inside the context.Context. +type Key struct{} + +func withInformer(ctx context.Context) (context.Context, controller.Informer) { + f := factory.Get(ctx) + inf := f.Core().V1().ConfigMaps() + return context.WithValue(ctx, Key{}, inf), inf.Informer() +} + +// Get extracts the typed informer from the context. +func Get(ctx context.Context) v1.ConfigMapInformer { + untyped := ctx.Value(Key{}) + if untyped == nil { + logging.FromContext(ctx).Panic( + "Unable to fetch k8s.io/client-go/informers/core/v1.ConfigMapInformer from context.") + } + return untyped.(v1.ConfigMapInformer) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 167797fe8afb..ac380af61a2b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1430,6 +1430,7 @@ knative.dev/pkg/hash knative.dev/pkg/injection knative.dev/pkg/injection/clients/dynamicclient knative.dev/pkg/injection/clients/dynamicclient/fake +knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/configmap knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret knative.dev/pkg/injection/clients/namespacedkube/informers/factory knative.dev/pkg/injection/sharedmain