test: add TLS profile unit and e2e tests

tmshort · claude · tmshort · commit 1459482cb451 · 2026-04-15T14:54:40.000-04:00
- Unit tests in tlsprofiles package verify cipher negotiation, cipher
  rejection, min-version enforcement, and curve acceptance/rejection
  by starting a local TLS server with a custom profile and connecting
  to it with a restricted client config.
- e2e feature (tls.feature) patches the catalogd deployment with
  specific custom TLS settings for each scenario, asserts the expected
  connection behaviour, then restores the original args on cleanup.
  Covers min-version enforcement (TLSv1.3), cipher negotiation and
  rejection (TLS 1.2 + ECDHE_ECDSA), and curve enforcement (prime256v1
  accepted, secp521r1 rejected).
- GODOG_ARGS variable added to the e2e Makefile target so a single
  feature file can be run with: make test-e2e GODOG_ARGS=features/tls.feature

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
Signed-off-by: Todd Short &lt;tshort@redhat.com&gt;
diff --git a/Makefile b/Makefile
@@ -255,9 +255,15 @@ $(eval $(call install-sh,standard,operator-controller-standard.yaml))
 test: manifests generate fmt lint test-unit test-e2e test-regression #HELP Run all tests.
 
 E2E_TIMEOUT ?= 10m
+# GODOG_ARGS can be used to pass extra arguments to the Godog test runner,
+# for example to run a single feature file:
+#   make test-e2e GODOG_ARGS=features/tls.feature
+# or to run scenarios matching a tag:
+#   make test-e2e GODOG_ARGS="--godog.tags=@smoke"
+GODOG_ARGS ?=
 .PHONY: e2e
 e2e: #EXHELP Run the e2e tests.
-	go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT)
+	go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT) -- $(GODOG_ARGS)
 
 E2E_REGISTRY_NAME := docker-registry
 E2E_REGISTRY_NAMESPACE := operator-controller-e2e
