diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1b1f337a..09779a71 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,7 +11,7 @@ repos: - id: check-json - id: end-of-file-fixer - id: trailing-whitespace - exclude: 'securetls*' + exclude: 'securetls*' # prevent the linter from running in this file because it's not smart enough not to trim the nmap test output. - id: check-merge-conflict - id: check-added-large-files - id: check-byte-order-marker diff --git a/internal/crypto/ptls/fips_strict.go b/internal/crypto/ptls/fips_strict.go index 5c1e21df..94c23fdf 100644 --- a/internal/crypto/ptls/fips_strict.go +++ b/internal/crypto/ptls/fips_strict.go @@ -32,6 +32,7 @@ func Default(rootCAs *x509.CertPool) *tls.Config { return &tls.Config{ // goboring requires TLS 1.2 and only TLS 1.2 MinVersion: SecureTLSConfigMinTLSVersion, + MaxVersion: SecureTLSConfigMinTLSVersion, // enable HTTP2 for go's 1.7 HTTP Server // setting this explicitly is only required in very specific circumstances diff --git a/internal/testutil/tlsserver/tlsserver.go b/internal/testutil/tlsserver/tlsserver.go index dad9de71..38ef025c 100644 --- a/internal/testutil/tlsserver/tlsserver.go +++ b/internal/testutil/tlsserver/tlsserver.go @@ -12,7 +12,6 @@ import ( "net/http" "net/http/httptest" "reflect" - "sort" "sync" "testing" @@ -100,12 +99,9 @@ func AssertTLSConfig(t *testing.T, r *http.Request, tlsConfig *tls.Config) { protos = tlsConfig.NextProtos[1:] } - helloInfoCiphers := info.CipherSuites - sort.Slice(helloInfoCiphers, func(i, j int) bool { return helloInfoCiphers[i] < helloInfoCiphers[j] }) - sort.Slice(ciphers, func(i, j int) bool { return ciphers[i] < ciphers[j] }) // use assert instead of require to not break the http.Handler with a panic ok1 := assert.Equal(t, supportedVersions, info.SupportedVersions) - ok2 := assert.Equal(t, ciphers, helloInfoCiphers) + ok2 := assert.Equal(t, ciphers, info.CipherSuites) ok3 := assert.Equal(t, protos, info.SupportedProtos) if all := ok1 && ok2 && ok3; !all { diff --git a/test/integration/concierge_impersonation_proxy_test.go b/test/integration/concierge_impersonation_proxy_test.go index 352a473a..c6dbd2fb 100644 --- a/test/integration/concierge_impersonation_proxy_test.go +++ b/test/integration/concierge_impersonation_proxy_test.go @@ -1503,10 +1503,10 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl startKubectlPortForward(cancelCtx, t, "10445", "443", env.ConciergeAppName+"-proxy", env.ConciergeNamespace) - stdout, stderr := runNmapSSLEnum(t, "127.0.0.1", 10445) + stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10445) require.Empty(t, stderr) - require.Contains(t, stdout, getExpectedCiphers(ptls.Default), "stdout:\n%s", stdout) + require.Contains(t, stdout, testlib.GetExpectedCiphers(ptls.Default(nil), "client"), "stdout:\n%s", stdout) }) }) diff --git a/test/integration/securetls_fips_test.go b/test/integration/securetls_fips_test.go index 6209b5ec..f60ff17d 100644 --- a/test/integration/securetls_fips_test.go +++ b/test/integration/securetls_fips_test.go @@ -7,21 +7,14 @@ package integration import ( - "bytes" "context" "crypto/tls" - "crypto/x509" "encoding/base64" "fmt" "net/http" - "os/exec" - "regexp" - "strconv" "strings" "testing" - "time" - "github.com/mohae/deepcopy" "github.com/stretchr/testify/require" "go.pinniped.dev/internal/crypto/ptls" @@ -34,10 +27,10 @@ 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. -var defaultCipherSuitesFIPS []uint16 = []uint16{ +var defaultCipherSuitesFIPS = []uint16{ tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, tls.TLS_RSA_WITH_AES_128_GCM_SHA256, tls.TLS_RSA_WITH_AES_256_GCM_SHA384, @@ -56,7 +49,7 @@ func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) { // 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) - secure.CipherSuites = deepcopy.Copy(defaultCipherSuitesFIPS).([]uint16) + secure.CipherSuites = defaultCipherSuitesFIPS tlsserver.AssertTLSConfig(t, r, secure) w.Header().Set("content-type", "application/json") fmt.Fprint(w, `{"kind":"TokenCredentialRequest","apiVersion":"login.concierge.pinniped.dev/v1alpha1",`+ @@ -92,7 +85,7 @@ func TestSecureTLSPinnipedCLIToSupervisor_Parallel(t *testing.T) { // 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) - defaultTLS.CipherSuites = deepcopy.Copy(defaultCipherSuitesFIPS).([]uint16) + defaultTLS.CipherSuites = defaultCipherSuitesFIPS tlsserver.AssertTLSConfig(t, r, defaultTLS) w.Header().Set("content-type", "application/json") fmt.Fprint(w, `{"issuer":"https://not-a-good-issuer"}`) @@ -127,10 +120,12 @@ func TestSecureTLSConciergeAggregatedAPI_Parallel(t *testing.T) { startKubectlPortForward(cancelCtx, t, "10446", "443", env.ConciergeAppName+"-api", env.ConciergeNamespace) - stdout, stderr := runNmapSSLEnum(t, "127.0.0.1", 10446) + stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10446) require.Empty(t, stderr) - require.Contains(t, stdout, getExpectedCiphers(ptls.Secure), "stdout:\n%s", stdout) + secure := ptls.Secure(nil) + secure.CipherSuites = defaultCipherSuitesFIPS + require.Contains(t, stdout, testlib.GetExpectedCiphers(secure, "server"), "stdout:\n%s", stdout) } func TestSecureTLSSupervisor(t *testing.T) { // does not run in parallel because of the createSupervisorDefaultTLSCertificateSecretIfNeeded call @@ -141,25 +136,22 @@ func TestSecureTLSSupervisor(t *testing.T) { // does not run in parallel because startKubectlPortForward(ctx, t, "10447", "443", env.SupervisorAppName+"-nodeport", env.SupervisorNamespace) - stdout, stderr := runNmapSSLEnum(t, "127.0.0.1", 10447) + stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10447) // supervisor's cert is ECDSA - defaultECDSAOnly := func(rootCAs *x509.CertPool) *tls.Config { - c := ptls.Default(rootCAs) - ciphers := make([]uint16, 0, len(defaultCipherSuitesFIPS)/3) - for _, id := range defaultCipherSuitesFIPS { - id := id - if !strings.Contains(tls.CipherSuiteName(id), "_ECDSA_") { - continue - } - ciphers = append(ciphers, id) + defaultECDSAOnly := ptls.Default(nil) + ciphers := make([]uint16, 0, len(defaultCipherSuitesFIPS)/3) + for _, id := range defaultCipherSuitesFIPS { + id := id + if !strings.Contains(tls.CipherSuiteName(id), "_ECDSA_") { + continue } - c.CipherSuites = ciphers - return c + ciphers = append(ciphers, id) } + defaultECDSAOnly.CipherSuites = ciphers require.Empty(t, stderr) - require.Contains(t, stdout, getExpectedCiphers(defaultECDSAOnly), "stdout:\n%s", stdout) + require.Contains(t, stdout, testlib.GetExpectedCiphers(defaultECDSAOnly, "server"), "stdout:\n%s", stdout) } type fakeT struct { @@ -178,98 +170,3 @@ func (t *fakeT) Errorf(format string, args ...interface{}) { t.Logf("reporting previously ignored errors since main test failed:\n"+format, args...) }) } - -func runNmapSSLEnum(t *testing.T, host string, port uint16) (string, string) { - t.Helper() - - ctx, cancel := context.WithTimeout(context.Background(), time.Minute) - defer cancel() - - version, err := exec.CommandContext(ctx, "nmap", "-V").CombinedOutput() - require.NoError(t, err) - - versionMatches := regexp.MustCompile(`Nmap version 7\.(?P\d+)`).FindStringSubmatch(string(version)) - require.Len(t, versionMatches, 2) - minorVersion, err := strconv.Atoi(versionMatches[1]) - require.NoError(t, err) - require.GreaterOrEqual(t, minorVersion, 92, "nmap >= 7.92.x is required") - - var stdout, stderr bytes.Buffer - //nolint:gosec // we are not performing malicious argument injection against ourselves - cmd := exec.CommandContext(ctx, "nmap", "--script", "ssl-enum-ciphers", - "-p", strconv.FormatUint(uint64(port), 10), - host, - ) - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - require.NoErrorf(t, cmd.Run(), "stderr:\n%s\n\nstdout:\n%s\n\n", stderr.String(), stdout.String()) - - return stdout.String(), stderr.String() -} - -// Note that the fips version doesn't do the same TLS 1.3 checks. -// This is because goboring's maxtlsversion is 1.2. -func getExpectedCiphers(configFunc ptls.ConfigFunc) string { - config := configFunc(nil) - // Cipher suites may be nil, in which case - // we should use the default fips cipher - // suites. - cipherSuites := config.CipherSuites - if cipherSuites == nil { - cipherSuites = defaultCipherSuitesFIPS - } - - var tls12Bit, tls13Bit string - - // use the TLS 1.2 ciphers to create the output in nmap's format. - var s strings.Builder - for i, id := range cipherSuites { - name := tls.CipherSuiteName(id) - description := "" - if strings.Contains(name, "_ECDHE_") { - description = secp256r1 - } else { - description = rsa2048 - } - s.WriteString(fmt.Sprintf(tls12Item, name, description)) - if i == len(cipherSuites)-1 { - break - } - s.WriteString("\n") - } - tls12Bit = fmt.Sprintf(tls12Base, s.String()) - - // There should be no TLS 1.3 ciphers. - // goboring disallows TLS 1.3 - tls13Bit = "" - - return fmt.Sprintf(baseItem, tls12Bit, tls13Bit) -} - -const ( - // this surrounds the tls 1.2 and 1.3 text in a way that guarantees that other TLS versions are not supported. - baseItem = `/tcp open unknown -| ssl-enum-ciphers: %s%s -|_ least strength: A - -Nmap done: 1 IP address (1 host up) scanned in` - - // the "cipher preference: client" bit a bug in nmap. - // https://github.com/nmap/nmap/issues/1691#issuecomment-536919978 - tls12Base = ` -| TLSv1.2: -| ciphers: -%s -| compressors: -| NULL -| cipher preference: server` - - tls12Item = `| %s (%s) - A` - - // This curve name is part of the output for each of our elliptic curve ciphers. - // secp256r1 is also known as P-256. - secp256r1 = "secp256r1" - // For the RSA ciphers, we expect this output to be RSA 2048. - rsa2048 = "rsa 2048" -) diff --git a/test/integration/securetls_test.go b/test/integration/securetls_test.go index 9c5b3c1d..f00aa9cd 100644 --- a/test/integration/securetls_test.go +++ b/test/integration/securetls_test.go @@ -7,20 +7,13 @@ package integration import ( - "bytes" "context" "crypto/tls" - "crypto/x509" "encoding/base64" "fmt" "net/http" - "os/exec" - "regexp" - "sort" - "strconv" "strings" "testing" - "time" "github.com/stretchr/testify/require" @@ -99,10 +92,10 @@ func TestSecureTLSConciergeAggregatedAPI_Parallel(t *testing.T) { startKubectlPortForward(cancelCtx, t, "10446", "443", env.ConciergeAppName+"-api", env.ConciergeNamespace) - stdout, stderr := runNmapSSLEnum(t, "127.0.0.1", 10446) + stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10446) require.Empty(t, stderr) - require.Contains(t, stdout, getExpectedCiphers(ptls.Secure), "stdout:\n%s", stdout) + require.Contains(t, stdout, testlib.GetExpectedCiphers(ptls.Secure(nil), "client"), "stdout:\n%s", stdout) } func TestSecureTLSSupervisor(t *testing.T) { // does not run in parallel because of the createSupervisorDefaultTLSCertificateSecretIfNeeded call @@ -113,25 +106,22 @@ func TestSecureTLSSupervisor(t *testing.T) { // does not run in parallel because startKubectlPortForward(ctx, t, "10447", "443", env.SupervisorAppName+"-nodeport", env.SupervisorNamespace) - stdout, stderr := runNmapSSLEnum(t, "127.0.0.1", 10447) + stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10447) // supervisor's cert is ECDSA - defaultECDSAOnly := func(rootCAs *x509.CertPool) *tls.Config { - c := ptls.Default(rootCAs) - ciphers := make([]uint16, 0, len(c.CipherSuites)/2) - for _, id := range c.CipherSuites { - id := id - if !strings.Contains(tls.CipherSuiteName(id), "_ECDSA_") { - continue - } - ciphers = append(ciphers, id) + defaultECDSAOnly := ptls.Default(nil) + ciphers := make([]uint16, 0, len(defaultECDSAOnly.CipherSuites)/2) + for _, id := range defaultECDSAOnly.CipherSuites { + id := id + if !strings.Contains(tls.CipherSuiteName(id), "_ECDSA_") { + continue } - c.CipherSuites = ciphers - return c + ciphers = append(ciphers, id) } + defaultECDSAOnly.CipherSuites = ciphers require.Empty(t, stderr) - require.Contains(t, stdout, getExpectedCiphers(defaultECDSAOnly), "stdout:\n%s", stdout) + require.Contains(t, stdout, testlib.GetExpectedCiphers(defaultECDSAOnly, "client"), "stdout:\n%s", stdout) } type fakeT struct { @@ -150,107 +140,3 @@ func (t *fakeT) Errorf(format string, args ...interface{}) { t.Logf("reporting previously ignored errors since main test failed:\n"+format, args...) }) } - -func runNmapSSLEnum(t *testing.T, host string, port uint16) (string, string) { - t.Helper() - - ctx, cancel := context.WithTimeout(context.Background(), time.Minute) - defer cancel() - - version, err := exec.CommandContext(ctx, "nmap", "-V").CombinedOutput() - require.NoError(t, err) - - versionMatches := regexp.MustCompile(`Nmap version 7\.(?P\d+)`).FindStringSubmatch(string(version)) - require.Len(t, versionMatches, 2) - minorVersion, err := strconv.Atoi(versionMatches[1]) - require.NoError(t, err) - require.GreaterOrEqual(t, minorVersion, 92, "nmap >= 7.92.x is required") - - var stdout, stderr bytes.Buffer - //nolint:gosec // we are not performing malicious argument injection against ourselves - cmd := exec.CommandContext(ctx, "nmap", "--script", "ssl-enum-ciphers", - "-p", strconv.FormatUint(uint64(port), 10), - host, - ) - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - require.NoErrorf(t, cmd.Run(), "stderr:\n%s\n\nstdout:\n%s\n\n", stderr.String(), stdout.String()) - - return stdout.String(), stderr.String() -} - -func getExpectedCiphers(configFunc ptls.ConfigFunc) string { - config := configFunc(nil) - secureConfig := ptls.Secure(nil) - - skip12 := config.MinVersion == secureConfig.MinVersion - - var tls12Bit, tls13Bit string - - if !skip12 { - sort.SliceStable(config.CipherSuites, func(i, j int) bool { - a := tls.CipherSuiteName(config.CipherSuites[i]) - b := tls.CipherSuiteName(config.CipherSuites[j]) - - ok1 := strings.Contains(a, "_ECDSA_") - ok2 := strings.Contains(b, "_ECDSA_") - - if ok1 && ok2 { - return false - } - - return ok1 - }) - - var s strings.Builder - for i, id := range config.CipherSuites { - s.WriteString(fmt.Sprintf(tls12Item, tls.CipherSuiteName(id))) - if i == len(config.CipherSuites)-1 { - break - } - s.WriteString("\n") - } - tls12Bit = fmt.Sprintf(tls12Base, s.String()) - } - - var s strings.Builder - for i, id := range secureConfig.CipherSuites { - s.WriteString(fmt.Sprintf(tls13Item, strings.Replace(tls.CipherSuiteName(id), "TLS_", "TLS_AKE_WITH_", 1))) - if i == len(secureConfig.CipherSuites)-1 { - break - } - s.WriteString("\n") - } - tls13Bit = fmt.Sprintf(tls13Base, s.String()) - - return fmt.Sprintf(baseItem, tls12Bit, tls13Bit) -} - -const ( - // this surrounds the tls 1.2 and 1.3 text in a way that guarantees that other TLS versions are not supported. - baseItem = `/tcp open unknown -| ssl-enum-ciphers: %s%s -|_ least strength: A - -Nmap done: 1 IP address (1 host up) scanned in` - - // the "cipher preference: client" bit a bug in nmap. - // https://github.com/nmap/nmap/issues/1691#issuecomment-536919978 - tls12Base = ` -| TLSv1.2: -| ciphers: -%s -| compressors: -| NULL -| cipher preference: client` - - tls13Base = ` -| TLSv1.3: -| ciphers: -%s -| cipher preference: server` - - tls12Item = `| %s (secp256r1) - A` - tls13Item = `| %s (ecdh_x25519) - A` -) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 6fc78990..9991fa01 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -18,8 +18,6 @@ import ( "testing" "time" - "go.pinniped.dev/internal/crypto/ptls" - "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -30,6 +28,7 @@ import ( "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" "go.pinniped.dev/internal/certauthority" + "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/here" "go.pinniped.dev/test/testlib" ) diff --git a/test/testlib/securetls.go b/test/testlib/securetls.go new file mode 100644 index 00000000..00d07e73 --- /dev/null +++ b/test/testlib/securetls.go @@ -0,0 +1,142 @@ +// Copyright 2022 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package testlib + +import ( + "bytes" + "context" + "crypto/tls" + "fmt" + "os/exec" + "regexp" + "sort" + "strconv" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "go.pinniped.dev/internal/crypto/ptls" +) + +func RunNmapSSLEnum(t *testing.T, host string, port uint16) (string, string) { + t.Helper() + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + + version, err := exec.CommandContext(ctx, "nmap", "-V").CombinedOutput() + require.NoError(t, err) + + versionMatches := regexp.MustCompile(`Nmap version 7\.(?P\d+)`).FindStringSubmatch(string(version)) + require.Len(t, versionMatches, 2) + minorVersion, err := strconv.Atoi(versionMatches[1]) + require.NoError(t, err) + require.GreaterOrEqual(t, minorVersion, 92, "nmap >= 7.92.x is required") + + var stdout, stderr bytes.Buffer + //nolint:gosec // we are not performing malicious argument injection against ourselves + cmd := exec.CommandContext(ctx, "nmap", "--script", "ssl-enum-ciphers", + "-p", strconv.FormatUint(uint64(port), 10), + host, + ) + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + require.NoErrorf(t, cmd.Run(), "stderr:\n%s\n\nstdout:\n%s\n\n", stderr.String(), stdout.String()) + + return stdout.String(), stderr.String() +} + +func GetExpectedCiphers(config *tls.Config, cipherPreference12 string) string { + secureConfig := ptls.Secure(nil) + + skip12 := config.MinVersion == tls.VersionTLS13 + + var tls12Bit, tls13Bit string + + if !skip12 { + sort.SliceStable(config.CipherSuites, func(i, j int) bool { + a := tls.CipherSuiteName(config.CipherSuites[i]) + b := tls.CipherSuiteName(config.CipherSuites[j]) + + ok1 := strings.Contains(a, "_ECDSA_") + ok2 := strings.Contains(b, "_ECDSA_") + + if ok1 && ok2 { + return false + } + + return ok1 + }) + + var s strings.Builder + for i, id := range config.CipherSuites { + name := tls.CipherSuiteName(id) + group := "" + if strings.Contains(name, "_ECDHE_") { + group = secp256r1 + } else { + group = rsa2048 + } + s.WriteString(fmt.Sprintf(tls12Item, name, group)) + if i == len(config.CipherSuites)-1 { + break + } + s.WriteString("\n") + } + tls12Bit = fmt.Sprintf(tls12Base, s.String(), cipherPreference12) + } + + skip13 := config.MaxVersion == tls.VersionTLS12 + if !skip13 { + var s strings.Builder + for i, id := range secureConfig.CipherSuites { + s.WriteString(fmt.Sprintf(tls13Item, strings.Replace(tls.CipherSuiteName(id), "TLS_", "TLS_AKE_WITH_", 1))) + if i == len(secureConfig.CipherSuites)-1 { + break + } + s.WriteString("\n") + } + tls13Bit = fmt.Sprintf(tls13Base, s.String()) + } + + return fmt.Sprintf(baseItem, tls12Bit, tls13Bit) +} + +const ( + // this surrounds the tls 1.2 and 1.3 text in a way that guarantees that other TLS versions are not supported. + baseItem = `/tcp open unknown +| ssl-enum-ciphers: %s%s +|_ least strength: A + +Nmap done: 1 IP address (1 host up) scanned in` + + // cipher preference is a variable because in FIPs mode it is server + // but in normal mode it is client. + tls12Base = ` +| TLSv1.2: +| ciphers: +%s +| compressors: +| NULL +| cipher preference: %s` + + tls12Item = `| %s (%s) - A` + + tls13Base = ` +| TLSv1.3: +| ciphers: +%s +| cipher preference: server` + + // This curve name is part of the output for each of our elliptic curve ciphers. + // secp256r1 is also known as P-256. + secp256r1 = "secp256r1" + // For the RSA ciphers, we expect this output to be RSA 2048. + rsa2048 = "rsa 2048" + + tls13Item = `| %s (ecdh_x25519) - A` +)