From bcf070cb731119fff9b4e8adc41aaffd0f1dd17d Mon Sep 17 00:00:00 2001 From: Pinny Date: Tue, 16 Jan 2024 09:09:12 +0000 Subject: [PATCH 1/2] Bump dependencies --- Dockerfile | 2 +- go.mod | 8 ++++---- go.sum | 17 ++++++++--------- hack/Dockerfile_fips | 2 +- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Dockerfile b/Dockerfile index 3a62ce3c4..e596b7f99 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ # Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -ARG BUILD_IMAGE=golang:1.21.5@sha256:672a2286da3ee7a854c3e0a56e0838918d0dbb1c18652992930293312de898a6 +ARG BUILD_IMAGE=golang:1.21.6@sha256:6fbd2d3398db924f8d708cf6e94bd3a436bb468195daa6a96e80504e0a9615f2 ARG BASE_IMAGE=gcr.io/distroless/static:nonroot@sha256:112a87f19e83c83711cc81ce8ed0b4d79acd65789682a6a272df57c4a0858534 # Prepare to cross-compile by always running the build stage in the build platform, not the target platform. diff --git a/go.mod b/go.mod index 41c034ae9..a88a048e8 100644 --- a/go.mod +++ b/go.mod @@ -53,7 +53,7 @@ require ( github.com/spf13/cobra v1.8.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.4 - github.com/tdewolff/minify/v2 v2.20.12 + github.com/tdewolff/minify/v2 v2.20.14 go.uber.org/zap v1.26.0 golang.org/x/crypto v0.18.0 golang.org/x/net v0.20.0 @@ -67,8 +67,8 @@ require ( k8s.io/apiserver v0.29.0 k8s.io/client-go v0.29.0 k8s.io/component-base v0.29.0 - k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 - k8s.io/klog/v2 v2.110.1 + k8s.io/gengo v0.0.0-20240110203215-22eea95d1e7a + k8s.io/klog/v2 v2.120.0 k8s.io/kube-aggregator v0.29.0 k8s.io/kube-openapi v0.0.0-20240105020646-a37d4de58910 k8s.io/utils v0.0.0-20240102154912-e7106e64919e @@ -151,7 +151,7 @@ require ( github.com/spf13/viper v1.16.0 // indirect github.com/stoewer/go-strcase v1.2.0 // indirect github.com/subosito/gotenv v1.4.2 // indirect - github.com/tdewolff/parse/v2 v2.7.7 // indirect + github.com/tdewolff/parse/v2 v2.7.8 // indirect go.etcd.io/etcd/api/v3 v3.5.10 // indirect go.etcd.io/etcd/client/pkg/v3 v3.5.10 // indirect go.etcd.io/etcd/client/v3 v3.5.10 // indirect diff --git a/go.sum b/go.sum index 177c6efdc..3de557c41 100644 --- a/go.sum +++ b/go.sum @@ -157,7 +157,6 @@ github.com/go-ldap/ldap/v3 v3.4.6/go.mod h1:IGMQANNtxpsOzj7uUAMjpGBaOVTC4DYyIy8V github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= github.com/go-logr/logr v0.2.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= @@ -553,10 +552,10 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/subosito/gotenv v1.4.2 h1:X1TuBLAMDFbaTAChgCBLu3DU3UPyELpnF2jjJ2cz/S8= github.com/subosito/gotenv v1.4.2/go.mod h1:ayKnFf/c6rvx/2iiLrJUk1e6plDbT3edrFNGqEflhK0= -github.com/tdewolff/minify/v2 v2.20.12 h1:ie5+91QGUUeEDbLkexhx2tlI9BQgwwnfY+/Qdj4BlQ4= -github.com/tdewolff/minify/v2 v2.20.12/go.mod h1:8ktdncc9Rh41MkTX2KYaicHT9+VnpvIDjCyIVsr/nN8= -github.com/tdewolff/parse/v2 v2.7.7 h1:V+50eFDH7Piw4IBwH8D8FtYeYbZp3T4SCtIvmBSIMyc= -github.com/tdewolff/parse/v2 v2.7.7/go.mod h1:3FbJWZp3XT9OWVN3Hmfp0p/a08v4h8J9W1aghka0soA= +github.com/tdewolff/minify/v2 v2.20.14 h1:sktSuVixRwk0ryQjqvKBu/uYS+MWmkwEFMEWtFZ+TdE= +github.com/tdewolff/minify/v2 v2.20.14/go.mod h1:qnIJbnG2dSzk7LIa/UUwgN2OjS8ir6RRlqc0T/1q2xY= +github.com/tdewolff/parse/v2 v2.7.8 h1:1cnVqa8L63xFkc2vfRsZTM6Qy35nJpTvQ2Uvdv3vbvs= +github.com/tdewolff/parse/v2 v2.7.8/go.mod h1:3FbJWZp3XT9OWVN3Hmfp0p/a08v4h8J9W1aghka0soA= github.com/tdewolff/test v1.0.11-0.20231101010635-f1265d231d52/go.mod h1:6DAvZliBAAnD7rhVgwaM7DE5/d9NMOAJ09SqYqeK4QE= github.com/tdewolff/test v1.0.11-0.20240106005702-7de5f7df4739 h1:IkjBCtQOOjIn03u/dMQK9g+Iw9ewps4mCl1nB8Sscbo= github.com/tdewolff/test v1.0.11-0.20240106005702-7de5f7df4739/go.mod h1:XPuWBzvdUzhCuxWO1ojpXsyzsA5bFoS3tO/Q3kFuTG8= @@ -1078,11 +1077,11 @@ k8s.io/client-go v0.29.0 h1:KmlDtFcrdUzOYrBhXHgKw5ycWzc3ryPX5mQe0SkG3y8= k8s.io/client-go v0.29.0/go.mod h1:yLkXH4HKMAywcrD82KMSmfYg2DlE8mepPR4JGSo5n38= k8s.io/component-base v0.29.0 h1:T7rjd5wvLnPBV1vC4zWd/iWRbV8Mdxs+nGaoaFzGw3s= k8s.io/component-base v0.29.0/go.mod h1:sADonFTQ9Zc9yFLghpDpmNXEdHyQmFIGbiuZbqAXQ1M= -k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 h1:pWEwq4Asjm4vjW7vcsmijwBhOr1/shsbSYiWXmNGlks= -k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= +k8s.io/gengo v0.0.0-20240110203215-22eea95d1e7a h1:zCwpCC6Ghs+RstFfEJZ3arc7dLZ9z0tlmLHDXGCkINY= +k8s.io/gengo v0.0.0-20240110203215-22eea95d1e7a/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= -k8s.io/klog/v2 v2.110.1 h1:U/Af64HJf7FcwMcXyKm2RPM22WZzyR7OSpYj5tg3cL0= -k8s.io/klog/v2 v2.110.1/go.mod h1:YGtd1984u+GgbuZ7e08/yBuAfKLSO0+uR1Fhi6ExXjo= +k8s.io/klog/v2 v2.120.0 h1:z+q5mfovBj1fKFxiRzsa2DsJLPIVMk/KFL81LMOfK+8= +k8s.io/klog/v2 v2.120.0/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kms v0.29.0 h1:KJ1zaZt74CgvgV3NR7tnURJ/mJOKC5X3nwon/WdwgxI= k8s.io/kms v0.29.0/go.mod h1:mB0f9HLxRXeXUfHfn1A7rpwOlzXI1gIWu86z6buNoYA= k8s.io/kube-aggregator v0.29.0 h1:N4fmtePxOZ+bwiK1RhVEztOU+gkoVkvterHgpwAuiTw= diff --git a/hack/Dockerfile_fips b/hack/Dockerfile_fips index 1802c9904..cde507aff 100644 --- a/hack/Dockerfile_fips +++ b/hack/Dockerfile_fips @@ -16,7 +16,7 @@ # See https://go.googlesource.com/go/+/dev.boringcrypto/README.boringcrypto.md # and https://kupczynski.info/posts/fips-golang/ for details. -ARG BUILD_IMAGE=golang:1.21.5@sha256:672a2286da3ee7a854c3e0a56e0838918d0dbb1c18652992930293312de898a6 +ARG BUILD_IMAGE=golang:1.21.6@sha256:6fbd2d3398db924f8d708cf6e94bd3a436bb468195daa6a96e80504e0a9615f2 ARG BASE_IMAGE=gcr.io/distroless/static:nonroot@sha256:112a87f19e83c83711cc81ce8ed0b4d79acd65789682a6a272df57c4a0858534 # This is not currently using --platform to prepare to cross-compile because we use gcc below to build From 50e4d6db6cd7eb9a93036f4d1fd2bc021767d986 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 17 Jan 2024 14:29:08 -0800 Subject: [PATCH 2/2] Support the new Go FIPS compiler which was upgraded inside Go 1.21.6 The release of Go 1.21.6 includes the new boring crypto when compiling with FIPS enabled. See https://go.dev/doc/devel/release#go1.21.0 and https://github.com/golang/go/issues/64717. This new version of boring crypto allows the use of TLS v1.3 for the first time, so we changed the Pinniped code to use TLS v1.3 where appropriate when compiled with the FIPS compiler. It also changed the allowed TLS v1.2 ciphers, so we updated those as well. After this commit, the project must be compiled by at least Go v1.21.6 when compiling in fips mode. The hack/Dockerfile_fips was already updated to use that version of Go in a previous commit. Co-authored-by: Benjamin A. Petersen --- internal/crypto/ptls/fips_strict.go | 51 ++++++++----------- internal/crypto/ptls/ptls_test.go | 42 +++++++-------- internal/crypto/ptls/secure.go | 12 +---- internal/testutil/tlsserver/tls13_ciphers.go | 31 +++++++++++ .../testutil/tlsserver/tls13_ciphers_fips.go | 30 +++++++++++ internal/testutil/tlsserver/tlsserver.go | 51 +++++++++++++------ test/integration/securetls_test.go | 35 +++++++++---- test/testlib/securetls.go | 22 ++++---- 8 files changed, 175 insertions(+), 99 deletions(-) create mode 100644 internal/testutil/tlsserver/tls13_ciphers.go create mode 100644 internal/testutil/tlsserver/tls13_ciphers_fips.go diff --git a/internal/crypto/ptls/fips_strict.go b/internal/crypto/ptls/fips_strict.go index d5039854b..6cf4bfd82 100644 --- a/internal/crypto/ptls/fips_strict.go +++ b/internal/crypto/ptls/fips_strict.go @@ -1,9 +1,8 @@ -// Copyright 2022-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2022-2024 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 +// The configurations here override the usual 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 package ptls @@ -15,16 +14,16 @@ import ( "path/filepath" "runtime" - "k8s.io/apiserver/pkg/server/options" - // Cause fipsonly tls mode with this side effect import. _ "go.pinniped.dev/internal/crypto/fips" "go.pinniped.dev/internal/plog" ) -// Always use TLS 1.2 for FIPs -const secureServingOptionsMinTLSVersion = "VersionTLS12" -const SecureTLSConfigMinTLSVersion = tls.VersionTLS12 +// goboring now also supports TLS 1.3 starting in Golang 1.21.6 +// (see https://github.com/golang/go/issues/64717), +// so we can use TLS 1.3 as the minimum TLS version for our "secure" configuration +// profile in both FIPS and non-FIPS compiled binaries. +// Hence, we no longer redefine the Secure() function in this file. func init() { switch filepath.Base(os.Args[0]) { @@ -40,10 +39,22 @@ func init() { func Default(rootCAs *x509.CertPool) *tls.Config { return &tls.Config{ - // goboring requires TLS 1.2 and only TLS 1.2 - MinVersion: SecureTLSConfigMinTLSVersion, + MinVersion: tls.VersionTLS12, + // goboring now also supports TLS 1.3 (see https://github.com/golang/go/issues/64717) + // so this default configuration can allow either 1.2 or 1.3 MaxVersion: SecureTLSConfigMinTLSVersion, + // This is all the fips-approved TLS 1.2 ciphers. + // The list is hard-coded for convenience of testing. + // If this list does not match the boring crypto compiler's list then the TestFIPSCipherSuites integration + // test should fail, which indicates that this list needs to be updated. + CipherSuites: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + 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, + }, + // enable HTTP2 for go's 1.7 HTTP Server // setting this explicitly is only required in very specific circumstances // it is simpler to just set it here than to try and determine if we need to @@ -51,29 +62,9 @@ func Default(rootCAs *x509.CertPool) *tls.Config { // optional root CAs, nil means use the host's root CA set RootCAs: rootCAs, - - // This is all of the fips-approved ciphers. - // The list is hard-coded for convenience of testing. - // This is kept in sync with the boring crypto compiler via TestFIPSCipherSuites. - CipherSuites: []uint16{ - tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - 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, - }, } } -func Secure(rootCAs *x509.CertPool) *tls.Config { - return Default(rootCAs) -} - func DefaultLDAP(rootCAs *x509.CertPool) *tls.Config { return Default(rootCAs) } - -func secureServing(opts *options.SecureServingOptionsWithLoopback) { - defaultServing(opts) -} diff --git a/internal/crypto/ptls/ptls_test.go b/internal/crypto/ptls/ptls_test.go index e8475b958..27aa13231 100644 --- a/internal/crypto/ptls/ptls_test.go +++ b/internal/crypto/ptls/ptls_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package ptls @@ -53,7 +53,7 @@ func TestMerge(t *testing.T) { want *tls.Config }{ { - name: "default no protos", + name: "default without NextProtos", tlsConfigFunc: Default, tlsConfig: &tls.Config{ //nolint:gosec // not concerned with TLS MinVersion here ServerName: "something-to-check-passthrough", @@ -73,7 +73,7 @@ func TestMerge(t *testing.T) { }, }, { - name: "default with protos", + name: "default with NextProtos", tlsConfigFunc: Default, tlsConfig: &tls.Config{ //nolint:gosec // not concerned with TLS MinVersion here ServerName: "a different thing for passthrough", @@ -94,42 +94,34 @@ func TestMerge(t *testing.T) { }, }, { - name: "secure no protos", + name: "secure without NextProtos", tlsConfigFunc: Secure, tlsConfig: &tls.Config{ //nolint:gosec // not concerned with TLS MinVersion here ServerName: "something-to-check-passthrough", }, want: &tls.Config{ - ServerName: "something-to-check-passthrough", - MinVersion: tls.VersionTLS13, - CipherSuites: []uint16{ - tls.TLS_AES_128_GCM_SHA256, - tls.TLS_AES_256_GCM_SHA384, - tls.TLS_CHACHA20_POLY1305_SHA256, - }, - NextProtos: []string{"h2", "http/1.1"}, + ServerName: "something-to-check-passthrough", + MinVersion: tls.VersionTLS13, + CipherSuites: nil, + NextProtos: []string{"h2", "http/1.1"}, }, }, { - name: "secure with protos", + name: "secure with NextProtos", tlsConfigFunc: Secure, tlsConfig: &tls.Config{ //nolint:gosec // not concerned with TLS MinVersion here ServerName: "a different thing for passthrough", NextProtos: []string{"panda"}, }, want: &tls.Config{ - ServerName: "a different thing for passthrough", - MinVersion: tls.VersionTLS13, - CipherSuites: []uint16{ - tls.TLS_AES_128_GCM_SHA256, - tls.TLS_AES_256_GCM_SHA384, - tls.TLS_CHACHA20_POLY1305_SHA256, - }, - NextProtos: []string{"panda"}, + ServerName: "a different thing for passthrough", + MinVersion: tls.VersionTLS13, + CipherSuites: nil, + NextProtos: []string{"panda"}, }, }, { - name: "default ldap no protos", + name: "default ldap without NextProtos", tlsConfigFunc: DefaultLDAP, tlsConfig: &tls.Config{ //nolint:gosec // not concerned with TLS MinVersion here ServerName: "something-to-check-passthrough", @@ -153,7 +145,7 @@ func TestMerge(t *testing.T) { }, }, { - name: "default ldap with protos", + name: "default ldap with NextProtos", tlsConfigFunc: DefaultLDAP, tlsConfig: &tls.Config{ ServerName: "a different thing for passthrough", @@ -178,7 +170,7 @@ func TestMerge(t *testing.T) { }, }, { - name: "legacy no protos", + name: "legacy without NextProtos", tlsConfigFunc: Legacy, tlsConfig: &tls.Config{ ServerName: "something-to-check-passthrough", @@ -209,7 +201,7 @@ func TestMerge(t *testing.T) { }, }, { - name: "legacy with protos", + name: "legacy with NextProtos", tlsConfigFunc: Legacy, tlsConfig: &tls.Config{ ServerName: "a different thing for passthrough", diff --git a/internal/crypto/ptls/secure.go b/internal/crypto/ptls/secure.go index 68e65b0ec..7f628f18f 100644 --- a/internal/crypto/ptls/secure.go +++ b/internal/crypto/ptls/secure.go @@ -1,8 +1,6 @@ -// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -//go:build !fips_strict - package ptls import ( @@ -34,13 +32,7 @@ func Secure(rootCAs *x509.CertPool) *tls.Config { // https://ssl-config.mozilla.org/#server=go&version=1.17.2&config=modern&guideline=5.6 c := Default(rootCAs) c.MinVersion = SecureTLSConfigMinTLSVersion // max out the security - c.CipherSuites = []uint16{ - // TLS 1.3 ciphers are not configurable, but we need to explicitly set them here to make our client hello behave correctly - // See https://github.com/golang/go/pull/49293 - tls.TLS_AES_128_GCM_SHA256, - tls.TLS_AES_256_GCM_SHA384, - tls.TLS_CHACHA20_POLY1305_SHA256, - } + c.CipherSuites = nil // TLS 1.3 ciphers are not configurable return c } diff --git a/internal/testutil/tlsserver/tls13_ciphers.go b/internal/testutil/tlsserver/tls13_ciphers.go new file mode 100644 index 000000000..8f0b0647e --- /dev/null +++ b/internal/testutil/tlsserver/tls13_ciphers.go @@ -0,0 +1,31 @@ +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build !fips_strict + +package tlsserver + +import "crypto/tls" + +// GetExpectedTLS13Ciphers returns the expected TLS 1.3 cipher for a non-FIPS build. +func GetExpectedTLS13Ciphers() []uint16 { + // TLS 1.3 ciphers are not configurable, so we can hard-code them here. + return []uint16{ + tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384, + tls.TLS_CHACHA20_POLY1305_SHA256, + } +} + +// GetExpectedTLS13CipherNMapKeyExchangeInfoValue returns the expected key exchange info value +// which is shown by nmap in parenthesis next to the cipher name for a non-FIPS build. +func GetExpectedTLS13CipherNMapKeyExchangeInfoValue(cipher uint16) string { + switch cipher { + case tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384, + tls.TLS_CHACHA20_POLY1305_SHA256: + return "ecdh_x25519" + default: + return "unknown key exchange value" + } +} diff --git a/internal/testutil/tlsserver/tls13_ciphers_fips.go b/internal/testutil/tlsserver/tls13_ciphers_fips.go new file mode 100644 index 000000000..ead0e3ffd --- /dev/null +++ b/internal/testutil/tlsserver/tls13_ciphers_fips.go @@ -0,0 +1,30 @@ +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build fips_strict + +package tlsserver + +import "crypto/tls" + +// GetExpectedTLS13Ciphers returns the expected TLS 1.3 cipher for a FIPS build. +func GetExpectedTLS13Ciphers() []uint16 { + // TLS 1.3 ciphers are not configurable, so we can hard-code them here. + return []uint16{ + tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384, + // tls.TLS_CHACHA20_POLY1305_SHA256 is not supported by boring crypto + } +} + +// GetExpectedTLS13CipherNMapKeyExchangeInfoValue returns the expected key exchange info value +// which is shown by nmap in parenthesis next to the cipher name for a FIPS build. +func GetExpectedTLS13CipherNMapKeyExchangeInfoValue(cipher uint16) string { + switch cipher { + case tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384: + return "secp256r1" + default: + return "unknown key exchange value" + } +} diff --git a/internal/testutil/tlsserver/tlsserver.go b/internal/testutil/tlsserver/tlsserver.go index 1585284da..56eddba39 100644 --- a/internal/testutil/tlsserver/tlsserver.go +++ b/internal/testutil/tlsserver/tlsserver.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package tlsserver @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/util/httpstream" + "k8s.io/apimachinery/pkg/util/sets" "go.pinniped.dev/internal/crypto/ptls" ) @@ -66,7 +67,7 @@ func RecordTLSHello(server *httptest.Server) { } } -func AssertTLS(t *testing.T, r *http.Request, tlsConfigFunc ptls.ConfigFunc) { +func AssertTLS(t *testing.T, r *http.Request, clientTLSConfigFunc ptls.ConfigFunc) { t.Helper() m, ok := getCtxMap(r.Context()) @@ -75,35 +76,55 @@ func AssertTLS(t *testing.T, r *http.Request, tlsConfigFunc ptls.ConfigFunc) { h, ok := m.Load(helloKey) require.True(t, ok) - info, ok := h.(*tls.ClientHelloInfo) + actualClientHello, ok := h.(*tls.ClientHelloInfo) require.True(t, ok) - tlsConfig := tlsConfigFunc(nil) + clientTLSConfig := clientTLSConfigFunc(nil) - supportedVersions := []uint16{tlsConfig.MinVersion} - ciphers := tlsConfig.CipherSuites + var wantClientSupportedVersions []uint16 + var wantClientSupportedCiphers []uint16 - if secureTLSConfig := ptls.Secure(nil); tlsConfig.MinVersion != secureTLSConfig.MinVersion { - supportedVersions = append([]uint16{secureTLSConfig.MinVersion}, supportedVersions...) - ciphers = append(ciphers, secureTLSConfig.CipherSuites...) + switch { + // When the provided config only supports TLS 1.3, then set up the expected values for TLS 1.3. + case clientTLSConfig.MinVersion == tls.VersionTLS13: + wantClientSupportedVersions = []uint16{tls.VersionTLS13} + wantClientSupportedCiphers = GetExpectedTLS13Ciphers() + // When the provided config supports both TLS 1.2 and 1.3, then set up the expected values for both. + case clientTLSConfig.MinVersion == tls.VersionTLS12 && (clientTLSConfig.MaxVersion == 0 || clientTLSConfig.MaxVersion == tls.VersionTLS13): + wantClientSupportedVersions = []uint16{tls.VersionTLS13, tls.VersionTLS12} + wantClientSupportedCiphers = appendIfNotAlreadyIncluded(clientTLSConfig.CipherSuites, GetExpectedTLS13Ciphers()) + default: + require.Fail(t, "incorrect test setup: clientTLSConfig supports an unexpected combination of TLS versions") } - protos := tlsConfig.NextProtos + wantClientProtos := clientTLSConfig.NextProtos if httpstream.IsUpgradeRequest(r) { - protos = tlsConfig.NextProtos[1:] + wantClientProtos = clientTLSConfig.NextProtos[1:] } // 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, cipherSuiteIDsToStrings(ciphers), cipherSuiteIDsToStrings(info.CipherSuites)) - ok3 := assert.Equal(t, protos, info.SupportedProtos) + ok1 := assert.Equal(t, wantClientSupportedVersions, actualClientHello.SupportedVersions) + ok2 := assert.Equal(t, cipherSuiteIDsToStrings(wantClientSupportedCiphers), cipherSuiteIDsToStrings(actualClientHello.CipherSuites)) + ok3 := assert.Equal(t, wantClientProtos, actualClientHello.SupportedProtos) if all := ok1 && ok2 && ok3; !all { - t.Errorf("insecure TLS detected for %q %q %q upgrade=%v supportedVersions=%v ciphers=%v protos=%v", + t.Errorf("insecure TLS detected for %q %q %q upgrade=%v wantClientSupportedVersions=%v wantClientSupportedCiphers=%v wantClientProtos=%v", r.Proto, r.Method, r.URL.String(), httpstream.IsUpgradeRequest(r), ok1, ok2, ok3) } } +// appendIfNotAlreadyIncluded only adds the newItems to the list if they are not already included +// in this list. It returns the potentially updated list. +func appendIfNotAlreadyIncluded(list []uint16, newItems []uint16) []uint16 { + originals := sets.New(list...) + for _, newItem := range newItems { + if !originals.Has(newItem) { + list = append(list, newItem) + } + } + return list +} + func cipherSuiteIDsToStrings(ids []uint16) []string { cipherSuites := make([]string, 0, len(ids)) for _, id := range ids { diff --git a/test/integration/securetls_test.go b/test/integration/securetls_test.go index e4324692b..0d2187bf5 100644 --- a/test/integration/securetls_test.go +++ b/test/integration/securetls_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -25,8 +25,8 @@ func TestSecureTLSPinnipedCLIToKAS_Parallel(t *testing.T) { server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // 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 + // 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",`+ @@ -59,8 +59,8 @@ func TestSecureTLSPinnipedCLIToSupervisor_Parallel(t *testing.T) { server := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // 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 + // 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"}`) @@ -101,17 +101,34 @@ func TestSecureTLSConciergeAggregatedAPI_Parallel(t *testing.T) { require.Contains(t, stdout, testlib.GetExpectedCiphers(ptls.Secure(nil)), "stdout:\n%s", stdout) } -func TestSecureTLSSupervisor(t *testing.T) { // does not run in parallel because of the createSupervisorDefaultTLSCertificateSecretIfNeeded call +// TLS checks safe to run in parallel with serial tests, see main_test.go. +func TestSecureTLSSupervisorAggregatedAPI_Parallel(t *testing.T) { + env := testlib.IntegrationEnv(t) + + cancelCtx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + startKubectlPortForward(cancelCtx, t, "10447", "443", env.SupervisorAppName+"-api", env.SupervisorNamespace) + + stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10447) + + require.Empty(t, stderr) + require.Contains(t, stdout, testlib.GetExpectedCiphers(ptls.Secure(nil)), "stdout:\n%s", stdout) +} + +func TestSecureTLSSupervisor(t *testing.T) { env := testlib.IntegrationEnv(t) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) - startKubectlPortForward(ctx, t, "10447", "443", env.SupervisorAppName+"-nodeport", env.SupervisorNamespace) + startKubectlPortForward(ctx, t, "10448", "443", env.SupervisorAppName+"-nodeport", env.SupervisorNamespace) - stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10447) + stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10448) - // supervisor's cert is ECDSA + // The Supervisor's auto-generated bootstrap TLS cert is ECDSA, so we think that only the ECDSA ciphers + // will be available on the server for TLS 1.2. Therefore, filter the list of expected ciphers to only + // include the ECDSA ciphers. defaultECDSAOnly := ptls.Default(nil) ciphers := make([]uint16, 0, len(defaultECDSAOnly.CipherSuites)/2) for _, id := range defaultECDSAOnly.CipherSuites { diff --git a/test/testlib/securetls.go b/test/testlib/securetls.go index b7f45b924..3111c39fc 100644 --- a/test/testlib/securetls.go +++ b/test/testlib/securetls.go @@ -1,4 +1,4 @@ -// Copyright 2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2022-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package testlib @@ -18,7 +18,7 @@ import ( "github.com/stretchr/testify/require" - "go.pinniped.dev/internal/crypto/ptls" + "go.pinniped.dev/internal/testutil/tlsserver" ) func RunNmapSSLEnum(t *testing.T, host string, port uint16) (string, string) { @@ -51,9 +51,8 @@ func RunNmapSSLEnum(t *testing.T, host string, port uint16) (string, string) { } func GetExpectedCiphers(config *tls.Config) string { - secureConfig := ptls.Secure(nil) - skip12 := config.MinVersion == tls.VersionTLS13 + skip13 := config.MaxVersion == tls.VersionTLS12 var tls12Bit, tls13Bit string @@ -90,12 +89,15 @@ func GetExpectedCiphers(config *tls.Config) string { tls12Bit = fmt.Sprintf(tls12Base, s.String(), cipherSuitePreference) } - 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 { + tls13CipherSuites := tlsserver.GetExpectedTLS13Ciphers() + for i, id := range tls13CipherSuites { + s.WriteString(fmt.Sprintf(tls13Item, + strings.Replace(tls.CipherSuiteName(id), "TLS_", "TLS_AKE_WITH_", 1), + tlsserver.GetExpectedTLS13CipherNMapKeyExchangeInfoValue(id)), + ) + if i == len(tls13CipherSuites)-1 { break } s.WriteString("\n") @@ -114,7 +116,7 @@ const ( Nmap done: 1 IP address (1 host up) scanned in` - // cipher preference is a variable because in FIPs mode it is server + // cipher preference is a variable because in FIPS mode it is server // but in normal mode it is client. tls12Base = ` | TLSv1.2: @@ -138,5 +140,5 @@ Nmap done: 1 IP address (1 host up) scanned in` // For the RSA ciphers, we expect this output to be RSA 2048. rsa2048 = "rsa 2048" - tls13Item = `| %s (ecdh_x25519) - A` + tls13Item = `| %s (%s) - A` )