Impersonation proxy detects when the user has configured an externall…

…y provided TLS secret to serve TLS

- https://github.com/vmware-tanzu/pinniped/tree/main/proposals/1547_impersonation-proxy-external-certs
- https://joshuatcasey.medium.com/k8s-mtls-auth-with-tls-passthrough-1bc25e750f52

internal/controller/impersonatorconfig/impersonator_config.go

@@ -248,7 +248,7 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre
 	}
 
 	// Make a live API call to avoid the cost of having an informer watch all node changes on the cluster,
-	// since there could be lots and we don't especially care about node changes.
+	// since there could be lots, and we don't especially care about node changes.
 	// Once we have concluded that there is or is not a visible control plane, then cache that decision
 	// to avoid listing nodes very often.
 	if c.hasControlPlaneNodes == nil {
@@ -296,8 +296,13 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre
 	}
 
 	var impersonationCABundle []byte
-	if c.shouldHaveImpersonator(impersonationSpec) {
-		impersonationCABundle, err = c.ensureCAAndTLSSecrets(ctx, nameInfo)
+	if c.shouldHaveImpersonator(impersonationSpec) { //nolint:nestif // This is complex but readable
+		if impersonationSpec.TLS != nil {
+			impersonationCABundle, err = c.evaluateExternallyProvidedTLSSecret(ctx, impersonationSpec.TLS)
+		} else {
+			impersonationCABundle, err = c.ensureCAAndTLSSecrets(ctx, nameInfo)
+		}
+
 		if err != nil {
 			return nil, err
 		}
@@ -321,7 +326,9 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre
 	return credentialIssuerStrategyResult, nil
 }
 
-func (c *impersonatorConfigController) ensureCAAndTLSSecrets(ctx context.Context, nameInfo *certNameInfo) ([]byte, error) {
+func (c *impersonatorConfigController) ensureCAAndTLSSecrets(
+	ctx context.Context,
+	nameInfo *certNameInfo) ([]byte, error) {
 	var (
 		impersonationCA *certauthority.CA
 		err             error
@@ -340,6 +347,44 @@ func (c *impersonatorConfigController) ensureCAAndTLSSecrets(ctx context.Context
 	return nil, nil
 }
 
+func (c *impersonatorConfigController) evaluateExternallyProvidedTLSSecret(
+	ctx context.Context,
+	tlsSpec *v1alpha1.ImpersonationProxyTLSSpec) ([]byte, error) {
+	if tlsSpec.SecretName == "" {
+		return nil, fmt.Errorf("must provide impersonationSpec.TLS.secretName if impersonationSpec.TLS is provided")
+	}
+
+	c.infoLog.Info("configuring the impersonation proxy to use an externally provided TLS secret",
+		"secretName", tlsSpec.SecretName)
+
+	// Ensure that any TLS secret generated by this controller is removed
+	err := c.ensureTLSSecretIsRemoved(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("unable to remove generated TLS secret with name %s: %w", c.tlsSecretName, err)
+	}
+
+	// The CA Bundle may come from either the TLS secret or the CertificateAuthorityData.
+	// Check CertificateAuthorityData last so that it will take priority.
+
+	var caBundle []byte
+	caBundle, err = c.readExternalTLSSecret(tlsSpec.SecretName)
+	if err != nil {
+		return nil, fmt.Errorf("could not load the externally provided TLS secret for the impersonation proxy: %w", err)
+	}
+
+	if tlsSpec.CertificateAuthorityData != "" {
+		caBundle, err = base64.StdEncoding.DecodeString(tlsSpec.CertificateAuthorityData)
+		if err != nil {
+			return nil, fmt.Errorf("could not decode impersonationSpec.TLS.certificateAuthorityData: %w", err)
+		}
+
+		c.infoLog.Info("the impersonation proxy will advertise its CA Bundle from impersonationSpec.TLS.CertificateAuthorityData",
+			"CertificateAuthorityData", caBundle)
+	}
+
+	return caBundle, nil
+}
+
 func (c *impersonatorConfigController) loadImpersonationProxyConfiguration(credIssuer *v1alpha1.CredentialIssuer) (*v1alpha1.ImpersonationProxySpec, error) {
 	// Make a copy of the spec since we got this object from informer cache.
 	spec := credIssuer.Spec.DeepCopy().ImpersonationProxy
@@ -661,6 +706,26 @@ func (c *impersonatorConfigController) createOrUpdateService(ctx context.Context
 	return err
 }
 
+func (c *impersonatorConfigController) readExternalTLSSecret(externalTLSSecretName string) (impersonationCABundle []byte, err error) {
+	secretFromInformer, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(externalTLSSecretName)
+	if err != nil {
+		c.infoLog.Info("could not find externally provided TLS secret for the impersonation proxy",
+			"secretName", externalTLSSecretName)
+		return nil, err
+	}
+
+	c.infoLog.Info("found externally provided TLS secret for the impersonation proxy",
+		"secretName", externalTLSSecretName)
+
+	err = c.loadTLSCertFromSecret(secretFromInformer)
+	if err != nil {
+		plog.Error("error loading cert from externally provided TLS secret for the impersonation proxy", err)
+		return nil, err
+	}
+
+	return secretFromInformer.Data[caCrtKey], nil
+}
+
 func (c *impersonatorConfigController) ensureTLSSecret(ctx context.Context, nameInfo *certNameInfo, ca *certauthority.CA) error {
 	secretFromInformer, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(c.tlsSecretName)
 	notFound := k8serrors.IsNotFound(err)

internal/controller/impersonatorconfig/impersonator_config_test.go

@@ -271,6 +271,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 		const localhostIP = "127.0.0.1"
 		const httpsPort = ":443"
 		const fakeServerResponseBody = "hello, world!"
+		const externallyProvidedTLSSecretName = "external-tls-secret" //nolint:gosec // this is not a credential
 		var labels = map[string]string{"app": "app-name", "other-key": "other-value"}
 
 		var r *require.Assertions
@@ -300,6 +301,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 		var testHTTPServerInterruptCh chan struct{}
 		var queue *testQueue
 		var validClientCert *tls.Certificate
+		var externalCA *certauthority.CA
+		var externalTLSSecret *corev1.Secret
 
 		var impersonatorFunc = func(
 			port int,
@@ -336,7 +339,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 					// and that the second parameter will always be nil in that case.
 					// rawCerts will be raw ASN.1 certificates provided by the peer.
 					if len(rawCerts) != 1 {
-						return fmt.Errorf("expected to get one client cert on incoming request to test server")
+						return fmt.Errorf("expected to get one client cert on incoming request to test server, found %d", len(rawCerts))
 					}
 					clientCert := rawCerts[0]
 					currentClientCertCA := impersonationProxySignerCAProvider.CurrentCABundleContent()
@@ -464,8 +467,12 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			var tr *http.Transport
 			if caCrt == nil {
 				tr = &http.Transport{
-					TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec
-					DialContext:     overrideDialContext,
+					TLSClientConfig: &tls.Config{
+						InsecureSkipVerify: true, //nolint:gosec // this is used to test when the impersonation proxy does not advertise a CA bundle
+						// Client cert which is supposed to work against the server's dynamic CAContentProvider
+						Certificates: []tls.Certificate{*validClientCert},
+					},
+					DialContext: overrideDialContext,
 				}
 			} else {
 				rootCAs := x509.NewCertPool()
@@ -1122,14 +1129,17 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			frozenNow = time.Date(2021, time.March, 2, 7, 42, 0, 0, time.Local)
 			signingCertProvider = dynamiccert.NewCA(name)
 
-			ca := newCA()
-			signingCACertPEM = ca.Bundle()
+			signingCA := newCA()
+			signingCACertPEM = signingCA.Bundle()
 			var err error
-			signingCAKeyPEM, err = ca.PrivateKeyToPEM()
+			signingCAKeyPEM, err = signingCA.PrivateKeyToPEM()
 			r.NoError(err)
 			signingCASecret = newSigningKeySecret(caSignerName, signingCACertPEM, signingCAKeyPEM)
-			validClientCert, err = ca.IssueClientCert("username", nil, time.Hour)
+			validClientCert, err = signingCA.IssueClientCert("username", nil, time.Hour)
 			r.NoError(err)
+
+			externalCA = newCA()
+			externalTLSSecret = newActualTLSSecret(externalCA, externallyProvidedTLSSecretName, localhostIP)
 		})
 
 		it.After(func() {
@@ -1159,7 +1169,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			})
 		})
 
-		when("the configuration is auto mode with an endpoint and service type none", func() {
+		when("the configuration is auto mode with an endpoint and service type none, using generated TLS secrets", func() {
 			it.Before(func() {
 				addSecretToTrackers(signingCASecret, kubeInformerClient)
 				addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
@@ -1211,6 +1221,133 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			})
 		})
 
+		when("using external TLS secrets", func() {
+			when("the configuration is auto mode with an endpoint and service type none", func() {
+				it.Before(func() {
+					addSecretToTrackers(signingCASecret, kubeInformerClient)
+					addSecretToTrackers(externalTLSSecret, kubeInformerClient)
+					addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
+						ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
+						Spec: v1alpha1.CredentialIssuerSpec{
+							ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{
+								Mode:             v1alpha1.ImpersonationProxyModeAuto,
+								ExternalEndpoint: localhostIP,
+								Service: v1alpha1.ImpersonationProxyServiceSpec{
+									Type: v1alpha1.ImpersonationProxyServiceTypeNone,
+								},
+								TLS: &v1alpha1.ImpersonationProxyTLSSpec{
+									CertificateAuthorityData: base64.StdEncoding.EncodeToString(externalCA.Bundle()),
+									SecretName:               externallyProvidedTLSSecretName,
+								},
+							},
+						},
+					}, pinnipedInformerClient, pinnipedAPIClient)
+				})
+
+				when("there are not visible control plane nodes", func() {
+					it.Before(func() {
+						addNodeWithRoleToTracker("worker", kubeAPIClient)
+					})
+
+					it("starts the impersonator according to the settings in the CredentialIssuer", func() {
+						startInformersAndController()
+						r.NoError(runControllerSync())
+						r.Len(kubeAPIClient.Actions(), 1)
+						requireNodesListed(kubeAPIClient.Actions()[0])
+						requireTLSServerIsRunning(externalCA.Bundle(), testServerAddr(), nil)
+						requireCredentialIssuer(newSuccessStrategy(localhostIP, externalCA.Bundle()))
+						requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM)
+					})
+
+					when("there is an existing generated TLS secret", func() {
+						it.Before(func() {
+							addSecretToTrackers(newEmptySecret(tlsSecretName), kubeInformerClient)
+						})
+
+						it("removes the existing generated TLS secret", func() {
+							startInformersAndController()
+							r.NoError(runControllerSync())
+							r.Len(kubeAPIClient.Actions(), 2)
+							requireNodesListed(kubeAPIClient.Actions()[0])
+							requireTLSSecretWasDeleted(kubeAPIClient.Actions()[1])
+							requireTLSServerIsRunning(externalCA.Bundle(), testServerAddr(), nil)
+							requireCredentialIssuer(newSuccessStrategy(localhostIP, externalCA.Bundle()))
+							requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM)
+						})
+					})
+				})
+			})
+
+			when("the CertificateAuthorityData is not configured", func() {
+				when("the externally provided TLS secret has a ca.crt field", func() {
+					it.Before(func() {
+						addSecretToTrackers(signingCASecret, kubeInformerClient)
+						externalTLSSecret.Data["ca.crt"] = externalCA.Bundle()
+						addSecretToTrackers(externalTLSSecret, kubeInformerClient)
+						addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
+							ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
+							Spec: v1alpha1.CredentialIssuerSpec{
+								ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{
+									Mode:             v1alpha1.ImpersonationProxyModeAuto,
+									ExternalEndpoint: localhostIP,
+									Service: v1alpha1.ImpersonationProxyServiceSpec{
+										Type: v1alpha1.ImpersonationProxyServiceTypeNone,
+									},
+									TLS: &v1alpha1.ImpersonationProxyTLSSpec{
+										SecretName: externallyProvidedTLSSecretName,
+									},
+								},
+							},
+						}, pinnipedInformerClient, pinnipedAPIClient)
+						addNodeWithRoleToTracker("worker", kubeAPIClient)
+					})
+
+					it("will advertise ca.crt from the externally provided secret", func() {
+						startInformersAndController()
+						r.NoError(runControllerSync())
+						r.Len(kubeAPIClient.Actions(), 1)
+						requireNodesListed(kubeAPIClient.Actions()[0])
+						requireTLSServerIsRunning(externalTLSSecret.Data["ca.crt"], testServerAddr(), nil)
+						requireCredentialIssuer(newSuccessStrategy(localhostIP, externalTLSSecret.Data["ca.crt"]))
+						requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM)
+					})
+				})
+
+				when("the externally provided TLS secret does not have a ca.crt field", func() {
+					it.Before(func() {
+						addSecretToTrackers(signingCASecret, kubeInformerClient)
+						addSecretToTrackers(externalTLSSecret, kubeInformerClient)
+						addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{
+							ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName},
+							Spec: v1alpha1.CredentialIssuerSpec{
+								ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{
+									Mode:             v1alpha1.ImpersonationProxyModeAuto,
+									ExternalEndpoint: localhostIP,
+									Service: v1alpha1.ImpersonationProxyServiceSpec{
+										Type: v1alpha1.ImpersonationProxyServiceTypeNone,
+									},
+									TLS: &v1alpha1.ImpersonationProxyTLSSpec{
+										SecretName: externallyProvidedTLSSecretName,
+									},
+								},
+							},
+						}, pinnipedInformerClient, pinnipedAPIClient)
+						addNodeWithRoleToTracker("worker", kubeAPIClient)
+					})
+
+					it("will advertise an empty CA bundle", func() {
+						startInformersAndController()
+						r.NoError(runControllerSync())
+						r.Len(kubeAPIClient.Actions(), 1)
+						requireNodesListed(kubeAPIClient.Actions()[0])
+						requireTLSServerIsRunning(nil, testServerAddr(), nil)
+						requireCredentialIssuer(newSuccessStrategy(localhostIP, nil))
+						requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM)
+					})
+				})
+			})
+		})
+
 		when("the configuration is auto mode", func() {
 			it.Before(func() {
 				addSecretToTrackers(signingCASecret, kubeInformerClient)

test/integration/kube_api_discovery_test.go

@@ -441,7 +441,7 @@ func TestGetAPIResourceList(t *testing.T) { //nolint:gocyclo // each t.Run is pr
 		// over time, make a rudimentary assertion that this test exercised the whole tree of all fields of all
 		// Pinniped API resources. Without this, the test could accidentally skip parts of the tree if the
 		// format has changed.
-		require.Equal(t, 227, foundFieldNames,
+		require.Equal(t, 230, foundFieldNames,
 			"Expected to find all known fields of all Pinniped API resources. "+
 				"You may will need to update this expectation if you added new fields to the API types.",
 		)