diff --git a/internal/concierge/impersonator/impersonator_test.go b/internal/concierge/impersonator/impersonator_test.go index 78e6e3f2..6eedd950 100644 --- a/internal/concierge/impersonator/impersonator_test.go +++ b/internal/concierge/impersonator/impersonator_test.go @@ -701,7 +701,7 @@ func TestImpersonator(t *testing.T) { testKubeAPIServerWasCalled := false var testKubeAPIServerSawHeaders http.Header testKubeAPIServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Secure(nil)) + tlsserver.AssertTLS(t, r, ptls.Secure) switch r.URL.Path { case "/api/v1/namespaces/kube-system/configmaps": @@ -1780,7 +1780,7 @@ func TestImpersonatorHTTPHandler(t *testing.T) { testKubeAPIServerWasCalled := false testKubeAPIServerSawHeaders := http.Header{} testKubeAPIServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Secure(nil)) + tlsserver.AssertTLS(t, r, ptls.Secure) testKubeAPIServerWasCalled = true testKubeAPIServerSawHeaders = r.Header diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 643ae03a..c482c6d6 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -59,7 +59,7 @@ func TestController(t *testing.T) { mux := http.NewServeMux() server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Default(nil)) + tlsserver.AssertTLS(t, r, ptls.Default) mux.ServeHTTP(w, r) }), tlsserver.RecordTLSHello) diff --git a/internal/controller/kubecertagent/pod_command_executor_test.go b/internal/controller/kubecertagent/pod_command_executor_test.go index f046f851..1133e97f 100644 --- a/internal/controller/kubecertagent/pod_command_executor_test.go +++ b/internal/controller/kubecertagent/pod_command_executor_test.go @@ -19,7 +19,7 @@ import ( func TestSecureTLS(t *testing.T) { var sawRequest bool server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, ptls.Secure(nil)) + tlsserver.AssertTLS(t, r, ptls.Secure) sawRequest = true }), tlsserver.RecordTLSHello) diff --git a/internal/crypto/ptls/fips_strict.go b/internal/crypto/ptls/fips_strict.go index 8fc3c10a..5c1e21df 100644 --- a/internal/crypto/ptls/fips_strict.go +++ b/internal/crypto/ptls/fips_strict.go @@ -1,6 +1,9 @@ // Copyright 2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +// The configurations here override the usual ptls.Secure, ptls.Default, and ptls.DefaultLDAP +// configs when Pinniped is built in fips-only mode. +// All of these are the same because FIPs is already so limited. //go:build fips_strict // +build fips_strict @@ -25,23 +28,9 @@ func init() { }() } -// FIPS does not support TLS 1.3. -// Therefore, we cannot use Pinniped's usual secure configuration, -// which requires TLS 1.3. -// Secure is just a wrapper for Default in this case. -func Secure(rootCAs *x509.CertPool) *tls.Config { - return Default(rootCAs) -} - func Default(rootCAs *x509.CertPool) *tls.Config { return &tls.Config{ - // Can't use SSLv3 because of POODLE and BEAST - // Can't use TLSv1.0 because of POODLE and BEAST using CBC cipher - // Can't use TLSv1.1 because of RC4 cipher usage - // - // The Kubernetes API Server must use TLS 1.2, at a minimum, - // to protect the confidentiality of sensitive data during electronic dissemination. - // https://stigviewer.com/stig/kubernetes/2021-06-17/finding/V-242378 + // goboring requires TLS 1.2 and only TLS 1.2 MinVersion: SecureTLSConfigMinTLSVersion, // enable HTTP2 for go's 1.7 HTTP Server @@ -51,10 +40,15 @@ func Default(rootCAs *x509.CertPool) *tls.Config { // optional root CAs, nil means use the host's root CA set RootCAs: rootCAs, + + // Don't set CipherSuites, which means it will default to the FIPS-compatible ones. } } -func DefaultLDAP(rootCAs *x509.CertPool) *tls.Config { - c := Default(rootCAs) - return c +func Secure(rootCAs *x509.CertPool) *tls.Config { + return Default(rootCAs) +} + +func DefaultLDAP(rootCAs *x509.CertPool) *tls.Config { + return Default(rootCAs) } diff --git a/internal/net/phttp/phttp_test.go b/internal/net/phttp/phttp_test.go index cb0f68a5..20346f3a 100644 --- a/internal/net/phttp/phttp_test.go +++ b/internal/net/phttp/phttp_test.go @@ -84,7 +84,7 @@ func TestClient(t *testing.T) { var sawRequest bool server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - tlsserver.AssertTLS(t, r, tt.configFunc(nil)) + tlsserver.AssertTLS(t, r, tt.configFunc) assertUserAgent(t, r) sawRequest = true }), tlsserver.RecordTLSHello) diff --git a/internal/testutil/fakekubeapi/fakekubeapi.go b/internal/testutil/fakekubeapi/fakekubeapi.go index a1ba2793..2b4e20c3 100644 --- a/internal/testutil/fakekubeapi/fakekubeapi.go +++ b/internal/testutil/fakekubeapi/fakekubeapi.go @@ -56,7 +56,7 @@ func Start(t *testing.T, resources map[string]runtime.Object) (*httptest.Server, } server := tlsserver.TLSTestServer(t, httperr.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { - tlsserver.AssertTLS(t, r, ptls.Secure(nil)) + tlsserver.AssertTLS(t, r, ptls.Secure) obj, err := decodeObj(r) if err != nil { diff --git a/internal/testutil/tlsserver/tlsserver.go b/internal/testutil/tlsserver/tlsserver.go index 677dbca1..dad9de71 100644 --- a/internal/testutil/tlsserver/tlsserver.go +++ b/internal/testutil/tlsserver/tlsserver.go @@ -67,9 +67,15 @@ func RecordTLSHello(server *httptest.Server) { } } -// TODO maybe change this back to just taking a configfunc and making a wrapper for the -// fips stuff -func AssertTLS(t *testing.T, r *http.Request, tlsConfig *tls.Config) { +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()) diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index 44989552..bb837339 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -1820,7 +1820,7 @@ func TestRealTLSDialing(t *testing.T) { _, _ = recordFunc(info) r, err := http.NewRequestWithContext(info.Context(), http.MethodGet, "/this-is-ldap", nil) require.NoError(t, err) - tlsserver.AssertTLS(t, r, ptls.DefaultLDAP(nil)) + tlsserver.AssertTLS(t, r, ptls.DefaultLDAP) return nil, nil } }) diff --git a/test/integration/securetls_fips_test.go b/test/integration/securetls_fips_test.go index 27c75ef3..6209b5ec 100644 --- a/test/integration/securetls_fips_test.go +++ b/test/integration/securetls_fips_test.go @@ -10,7 +10,6 @@ import ( "bytes" "context" "crypto/tls" - _ "crypto/tls/fipsonly" // restricts all TLS configuration to FIPS-approved settings. "crypto/x509" "encoding/base64" "fmt" @@ -35,8 +34,6 @@ import ( // The expected cipher suites should belong to this // hard-coded list, copied from here: // https://github.com/golang/go/blob/dev.boringcrypto/src/crypto/tls/boring.go. -// TODO this is a private variable in the tls package... is there a better -// way to get access to it than just copying? var defaultCipherSuitesFIPS []uint16 = []uint16{ tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, @@ -47,18 +44,20 @@ var defaultCipherSuitesFIPS []uint16 = []uint16{ } // This test mirrors securetls_test.go, but adapted for fips mode. -// e.g. checks for only TLS 1.2 ciphers +// 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) { - // in fips mode the ciphers are nil, so we need to replace them with what we actually expect. + // 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) - // TODO this is kind of ugly... but I want different sort orders... secure.CipherSuites = deepcopy.Copy(defaultCipherSuitesFIPS).([]uint16) - tlsserver.AssertTLS(t, r, secure) // pinniped CLI uses ptls.Secure when talking to KAS + 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"}}}`) @@ -89,10 +88,12 @@ 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) - // TODO this is kind of ugly... but I want different sort orders... defaultTLS.CipherSuites = deepcopy.Copy(defaultCipherSuitesFIPS).([]uint16) - tlsserver.AssertTLS(t, r, defaultTLS) // pinniped CLI uses ptls.Default when talking to supervisor + tlsserver.AssertTLSConfig(t, r, defaultTLS) w.Header().Set("content-type", "application/json") fmt.Fprint(w, `{"issuer":"https://not-a-good-issuer"}`) }), tlsserver.RecordTLSHello) diff --git a/test/integration/securetls_test.go b/test/integration/securetls_test.go index 03f8547d..9c5b3c1d 100644 --- a/test/integration/securetls_test.go +++ b/test/integration/securetls_test.go @@ -34,7 +34,7 @@ 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(nil)) // pinniped CLI uses ptls.Secure when talking to KAS + tlsserver.AssertTLS(t, r, ptls.Secure) // pinniped CLI uses ptls.Secure when talking to KAS w.Header().Set("content-type", "application/json") fmt.Fprint(w, `{"kind":"TokenCredentialRequest","apiVersion":"login.concierge.pinniped.dev/v1alpha1",`+ `"status":{"credential":{"token":"some-fancy-token"}}}`) @@ -65,7 +65,7 @@ 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(nil)) // pinniped CLI uses ptls.Default when talking to supervisor + tlsserver.AssertTLS(t, r, ptls.Default) // pinniped CLI uses ptls.Default when talking to supervisor w.Header().Set("content-type", "application/json") fmt.Fprint(w, `{"issuer":"https://not-a-good-issuer"}`) }), tlsserver.RecordTLSHello) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 6c9a6061..6fc78990 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -662,7 +662,7 @@ func newHTTPClient(t *testing.T, caBundle string, dnsOverrides map[string]string caCertPool.AppendCertsFromPEM([]byte(caBundle)) c.Transport = &http.Transport{ DialContext: overrideDialContext, - TLSClientConfig: &tls.Config{MinVersion: ptls.SecureTLSConfigMinTLSVersion, RootCAs: caCertPool}, //nolint: gosec // this seems to be a false flag, min tls version is 1.3 or 1.2 in fips mode + TLSClientConfig: &tls.Config{MinVersion: ptls.SecureTLSConfigMinTLSVersion, RootCAs: caCertPool}, //nolint: gosec // this seems to be a false flag, min tls version is 1.3 in normal mode or 1.2 in fips mode } } else { c.Transport = &http.Transport{