fix: use SecretBytes type for cert values to prevent accidental print…

…ing (#147)

We save the values of the provided certs that we retrieve from Kubernetes secrets in the `Certificates` attribute on the `Certificates` struct.

This is sensitive information that we want to make sure stays out of the logs and any stack traces. A common approach to this is to create a type definition for sensitive values that implements `Stringer` and `JSON` interfaces and cast the sensitive data to that value.

Fixes issues #145

.tool-versions

@@ -0,0 +1 @@
+golang 1.19.13

cmd/tls-config-factory-test-harness/main.go

@@ -6,6 +6,7 @@ import (
 	"github.com/nginxinc/kubernetes-nginx-ingress/internal/authentication"
 	"github.com/nginxinc/kubernetes-nginx-ingress/internal/certification"
 	"github.com/nginxinc/kubernetes-nginx-ingress/internal/configuration"
+	"github.com/nginxinc/kubernetes-nginx-ingress/internal/core"
 	"github.com/sirupsen/logrus"
 	"os"
 )
@@ -82,7 +83,7 @@ func buildConfigMap() map[string]TlsConfiguration {
 }
 
 func ssTlsConfig() configuration.Settings {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
 	certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
 
@@ -95,7 +96,7 @@ func ssTlsConfig() configuration.Settings {
 }
 
 func ssMtlsConfig() configuration.Settings {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
 	certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
 
@@ -114,7 +115,7 @@ func caTlsConfig() configuration.Settings {
 }
 
 func caMtlsConfig() configuration.Settings {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
 
 	return configuration.Settings{
@@ -215,15 +216,15 @@ z/3KkMx4uqJXZyvQrmkolSg=
 `
 }
 
-func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string][]byte {
-	return map[string][]byte{
-		certification.CertificateKey:    []byte(certificatePEM),
-		certification.CertificateKeyKey: []byte(keyPEM),
+func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string]core.SecretBytes {
+	return map[string]core.SecretBytes{
+		certification.CertificateKey:    core.SecretBytes([]byte(certificatePEM)),
+		certification.CertificateKeyKey: core.SecretBytes([]byte(keyPEM)),
 	}
 }
 
-func buildCaCertificateEntry(certificatePEM string) map[string][]byte {
-	return map[string][]byte{
-		certification.CertificateKey: []byte(certificatePEM),
+func buildCaCertificateEntry(certificatePEM string) map[string]core.SecretBytes {
+	return map[string]core.SecretBytes{
+		certification.CertificateKey: core.SecretBytes([]byte(certificatePEM)),
 	}
 }

internal/authentication/factory_test.go

@@ -6,9 +6,11 @@
 package authentication
 
 import (
+	"testing"
+
 	"github.com/nginxinc/kubernetes-nginx-ingress/internal/certification"
 	"github.com/nginxinc/kubernetes-nginx-ingress/internal/configuration"
-	"testing"
+	"github.com/nginxinc/kubernetes-nginx-ingress/internal/core"
 )
 
 const (
@@ -34,7 +36,7 @@ func TestTlsFactory_UnspecifiedModeDefaultsToNoTls(t *testing.T) {
 }
 
 func TestTlsFactory_SelfSignedTlsMode(t *testing.T) {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
 
 	settings := configuration.Settings{
@@ -69,7 +71,7 @@ func TestTlsFactory_SelfSignedTlsMode(t *testing.T) {
 }
 
 func TestTlsFactory_SelfSignedTlsModeCertPoolError(t *testing.T) {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificatePEM())
 
 	settings := configuration.Settings{
@@ -90,7 +92,7 @@ func TestTlsFactory_SelfSignedTlsModeCertPoolError(t *testing.T) {
 }
 
 func TestTlsFactory_SelfSignedTlsModeCertPoolCertificateParseError(t *testing.T) {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificateDataPEM())
 
 	settings := configuration.Settings{
@@ -113,7 +115,7 @@ func TestTlsFactory_SelfSignedTlsModeCertPoolCertificateParseError(t *testing.T)
 }
 
 func TestTlsFactory_SelfSignedMtlsMode(t *testing.T) {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
 	certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
 
@@ -149,7 +151,7 @@ func TestTlsFactory_SelfSignedMtlsMode(t *testing.T) {
 }
 
 func TestTlsFactory_SelfSignedMtlsModeCertPoolError(t *testing.T) {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificatePEM())
 	certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
 
@@ -171,7 +173,7 @@ func TestTlsFactory_SelfSignedMtlsModeCertPoolError(t *testing.T) {
 }
 
 func TestTlsFactory_SelfSignedMtlsModeClientCertificateError(t *testing.T) {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
 	certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), invalidCertificatePEM())
 
@@ -222,7 +224,7 @@ func TestTlsFactory_CaTlsMode(t *testing.T) {
 }
 
 func TestTlsFactory_CaMtlsMode(t *testing.T) {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
 
 	settings := configuration.Settings{
@@ -257,7 +259,7 @@ func TestTlsFactory_CaMtlsMode(t *testing.T) {
 }
 
 func TestTlsFactory_CaMtlsModeClientCertificateError(t *testing.T) {
-	certificates := make(map[string]map[string][]byte)
+	certificates := make(map[string]map[string]core.SecretBytes)
 	certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
 	certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), invalidCertificatePEM())
 
@@ -411,15 +413,15 @@ z/3KkMx4uqJXZyvQrmkolSg=
 `
 }
 
-func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string][]byte {
-	return map[string][]byte{
-		certification.CertificateKey:    []byte(certificatePEM),
-		certification.CertificateKeyKey: []byte(keyPEM),
+func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string]core.SecretBytes {
+	return map[string]core.SecretBytes{
+		certification.CertificateKey:    core.SecretBytes([]byte(certificatePEM)),
+		certification.CertificateKeyKey: core.SecretBytes([]byte(keyPEM)),
 	}
 }
 
-func buildCaCertificateEntry(certificatePEM string) map[string][]byte {
-	return map[string][]byte{
-		certification.CertificateKey: []byte(certificatePEM),
+func buildCaCertificateEntry(certificatePEM string) map[string]core.SecretBytes {
+	return map[string]core.SecretBytes{
+		certification.CertificateKey: core.SecretBytes([]byte(certificatePEM)),
 	}
 }

internal/certification/certificates.go

@@ -11,11 +11,14 @@ package certification
 import (
 	"context"
 	"fmt"
+
 	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
+
+	"github.com/nginxinc/kubernetes-nginx-ingress/internal/core"
 )
 
 const (
@@ -30,7 +33,7 @@ const (
 )
 
 type Certificates struct {
-	Certificates map[string]map[string][]byte
+	Certificates map[string]map[string]core.SecretBytes
 
 	// Context is the context used to control the application.
 	Context context.Context
@@ -61,14 +64,14 @@ func NewCertificates(ctx context.Context, k8sClient kubernetes.Interface) *Certi
 }
 
 // GetCACertificate returns the Certificate Authority certificate.
-func (c *Certificates) GetCACertificate() []byte {
+func (c *Certificates) GetCACertificate() core.SecretBytes {
 	bytes := c.Certificates[c.CaCertificateSecretKey][CertificateKey]
 
 	return bytes
 }
 
 // GetClientCertificate returns the Client certificate and key.
-func (c *Certificates) GetClientCertificate() ([]byte, []byte) {
+func (c *Certificates) GetClientCertificate() (core.SecretBytes, core.SecretBytes) {
 	keyBytes := c.Certificates[c.ClientCertificateSecretKey][CertificateKeyKey]
 	certificateBytes := c.Certificates[c.ClientCertificateSecretKey][CertificateKey]
 
@@ -81,7 +84,7 @@ func (c *Certificates) Initialize() error {
 
 	var err error
 
-	c.Certificates = make(map[string]map[string][]byte)
+	c.Certificates = make(map[string]map[string]core.SecretBytes)
 
 	informer, err := c.buildInformer()
 	if err != nil {
@@ -151,10 +154,14 @@ func (c *Certificates) handleAddEvent(obj interface{}) {
 		return
 	}
 
-	c.Certificates[secret.Name] = map[string][]byte{}
+	c.Certificates[secret.Name] = map[string]core.SecretBytes{}
 
+	// Input from the secret comes in the form
+	//   tls.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVCVEN...
+	//   tls.key: LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2Z0l...
+	// Where the keys are `tls.crt` and `tls.key` and the values are []byte
 	for k, v := range secret.Data {
-		c.Certificates[secret.Name][k] = v
+		c.Certificates[secret.Name][k] = core.SecretBytes(v)
 	}
 
 	logrus.Debugf("Certificates::handleAddEvent: certificates (%d)", len(c.Certificates))

internal/core/secret_bytes.go

@@ -0,0 +1,21 @@
+package core
+
+import (
+	"encoding/json"
+)
+
+// Wraps byte slices which potentially could contain
+// sensitive data that should not be output to the logs.
+// This will output [REDACTED] if attempts are made
+// to print this type in logs, serialize to JSON, or
+// otherwise convert it to a string.
+// Usage: core.SecretBytes(myByteSlice)
+type SecretBytes []byte
+
+func (sb SecretBytes) String() string {
+	return "[REDACTED]"
+}
+
+func (sb SecretBytes) MarshalJSON() ([]byte, error) {
+	return json.Marshal("[REDACTED]")
+}

internal/core/secret_bytes_test.go

@@ -0,0 +1,31 @@
+/*
+ * 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 core
+
+import (
+	"encoding/json"
+	"fmt"
+	"testing"
+)
+
+func TestSecretBytesToString(t *testing.T) {
+	sensitive := SecretBytes([]byte("If you can see this we have a problem"))
+
+	expected := "foo [REDACTED] bar"
+	result := fmt.Sprintf("foo %v bar", sensitive)
+	if result != expected {
+		t.Errorf("Expected %s, got %s", expected, result)
+	}
+}
+
+func TestSecretBytesToJSON(t *testing.T) {
+	sensitive, _ := json.Marshal(SecretBytes([]byte("If you can see this we have a problem")))
+	expected := `foo "[REDACTED]" bar`
+	result := fmt.Sprintf("foo %v bar", string(sensitive))
+	if result != expected {
+		t.Errorf("Expected %s, got %s", expected, result)
+	}
+}