From 53031ad8d4ae39750fcc9b8195ddcf9d6fa5a720 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 14 May 2024 09:17:14 -0500 Subject: [PATCH] User can now configured allowed ciphers, to restrict the ciphers used by the Default profile --- deploy/concierge/deployment.yaml | 3 + deploy/concierge/values.yaml | 13 + deploy/supervisor/helpers.lib.yaml | 5 + deploy/supervisor/values.yaml | 13 + internal/concierge/server/server.go | 4 +- internal/config/concierge/config.go | 7 +- internal/config/concierge/config_test.go | 71 ++- internal/config/concierge/types.go | 12 + internal/config/supervisor/config.go | 6 +- internal/config/supervisor/config_test.go | 59 ++- internal/config/supervisor/types.go | 12 + internal/crypto/ptls/common.go | 193 ++++++++ internal/crypto/ptls/common_test.go | 468 +++++++++++++++++++ internal/crypto/ptls/log_profiles.go | 41 ++ internal/crypto/ptls/log_profiles_test.go | 30 ++ internal/crypto/ptls/profiles.go | 123 ++--- internal/crypto/ptls/profiles_fips_strict.go | 62 +-- internal/crypto/ptls/profiles_test.go | 86 ++++ internal/supervisor/server/server.go | 4 +- test/integration/pod_shutdown_test.go | 19 +- 20 files changed, 1114 insertions(+), 117 deletions(-) create mode 100644 internal/crypto/ptls/common.go create mode 100644 internal/crypto/ptls/common_test.go create mode 100644 internal/crypto/ptls/log_profiles.go create mode 100644 internal/crypto/ptls/log_profiles_test.go diff --git a/deploy/concierge/deployment.yaml b/deploy/concierge/deployment.yaml index 26b892524..6aac1fd1e 100644 --- a/deploy/concierge/deployment.yaml +++ b/deploy/concierge/deployment.yaml @@ -100,6 +100,9 @@ data: log: level: (@= getAndValidateLogLevel() @) (@ end @) + tls: + onedottwo: + allowedCiphers: (@= str(data.values.allowed_ciphers_for_tls_onedottwo) @) --- #@ if data.values.image_pull_dockerconfigjson and data.values.image_pull_dockerconfigjson != "": apiVersion: v1 diff --git a/deploy/concierge/values.yaml b/deploy/concierge/values.yaml index 0e5708956..92bd2db6d 100644 --- a/deploy/concierge/values.yaml +++ b/deploy/concierge/values.yaml @@ -214,3 +214,16 @@ https_proxy: "" #@ localhost endpoints, and the known instance metadata IP address for public cloud providers." #@schema/desc no_proxy_desc no_proxy: "$(KUBERNETES_SERVICE_HOST),169.254.169.254,127.0.0.1,localhost,.svc,.cluster.local" + +#@schema/title "Allowed Ciphers for TLS 1.2" +#@ allowed_ciphers_for_tls_onedottwo_desc = "When specified, only the ciphers listed will be used for TLS 1.2. \ +#@ This includes both server-side and client-side TLS connections. \ +#@ Specify only secure cipher names from golang's crypto/tls package. \ +#@ This list must only include cipher suites that Pinniped is configured to accept (see the internal/crypto/ptls package). \ +#@ An empty array means accept Pinniped's defaults." +#@schema/desc allowed_ciphers_for_tls_onedottwo_desc +#@schema/examples ("Example with a few secure ciphers", ["TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256"]) +#! No type, default, or validation is required here. +#! An empty array is perfectly valid, as is any array of strings. +allowed_ciphers_for_tls_onedottwo: +- "" diff --git a/deploy/supervisor/helpers.lib.yaml b/deploy/supervisor/helpers.lib.yaml index 818b673a8..693dedaa1 100644 --- a/deploy/supervisor/helpers.lib.yaml +++ b/deploy/supervisor/helpers.lib.yaml @@ -53,6 +53,11 @@ _: #@ template.replace(data.values.custom_labels) #@ "apiService": defaultResourceNameWithSuffix("api"), #@ }, #@ "labels": labels(), +#@ "tls": { +#@ "onedottwo": { +#@ "allowedCiphers": data.values.allowed_ciphers_for_tls_onedottwo +#@ } +#@ } #@ } #@ if data.values.log_level: #@ config["log"] = {} diff --git a/deploy/supervisor/values.yaml b/deploy/supervisor/values.yaml index 750a5e075..912ad3932 100644 --- a/deploy/supervisor/values.yaml +++ b/deploy/supervisor/values.yaml @@ -203,3 +203,16 @@ no_proxy: "$(KUBERNETES_SERVICE_HOST),169.254.169.254,127.0.0.1,localhost,.svc,. #@schema/nullable #@schema/validation ("a map with keys 'http' and 'https', whose values are either the string 'disabled' or a map having keys 'network' and 'address', and the value of 'network' must be one of the allowed values", validate_endpoints) endpoints: { } + +#@schema/title "Allowed Ciphers for TLS 1.2" +#@ allowed_ciphers_for_tls_onedottwo_desc = "When specified, only the ciphers listed will be used for TLS 1.2. \ +#@ This includes both server-side and client-side TLS connections. \ +#@ Specify only secure cipher names from golang's crypto/tls package. \ +#@ This list must only include cipher suites that Pinniped is configured to accept (see the internal/crypto/ptls package). \ +#@ An empty array means accept Pinniped's defaults." +#@schema/desc allowed_ciphers_for_tls_onedottwo_desc +#@schema/examples ("Example with a few secure ciphers", ["TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256"]) +#! No type, default, or validation is required here. +#! An empty array is perfectly valid, as is any array of strings. +allowed_ciphers_for_tls_onedottwo: +- "" diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index 728acb353..78d8ca125 100644 --- a/internal/concierge/server/server.go +++ b/internal/concierge/server/server.go @@ -112,11 +112,13 @@ func (a *App) runServer(ctx context.Context) error { featuregates.DisableKubeFeatureGate(features.UnauthenticatedHTTP2DOSMitigation) // Read the server config file. - cfg, err := concierge.FromPath(ctx, a.configPath) + cfg, err := concierge.FromPath(ctx, a.configPath, ptls.SetUserConfiguredCiphersForTLSOneDotTwo) if err != nil { return fmt.Errorf("could not load config: %w", err) } + ptls.LogAllProfiles(plog.New()) + // Discover in which namespace we are installed. podInfo, err := downward.Load(a.downwardAPIPath) if err != nil { diff --git a/internal/config/concierge/config.go b/internal/config/concierge/config.go index a95619565..5b0090402 100644 --- a/internal/config/concierge/config.go +++ b/internal/config/concierge/config.go @@ -15,6 +15,7 @@ import ( "sigs.k8s.io/yaml" "go.pinniped.dev/internal/constable" + "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/groupsuffix" "go.pinniped.dev/internal/plog" ) @@ -42,7 +43,7 @@ const ( // Note! The Config file should contain base64-encoded WebhookCABundle data. // This function will decode that base64-encoded data to PEM bytes to be stored // in the Config. -func FromPath(ctx context.Context, path string) (*Config, error) { +func FromPath(ctx context.Context, path string, setAllowedCiphers ptls.SetAllowedCiphers) (*Config, error) { data, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("read file: %w", err) @@ -83,6 +84,10 @@ func FromPath(ctx context.Context, path string) (*Config, error) { return nil, fmt.Errorf("validate log level: %w", err) } + if err := setAllowedCiphers(config.TLS.OneDotTwo.AllowedCiphers); err != nil { + return nil, fmt.Errorf("validate tls: %w", err) + } + if config.Labels == nil { config.Labels = make(map[string]string) } diff --git a/internal/config/concierge/config_test.go b/internal/config/concierge/config_test.go index f1d52c3ed..37eacd150 100644 --- a/internal/config/concierge/config_test.go +++ b/internal/config/concierge/config_test.go @@ -5,6 +5,7 @@ package concierge import ( "context" + "fmt" "os" "testing" @@ -17,10 +18,11 @@ import ( func TestFromPath(t *testing.T) { tests := []struct { - name string - yaml string - wantConfig *Config - wantError string + name string + yaml string + allowedCiphersError error + wantConfig *Config + wantError string }{ { name: "Fully filled out", @@ -59,6 +61,12 @@ func TestFromPath(t *testing.T) { imagePullSecrets: [kube-cert-agent-image-pull-secret] log: level: debug + tls: + onedottwo: + allowedCiphers: + - foo + - bar + - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305 `), wantConfig: &Config{ DiscoveryInfo: DiscoveryInfoSpec{ @@ -98,6 +106,15 @@ func TestFromPath(t *testing.T) { Log: plog.LogSpec{ Level: plog.LevelDebug, }, + TLS: TLSSpec{ + OneDotTwo: TLSProtocolSpec{ + AllowedCiphers: []string{ + "foo", + "bar", + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", + }, + }, + }, }, }, { @@ -587,6 +604,31 @@ func TestFromPath(t *testing.T) { `), wantError: "validate apiGroupSuffix: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')", }, + { + name: "returns setAllowedCiphers errors", + yaml: here.Doc(` + --- + names: + servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate + credentialIssuer: pinniped-config + apiService: pinniped-api + impersonationLoadBalancerService: impersonationLoadBalancerService-value + impersonationClusterIPService: impersonationClusterIPService-value + impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value + impersonationCACertificateSecret: impersonationCACertificateSecret-value + impersonationSignerSecret: impersonationSignerSecret-value + impersonationSignerSecret: impersonationSignerSecret-value + agentServiceAccount: agentServiceAccount-value + impersonationProxyServiceAccount: impersonationProxyServiceAccount-value + impersonationProxyLegacySecret: impersonationProxyLegacySecret-value + tls: + onedottwo: + allowedCiphers: + - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 + `), + allowedCiphersError: fmt.Errorf("some error from setAllowedCiphers"), + wantError: "validate tls: some error from setAllowedCiphers", + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -595,10 +637,10 @@ func TestFromPath(t *testing.T) { // Write yaml to temp file f, err := os.CreateTemp("", "pinniped-test-config-yaml-*") require.NoError(t, err) - defer func() { + t.Cleanup(func() { err := os.Remove(f.Name()) require.NoError(t, err) - }() + }) _, err = f.WriteString(test.yaml) require.NoError(t, err) err = f.Close() @@ -607,14 +649,23 @@ func TestFromPath(t *testing.T) { // Test FromPath() ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) - config, err := FromPath(ctx, f.Name()) + + var actualCiphers []string + setAllowedCiphers := func(ciphers []string) error { + actualCiphers = ciphers + return test.allowedCiphersError + } + + config, err := FromPath(ctx, f.Name(), setAllowedCiphers) if test.wantError != "" { require.EqualError(t, err, test.wantError) - } else { - require.NoError(t, err) - require.Equal(t, test.wantConfig, config) + return } + + require.NoError(t, err) + require.Equal(t, test.wantConfig, config) + require.Equal(t, test.wantConfig.TLS.OneDotTwo.AllowedCiphers, actualCiphers) }) } } diff --git a/internal/config/concierge/types.go b/internal/config/concierge/types.go index ffcaac5b0..b4b814845 100644 --- a/internal/config/concierge/types.go +++ b/internal/config/concierge/types.go @@ -16,6 +16,18 @@ type Config struct { KubeCertAgentConfig KubeCertAgentSpec `json:"kubeCertAgent"` Labels map[string]string `json:"labels"` Log plog.LogSpec `json:"log"` + TLS TLSSpec `json:"tls"` +} + +type TLSSpec struct { + OneDotTwo TLSProtocolSpec `json:"onedottwo"` +} + +type TLSProtocolSpec struct { + // AllowedCiphers will permit Pinniped to use only the listed ciphers. + // This affects Pinniped both when it acts as a client and as a server. + // If empty, Pinniped will use a built-in list of ciphers. + AllowedCiphers []string `json:"allowedCiphers"` } // DiscoveryInfoSpec contains configuration knobs specific to diff --git a/internal/config/supervisor/config.go b/internal/config/supervisor/config.go index 1e7c60d1b..d3eb77565 100644 --- a/internal/config/supervisor/config.go +++ b/internal/config/supervisor/config.go @@ -16,6 +16,7 @@ import ( "sigs.k8s.io/yaml" "go.pinniped.dev/internal/constable" + "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/groupsuffix" "go.pinniped.dev/internal/plog" ) @@ -35,7 +36,7 @@ const ( // FromPath loads an Config from a provided local file path, inserts any // defaults (from the Config documentation), and verifies that the config is // valid (Config documentation). -func FromPath(ctx context.Context, path string) (*Config, error) { +func FromPath(ctx context.Context, path string, setAllowedCiphers ptls.SetAllowedCiphers) (*Config, error) { data, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("read file: %w", err) @@ -95,6 +96,9 @@ func FromPath(ctx context.Context, path string) (*Config, error) { if err := validateAtLeastOneEnabledEndpoint(*config.Endpoints.HTTPS, *config.Endpoints.HTTP); err != nil { return nil, fmt.Errorf("validate endpoints: %w", err) } + if err := setAllowedCiphers(config.TLS.OneDotTwo.AllowedCiphers); err != nil { + return nil, fmt.Errorf("validate tls: %w", err) + } return &config, nil } diff --git a/internal/config/supervisor/config_test.go b/internal/config/supervisor/config_test.go index 61f83aed8..d88344f3b 100644 --- a/internal/config/supervisor/config_test.go +++ b/internal/config/supervisor/config_test.go @@ -18,10 +18,11 @@ import ( func TestFromPath(t *testing.T) { tests := []struct { - name string - yaml string - wantConfig *Config - wantError string + name string + yaml string + allowedCiphersError error + wantConfig *Config + wantError string }{ { name: "Happy", @@ -45,6 +46,12 @@ func TestFromPath(t *testing.T) { level: info format: json aggregatedAPIServerPort: 12345 + tls: + onedottwo: + allowedCiphers: + - foo + - bar + - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305 `), wantConfig: &Config{ APIGroupSuffix: ptr.To("some.suffix.com"), @@ -70,6 +77,15 @@ func TestFromPath(t *testing.T) { Format: plog.FormatJSON, }, AggregatedAPIServerPort: ptr.To[int64](12345), + TLS: TLSSpec{ + OneDotTwo: TLSProtocolSpec{ + AllowedCiphers: []string{ + "foo", + "bar", + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", + }, + }, + }, }, }, { @@ -251,6 +267,20 @@ func TestFromPath(t *testing.T) { `), wantError: "validate aggregatedAPIServerPort: must be within range 1024 to 65535", }, + { + name: "returns setAllowedCiphers errors", + yaml: here.Doc(` + --- + names: + defaultTLSCertificateSecret: my-secret-name + tls: + onedottwo: + allowedCiphers: + - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 + `), + allowedCiphersError: fmt.Errorf("some error from setAllowedCiphers"), + wantError: "validate tls: some error from setAllowedCiphers", + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -259,10 +289,10 @@ func TestFromPath(t *testing.T) { // Write yaml to temp file f, err := os.CreateTemp("", "pinniped-test-config-yaml-*") require.NoError(t, err) - defer func() { + t.Cleanup(func() { err := os.Remove(f.Name()) require.NoError(t, err) - }() + }) _, err = f.WriteString(test.yaml) require.NoError(t, err) err = f.Close() @@ -271,14 +301,23 @@ func TestFromPath(t *testing.T) { // Test FromPath() ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) - config, err := FromPath(ctx, f.Name()) + + var actualCiphers []string + setAllowedCiphers := func(ciphers []string) error { + actualCiphers = ciphers + return test.allowedCiphersError + } + + config, err := FromPath(ctx, f.Name(), setAllowedCiphers) if test.wantError != "" { require.EqualError(t, err, test.wantError) - } else { - require.NoError(t, err) - require.Equal(t, test.wantConfig, config) + return } + + require.NoError(t, err) + require.Equal(t, test.wantConfig, config) + require.Equal(t, test.wantConfig.TLS.OneDotTwo.AllowedCiphers, actualCiphers) }) } } diff --git a/internal/config/supervisor/types.go b/internal/config/supervisor/types.go index 918375f67..f1b2870ec 100644 --- a/internal/config/supervisor/types.go +++ b/internal/config/supervisor/types.go @@ -15,6 +15,18 @@ type Config struct { Log plog.LogSpec `json:"log"` Endpoints *Endpoints `json:"endpoints"` AggregatedAPIServerPort *int64 `json:"aggregatedAPIServerPort"` + TLS TLSSpec `json:"tls"` +} + +type TLSSpec struct { + OneDotTwo TLSProtocolSpec `json:"onedottwo"` +} + +type TLSProtocolSpec struct { + // AllowedCiphers will permit Pinniped to use only the listed ciphers. + // This affects Pinniped both when it acts as a client and as a server. + // If empty, Pinniped will use a built-in list of ciphers. + AllowedCiphers []string `json:"allowedCiphers"` } // NamesConfigSpec configures the names of some Kubernetes resources for the Supervisor. diff --git a/internal/crypto/ptls/common.go b/internal/crypto/ptls/common.go new file mode 100644 index 000000000..a50811c56 --- /dev/null +++ b/internal/crypto/ptls/common.go @@ -0,0 +1,193 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package ptls + +import ( + "crypto/tls" + "crypto/x509" + "fmt" + "slices" + "strings" + "sync/atomic" + + "k8s.io/apimachinery/pkg/util/sets" + + "go.pinniped.dev/internal/plog" +) + +// allowedCiphersForTLSOneDotTwo will only contain ciphers that meet the following criteria: +// 1. They are secure +// 2. They are returned by tls.CipherSuites +// 3. They are returned by cipherSuitesForDefault or cipherSuitesForDefaultLDAP +// This is atomic so that it can not be set and read at the same time. +// +//nolint:gochecknoglobals // This needs to be global because it will be set at application startup from configuration values +var allowedCiphersForTLSOneDotTwo atomic.Value + +type SetAllowedCiphers func([]string) error + +// SetUserConfiguredCiphersForTLSOneDotTwo allows configuration/setup components to constrain the allowed TLS ciphers +// for TLS1.2. +func SetUserConfiguredCiphersForTLSOneDotTwo(userConfiguredCiphersForTLSOneDotTwo []string) error { + plog.Debug("setting user-configured allowed ciphers for TLS 1.2", "userConfiguredAllowedCipherSuites", userConfiguredCiphersForTLSOneDotTwo) + + validatedUserConfiguredAllowedCipherSuites, err := validateAllowedCiphers(hardcodedCipherSuites(), userConfiguredCiphersForTLSOneDotTwo) + if err != nil { + return err + } + allowedCiphersForTLSOneDotTwo.Store(validatedUserConfiguredAllowedCipherSuites) + return nil +} + +// getUserConfiguredCiphersAllowList returns the user-configured list of allowed ciphers for TLS1.2. +// It is not exported so that it is only available to this package. +func getUserConfiguredCiphersAllowList() []*tls.CipherSuite { + userConfiguredCiphersAllowList, ok := (allowedCiphersForTLSOneDotTwo.Load()).([]*tls.CipherSuite) + if ok { + return userConfiguredCiphersAllowList + } + return nil +} + +// constrainCipherSuites returns the intersection of its parameters, as a list of cipher suite IDs. +// If userConfiguredCiphersAllowList is empty, it will return all the hardcodedCipherSuites. +func constrainCipherSuites( + hardcodedCipherSuites []*tls.CipherSuite, + userConfiguredAllowedCipherSuites []*tls.CipherSuite, +) []uint16 { + allowedCipherSuiteIDs := sets.New[uint16]() + for _, allowedCipherSuite := range userConfiguredAllowedCipherSuites { + allowedCipherSuiteIDs.Insert(allowedCipherSuite.ID) + } + + configuredCipherSuiteIDs := sets.New[uint16]() + for _, configuredCipherSuite := range hardcodedCipherSuites { + configuredCipherSuiteIDs.Insert(configuredCipherSuite.ID) + } + + intersection := configuredCipherSuiteIDs.Intersection(allowedCipherSuiteIDs) + + // If the user did not provide any valid allowed cipher suites, use configuredCipherSuiteIDs. + // Note that the user-configured allowed cipher suites are validated elsewhere, so this should only happen when the + // user chose not to specify any allowed cipher suites. + if len(intersection) == 0 { + intersection = configuredCipherSuiteIDs + } + + result := intersection.UnsortedList() + // Preserve the order as shown in configuredCipherSuites + slices.SortFunc(result, func(a, b uint16) int { + return slices.IndexFunc(hardcodedCipherSuites, func(cipher *tls.CipherSuite) bool { return cipher.ID == a }) - + slices.IndexFunc(hardcodedCipherSuites, func(cipher *tls.CipherSuite) bool { return cipher.ID == b }) + }) + + return result +} + +func translateIDIntoSecureCipherSuites(ids []uint16) []*tls.CipherSuite { + golangSecureCipherSuites := tls.CipherSuites() + result := make([]*tls.CipherSuite, 0) + + for _, golangSecureCipherSuite := range golangSecureCipherSuites { + // As of golang 1.22.2, all cipher suites from tls.CipherSuites are secure, so this is just future-proofing. + if golangSecureCipherSuite.Insecure { // untested + continue + } + + if !slices.Contains(golangSecureCipherSuite.SupportedVersions, tls.VersionTLS12) { + continue + } + + if slices.Contains(ids, golangSecureCipherSuite.ID) { + result = append(result, golangSecureCipherSuite) + } + } + + // Preserve the order as shown in ids + slices.SortFunc(result, func(a, b *tls.CipherSuite) int { + return slices.Index(ids, a.ID) - slices.Index(ids, b.ID) + }) + + return result +} + +// buildTLSConfig will return a tls.Config with CipherSuites from the intersection of configuredCipherSuites and allowedCipherSuites. +func buildTLSConfig( + rootCAs *x509.CertPool, + hardcodedCipherSuites []*tls.CipherSuite, + userConfiguredCiphersAllowList []*tls.CipherSuite, +) *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 + MinVersion: tls.VersionTLS12, + + CipherSuites: constrainCipherSuites(hardcodedCipherSuites, userConfiguredCiphersAllowList), + + // 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 + NextProtos: []string{"h2", "http/1.1"}, + + // optional root CAs, nil means use the host's root CA set + RootCAs: rootCAs, + } +} + +// validateAllowedCiphers will take in the user-configured allowed cipher names and validate them against Pinniped's configured list of ciphers. +// If any allowed cipher names are not configured, return a descriptive error. +// An empty list of allowed cipher names is perfectly valid. +// Returns the tls.CipherSuite representation when all allowedCipherNames are accepted. +func validateAllowedCiphers( + hardcodedCipherSuites []*tls.CipherSuite, + userConfiguredCiphersAllowList []string, +) ([]*tls.CipherSuite, error) { + if len(userConfiguredCiphersAllowList) < 1 { + return nil, nil + } + + hardcodedCipherSuiteNames := make([]string, len(hardcodedCipherSuites)) + for i, cipher := range hardcodedCipherSuites { + hardcodedCipherSuiteNames[i] = cipher.Name + } + + // Allow some loosening of the names for legacy reasons. + for i := range userConfiguredCiphersAllowList { + switch userConfiguredCiphersAllowList[i] { + case "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305": + userConfiguredCiphersAllowList[i] = "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256" + case "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305": + userConfiguredCiphersAllowList[i] = "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256" + } + } + + // Make sure that all allowedCipherNames are actually configured within Pinniped + var invalidCipherNames []string + for _, allowedCipherName := range userConfiguredCiphersAllowList { + if !slices.Contains(hardcodedCipherSuiteNames, allowedCipherName) { + invalidCipherNames = append(invalidCipherNames, allowedCipherName) + } + } + + if len(invalidCipherNames) > 0 { + return nil, fmt.Errorf("unrecognized ciphers [%s], ciphers must be from list [%s]", + strings.Join(invalidCipherNames, ", "), + strings.Join(hardcodedCipherSuiteNames, ", ")) + } + + // Now translate the allowedCipherNames into their *tls.CipherSuite representation + var validCiphers []*tls.CipherSuite + for _, cipher := range hardcodedCipherSuites { + if slices.Contains(userConfiguredCiphersAllowList, cipher.Name) { + validCiphers = append(validCiphers, cipher) + } + } + + return validCiphers, nil +} diff --git a/internal/crypto/ptls/common_test.go b/internal/crypto/ptls/common_test.go new file mode 100644 index 000000000..61925ccf6 --- /dev/null +++ b/internal/crypto/ptls/common_test.go @@ -0,0 +1,468 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package ptls + +import ( + "crypto/tls" + "crypto/x509" + "regexp" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSetAllowedCiphersForTLSOneDotTwo(t *testing.T) { + t.Run("with valid ciphers, mutates the global state", func(t *testing.T) { + require.Empty(t, getUserConfiguredCiphersAllowList()) + // With no user-configured allowed ciphers, expect all the hardcoded ciphers + require.Equal(t, []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_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + }, Default(nil).CipherSuites) + require.Equal(t, []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_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + }, DefaultLDAP(nil).CipherSuites) + require.Empty(t, Secure(nil).CipherSuites) + + t.Cleanup(func() { + err := SetUserConfiguredCiphersForTLSOneDotTwo(nil) + require.NoError(t, err) + require.Nil(t, getUserConfiguredCiphersAllowList()) + + require.Equal(t, []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_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + }, Default(nil).CipherSuites) + require.Equal(t, []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_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + }, DefaultLDAP(nil).CipherSuites) + require.Empty(t, Secure(nil).CipherSuites) + }) + + userConfiguredAllowedCipherSuites := []string{ + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", // this is an LDAP-only cipher + } + err := SetUserConfiguredCiphersForTLSOneDotTwo(userConfiguredAllowedCipherSuites) + require.NoError(t, err) + stored := getUserConfiguredCiphersAllowList() + var storedNames []string + for _, suite := range stored { + storedNames = append(storedNames, suite.Name) + } + require.Equal(t, userConfiguredAllowedCipherSuites, storedNames) + + require.Equal(t, []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + }, Default(nil).CipherSuites) + require.Equal(t, []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + }, DefaultLDAP(nil).CipherSuites) + require.Empty(t, Secure(nil).CipherSuites) + }) + + t.Run("SetUserConfiguredCiphersForTLSOneDotTwo calls validateAllowedCiphers and returns error", func(t *testing.T) { + err := SetUserConfiguredCiphersForTLSOneDotTwo([]string{"foo"}) + require.Regexp(t, regexp.QuoteMeta("unrecognized ciphers [foo], ciphers must be from list [TLS")+".*"+regexp.QuoteMeta("]"), err.Error()) + }) +} + +func TestConstrainCipherSuites(t *testing.T) { + tests := []struct { + name string + hardcodedCipherSuites []*tls.CipherSuite + userConfiguredAllowedCipherSuites []*tls.CipherSuite + wantCipherSuites []uint16 + }{ + { + name: "with empty inputs, returns empty output", + wantCipherSuites: make([]uint16, 0), + }, + { + name: "with empty userConfiguredAllowedCipherSuites, returns hardcodedCipherSuites", + hardcodedCipherSuites: []*tls.CipherSuite{ + {ID: 0}, + {ID: 1}, + {ID: 2}, + }, + wantCipherSuites: []uint16{0, 1, 2}, + }, + { + name: "with userConfiguredAllowedCipherSuites, returns only ciphers found in both inputs", + hardcodedCipherSuites: []*tls.CipherSuite{ + {ID: 0}, + {ID: 1}, + {ID: 2}, + {ID: 3}, + {ID: 4}, + }, + userConfiguredAllowedCipherSuites: []*tls.CipherSuite{ + {ID: 1}, + {ID: 3}, + {ID: 999}, + }, + wantCipherSuites: []uint16{1, 3}, + }, + { + name: "with all invalid userConfiguredAllowedCipherSuites, returns hardcodedCipherSuites", + hardcodedCipherSuites: []*tls.CipherSuite{ + {ID: 0}, + {ID: 1}, + {ID: 2}, + {ID: 3}, + {ID: 4}, + }, + userConfiguredAllowedCipherSuites: []*tls.CipherSuite{ + {ID: 1000}, + {ID: 2000}, + {ID: 3000}, + }, + wantCipherSuites: []uint16{0, 1, 2, 3, 4}, + }, + { + name: "preserves order from hardcodedCipherSuites", + hardcodedCipherSuites: []*tls.CipherSuite{ + {ID: 0}, + {ID: 1}, + {ID: 2}, + {ID: 3}, + {ID: 4}, + }, + userConfiguredAllowedCipherSuites: []*tls.CipherSuite{ + {ID: 5}, + {ID: 4}, + {ID: 3}, + {ID: 2}, + }, + wantCipherSuites: []uint16{2, 3, 4}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + actual := constrainCipherSuites(test.hardcodedCipherSuites, test.userConfiguredAllowedCipherSuites) + require.Equal(t, test.wantCipherSuites, actual) + }) + } +} + +// TestTLSSecureCipherSuites checks whether golang has changed the list of secure ciphers. +// This was written against golang 1.22.2. +// Pinniped has chosen to use only secure ciphers returned by tls.CipherSuites, so in the future we want to be aware of +// changes to this list of ciphers (additions or removals). +// +// If golang adds ciphers, we should consider adding them to the list of possible ciphers for Pinniped's profiles. +// If golang removes ciphers, we should consider removing them from the list of possible ciphers for Pinniped's profiles. +// Any cipher modifications should be added to release notes so that Pinniped admins can choose to modify their +// allowedCiphers accordingly. +func TestTLSSecureCipherSuites(t *testing.T) { + expectedCipherSuites := []uint16{ + tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384, + tls.TLS_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + 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_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + } + + tlsSecureCipherSuites := tls.CipherSuites() + require.Equal(t, len(expectedCipherSuites), len(tlsSecureCipherSuites)) + for _, suite := range tlsSecureCipherSuites { + require.False(t, suite.Insecure) + require.Contains(t, expectedCipherSuites, suite.ID) + } +} + +func TestBuildTLSConfig(t *testing.T) { + t.Parallel() + + aCertPool := x509.NewCertPool() + + tests := []struct { + name string + rootCAs *x509.CertPool + configuredCipherSuites []*tls.CipherSuite + allowedCipherIDs []uint16 + wantConfig *tls.Config + }{ + { + name: "happy path", + rootCAs: aCertPool, + configuredCipherSuites: tls.CipherSuites(), + wantConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + CipherSuites: []uint16{ + tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384, + tls.TLS_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, //nolint:gosec // this is for testing purposes + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + 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_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + }, + NextProtos: []string{"h2", "http/1.1"}, + RootCAs: aCertPool, + }, + }, + { + name: "with no allowedCipherSuites, returns configuredCipherSuites", + configuredCipherSuites: func() []*tls.CipherSuite { + result := tls.CipherSuites() + return result[:2] + }(), + wantConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + CipherSuites: []uint16{ + tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384, + }, + NextProtos: []string{"h2", "http/1.1"}, + }, + }, + { + name: "with allowed Ciphers, restricts CipherSuites to just those ciphers", + rootCAs: aCertPool, + configuredCipherSuites: tls.CipherSuites(), + allowedCipherIDs: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + }, + wantConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + CipherSuites: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, //nolint:gosec // this is a test + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + }, + NextProtos: []string{"h2", "http/1.1"}, + RootCAs: aCertPool, + }, + }, + { + name: "with allowed ciphers in random order, returns ciphers in the order from configuredCipherSuites", + configuredCipherSuites: tls.CipherSuites(), + allowedCipherIDs: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + }, + wantConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + CipherSuites: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, //nolint:gosec // this is for testing purposes + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + }, + NextProtos: []string{"h2", "http/1.1"}, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + allowedCipherSuites := make([]*tls.CipherSuite, 0) + for _, allowedCipher := range test.allowedCipherIDs { + for _, cipherSuite := range tls.CipherSuites() { + if allowedCipher == cipherSuite.ID { + allowedCipherSuites = append(allowedCipherSuites, cipherSuite) + } + } + } + + actualConfig := buildTLSConfig(test.rootCAs, test.configuredCipherSuites, allowedCipherSuites) + require.Equal(t, test.wantConfig, actualConfig) + }) + } +} + +func TestValidateAllowedCiphers(t *testing.T) { + cipherSuites := tls.CipherSuites() + + tests := []struct { + name string + hardCodedCipherSuites []*tls.CipherSuite + userConfiguredCiphersAllowList []string + wantCipherSuites []*tls.CipherSuite + wantErr string + }{ + { + name: "empty inputs result in empty outputs", + }, + { + name: "with all valid inputs, returns the ciphers", + hardCodedCipherSuites: cipherSuites, + userConfiguredCiphersAllowList: []string{ + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + }, + wantCipherSuites: func() []*tls.CipherSuite { + var result []*tls.CipherSuite + for _, suite := range cipherSuites { + switch suite.Name { + case "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA": + result = append(result, suite) + default: + } + } + return result + }(), + }, + { + name: "with all valid inputs, allows some legacy cipher names and returns the ciphers", + hardCodedCipherSuites: cipherSuites, + userConfiguredCiphersAllowList: []string{ + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305", + }, + wantCipherSuites: func() []*tls.CipherSuite { + var result []*tls.CipherSuite + for _, suite := range cipherSuites { + switch suite.Name { + case "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256", + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256": + result = append(result, suite) + default: + } + } + return result + }(), + }, + { + name: "with invalid input, return an error with all known ciphers", + hardCodedCipherSuites: cipherSuites[:2], + userConfiguredCiphersAllowList: []string{"foo"}, + wantErr: "unrecognized ciphers [foo], ciphers must be from list [TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384]", + }, + { + name: "with some valid and some invalid input, return an error with all known ciphers", + hardCodedCipherSuites: cipherSuites[6:9], + userConfiguredCiphersAllowList: []string{ + "foo", + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "bar", + }, + wantErr: "unrecognized ciphers [foo, bar], ciphers must be from list [TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384]", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + actual, err := validateAllowedCiphers(test.hardCodedCipherSuites, test.userConfiguredCiphersAllowList) + if len(test.wantErr) > 0 { + require.ErrorContains(t, err, test.wantErr) + } else { + require.NoError(t, err) + require.ElementsMatch(t, test.wantCipherSuites, actual) + } + }) + } +} + +func TestTranslateIDIntoSecureCipherSuites(t *testing.T) { + tests := []struct { + name string + inputs []uint16 + wantOutputs []uint16 + }{ + { + name: "returns ciphers found in tls.CipherSuites", + inputs: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + }, + wantOutputs: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + }, + }, + { + name: "returns ciphers in the input order", + inputs: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + }, + wantOutputs: []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + }, + }, + { + name: "ignores cipher suites not returned by tls.CipherSuites", + inputs: []uint16{ + tls.TLS_RSA_WITH_AES_128_CBC_SHA, + }, + }, + { + name: "ignores cipher suites that only support TLS1.3", + inputs: []uint16{ + tls.TLS_AES_128_GCM_SHA256, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + actual := translateIDIntoSecureCipherSuites(test.inputs) + + require.Len(t, actual, len(test.wantOutputs)) + for i, suite := range actual { + require.Equal(t, test.wantOutputs[i], suite.ID) + } + }) + } +} diff --git a/internal/crypto/ptls/log_profiles.go b/internal/crypto/ptls/log_profiles.go new file mode 100644 index 000000000..22442dd52 --- /dev/null +++ b/internal/crypto/ptls/log_profiles.go @@ -0,0 +1,41 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package ptls + +import ( + "crypto/tls" + + "go.pinniped.dev/internal/plog" +) + +func cipherSuiteNames(ciphers []uint16) []string { + names := make([]string, len(ciphers)) + for i, suite := range ciphers { + names[i] = tls.CipherSuiteName(suite) + } + return names +} + +func versionName(tlsVersion uint16) string { + if tlsVersion == 0 { + return "NONE" + } + return tls.VersionName(tlsVersion) +} + +func logProfile(name string, log plog.Logger, profile *tls.Config) { + log.Info("tls configuration", + "profile name", name, + "MinVersion", versionName(profile.MinVersion), + "MaxVersion", versionName(profile.MaxVersion), + "CipherSuites", cipherSuiteNames(profile.CipherSuites), + "NextProtos", profile.NextProtos, + ) +} + +func LogAllProfiles(log plog.Logger) { + logProfile("Default", log, Default(nil)) + logProfile("DefaultLDAP", log, DefaultLDAP(nil)) + logProfile("Secure", log, Secure(nil)) +} diff --git a/internal/crypto/ptls/log_profiles_test.go b/internal/crypto/ptls/log_profiles_test.go new file mode 100644 index 000000000..f5dac9b59 --- /dev/null +++ b/internal/crypto/ptls/log_profiles_test.go @@ -0,0 +1,30 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package ptls + +import ( + "bytes" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "go.pinniped.dev/internal/plog" +) + +func TestLogAllProfiles(t *testing.T) { + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + LogAllProfiles(logger) + + expectedLines := []string{ + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","caller":"ptls/log_profiles.go:$ptls.logProfile","message":"tls configuration","profile name":"Default","MinVersion":"TLS 1.2","MaxVersion":"NONE","CipherSuites":["TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256","TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256","TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384","TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384","TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256","TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256"],"NextProtos":["h2","http/1.1"]}`, + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","caller":"ptls/log_profiles.go:$ptls.logProfile","message":"tls configuration","profile name":"DefaultLDAP","MinVersion":"TLS 1.2","MaxVersion":"NONE","CipherSuites":["TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256","TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256","TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384","TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384","TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256","TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256","TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA","TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA","TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA","TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA"],"NextProtos":["h2","http/1.1"]}`, + `{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","caller":"ptls/log_profiles.go:$ptls.logProfile","message":"tls configuration","profile name":"Secure","MinVersion":"TLS 1.3","MaxVersion":"NONE","CipherSuites":[],"NextProtos":["h2","http/1.1"]}`, + } + expectedOutput := strings.Join(expectedLines, "\n") + "\n" + + require.Equal(t, expectedOutput, log.String()) +} diff --git a/internal/crypto/ptls/profiles.go b/internal/crypto/ptls/profiles.go index 1adb6a4b6..185acb193 100644 --- a/internal/crypto/ptls/profiles.go +++ b/internal/crypto/ptls/profiles.go @@ -39,73 +39,16 @@ const SecureTLSConfigMinTLSVersion = tls.VersionTLS13 // A. servers whose clients are outside our control and who may reasonably wish to use TLS 1.2, and // B. clients who need to interact with servers that might not support TLS 1.3. // Note that this will behave differently when compiled in FIPS mode (see profiles_fips_strict.go). +// Default returns a tls.Config with a minimum of TLS1.2+ and a few ciphers that can be further constrained by configuration. 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 - MinVersion: tls.VersionTLS12, - - // the order does not matter in go 1.17+ https://go.dev/blog/tls-cipher-suites - // we match crypto/tls.cipherSuitesPreferenceOrder because it makes unit tests easier to write - // this list is ignored when TLS 1.3 is used - // - // as of 2021-10-19, Mozilla Guideline v5.6, Go 1.17.2, intermediate configuration, supports: - // - Firefox 27 - // - Android 4.4.2 - // - Chrome 31 - // - Edge - // - IE 11 on Windows 7 - // - Java 8u31 - // - OpenSSL 1.0.1 - // - Opera 20 - // - Safari 9 - // https://ssl-config.mozilla.org/#server=go&version=1.17.2&config=intermediate&guideline=5.6 - // - // The Kubernetes API server must use approved cipher suites. - // https://stigviewer.com/stig/kubernetes/2021-06-17/finding/V-242418 - CipherSuites: []uint16{ - // these are all AEADs with ECDHE, some use ChaCha20Poly1305 while others use AES-GCM - // this provides forward secrecy, confidentiality and authenticity of data - 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_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, - tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, - }, - - // 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 - NextProtos: []string{"h2", "http/1.1"}, - - // optional root CAs, nil means use the host's root CA set - RootCAs: rootCAs, - } + return buildTLSConfig(rootCAs, cipherSuitesForDefault(), getUserConfiguredCiphersAllowList()) } // DefaultLDAP TLS profile should be used by clients who need to interact with potentially old LDAP servers // that might not support TLS 1.3 and that might use older ciphers. // Note that this will behave differently when compiled in FIPS mode (see profiles_fips_strict.go). func DefaultLDAP(rootCAs *x509.CertPool) *tls.Config { - c := Default(rootCAs) - // add less secure ciphers to support the default AWS Active Directory config - c.CipherSuites = append(c.CipherSuites, - // CBC with ECDHE - // this provides forward secrecy and confidentiality of data but not authenticity - // MAC-then-Encrypt CBC ciphers are susceptible to padding oracle attacks - // See https://crypto.stackexchange.com/a/205 and https://crypto.stackexchange.com/a/224 - tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, - tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, - tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, - ) - return c + return buildTLSConfig(rootCAs, cipherSuitesForDefaultLDAP(), getUserConfiguredCiphersAllowList()) } // Secure TLS profile should be used by: @@ -142,3 +85,63 @@ func SecureServing(opts *options.SecureServingOptionsWithLoopback) { opts.MinTLSVersion = "VersionTLS13" opts.CipherSuites = nil } + +func hardcodedCipherSuites() []*tls.CipherSuite { + return cipherSuitesForDefaultLDAP() +} + +// cipherSuitesForDefault are the ciphers that Pinniped allows. +// It will be a strict subset of tls.CipherSuites. +func cipherSuitesForDefault() []*tls.CipherSuite { + // the order does not matter in go 1.17+ https://go.dev/blog/tls-cipher-suites + // we match crypto/tls.cipherSuitesPreferenceOrder because it makes unit tests easier to write + // this list is ignored when TLS 1.3 is used + // + // as of 2021-10-19, Mozilla Guideline v5.6, Go 1.17.2, intermediate configuration, supports: + // - Firefox 27 + // - Android 4.4.2 + // - Chrome 31 + // - Edge + // - IE 11 on Windows 7 + // - Java 8u31 + // - OpenSSL 1.0.1 + // - Opera 20 + // - Safari 9 + // https://ssl-config.mozilla.org/#server=go&version=1.17.2&config=intermediate&guideline=5.6 + // + // The Kubernetes API server must use approved cipher suites. + // https://stigviewer.com/stig/kubernetes/2021-06-17/finding/V-242418 + + // These are all AEADs with ECDHE, some use ChaCha20Poly1305 while others use AES-GCM, + // which provides forward secrecy, confidentiality and authenticity of data. + cipherSuiteIDsForDefault := []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_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + } + + return translateIDIntoSecureCipherSuites(cipherSuiteIDsForDefault) +} + +// cipherSuitesForDefaultLDAP are some additional ciphers that Pinniped allows only for LDAP. +// It will be a strict subset of tls.CipherSuites. +func cipherSuitesForDefaultLDAP() []*tls.CipherSuite { + // Add less secure ciphers to support the default AWS Active Directory config + // + // CBC with ECDHE + // this provides forward secrecy and confidentiality of data but not authenticity + // MAC-then-Encrypt CBC ciphers are susceptible to padding oracle attacks + // See https://crypto.stackexchange.com/a/205 and https://crypto.stackexchange.com/a/224 + cipherSuiteIDsForDefaultLDAP := []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + } + result := cipherSuitesForDefault() + result = append(result, translateIDIntoSecureCipherSuites(cipherSuiteIDsForDefaultLDAP)...) + return result +} diff --git a/internal/crypto/ptls/profiles_fips_strict.go b/internal/crypto/ptls/profiles_fips_strict.go index 03da3b5b7..0ffad2cfd 100644 --- a/internal/crypto/ptls/profiles_fips_strict.go +++ b/internal/crypto/ptls/profiles_fips_strict.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "runtime" + "slices" "k8s.io/apiserver/pkg/server/options" @@ -41,32 +42,10 @@ const SecureTLSConfigMinTLSVersion = tls.VersionTLS12 // Default: see comment in profiles.go. // This chooses different cipher suites and/or TLS versions compared to non-FIPS mode. func Default(rootCAs *x509.CertPool) *tls.Config { - return &tls.Config{ - MinVersion: tls.VersionTLS12, - // Until goboring supports TLS 1.3, make the max version 1.2. - MaxVersion: tls.VersionTLS12, - - // 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, - tls.TLS_RSA_WITH_AES_128_GCM_SHA256, - tls.TLS_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 - NextProtos: []string{"h2", "http/1.1"}, - - // optional root CAs, nil means use the host's root CA set - RootCAs: rootCAs, - } + config := buildTLSConfig(rootCAs, hardcodedCipherSuites(), getUserConfiguredCiphersAllowList()) + // Until goboring supports TLS 1.3, make the max version 1.2. + config.MaxVersion = tls.VersionTLS12 + return config } // DefaultLDAP: see comment in profiles.go. @@ -88,3 +67,34 @@ func Secure(rootCAs *x509.CertPool) *tls.Config { func SecureServing(opts *options.SecureServingOptionsWithLoopback) { defaultServing(opts) } + +func hardcodedCipherSuites() []*tls.CipherSuite { + // 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. + secureCipherSuiteIDsForFIPS := []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, + } + + insecureCipherSuiteIDsForFIPS := []uint16{ + tls.TLS_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_RSA_WITH_AES_256_GCM_SHA384, + } + + result := translateIDIntoSecureCipherSuites(secureCipherSuiteIDsForFIPS) + + for _, golangInsecureCipherSuite := range tls.InsecureCipherSuites() { + if !slices.Contains(golangInsecureCipherSuite.SupportedVersions, tls.VersionTLS12) { + continue + } + + if slices.Contains(insecureCipherSuiteIDsForFIPS, golangInsecureCipherSuite.ID) { + result = append(result, golangInsecureCipherSuite) + } + } + return result +} diff --git a/internal/crypto/ptls/profiles_test.go b/internal/crypto/ptls/profiles_test.go index ac175c06b..972bf4606 100644 --- a/internal/crypto/ptls/profiles_test.go +++ b/internal/crypto/ptls/profiles_test.go @@ -6,9 +6,11 @@ package ptls import ( "crypto/tls" "crypto/x509" + "slices" "testing" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/server/options" ) @@ -89,3 +91,87 @@ func TestSecureServing(t *testing.T) { }, }, *opts) } + +func TestCipherSuitesForDefault(t *testing.T) { + t.Run("contains exactly and only the expected values", func(t *testing.T) { + expected := []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_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + } + + actual := cipherSuitesForDefault() + + require.Equal(t, len(expected), len(actual)) + for _, suite := range actual { + require.True(t, slices.Contains(expected, suite.ID)) + } + }) + + t.Run("is a subset of TestCipherSuitesForDefaultLDAP", func(t *testing.T) { + a1 := cipherSuitesForDefault() + a2 := cipherSuitesForDefaultLDAP() + + require.Greater(t, len(a1), 0) + require.GreaterOrEqual(t, len(a2), len(a1)) + + a1ids := sets.New[uint16]() + for _, suite := range a1 { + a1ids.Insert(suite.ID) + } + a2ids := sets.New[uint16]() + for _, suite := range a1 { + a2ids.Insert(suite.ID) + } + + require.Equal(t, 0, a1ids.Difference(a2ids).Len()) + }) +} + +func TestCipherSuitesForDefaultLDAP(t *testing.T) { + t.Run("contains exactly and only the expected values", func(t *testing.T) { + expected := []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_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + + // Add these for LDAP + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, + } + + actual := cipherSuitesForDefaultLDAP() + + require.Equal(t, len(expected), len(actual)) + for _, suite := range actual { + require.True(t, slices.Contains(expected, suite.ID)) + } + }) + + t.Run("is a superset of TestCipherSuitesForDefault", func(t *testing.T) { + a1 := cipherSuitesForDefault() + a2 := cipherSuitesForDefaultLDAP() + + require.Greater(t, len(a1), 0) + require.GreaterOrEqual(t, len(a2), len(a1)) + + a1ids := sets.New[uint16]() + for _, suite := range a1 { + a1ids.Insert(suite.ID) + } + a2ids := sets.New[uint16]() + for _, suite := range a1 { + a2ids.Insert(suite.ID) + } + + require.True(t, a2ids.IsSuperset(a1ids)) + }) +} diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index b83922faa..19c9140bb 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -742,11 +742,13 @@ func main() error { // return an error instead of plog.Fatal to allow defer stat ctx := signalCtx() // Read the server config file. - cfg, err := supervisor.FromPath(ctx, os.Args[2]) + cfg, err := supervisor.FromPath(ctx, os.Args[2], ptls.SetUserConfiguredCiphersForTLSOneDotTwo) if err != nil { return fmt.Errorf("could not load config: %w", err) } + ptls.LogAllProfiles(plog.New()) + return runSupervisor(ctx, podInfo, cfg) } diff --git a/test/integration/pod_shutdown_test.go b/test/integration/pod_shutdown_test.go index 77ad4d1e9..a3893d0d7 100644 --- a/test/integration/pod_shutdown_test.go +++ b/test/integration/pod_shutdown_test.go @@ -24,18 +24,23 @@ import ( // before they die. // Never run this test in parallel since deleting the pods is disruptive, see main_test.go. func TestPodShutdown_Disruptive(t *testing.T) { - // Only run this test in CI on Kind clusters, because something about restarting the pods - // in this test breaks the "kubectl port-forward" commands that we are using in CI for - // AKS, EKS, and GKE clusters. The Go code that we wrote for graceful pod shutdown should - // not be sensitive to which distribution it runs on, so running this test only on Kind - // should give us sufficient coverage for what we are trying to test here. - env := testlib.IntegrationEnv(t, testlib.SkipPodRestartAssertions()). - WithKubeDistribution(testlib.KindDistro) + env := testOnKindWithPodShutdown(t) shutdownAllPodsOfApp(t, env, env.ConciergeNamespace, env.ConciergeAppName, true) shutdownAllPodsOfApp(t, env, env.SupervisorNamespace, env.SupervisorAppName, false) } +// testOnKindWithPodShutdown builds an env with the following description: +// Only run this test in CI on Kind clusters, because something about restarting the pods +// in this test breaks the "kubectl port-forward" commands that we are using in CI for +// AKS, EKS, and GKE clusters. The Go code that we wrote for graceful pod shutdown should +// not be sensitive to which distribution it runs on, so running this test only on Kind +// should give us sufficient coverage for what we are trying to test here. +func testOnKindWithPodShutdown(t *testing.T) *testlib.TestEnv { + return testlib.IntegrationEnv(t, testlib.SkipPodRestartAssertions()). + WithKubeDistribution(testlib.KindDistro) +} + func shutdownAllPodsOfApp( t *testing.T, env *testlib.TestEnv,