Skip to content

Commit 2ee0441

Browse files
authored
fix: use SecretBytes type for cert values to prevent accidental printing (#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
1 parent 24ea3e7 commit 2ee0441

File tree

6 files changed

+95
-32
lines changed

6 files changed

+95
-32
lines changed

.tool-versions

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
golang 1.19.13

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

+11-10
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/nginxinc/kubernetes-nginx-ingress/internal/authentication"
77
"github.com/nginxinc/kubernetes-nginx-ingress/internal/certification"
88
"github.com/nginxinc/kubernetes-nginx-ingress/internal/configuration"
9+
"github.com/nginxinc/kubernetes-nginx-ingress/internal/core"
910
"github.com/sirupsen/logrus"
1011
"os"
1112
)
@@ -82,7 +83,7 @@ func buildConfigMap() map[string]TlsConfiguration {
8283
}
8384

8485
func ssTlsConfig() configuration.Settings {
85-
certificates := make(map[string]map[string][]byte)
86+
certificates := make(map[string]map[string]core.SecretBytes)
8687
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
8788
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
8889

@@ -95,7 +96,7 @@ func ssTlsConfig() configuration.Settings {
9596
}
9697

9798
func ssMtlsConfig() configuration.Settings {
98-
certificates := make(map[string]map[string][]byte)
99+
certificates := make(map[string]map[string]core.SecretBytes)
99100
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
100101
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
101102

@@ -114,7 +115,7 @@ func caTlsConfig() configuration.Settings {
114115
}
115116

116117
func caMtlsConfig() configuration.Settings {
117-
certificates := make(map[string]map[string][]byte)
118+
certificates := make(map[string]map[string]core.SecretBytes)
118119
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
119120

120121
return configuration.Settings{
@@ -215,15 +216,15 @@ z/3KkMx4uqJXZyvQrmkolSg=
215216
`
216217
}
217218

218-
func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string][]byte {
219-
return map[string][]byte{
220-
certification.CertificateKey: []byte(certificatePEM),
221-
certification.CertificateKeyKey: []byte(keyPEM),
219+
func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string]core.SecretBytes {
220+
return map[string]core.SecretBytes{
221+
certification.CertificateKey: core.SecretBytes([]byte(certificatePEM)),
222+
certification.CertificateKeyKey: core.SecretBytes([]byte(keyPEM)),
222223
}
223224
}
224225

225-
func buildCaCertificateEntry(certificatePEM string) map[string][]byte {
226-
return map[string][]byte{
227-
certification.CertificateKey: []byte(certificatePEM),
226+
func buildCaCertificateEntry(certificatePEM string) map[string]core.SecretBytes {
227+
return map[string]core.SecretBytes{
228+
certification.CertificateKey: core.SecretBytes([]byte(certificatePEM)),
228229
}
229230
}

internal/authentication/factory_test.go

+18-16
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
package authentication
77

88
import (
9+
"testing"
10+
911
"github.com/nginxinc/kubernetes-nginx-ingress/internal/certification"
1012
"github.com/nginxinc/kubernetes-nginx-ingress/internal/configuration"
11-
"testing"
13+
"github.com/nginxinc/kubernetes-nginx-ingress/internal/core"
1214
)
1315

1416
const (
@@ -34,7 +36,7 @@ func TestTlsFactory_UnspecifiedModeDefaultsToNoTls(t *testing.T) {
3436
}
3537

3638
func TestTlsFactory_SelfSignedTlsMode(t *testing.T) {
37-
certificates := make(map[string]map[string][]byte)
39+
certificates := make(map[string]map[string]core.SecretBytes)
3840
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
3941

4042
settings := configuration.Settings{
@@ -69,7 +71,7 @@ func TestTlsFactory_SelfSignedTlsMode(t *testing.T) {
6971
}
7072

7173
func TestTlsFactory_SelfSignedTlsModeCertPoolError(t *testing.T) {
72-
certificates := make(map[string]map[string][]byte)
74+
certificates := make(map[string]map[string]core.SecretBytes)
7375
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificatePEM())
7476

7577
settings := configuration.Settings{
@@ -90,7 +92,7 @@ func TestTlsFactory_SelfSignedTlsModeCertPoolError(t *testing.T) {
9092
}
9193

9294
func TestTlsFactory_SelfSignedTlsModeCertPoolCertificateParseError(t *testing.T) {
93-
certificates := make(map[string]map[string][]byte)
95+
certificates := make(map[string]map[string]core.SecretBytes)
9496
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificateDataPEM())
9597

9698
settings := configuration.Settings{
@@ -113,7 +115,7 @@ func TestTlsFactory_SelfSignedTlsModeCertPoolCertificateParseError(t *testing.T)
113115
}
114116

115117
func TestTlsFactory_SelfSignedMtlsMode(t *testing.T) {
116-
certificates := make(map[string]map[string][]byte)
118+
certificates := make(map[string]map[string]core.SecretBytes)
117119
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
118120
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
119121

@@ -149,7 +151,7 @@ func TestTlsFactory_SelfSignedMtlsMode(t *testing.T) {
149151
}
150152

151153
func TestTlsFactory_SelfSignedMtlsModeCertPoolError(t *testing.T) {
152-
certificates := make(map[string]map[string][]byte)
154+
certificates := make(map[string]map[string]core.SecretBytes)
153155
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificatePEM())
154156
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
155157

@@ -171,7 +173,7 @@ func TestTlsFactory_SelfSignedMtlsModeCertPoolError(t *testing.T) {
171173
}
172174

173175
func TestTlsFactory_SelfSignedMtlsModeClientCertificateError(t *testing.T) {
174-
certificates := make(map[string]map[string][]byte)
176+
certificates := make(map[string]map[string]core.SecretBytes)
175177
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
176178
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), invalidCertificatePEM())
177179

@@ -222,7 +224,7 @@ func TestTlsFactory_CaTlsMode(t *testing.T) {
222224
}
223225

224226
func TestTlsFactory_CaMtlsMode(t *testing.T) {
225-
certificates := make(map[string]map[string][]byte)
227+
certificates := make(map[string]map[string]core.SecretBytes)
226228
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
227229

228230
settings := configuration.Settings{
@@ -257,7 +259,7 @@ func TestTlsFactory_CaMtlsMode(t *testing.T) {
257259
}
258260

259261
func TestTlsFactory_CaMtlsModeClientCertificateError(t *testing.T) {
260-
certificates := make(map[string]map[string][]byte)
262+
certificates := make(map[string]map[string]core.SecretBytes)
261263
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
262264
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), invalidCertificatePEM())
263265

@@ -411,15 +413,15 @@ z/3KkMx4uqJXZyvQrmkolSg=
411413
`
412414
}
413415

414-
func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string][]byte {
415-
return map[string][]byte{
416-
certification.CertificateKey: []byte(certificatePEM),
417-
certification.CertificateKeyKey: []byte(keyPEM),
416+
func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string]core.SecretBytes {
417+
return map[string]core.SecretBytes{
418+
certification.CertificateKey: core.SecretBytes([]byte(certificatePEM)),
419+
certification.CertificateKeyKey: core.SecretBytes([]byte(keyPEM)),
418420
}
419421
}
420422

421-
func buildCaCertificateEntry(certificatePEM string) map[string][]byte {
422-
return map[string][]byte{
423-
certification.CertificateKey: []byte(certificatePEM),
423+
func buildCaCertificateEntry(certificatePEM string) map[string]core.SecretBytes {
424+
return map[string]core.SecretBytes{
425+
certification.CertificateKey: core.SecretBytes([]byte(certificatePEM)),
424426
}
425427
}

internal/certification/certificates.go

+13-6
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@ package certification
1111
import (
1212
"context"
1313
"fmt"
14+
1415
"github.com/sirupsen/logrus"
1516
corev1 "k8s.io/api/core/v1"
1617
"k8s.io/client-go/informers"
1718
"k8s.io/client-go/kubernetes"
1819
"k8s.io/client-go/tools/cache"
20+
21+
"github.com/nginxinc/kubernetes-nginx-ingress/internal/core"
1922
)
2023

2124
const (
@@ -30,7 +33,7 @@ const (
3033
)
3134

3235
type Certificates struct {
33-
Certificates map[string]map[string][]byte
36+
Certificates map[string]map[string]core.SecretBytes
3437

3538
// Context is the context used to control the application.
3639
Context context.Context
@@ -61,14 +64,14 @@ func NewCertificates(ctx context.Context, k8sClient kubernetes.Interface) *Certi
6164
}
6265

6366
// GetCACertificate returns the Certificate Authority certificate.
64-
func (c *Certificates) GetCACertificate() []byte {
67+
func (c *Certificates) GetCACertificate() core.SecretBytes {
6568
bytes := c.Certificates[c.CaCertificateSecretKey][CertificateKey]
6669

6770
return bytes
6871
}
6972

7073
// GetClientCertificate returns the Client certificate and key.
71-
func (c *Certificates) GetClientCertificate() ([]byte, []byte) {
74+
func (c *Certificates) GetClientCertificate() (core.SecretBytes, core.SecretBytes) {
7275
keyBytes := c.Certificates[c.ClientCertificateSecretKey][CertificateKeyKey]
7376
certificateBytes := c.Certificates[c.ClientCertificateSecretKey][CertificateKey]
7477

@@ -81,7 +84,7 @@ func (c *Certificates) Initialize() error {
8184

8285
var err error
8386

84-
c.Certificates = make(map[string]map[string][]byte)
87+
c.Certificates = make(map[string]map[string]core.SecretBytes)
8588

8689
informer, err := c.buildInformer()
8790
if err != nil {
@@ -151,10 +154,14 @@ func (c *Certificates) handleAddEvent(obj interface{}) {
151154
return
152155
}
153156

154-
c.Certificates[secret.Name] = map[string][]byte{}
157+
c.Certificates[secret.Name] = map[string]core.SecretBytes{}
155158

159+
// Input from the secret comes in the form
160+
// tls.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVCVEN...
161+
// tls.key: LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2Z0l...
162+
// Where the keys are `tls.crt` and `tls.key` and the values are []byte
156163
for k, v := range secret.Data {
157-
c.Certificates[secret.Name][k] = v
164+
c.Certificates[secret.Name][k] = core.SecretBytes(v)
158165
}
159166

160167
logrus.Debugf("Certificates::handleAddEvent: certificates (%d)", len(c.Certificates))

internal/core/secret_bytes.go

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package core
2+
3+
import (
4+
"encoding/json"
5+
)
6+
7+
// Wraps byte slices which potentially could contain
8+
// sensitive data that should not be output to the logs.
9+
// This will output [REDACTED] if attempts are made
10+
// to print this type in logs, serialize to JSON, or
11+
// otherwise convert it to a string.
12+
// Usage: core.SecretBytes(myByteSlice)
13+
type SecretBytes []byte
14+
15+
func (sb SecretBytes) String() string {
16+
return "[REDACTED]"
17+
}
18+
19+
func (sb SecretBytes) MarshalJSON() ([]byte, error) {
20+
return json.Marshal("[REDACTED]")
21+
}

internal/core/secret_bytes_test.go

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright 2023 F5 Inc. All rights reserved.
3+
* Use of this source code is governed by the Apache License that can be found in the LICENSE file.
4+
*/
5+
6+
package core
7+
8+
import (
9+
"encoding/json"
10+
"fmt"
11+
"testing"
12+
)
13+
14+
func TestSecretBytesToString(t *testing.T) {
15+
sensitive := SecretBytes([]byte("If you can see this we have a problem"))
16+
17+
expected := "foo [REDACTED] bar"
18+
result := fmt.Sprintf("foo %v bar", sensitive)
19+
if result != expected {
20+
t.Errorf("Expected %s, got %s", expected, result)
21+
}
22+
}
23+
24+
func TestSecretBytesToJSON(t *testing.T) {
25+
sensitive, _ := json.Marshal(SecretBytes([]byte("If you can see this we have a problem")))
26+
expected := `foo "[REDACTED]" bar`
27+
result := fmt.Sprintf("foo %v bar", string(sensitive))
28+
if result != expected {
29+
t.Errorf("Expected %s, got %s", expected, result)
30+
}
31+
}

0 commit comments

Comments
 (0)