From 537b14a26182d7e0b9bd6d2a17551316880b93f8 Mon Sep 17 00:00:00 2001 From: Steve Wagner Date: Thu, 7 Dec 2023 15:30:38 -0800 Subject: [PATCH] TLS Mode is now an enum - Improve enum readability - Improve TLS Mode configuration validation and application - Better error message --- cmd/tls-config-factory-test-harness/main.go | 8 +-- internal/authentication/factory.go | 16 +++-- internal/authentication/factory_test.go | 37 +++-------- internal/configuration/settings.go | 28 +++++--- internal/configuration/tlsmodes.go | 46 +++++++++++++ internal/configuration/tlsmodes_test.go | 74 +++++++++++++++++++++ 6 files changed, 163 insertions(+), 46 deletions(-) create mode 100644 internal/configuration/tlsmodes.go create mode 100644 internal/configuration/tlsmodes_test.go diff --git a/cmd/tls-config-factory-test-harness/main.go b/cmd/tls-config-factory-test-harness/main.go index 3e9ef48..b5fda78 100644 --- a/cmd/tls-config-factory-test-harness/main.go +++ b/cmd/tls-config-factory-test-harness/main.go @@ -87,7 +87,7 @@ func ssTlsConfig() configuration.Settings { certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM()) return configuration.Settings{ - TlsMode: "ss-tls", + TlsMode: configuration.SelfSignedTLS, Certificates: &certification.Certificates{ Certificates: certificates, }, @@ -100,7 +100,7 @@ func ssMtlsConfig() configuration.Settings { certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM()) return configuration.Settings{ - TlsMode: "ss-mtls", + TlsMode: configuration.SelfSignedMutualTLS, Certificates: &certification.Certificates{ Certificates: certificates, }, @@ -109,7 +109,7 @@ func ssMtlsConfig() configuration.Settings { func caTlsConfig() configuration.Settings { return configuration.Settings{ - TlsMode: "ca-tls", + TlsMode: configuration.CertificateAuthorityTLS, } } @@ -118,7 +118,7 @@ func caMtlsConfig() configuration.Settings { certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM()) return configuration.Settings{ - TlsMode: "ca-mtls", + TlsMode: configuration.CertificateAuthorityMutualTLS, Certificates: &certification.Certificates{ Certificates: certificates, }, diff --git a/internal/authentication/factory.go b/internal/authentication/factory.go index 293989b..8b8d06e 100644 --- a/internal/authentication/factory.go +++ b/internal/authentication/factory.go @@ -20,20 +20,24 @@ import ( func NewTlsConfig(settings *configuration.Settings) (*tls.Config, error) { logrus.Debugf("authentication::NewTlsConfig Creating TLS config for mode: '%s'", settings.TlsMode) switch settings.TlsMode { - case "ss-tls": // needs ca cert + + case configuration.NoTLS: + return buildBasicTlsConfig(true), nil + + case configuration.SelfSignedTLS: // needs ca cert return buildSelfSignedTlsConfig(settings.Certificates) - case "ss-mtls": // needs ca cert and client cert + case configuration.SelfSignedMutualTLS: // needs ca cert and client cert return buildSelfSignedMtlsConfig(settings.Certificates) - case "ca-tls": // needs nothing + case configuration.CertificateAuthorityTLS: // needs nothing return buildBasicTlsConfig(false), nil - case "ca-mtls": // needs client cert + case configuration.CertificateAuthorityMutualTLS: // needs client cert return buildCaTlsConfig(settings.Certificates) - default: // no-tls, needs nothing - return buildBasicTlsConfig(true), nil + default: + return nil, fmt.Errorf("unknown TLS mode: %s", settings.TlsMode) } } diff --git a/internal/authentication/factory_test.go b/internal/authentication/factory_test.go index d106eb7..7ecd610 100644 --- a/internal/authentication/factory_test.go +++ b/internal/authentication/factory_test.go @@ -16,25 +16,6 @@ const ( ClientCertificateSecretKey = "nlk-tls-client-secret" ) -func TestTlsFactory_EmptyStringModeDefaultsToNoTls(t *testing.T) { - settings := configuration.Settings{ - TlsMode: "", - } - - tlsConfig, err := NewTlsConfig(&settings) - if err != nil { - t.Fatalf(`Unexpected error: %v`, err) - } - - if tlsConfig == nil { - t.Fatalf(`tlsConfig should not be nil`) - } - - if tlsConfig.InsecureSkipVerify != true { - t.Fatalf(`tlsConfig.InsecureSkipVerify should be true`) - } -} - func TestTlsFactory_UnspecifiedModeDefaultsToNoTls(t *testing.T) { settings := configuration.Settings{} @@ -57,7 +38,7 @@ func TestTlsFactory_SelfSignedTlsMode(t *testing.T) { certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM()) settings := configuration.Settings{ - TlsMode: "ss-tls", + TlsMode: configuration.SelfSignedTLS, Certificates: &certification.Certificates{ Certificates: certificates, CaCertificateSecretKey: CaCertificateSecretKey, @@ -92,7 +73,7 @@ func TestTlsFactory_SelfSignedTlsModeCertPoolError(t *testing.T) { certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificatePEM()) settings := configuration.Settings{ - TlsMode: "ss-tls", + TlsMode: configuration.SelfSignedTLS, Certificates: &certification.Certificates{ Certificates: certificates, }, @@ -113,7 +94,7 @@ func TestTlsFactory_SelfSignedTlsModeCertPoolCertificateParseError(t *testing.T) certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificateDataPEM()) settings := configuration.Settings{ - TlsMode: "ss-tls", + TlsMode: configuration.SelfSignedTLS, Certificates: &certification.Certificates{ Certificates: certificates, CaCertificateSecretKey: CaCertificateSecretKey, @@ -137,7 +118,7 @@ func TestTlsFactory_SelfSignedMtlsMode(t *testing.T) { certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM()) settings := configuration.Settings{ - TlsMode: "ss-mtls", + TlsMode: configuration.SelfSignedMutualTLS, Certificates: &certification.Certificates{ Certificates: certificates, CaCertificateSecretKey: CaCertificateSecretKey, @@ -173,7 +154,7 @@ func TestTlsFactory_SelfSignedMtlsModeCertPoolError(t *testing.T) { certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM()) settings := configuration.Settings{ - TlsMode: "ss-mtls", + TlsMode: configuration.SelfSignedMutualTLS, Certificates: &certification.Certificates{ Certificates: certificates, }, @@ -195,7 +176,7 @@ func TestTlsFactory_SelfSignedMtlsModeClientCertificateError(t *testing.T) { certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), invalidCertificatePEM()) settings := configuration.Settings{ - TlsMode: "ss-mtls", + TlsMode: configuration.SelfSignedMutualTLS, Certificates: &certification.Certificates{ Certificates: certificates, CaCertificateSecretKey: CaCertificateSecretKey, @@ -215,7 +196,7 @@ func TestTlsFactory_SelfSignedMtlsModeClientCertificateError(t *testing.T) { func TestTlsFactory_CaTlsMode(t *testing.T) { settings := configuration.Settings{ - TlsMode: "ca-tls", + TlsMode: configuration.CertificateAuthorityTLS, } tlsConfig, err := NewTlsConfig(&settings) @@ -245,7 +226,7 @@ func TestTlsFactory_CaMtlsMode(t *testing.T) { certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM()) settings := configuration.Settings{ - TlsMode: "ca-mtls", + TlsMode: configuration.CertificateAuthorityMutualTLS, Certificates: &certification.Certificates{ Certificates: certificates, CaCertificateSecretKey: CaCertificateSecretKey, @@ -281,7 +262,7 @@ func TestTlsFactory_CaMtlsModeClientCertificateError(t *testing.T) { certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), invalidCertificatePEM()) settings := configuration.Settings{ - TlsMode: "ca-mtls", + TlsMode: configuration.CertificateAuthorityMutualTLS, Certificates: &certification.Certificates{ Certificates: certificates, }, diff --git a/internal/configuration/settings.go b/internal/configuration/settings.go index 624dfbc..d15ad84 100644 --- a/internal/configuration/settings.go +++ b/internal/configuration/settings.go @@ -110,7 +110,7 @@ type Settings struct { NginxPlusHosts []string // TlsMode is the value used to determine which of the five TLS modes will be used to communicate with the Border Servers (see: ../../docs/tls/README.md). - TlsMode string + TlsMode TLSMode // Certificates is the object used to retrieve the certificates and keys used to communicate with the Border Servers. Certificates *certification.Certificates @@ -139,7 +139,7 @@ func NewSettings(ctx context.Context, k8sClient kubernetes.Interface) (*Settings settings := &Settings{ Context: ctx, K8sClient: k8sClient, - TlsMode: "", + TlsMode: NoTLS, Certificates: nil, Handler: HandlerSettings{ RetryCount: 5, @@ -282,13 +282,12 @@ func (s *Settings) handleUpdateEvent(_ interface{}, newValue interface{}) { logrus.Warnf("Settings::handleUpdateEvent: nginx-hosts key not found in ConfigMap") } - tlsMode, found := configMap.Data["tls-mode"] - if found { - s.TlsMode = tlsMode - logrus.Debugf("Settings::handleUpdateEvent: tls-mode: %s", s.TlsMode) + tlsMode, err := validateTlsMode(configMap) + if err != nil { + // NOTE: the TLSMode defaults to NoTLS on startup, or the last known good value if previously set. + logrus.Errorf("There was an error with the configured TLS Mode. TLS Mode has NOT been changed. The current mode is: '%v'. Error: %v. ", s.TlsMode, err) } else { - s.TlsMode = "no-tls" - logrus.Warnf("Settings::handleUpdateEvent: tls-mode key not found in ConfigMap, defaulting to 'no-tls'") + s.TlsMode = tlsMode } caCertificateSecretKey, found := configMap.Data["ca-certificate"] @@ -314,6 +313,19 @@ func (s *Settings) handleUpdateEvent(_ interface{}, newValue interface{}) { logrus.Debugf("Settings::handleUpdateEvent: \n\tHosts: %v,\n\tSettings: %v ", s.NginxPlusHosts, configMap) } +func validateTlsMode(configMap *corev1.ConfigMap) (TLSMode, error) { + tlsConfigMode, tlsConfigModeFound := configMap.Data["tls-mode"] + if !tlsConfigModeFound { + return NoTLS, fmt.Errorf(`tls-mode key not found in ConfigMap`) + } + + if tlsMode, tlsModeFound := TLSModeMap[tlsConfigMode]; tlsModeFound { + return tlsMode, nil + } + + return NoTLS, fmt.Errorf(`invalid tls-mode value: %s`, tlsConfigMode) +} + func (s *Settings) parseHosts(hosts string) []string { return strings.Split(hosts, ",") } diff --git a/internal/configuration/tlsmodes.go b/internal/configuration/tlsmodes.go new file mode 100644 index 0000000..2f7271f --- /dev/null +++ b/internal/configuration/tlsmodes.go @@ -0,0 +1,46 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package configuration + +const ( + NoTLS TLSMode = iota + CertificateAuthorityTLS + CertificateAuthorityMutualTLS + SelfSignedTLS + SelfSignedMutualTLS +) + +const ( + NoTLSString = "no-tls" + CertificateAuthorityTLSString = "ca-tls" + CertificateAuthorityMutualTLSString = "ca-mtls" + SelfSignedTLSString = "ss-tls" + SelfSignedMutualTLSString = "ss-mtls" +) + +type TLSMode int + +var TLSModeMap = map[string]TLSMode{ + NoTLSString: NoTLS, + CertificateAuthorityTLSString: CertificateAuthorityTLS, + CertificateAuthorityMutualTLSString: CertificateAuthorityMutualTLS, + SelfSignedTLSString: SelfSignedTLS, + SelfSignedMutualTLSString: SelfSignedMutualTLS, +} + +func (t TLSMode) String() string { + modes := []string{ + NoTLSString, + CertificateAuthorityTLSString, + CertificateAuthorityMutualTLSString, + SelfSignedTLSString, + SelfSignedMutualTLSString, + } + if t < NoTLS || t > SelfSignedMutualTLS { + return "" + } + return modes[t] +} diff --git a/internal/configuration/tlsmodes_test.go b/internal/configuration/tlsmodes_test.go new file mode 100644 index 0000000..62abf96 --- /dev/null +++ b/internal/configuration/tlsmodes_test.go @@ -0,0 +1,74 @@ +/* + * Copyright 2023 F5 Inc. All rights reserved. + * Use of this source code is governed by the Apache License that can be found in the LICENSE file. + */ + +package configuration + +import ( + "testing" +) + +func Test_String(t *testing.T) { + mode := NoTLS.String() + if mode != "no-tls" { + t.Errorf("Expected TLSModeNoTLS to be 'no-tls', got '%s'", mode) + } + + mode = CertificateAuthorityTLS.String() + if mode != "ca-tls" { + t.Errorf("Expected TLSModeCaTLS to be 'ca-tls', got '%s'", mode) + } + + mode = CertificateAuthorityMutualTLS.String() + if mode != "ca-mtls" { + t.Errorf("Expected TLSModeCaMTLS to be 'ca-mtls', got '%s'", mode) + } + + mode = SelfSignedTLS.String() + if mode != "ss-tls" { + t.Errorf("Expected TLSModeSsTLS to be 'ss-tls', got '%s'", mode) + } + + mode = SelfSignedMutualTLS.String() + if mode != "ss-mtls" { + t.Errorf("Expected TLSModeSsMTLS to be 'ss-mtls', got '%s',", mode) + } + + mode = TLSMode(5).String() + if mode != "" { + t.Errorf("Expected TLSMode(5) to be '', got '%s'", mode) + } +} + +func Test_TLSModeMap(t *testing.T) { + mode := TLSModeMap["no-tls"] + if mode != NoTLS { + t.Errorf("Expected TLSModeMap['no-tls'] to be TLSModeNoTLS, got '%d'", mode) + } + + mode = TLSModeMap["ca-tls"] + if mode != CertificateAuthorityTLS { + t.Errorf("Expected TLSModeMap['ca-tls'] to be TLSModeCaTLS, got '%d'", mode) + } + + mode = TLSModeMap["ca-mtls"] + if mode != CertificateAuthorityMutualTLS { + t.Errorf("Expected TLSModeMap['ca-mtls'] to be TLSModeCaMTLS, got '%d'", mode) + } + + mode = TLSModeMap["ss-tls"] + if mode != SelfSignedTLS { + t.Errorf("Expected TLSModeMap['ss-tls'] to be TLSModeSsTLS, got '%d'", mode) + } + + mode = TLSModeMap["ss-mtls"] + if mode != SelfSignedMutualTLS { + t.Errorf("Expected TLSModeMap['ss-mtls'] to be TLSModeSsMTLS, got '%d'", mode) + } + + mode = TLSModeMap["invalid"] + if mode != TLSMode(0) { + t.Errorf("Expected TLSModeMap['invalid'] to be TLSMode(0), got '%d'", mode) + } +}