Remove duplication in secure TLS tests

Signed-off-by: Monis Khan <mok@vmware.com>

internal/testutil/tlsserver/tlsserver.go

@@ -69,14 +69,6 @@ func RecordTLSHello(server *httptest.Server) {
 func AssertTLS(t *testing.T, r *http.Request, tlsConfigFunc ptls.ConfigFunc) {
 	t.Helper()
 
-	tlsConfig := tlsConfigFunc(nil)
-
-	AssertTLSConfig(t, r, tlsConfig)
-}
-
-func AssertTLSConfig(t *testing.T, r *http.Request, tlsConfig *tls.Config) {
-	t.Helper()
-
 	m, ok := getCtxMap(r.Context())
 	require.True(t, ok)
 
@@ -86,6 +78,8 @@ func AssertTLSConfig(t *testing.T, r *http.Request, tlsConfig *tls.Config) {
 	info, ok := h.(*tls.ClientHelloInfo)
 	require.True(t, ok)
 
+	tlsConfig := tlsConfigFunc(nil)
+
 	supportedVersions := []uint16{tlsConfig.MinVersion}
 	ciphers := tlsConfig.CipherSuites
 

test/integration/securetls_fips_test.go

@@ -7,12 +7,8 @@
 package integration
 
 import (
-	"context"
 	"crypto/tls"
-	"encoding/base64"
-	"fmt"
 	"net/http"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -23,121 +19,6 @@ import (
 	"go.pinniped.dev/test/testlib"
 )
 
-// This test mirrors securetls_test.go, but adapted for fips mode.
-// e.g. checks for only TLS 1.2 ciphers and checks for the
-// list of fips-approved ciphers above.
-// TLS checks safe to run in parallel with serial tests, see main_test.go.
-func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) {
-	_ = testlib.IntegrationEnv(t)
-	t.Log("testing FIPs tls config")
-
-	server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// pinniped CLI uses ptls.Secure when talking to KAS,
-		// although the distinction doesn't matter much in FIPs mode because
-		// each of the configs is a wrapper for the same base FIPs config.
-		secure := ptls.Secure(nil)
-		tlsserver.AssertTLSConfig(t, r, secure)
-		w.Header().Set("content-type", "application/json")
-		fmt.Fprint(w, `{"kind":"TokenCredentialRequest","apiVersion":"login.concierge.pinniped.dev/v1alpha1",`+
-			`"status":{"credential":{"token":"some-fancy-token"}}}`)
-	}), tlsserver.RecordTLSHello)
-
-	ca := tlsserver.TLSTestServerCA(server)
-
-	pinnipedExe := testlib.PinnipedCLIPath(t)
-
-	stdout, stderr := runPinnipedCLI(t, nil, pinnipedExe, "login", "static",
-		"--token", "does-not-matter",
-		"--concierge-authenticator-type", "webhook",
-		"--concierge-authenticator-name", "does-not-matter",
-		"--concierge-ca-bundle-data", base64.StdEncoding.EncodeToString(ca),
-		"--concierge-endpoint", server.URL,
-		"--enable-concierge",
-		"--credential-cache", "",
-	)
-
-	require.Empty(t, stderr)
-	require.Equal(t, `{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1",`+
-		`"spec":{"interactive":false},"status":{"expirationTimestamp":null,"token":"some-fancy-token"}}
-`, stdout)
-}
-
-// TLS checks safe to run in parallel with serial tests, see main_test.go.
-func TestSecureTLSPinnipedCLIToSupervisor_Parallel(t *testing.T) {
-	_ = testlib.IntegrationEnv(t)
-
-	server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// pinniped CLI uses ptls.Default when talking to supervisor,
-		// although the distinction doesn't matter much in FIPs mode because
-		// each of the configs is a wrapper for the same base FIPs config.
-		defaultTLS := ptls.Default(nil)
-		tlsserver.AssertTLSConfig(t, r, defaultTLS)
-		w.Header().Set("content-type", "application/json")
-		fmt.Fprint(w, `{"issuer":"https://not-a-good-issuer"}`)
-	}), tlsserver.RecordTLSHello)
-
-	ca := tlsserver.TLSTestServerCA(server)
-
-	pinnipedExe := testlib.PinnipedCLIPath(t)
-
-	stdout, stderr := runPinnipedCLI(&fakeT{T: t}, nil, pinnipedExe, "login", "oidc",
-		"--ca-bundle-data", base64.StdEncoding.EncodeToString(ca),
-		"--issuer", server.URL,
-		"--credential-cache", "",
-		"--upstream-identity-provider-flow", "cli_password",
-		"--upstream-identity-provider-name", "does-not-matter",
-		"--upstream-identity-provider-type", "oidc",
-	)
-
-	require.Equal(t, `Error: could not complete Pinniped login: could not perform OIDC discovery for "`+
-		server.URL+`": oidc: issuer did not match the issuer returned by provider, expected "`+
-		server.URL+`" got "https://not-a-good-issuer"
-`, stderr)
-	require.Empty(t, stdout)
-}
-
-// TLS checks safe to run in parallel with serial tests, see main_test.go.
-func TestSecureTLSConciergeAggregatedAPI_Parallel(t *testing.T) {
-	env := testlib.IntegrationEnv(t)
-
-	cancelCtx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
-
-	startKubectlPortForward(cancelCtx, t, "10446", "443", env.ConciergeAppName+"-api", env.ConciergeNamespace)
-
-	stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10446)
-
-	require.Empty(t, stderr)
-	secure := ptls.Secure(nil)
-	require.Contains(t, stdout, testlib.GetExpectedCiphers(secure), "stdout:\n%s", stdout)
-}
-
-func TestSecureTLSSupervisor(t *testing.T) { // does not run in parallel because of the createSupervisorDefaultTLSCertificateSecretIfNeeded call
-	env := testlib.IntegrationEnv(t)
-
-	ctx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
-
-	startKubectlPortForward(ctx, t, "10447", "443", env.SupervisorAppName+"-nodeport", env.SupervisorNamespace)
-
-	stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10447)
-
-	// supervisor's cert is ECDSA
-	defaultECDSAOnly := ptls.Default(nil)
-	ciphers := make([]uint16, 0, len(defaultECDSAOnly.CipherSuites)/3)
-	for _, id := range defaultECDSAOnly.CipherSuites {
-		id := id
-		if !strings.Contains(tls.CipherSuiteName(id), "_ECDSA_") {
-			continue
-		}
-		ciphers = append(ciphers, id)
-	}
-	defaultECDSAOnly.CipherSuites = ciphers
-
-	require.Empty(t, stderr)
-	require.Contains(t, stdout, testlib.GetExpectedCiphers(defaultECDSAOnly), "stdout:\n%s", stdout)
-}
-
 // TestFIPSCipherSuites_Parallel ensures that if the list of default fips cipher suites changes,
 // we will know.  This is an integration test because we do not support build tags on unit tests.
 func TestFIPSCipherSuites_Parallel(t *testing.T) {
@@ -146,9 +27,8 @@ func TestFIPSCipherSuites_Parallel(t *testing.T) {
 	server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// use the default fips config which contains a hard coded list of cipher suites
 		// that should be equal to the default list of fips cipher suites.
-		defaultTLS := ptls.Default(nil)
 		// assert that the client hello response has the same tls config as this test server.
-		tlsserver.AssertTLSConfig(t, r, defaultTLS)
+		tlsserver.AssertTLS(t, r, ptls.Default)
 	}), tlsserver.RecordTLSHello)
 
 	ca := tlsserver.TLSTestServerCA(server)
