implement cluster-local-domain-tls in serving (#14610)

* use Knative certificates for Serving encryption

* Make namespace the owner of the Queue-Proxy certificate

* Lint error fixes

* Minor fixes

* cherry-pick #14706

* keep existing secret in upgrade/downgrade tests

* Rebase and update to upstream/main

* Review improvements

* Update net-istio

* Use a revision tracker to reconcile on KCerts

cmd/activator/main.go

@@ -167,8 +167,11 @@ func main() {
 	// At this moment activator with TLS does not disable HTTP.
 	// See also https://github.com/knative/serving/issues/12808.
 	if tlsEnabled {
-		logger.Info("Knative Internal TLS is enabled")
-		certCache = certificate.NewCertCache(ctx)
+		logger.Info("Knative system-internal-tls is enabled")
+		certCache, err = certificate.NewCertCache(ctx)
+		if err != nil {
+			logger.Fatalw("Failed to create certificate cache", zap.Error(err))
+		}
 		transport = pkgnet.NewProxyAutoTLSTransport(env.MaxIdleProxyConns, env.MaxIdleProxyConnsPerHost, certCache.TLSContext())
 	}
 

cmd/controller/main.go

@@ -20,7 +20,6 @@ import (
 	// The set of controllers this controller process runs.
 	"flag"
 
-	certificate "knative.dev/networking/pkg/certificates/reconciler"
 	"knative.dev/pkg/reconciler"
 	"knative.dev/pkg/signals"
 	"knative.dev/serving/pkg/reconciler/configuration"
@@ -32,18 +31,11 @@ import (
 	"knative.dev/serving/pkg/reconciler/serverlessservice"
 	"knative.dev/serving/pkg/reconciler/service"
 
-	// This defines the shared main for injected controllers.
-	filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"
 	"knative.dev/pkg/injection"
 	"knative.dev/pkg/injection/sharedmain"
-	"knative.dev/serving/pkg/networking"
 	"knative.dev/serving/pkg/reconciler/domainmapping"
 )
 
-const (
-	secretLabelNamePostfix = "-ctrl"
-)
-
 var ctors = []injection.ControllerConstructor{
 	configuration.NewController,
 	labeler.NewController,
@@ -53,7 +45,6 @@ var ctors = []injection.ControllerConstructor{
 	service.NewController,
 	gc.NewController,
 	nscert.NewController,
-	certificate.NewControllerFactory(networking.ServingCertName),
 	domainmapping.NewController,
 }
 
@@ -62,7 +53,5 @@ func main() {
 		"reconciliation-timeout", reconciler.DefaultTimeout,
 		"The amount of time to give each reconciliation of a resource to complete before its context is canceled.")
 
-	labelName := networking.ServingCertName + secretLabelNamePostfix
-	ctx := filteredFactory.WithSelectors(signals.NewContext(), labelName)
-	sharedmain.MainWithContext(ctx, "controller", ctors...)
+	sharedmain.MainWithContext(signals.NewContext(), "controller", ctors...)
 }

config/core/300-knativecertificate.yaml

@@ -0,0 +1,14 @@
+apiVersion: networking.internal.knative.dev/v1alpha1
+kind: Certificate
+metadata:
+  annotations:
+    networking.knative.dev/certificate.class: cert-manager.certificate.networking.knative.dev
+  labels:
+    networking.knative.dev/certificate-type: system-internal
+  name: routing-serving-certs
+  namespace: knative-serving
+spec:
+  dnsNames:
+    - kn-routing
+    - data-plane.knative.dev # for reverse-compatibility with net-* implementations that do not work with multi-SANs
+  secretName: routing-serving-certs

pkg/activator/certificate/cache.go

@@ -21,6 +21,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
+	"fmt"
 	"sync"
 
 	"go.uber.org/zap"
@@ -47,8 +48,8 @@ type CertCache struct {
 	certificatesMux sync.RWMutex
 }
 
-// NewCertCache starts secretInformer.
-func NewCertCache(ctx context.Context) *CertCache {
+// NewCertCache creates and starts the certificate cache that watches Activators certificate.
+func NewCertCache(ctx context.Context) (*CertCache, error) {
 	secretInformer := secretinformer.Get(ctx)
 
 	cr := &CertCache{
@@ -58,8 +59,8 @@ func NewCertCache(ctx context.Context) *CertCache {
 
 	secret, err := cr.secretInformer.Lister().Secrets(system.Namespace()).Get(netcfg.ServingRoutingCertName)
 	if err != nil {
-		cr.logger.Warnw("failed to get secret", zap.Error(err))
-		return 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",
+			system.Namespace(), netcfg.ServingRoutingCertName, err)
 	}
 
 	cr.updateCache(secret)
@@ -72,7 +73,7 @@ func NewCertCache(ctx context.Context) *CertCache {
 		},
 	})
 
-	return cr
+	return cr, nil
 }
 
 func (cr *CertCache) handleCertificateAdd(added interface{}) {

pkg/apis/serving/v1/route_lifecycle.go

@@ -195,11 +195,6 @@ const (
 	// RouteConditionCertificateProvisioned condition when it is set to True
 	// because external-domain-tls was not enabled.
 	ExternalDomainTLSNotEnabledMessage = "external-domain-tls is not enabled"
-
-	// TLSNotEnabledForClusterLocalMessage is the message which is set on the
-	// RouteConditionCertificateProvisioned condition when it is set to True
-	// because the domain is cluster-local.
-	TLSNotEnabledForClusterLocalMessage = "TLS is not enabled for cluster-local"
 )
 
 // MarkTLSNotEnabled sets RouteConditionCertificateProvisioned to true when

pkg/queue/certificate/watcher_test.go

@@ -17,9 +17,15 @@ limitations under the License.
 package certificate
 
 import (
+	"bytes"
 	"context"
+	"crypto/rand"
+	"crypto/rsa"
 	"crypto/tls"
 	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"math/big"
 	"os"
 	"testing"
 	"time"
@@ -94,26 +100,47 @@ func TestCertificateRotation(t *testing.T) {
 }
 
 func createAndSaveCertificate(san, dir string) error {
-	ca, err := certificates.CreateCACerts(1 * time.Hour)
-	if err != nil {
-		return err
+	cert := &x509.Certificate{
+		SerialNumber: big.NewInt(2019),
+		Subject: pkix.Name{
+			Organization: []string{"Knative"},
+		},
+		NotBefore:             time.Now(),
+		NotAfter:              time.Now().AddDate(10, 0, 0),
+		IsCA:                  false,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth},
+		KeyUsage:              x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
+		BasicConstraintsValid: true,
 	}
+	cert.DNSNames = []string{san}
 
-	caCert, caKey, err := ca.Parse()
+	pk, err := rsa.GenerateKey(rand.Reader, 4096)
 	if err != nil {
 		return err
 	}
 
-	cert, err := certificates.CreateCert(caKey, caCert, 1*time.Hour, san)
+	certBytes, err := x509.CreateCertificate(rand.Reader, cert, cert, &pk.PublicKey, pk)
 	if err != nil {
 		return err
 	}
 
-	if err := os.WriteFile(dir+"/"+certificates.CertName, cert.CertBytes(), 0644); err != nil {
+	caPEM := new(bytes.Buffer)
+	pem.Encode(caPEM, &pem.Block{
+		Type:  "CERTIFICATE",
+		Bytes: certBytes,
+	})
+
+	caPrivKeyPEM := new(bytes.Buffer)
+	pem.Encode(caPrivKeyPEM, &pem.Block{
+		Type:  "RSA PRIVATE KEY",
+		Bytes: x509.MarshalPKCS1PrivateKey(pk),
+	})
+
+	if err := os.WriteFile(dir+"/"+certificates.CertName, caPEM.Bytes(), 0644); err != nil {
 		return err
 	}
 
-	return os.WriteFile(dir+"/"+certificates.PrivateKeyName, cert.PrivateKeyBytes(), 0644)
+	return os.WriteFile(dir+"/"+certificates.PrivateKeyName, caPrivKeyPEM.Bytes(), 0644)
 }
 
 func getSAN(c *tls.Certificate) (string, error) {

pkg/reconciler/accessor/networking/certificate.go

@@ -63,10 +63,15 @@ func ReconcileCertificate(ctx context.Context, owner kmeta.Accessor, desired *v1
 		return nil, kaccessor.NewAccessorError(
 			fmt.Errorf("owner: %s with Type %T does not own Certificate: %q", owner.GetName(), owner, cert.Name),
 			kaccessor.NotOwnResource)
-	} else if !equality.Semantic.DeepEqual(cert.Spec, desired.Spec) {
+	} else if !equality.Semantic.DeepEqual(cert.Spec, desired.Spec) ||
+		!equality.Semantic.DeepEqual(cert.Annotations, desired.Annotations) ||
+		!equality.Semantic.DeepEqual(cert.Labels, desired.Labels) {
+
 		// Don't modify the informers copy
 		existing := cert.DeepCopy()
 		existing.Spec = desired.Spec
+		existing.Annotations = desired.Annotations
+		existing.Labels = desired.Labels
 		cert, err = certAccessor.GetNetworkingClient().NetworkingV1alpha1().Certificates(existing.Namespace).Update(ctx, existing, metav1.UpdateOptions{})
 		if err != nil {
 			recorder.Eventf(owner, corev1.EventTypeWarning, "UpdateFailed",

pkg/reconciler/domainmapping/resources/certificate_test.go

@@ -22,14 +22,14 @@ import (
 	"github.com/google/go-cmp/cmp"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/networking/pkg/config"
 
 	"knative.dev/networking/pkg/apis/networking"
 	networkingv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
 	"knative.dev/pkg/kmeta"
 	"knative.dev/serving/pkg/apis/serving"
 	"knative.dev/serving/pkg/apis/serving/v1beta1"
-	// . "knative.dev/serving/pkg/testing/v1"
 )
 
 func TestMakeCertificate(t *testing.T) {
@@ -58,7 +58,8 @@ func TestMakeCertificate(t *testing.T) {
 				Namespace:   "the-namespace",
 				Annotations: map[string]string{networking.CertificateClassAnnotationKey: certClass},
 				Labels: map[string]string{
-					serving.DomainMappingUIDLabelKey: "mapping.com",
+					serving.DomainMappingUIDLabelKey:   "mapping.com",
+					networking.CertificateTypeLabelKey: string(config.CertificateExternalDomain),
 				},
 			},
 			Spec: networkingv1alpha1.CertificateSpec{
@@ -96,7 +97,8 @@ func TestMakeCertificate(t *testing.T) {
 					"others":                                 "kept",
 				},
 				Labels: map[string]string{
-					serving.DomainMappingUIDLabelKey: "mapping.com",
+					serving.DomainMappingUIDLabelKey:   "mapping.com",
+					networking.CertificateTypeLabelKey: string(config.CertificateExternalDomain),
 				},
 			},
 			Spec: networkingv1alpha1.CertificateSpec{

pkg/reconciler/domainmapping/table_test.go

@@ -967,6 +967,7 @@ func TestReconcileTLSEnabled(t *testing.T) {
 					},
 					Labels: map[string]string{
 						serving.DomainMappingUIDLabelKey: "becomes.ready.run",
+						netapi.CertificateTypeLabelKey:   string(netcfg.CertificateExternalDomain),
 					},
 				},
 				Spec: netv1alpha1.CertificateSpec{
@@ -1114,6 +1115,7 @@ func TestReconcileTLSEnabled(t *testing.T) {
 					},
 					Labels: map[string]string{
 						serving.DomainMappingUIDLabelKey: "challenged.com",
+						netapi.CertificateTypeLabelKey:   string(netcfg.CertificateExternalDomain),
 					},
 				},
 				Spec: netv1alpha1.CertificateSpec{

pkg/reconciler/revision/controller.go

@@ -26,6 +26,9 @@ import (
 	"golang.org/x/time/rate"
 	cachingclient "knative.dev/caching/pkg/client/injection/client"
 	imageinformer "knative.dev/caching/pkg/client/injection/informers/caching/v1alpha1/image"
+	"knative.dev/networking/pkg/apis/networking/v1alpha1"
+	networkingclient "knative.dev/networking/pkg/client/injection/client"
+	certificateinformer "knative.dev/networking/pkg/client/injection/informers/networking/v1alpha1/certificate"
 	"knative.dev/pkg/changeset"
 	kubeclient "knative.dev/pkg/client/injection/kube/client"
 	deploymentinformer "knative.dev/pkg/client/injection/kube/informers/apps/v1/deployment"
@@ -73,15 +76,18 @@ func newControllerWithOptions(
 	deploymentInformer := deploymentinformer.Get(ctx)
 	imageInformer := imageinformer.Get(ctx)
 	paInformer := painformer.Get(ctx)
+	certificateInformer := certificateinformer.Get(ctx)
 
 	c := &Reconciler{
-		kubeclient:    kubeclient.Get(ctx),
-		client:        servingclient.Get(ctx),
-		cachingclient: cachingclient.Get(ctx),
+		kubeclient:       kubeclient.Get(ctx),
+		client:           servingclient.Get(ctx),
+		networkingclient: networkingclient.Get(ctx),
+		cachingclient:    cachingclient.Get(ctx),
 
 		podAutoscalerLister: paInformer.Lister(),
 		imageLister:         imageInformer.Lister(),
 		deploymentLister:    deploymentInformer.Lister(),
+		certificateLister:   certificateInformer.Lister(),
 	}
 
 	impl := revisionreconciler.NewImpl(ctx, c, func(impl *controller.Impl) controller.Options {
@@ -103,6 +109,8 @@ func newControllerWithOptions(
 		return controller.Options{ConfigStore: configStore}
 	})
 
+	c.tracker = impl.Tracker
+
 	transport := http.DefaultTransport
 	if rt, err := newResolverTransport(k8sCertPath, digestResolutionWorkers, digestResolutionWorkers); err != nil {
 		logging.FromContext(ctx).Errorw("Failed to create resolver transport", zap.Error(err))
@@ -131,6 +139,15 @@ func newControllerWithOptions(
 	}
 	deploymentInformer.Informer().AddEventHandler(handleMatchingControllers)
 	paInformer.Informer().AddEventHandler(handleMatchingControllers)
+	certificateInformer.Informer().AddEventHandler(controller.HandleAll(
+		// Call the tracker's OnChanged method, but we've seen the objects
+		// coming through this path missing TypeMeta, so ensure it is properly
+		// populated.
+		controller.EnsureTypeMeta(
+			c.tracker.OnChanged,
+			v1alpha1.SchemeGroupVersion.WithKind("Certificate"),
+		),
+	))
 
 	// We don't watch for changes to Image because we don't incorporate any of its
 	// properties into our own status and should work completely in the absence of