Skip to content

fix: use SecretBytes type for cert values to prevent accidental printing #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
golang 1.19.13
21 changes: 11 additions & 10 deletions cmd/tls-config-factory-test-harness/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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())

Expand All @@ -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())

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

Expand Down Expand Up @@ -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())

Expand All @@ -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())

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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)),
}
}
19 changes: 13 additions & 6 deletions internal/certification/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
Expand Down Expand Up @@ -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]

Expand All @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down
21 changes: 21 additions & 0 deletions internal/core/secret_bytes.go
Original file line number Diff line number Diff line change
@@ -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]")
}
31 changes: 31 additions & 0 deletions internal/core/secret_bytes_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}