From d0f970f5ba44e5d8b98c389dd0f138439a6cf68e Mon Sep 17 00:00:00 2001
From: Kenneth Jenkins <51246568+kenjenkins@users.noreply.github.com>
Date: Thu, 29 Aug 2024 12:22:59 -0700
Subject: [PATCH] config: generate fallback cert only as last resort

Currently Pomerium will generate a self-signed wildcard certificate for
use as a fallback certificate.

If any other certificate is configured, this self-signed certificate
will not normally be presented, except in the case of a TLS connection
where the client does not include the Server Name Indication (SNI)
extension. All modern browsers support SNI, so in practice this
certificate should never be presented to end users.

However, some network scanning tools will probe connections by IP
addresses, and so this self-signed certificate may be presented. The
presence of a self-signed certificate may be flagged as a problem.

Let's avoid generating this self-signed certificate if Pomerium has any
other certificate configured. This should prevent false positive reports
from these particular vulnerability scans.
---
 config/config.go          | 54 +++++++++++++++++++--------------------
 config/envoyconfig/tls.go | 20 ++++++++++++---
 2 files changed, 43 insertions(+), 31 deletions(-)

diff --git a/config/config.go b/config/config.go
index f2f7dd4d9..8ec1f1aec 100644
--- a/config/config.go
+++ b/config/config.go
@@ -155,38 +155,38 @@ func (cfg *Config) GetTLSClientConfig() (*tls.Config, error) {
 	}, nil
 }
 
-// GenerateCatchAllCertificate generates a catch-all certificate. If no derived CA is defined a
-// self-signed certificate will be generated.
-func (cfg *Config) GenerateCatchAllCertificate() (*tls.Certificate, error) {
-	if cfg.Options.DeriveInternalDomainCert != nil {
-		sharedKey, err := cfg.Options.GetSharedKey()
-		if err != nil {
-			return nil, fmt.Errorf("failed to generate cert, invalid shared key: %w", err)
-		}
-
-		ca, err := derivecert.NewCA(sharedKey)
-		if err != nil {
-			return nil, fmt.Errorf("failed to generate cert, invalid derived CA: %w", err)
-		}
-
-		pem, err := ca.NewServerCert([]string{"*"})
-		if err != nil {
-			return nil, fmt.Errorf("failed to generate cert, error creating server certificate: %w", err)
-		}
-
-		cert, err := pem.TLS()
-		if err != nil {
-			return nil, fmt.Errorf("failed to generate cert, error converting generated certificate into TLS certificate: %w", err)
-		}
-		return &cert, nil
-	}
-
+// GenerateDerivedCertificate generates a wildcard certificate from a CA
+// derived from the shared secret.
+func (cfg *Config) GenerateDerivedCertificate() (*tls.Certificate, error) {
 	sharedKey, err := cfg.Options.GetSharedKey()
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate cert, invalid shared key: %w", err)
 	}
 
-	// finally fall back to a generated, self-signed certificate
+	ca, err := derivecert.NewCA(sharedKey)
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate cert, invalid derived CA: %w", err)
+	}
+
+	pem, err := ca.NewServerCert([]string{"*"})
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate cert, error creating server certificate: %w", err)
+	}
+
+	cert, err := pem.TLS()
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate cert, error converting generated certificate into TLS certificate: %w", err)
+	}
+	return &cert, nil
+}
+
+// GenerateFallbackCertificate generates a self-signed certificate derived from
+// the shared secret.
+func (cfg *Config) GenerateFallbackCertificate() (*tls.Certificate, error) {
+	sharedKey, err := cfg.Options.GetSharedKey()
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate cert, invalid shared key: %w", err)
+	}
 	return cryptutil.GenerateCertificate(sharedKey, "*")
 }
 
diff --git a/config/envoyconfig/tls.go b/config/envoyconfig/tls.go
index abf1d51c7..09d25b2d0 100644
--- a/config/envoyconfig/tls.go
+++ b/config/envoyconfig/tls.go
@@ -219,12 +219,24 @@ func getAllCertificates(cfg *config.Config) ([]tls.Certificate, error) {
 		return nil, fmt.Errorf("error collecting all certificates: %w", err)
 	}
 
-	wc, err := cfg.GenerateCatchAllCertificate()
-	if err != nil {
-		return nil, fmt.Errorf("error getting wildcard certificate: %w", err)
+	if cfg.Options.DeriveInternalDomainCert != nil {
+		wc, err := cfg.GenerateDerivedCertificate()
+		if err != nil {
+			return nil, fmt.Errorf("error generating wildcard certificate: %w", err)
+		}
+		allCertificates = append(allCertificates, *wc)
 	}
 
-	return append(allCertificates, *wc), nil
+	// Generate a fallback certificate only as a last resort, if no other
+	// certificates are configured.
+	if len(allCertificates) == 0 {
+		wc, err := cfg.GenerateFallbackCertificate()
+		if err != nil {
+			return nil, fmt.Errorf("error generating fallback certificate: %w", err)
+		}
+		allCertificates = append(allCertificates, *wc)
+	}
+	return allCertificates, nil
 }
 
 // validateCertificate validates that a certificate can be used with Envoy's TLS stack.