diff --git a/internal/shared/util/tlsprofiles/tlsprofiles_connection_test.go b/internal/shared/util/tlsprofiles/tlsprofiles_connection_test.go
@@ -0,0 +1,247 @@
+package tlsprofiles
+
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/tls"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"math/big"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+// generateSelfSignedCert generates a self-signed ECDSA P-256 certificate for use in tests.
+func generateSelfSignedCert(t *testing.T) tls.Certificate {
+	t.Helper()
+
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+
+	template := x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		Subject:      pkix.Name{Organization: []string{"Test"}},
+		NotBefore:    time.Now().Add(-time.Hour),
+		NotAfter:     time.Now().Add(time.Hour),
+		KeyUsage:     x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:  []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		IPAddresses:  []net.IP{net.ParseIP("127.0.0.1")},
+	}
+
+	certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
+	require.NoError(t, err)
+
+	certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})
+	privDER, err := x509.MarshalECPrivateKey(priv)
+	require.NoError(t, err)
+	keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: privDER})
+
+	cert, err := tls.X509KeyPair(certPEM, keyPEM)
+	require.NoError(t, err)
+	return cert
+}
+
+// startTLSServer starts a TLS listener with cfgFn applied and serves connections in
+// the background. The listener is closed when the test completes.
+func startTLSServer(t *testing.T, cfgFn func(*tls.Config)) string {
+	t.Helper()
+
+	serverCfg := &tls.Config{Certificates: []tls.Certificate{generateSelfSignedCert(t)}}
+	cfgFn(serverCfg)
+
+	ln, err := tls.Listen("tcp", "127.0.0.1:0", serverCfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { ln.Close() })
+
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return // listener closed
+			}
+			go func() {
+				defer conn.Close()
+				_ = conn.(*tls.Conn).Handshake()
+			}()
+		}
+	}()
+
+	return ln.Addr().String()
+}
+
+// dialTLS connects to addr with the given config and returns the negotiated
+// ConnectionState. The caller must check err before using the state.
+func dialTLS(addr string, clientCfg *tls.Config) (tls.ConnectionState, error) {
+	conn, err := tls.Dial("tcp", addr, clientCfg)
+	if err != nil {
+		return tls.ConnectionState{}, err
+	}
+	defer conn.Close()
+	return conn.ConnectionState(), nil
+}
+
+// setCustomProfile configures the package-level custom TLS profile for the duration
+// of the test and restores the original state via t.Cleanup.
+func setCustomProfile(t *testing.T, cipherNames []string, curveNames []string, minVersion string) {
+	t.Helper()
+
+	origProfile := configuredProfile
+	origCustom := customTLSProfile
+	t.Cleanup(func() {
+		configuredProfile = origProfile
+		customTLSProfile = origCustom
+	})
+
+	configuredProfile = "custom"
+	customTLSProfile = tlsProfile{
+		ciphers: cipherSlice{},
+		curves:  curveSlice{},
+	}
+
+	for _, name := range cipherNames {
+		require.NoError(t, customTLSProfile.ciphers.Append(name))
+	}
+	for _, name := range curveNames {
+		require.NoError(t, customTLSProfile.curves.Append(name))
+	}
+	if minVersion != "" {
+		require.NoError(t, customTLSProfile.minTLSVersion.Set(minVersion))
+	}
+}
+
+// TestCustomTLSProfileCipherNegotiation verifies that when a custom profile
+// specifies a single cipher suite, that cipher is actually negotiated.
+func TestCustomTLSProfileCipherNegotiation(t *testing.T) {
+	const cipher = "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"
+	cipherID := cipherSuiteId(cipher)
+	require.NotZero(t, cipherID)
+
+	setCustomProfile(t, []string{cipher}, []string{"prime256v1"}, "TLSv1.2")
+
+	cfgFn, err := GetTLSConfigFunc()
+	require.NoError(t, err)
+
+	addr := startTLSServer(t, cfgFn)
+
+	// Client is restricted to TLS 1.2 with the same single cipher.
+	clientCfg := &tls.Config{
+		InsecureSkipVerify: true, //nolint:gosec // self-signed cert used only in tests
+		MaxVersion:         tls.VersionTLS12,
+		CipherSuites:       []uint16{cipherID},
+	}
+
+	state, err := dialTLS(addr, clientCfg)
+	require.NoError(t, err)
+	require.Equal(t, cipherID, state.CipherSuite, "expected cipher %s to be negotiated", cipher)
+}
+
+// TestCustomTLSProfileCipherRejection verifies that the server rejects a
+// connection when the client offers only a cipher not in the custom profile.
+func TestCustomTLSProfileCipherRejection(t *testing.T) {
+	// Server is configured with AES-256 only.
+	setCustomProfile(t,
+		[]string{"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"},
+		[]string{"prime256v1"},
+		"TLSv1.2",
+	)
+
+	cfgFn, err := GetTLSConfigFunc()
+	require.NoError(t, err)
+
+	addr := startTLSServer(t, cfgFn)
+
+	// Client offers only AES-128, which the server does not allow.
+	clientCfg := &tls.Config{
+		InsecureSkipVerify: true, //nolint:gosec // self-signed cert used only in tests
+		MaxVersion:         tls.VersionTLS12,
+		CipherSuites:       []uint16{cipherSuiteId("TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256")},
+	}
+
+	_, err = dialTLS(addr, clientCfg)
+	require.Error(t, err, "connection should fail when client offers no cipher from the custom profile")
+}
+
+// TestCustomTLSProfileMinVersionEnforcement verifies that a custom profile
+// configured with a TLS 1.3 minimum rejects TLS 1.2-only clients.
+func TestCustomTLSProfileMinVersionEnforcement(t *testing.T) {
+	setCustomProfile(t,
+		[]string{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"},
+		[]string{"prime256v1"},
+		"TLSv1.3",
+	)
+
+	cfgFn, err := GetTLSConfigFunc()
+	require.NoError(t, err)
+
+	addr := startTLSServer(t, cfgFn)
+
+	// Client advertises TLS 1.2 as its maximum; server requires TLS 1.3.
+	clientCfg := &tls.Config{
+		InsecureSkipVerify: true, //nolint:gosec // self-signed cert used only in tests
+		MaxVersion:         tls.VersionTLS12,
+	}
+
+	_, err = dialTLS(addr, clientCfg)
+	require.Error(t, err, "connection should fail when server requires TLS 1.3 and client only supports TLS 1.2")
+}
+
+// TestCustomTLSProfileCurveNegotiation verifies that a connection succeeds when
+// the client's curve preferences overlap with the custom profile's curve list.
+func TestCustomTLSProfileCurveNegotiation(t *testing.T) {
+	// Server allows only prime256v1 (P-256).
+	setCustomProfile(t,
+		[]string{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"},
+		[]string{"prime256v1"},
+		"TLSv1.2",
+	)
+
+	cfgFn, err := GetTLSConfigFunc()
+	require.NoError(t, err)
+
+	addr := startTLSServer(t, cfgFn)
+
+	// Client also only uses prime256v1 — there is an overlap.
+	clientCfg := &tls.Config{
+		InsecureSkipVerify: true, //nolint:gosec // self-signed cert used only in tests
+		MaxVersion:         tls.VersionTLS12,
+		CurvePreferences:   []tls.CurveID{tls.CurveP256},
+		CipherSuites:       []uint16{cipherSuiteId("TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256")},
+	}
+
+	_, err = dialTLS(addr, clientCfg)
+	require.NoError(t, err)
+}
+
+// TestCustomTLSProfileCurveRejection verifies that a connection fails when the
+// client's supported curves do not overlap with the custom profile's curve list.
+// TLS 1.2 is used because the curve negotiation failure is deterministic there;
+// TLS 1.3 can fall back via HelloRetryRequest.
+func TestCustomTLSProfileCurveRejection(t *testing.T) {
+	// Server allows only prime256v1 (P-256).
+	setCustomProfile(t,
+		[]string{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"},
+		[]string{"prime256v1"},
+		"TLSv1.2",
+	)
+
+	cfgFn, err := GetTLSConfigFunc()
+	require.NoError(t, err)
+
+	addr := startTLSServer(t, cfgFn)
+
+	// Client only supports X25519, which is not in the server's curve list.
+	clientCfg := &tls.Config{
+		InsecureSkipVerify: true, //nolint:gosec // self-signed cert used only in tests
+		MaxVersion:         tls.VersionTLS12,
+		CurvePreferences:   []tls.CurveID{tls.X25519},
+		CipherSuites:       []uint16{cipherSuiteId("TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256")},
+	}
+
+	_, err = dialTLS(addr, clientCfg)
+	require.Error(t, err, "connection should fail when client and server share no common curve")
+}
diff --git a/test/e2e/features/tls.feature b/test/e2e/features/tls.feature
@@ -0,0 +1,27 @@
+Feature: TLS profile enforcement on metrics endpoints
+
+  Background:
+    Given OLM is available
+
+  # Each scenario patches the deployment with the TLS settings under test and
+  # restores the original configuration during cleanup, so scenarios are independent.
+
+  # All three scenarios test catalogd only: the enforcement logic lives in the shared
+  # tlsprofiles package, so one component is sufficient. TLS 1.2 is used for cipher
+  # and curve enforcement because Go's crypto/tls does not allow the server to restrict
+  # TLS 1.3 cipher suites — CipherSuites config only applies to TLS 1.2. The e2e cert
+  # uses ECDSA, so ECDHE_ECDSA cipher families are required.
+  Scenario: catalogd metrics endpoint enforces configured minimum TLS version
+    Given the "catalogd" deployment is configured with custom TLS minimum version "TLSv1.3"
+    Then the "catalogd" metrics endpoint accepts a TLS 1.3 connection
+    And the "catalogd" metrics endpoint rejects a TLS 1.2 connection
+
+  Scenario: catalogd metrics endpoint negotiates and enforces configured cipher suite
+    Given the "catalogd" deployment is configured with custom TLS version "TLSv1.2", ciphers "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", and curves "prime256v1"
+    Then the "catalogd" metrics endpoint negotiates cipher "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" over TLS 1.2
+    And the "catalogd" metrics endpoint rejects a TLS 1.2 connection offering only cipher "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"
+
+  Scenario: catalogd metrics endpoint enforces configured curve preferences
+    Given the "catalogd" deployment is configured with custom TLS version "TLSv1.2", ciphers "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", and curves "prime256v1"
+    Then the "catalogd" metrics endpoint accepts a TLS 1.2 connection with cipher "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" and curve "prime256v1"
+    And the "catalogd" metrics endpoint rejects a TLS 1.2 connection with cipher "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" and only curve "secp521r1"
diff --git a/test/e2e/steps/hooks.go b/test/e2e/steps/hooks.go
@@ -27,6 +27,14 @@ type resource struct {
 	namespace string
 }
 
+// deploymentRestore records the original container args of a deployment so that
+// it can be patched back to its pre-test state during scenario cleanup.
+type deploymentRestore struct {
+	namespace      string
+	deploymentName string
+	originalArgs   []string
+}
+
 type scenarioContext struct {
 	id                   string
 	namespace            string
@@ -38,6 +46,7 @@ type scenarioContext struct {
 	backGroundCmds       []*exec.Cmd
 	metricsResponse      map[string]string
 	leaderPods           map[string]string // component name -> leader pod name
+	deploymentRestores   []deploymentRestore
 
 	extensionObjects []client.Object
 }
@@ -182,6 +191,21 @@ func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context
 			_ = p.Kill()
 		}
 	}
+
+	// Always restore deployments whose args were modified during the scenario,
+	// even when the scenario failed, so that a misconfigured TLS profile does
+	// not leak into subsequent scenarios.
+	for _, dr := range sc.deploymentRestores {
+		if err2 := patchDeploymentArgs(dr.namespace, dr.deploymentName, dr.originalArgs); err2 != nil {
+			logger.Info("Error restoring deployment args", "name", dr.deploymentName, "error", err2)
+			continue
+		}
+		if _, err2 := k8sClient("rollout", "status", "-n", dr.namespace,
+			fmt.Sprintf("deployment/%s", dr.deploymentName), "--timeout=2m"); err2 != nil {
+			logger.Info("Timeout waiting for deployment rollout after restore", "name", dr.deploymentName)
+		}
+	}
+
 	if err != nil {
 		return ctx, err
 	}
diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go
@@ -150,6 +150,18 @@ func RegisterSteps(sc *godog.ScenarioContext) {
 
 	sc.Step(`^(?i)the current ClusterExtension is tracked for cleanup$`, TrackCurrentClusterExtensionForCleanup)
 
+	// TLS profile enforcement steps — deployment configuration
+	sc.Step(`^(?i)the "([^"]+)" deployment is configured with custom TLS minimum version "([^"]+)"$`, ConfigureDeploymentWithCustomTLSVersion)
+	sc.Step(`^(?i)the "([^"]+)" deployment is configured with custom TLS version "([^"]+)", ciphers "([^"]+)", and curves "([^"]+)"$`, ConfigureDeploymentWithCustomTLSFull)
+
+	// TLS profile enforcement steps — connection assertions
+	sc.Step(`^(?i)the "([^"]+)" metrics endpoint accepts a TLS 1\.3 connection$`, MetricsEndpointAcceptsTLS13)
+	sc.Step(`^(?i)the "([^"]+)" metrics endpoint rejects a TLS 1\.2 connection$`, MetricsEndpointRejectsTLS12)
+	sc.Step(`^(?i)the "([^"]+)" metrics endpoint negotiates cipher "([^"]+)" over TLS 1\.2$`, MetricsEndpointNegotiatesTLS12Cipher)
+	sc.Step(`^(?i)the "([^"]+)" metrics endpoint rejects a TLS 1\.2 connection offering only cipher "([^"]+)"$`, MetricsEndpointRejectsTLS12ConnectionWithCipher)
+	sc.Step(`^(?i)the "([^"]+)" metrics endpoint accepts a TLS 1\.2 connection with cipher "([^"]+)" and curve "([^"]+)"$`, MetricsEndpointAcceptsTLS12ConnectionWithCurve)
+	sc.Step(`^(?i)the "([^"]+)" metrics endpoint rejects a TLS 1\.2 connection with cipher "([^"]+)" and only curve "([^"]+)"$`, MetricsEndpointRejectsTLS12ConnectionWithCurve)
+
 	// Upgrade-specific steps
 	sc.Step(`^(?i)the latest stable OLM release is installed$`, LatestStableOLMReleaseIsInstalled)
 	sc.Step(`^(?i)OLM is upgraded$`, OLMIsUpgraded)
diff --git a/test/e2e/steps/tls_steps.go b/test/e2e/steps/tls_steps.go