@@ -173,20 +53,3 @@ func TestFIPSCipherSuites_Parallel(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, http.StatusOK, response.StatusCode)
 }
-
-type fakeT struct {
-	*testing.T
-}
-
-func (t *fakeT) FailNow() {
-	t.Errorf("fakeT ignored FailNow")
-}
-
-func (t *fakeT) Errorf(format string, args ...interface{}) {
-	t.Cleanup(func() {
-		if !t.Failed() {
-			return
-		}
-		t.Logf("reporting previously ignored errors since main test failed:\n"+format, args...)
-	})
-}

test/integration/securetls_test.go

@@ -1,9 +1,6 @@
 // Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-//go:build !fips_strict
-// +build !fips_strict
-
 package integration
 
 import (
@@ -27,7 +24,10 @@ func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) {
 	_ = testlib.IntegrationEnv(t)
 
 	server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		tlsserver.AssertTLS(t, r, ptls.Secure) // pinniped CLI uses ptls.Secure when talking to KAS
+		// pinniped CLI uses ptls.Secure when talking to KAS
+		// in FIPs mode the distinction doesn't matter much because
+		// each of the configs is a wrapper for the same base FIPs config
+		tlsserver.AssertTLS(t, r, ptls.Secure)
 		w.Header().Set("content-type", "application/json")
 		fmt.Fprint(w, `{"kind":"TokenCredentialRequest","apiVersion":"login.concierge.pinniped.dev/v1alpha1",`+
 			`"status":{"credential":{"token":"some-fancy-token"}}}`)
@@ -58,7 +58,10 @@ func TestSecureTLSPinnipedCLIToSupervisor_Parallel(t *testing.T) {
 	_ = testlib.IntegrationEnv(t)
 
 	server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		tlsserver.AssertTLS(t, r, ptls.Default) // pinniped CLI uses ptls.Default when talking to supervisor
+		// pinniped CLI uses ptls.Default when talking to supervisor
+		// in FIPs mode the distinction doesn't matter much because
+		// each of the configs is a wrapper for the same base FIPs config
+		tlsserver.AssertTLS(t, r, ptls.Default)
 		w.Header().Set("content-type", "application/json")
 		fmt.Fprint(w, `{"issuer":"https://not-a-good-issuer"}`)
 	}), tlsserver.RecordTLSHello)