Skip to content

Commit

Permalink
TLS Mode is now an enum
Browse files Browse the repository at this point in the history
- Improve enum readability
- Improve TLS Mode configuration validation and application
- Better error message
  • Loading branch information
ciroque committed Dec 15, 2023
1 parent 360536b commit e5790ee
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 46 deletions.
8 changes: 4 additions & 4 deletions cmd/tls-config-factory-test-harness/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -109,7 +109,7 @@ func ssMtlsConfig() configuration.Settings {

func caTlsConfig() configuration.Settings {
return configuration.Settings{
TlsMode: "ca-tls",
TlsMode: configuration.CertificateAuthorityTLS,
}
}

Expand All @@ -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,
},
Expand Down
16 changes: 10 additions & 6 deletions internal/authentication/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
37 changes: 9 additions & 28 deletions internal/authentication/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand All @@ -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,
Expand Down Expand Up @@ -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,
},
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
},
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
},
Expand Down
28 changes: 20 additions & 8 deletions internal/configuration/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"]
Expand All @@ -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, ",")
}
Expand Down
46 changes: 46 additions & 0 deletions internal/configuration/tlsmodes.go
Original file line number Diff line number Diff line change
@@ -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]
}
74 changes: 74 additions & 0 deletions internal/configuration/tlsmodes_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit e5790ee

Please sign in to comment.