TLS Mode is now an enum

- Improve enum readability
- Improve TLS Mode configuration validation and application
- Better error message

Author: ciroque

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,
 		},

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)
 	}
 }
 

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,
 		},

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, no changes applied. Value in error: %v. the current mode: %v", err, s.TlsMode)
 	} 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, ",")
 }

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]
+}

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)
+	}
+}