Skip to content

Commit 23457dd

Browse files
4141done4141done
committed
fix: use SecretBytes type for cert values to prevent accidental printing. Fixes 145
1 parent f77cc0b commit 23457dd

File tree

7 files changed

+95
-32
lines changed

7 files changed

+95
-32
lines changed

‎.tool-versions

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
golang 1.19.13

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

Lines changed: 11 additions & 10 deletions
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

Lines changed: 18 additions & 16 deletions
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 (
@@ -53,7 +55,7 @@ func TestTlsFactory_UnspecifiedModeDefaultsToNoTls(t *testing.T) {
5355
}
5456

5557
func TestTlsFactory_SelfSignedTlsMode(t *testing.T) {
56-
certificates := make(map[string]map[string][]byte)
58+
certificates := make(map[string]map[string]core.SecretBytes)
5759
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
5860

5961
settings := configuration.Settings{
@@ -88,7 +90,7 @@ func TestTlsFactory_SelfSignedTlsMode(t *testing.T) {
8890
}
8991

9092
func TestTlsFactory_SelfSignedTlsModeCertPoolError(t *testing.T) {
91-
certificates := make(map[string]map[string][]byte)
93+
certificates := make(map[string]map[string]core.SecretBytes)
9294
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificatePEM())
9395

9496
settings := configuration.Settings{
@@ -109,7 +111,7 @@ func TestTlsFactory_SelfSignedTlsModeCertPoolError(t *testing.T) {
109111
}
110112

111113
func TestTlsFactory_SelfSignedTlsModeCertPoolCertificateParseError(t *testing.T) {
112-
certificates := make(map[string]map[string][]byte)
114+
certificates := make(map[string]map[string]core.SecretBytes)
113115
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificateDataPEM())
114116

115117
settings := configuration.Settings{
@@ -132,7 +134,7 @@ func TestTlsFactory_SelfSignedTlsModeCertPoolCertificateParseError(t *testing.T)
132134
}
133135

134136
func TestTlsFactory_SelfSignedMtlsMode(t *testing.T) {
135-
certificates := make(map[string]map[string][]byte)
137+
certificates := make(map[string]map[string]core.SecretBytes)
136138
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
137139
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
138140

@@ -168,7 +170,7 @@ func TestTlsFactory_SelfSignedMtlsMode(t *testing.T) {
168170
}
169171

170172
func TestTlsFactory_SelfSignedMtlsModeCertPoolError(t *testing.T) {
171-
certificates := make(map[string]map[string][]byte)
173+
certificates := make(map[string]map[string]core.SecretBytes)
172174
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(invalidCertificatePEM())
173175
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
174176

@@ -190,7 +192,7 @@ func TestTlsFactory_SelfSignedMtlsModeCertPoolError(t *testing.T) {
190192
}
191193

192194
func TestTlsFactory_SelfSignedMtlsModeClientCertificateError(t *testing.T) {
193-
certificates := make(map[string]map[string][]byte)
195+
certificates := make(map[string]map[string]core.SecretBytes)
194196
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
195197
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), invalidCertificatePEM())
196198

@@ -241,7 +243,7 @@ func TestTlsFactory_CaTlsMode(t *testing.T) {
241243
}
242244

243245
func TestTlsFactory_CaMtlsMode(t *testing.T) {
244-
certificates := make(map[string]map[string][]byte)
246+
certificates := make(map[string]map[string]core.SecretBytes)
245247
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), clientCertificatePEM())
246248

247249
settings := configuration.Settings{
@@ -276,7 +278,7 @@ func TestTlsFactory_CaMtlsMode(t *testing.T) {
276278
}
277279

278280
func TestTlsFactory_CaMtlsModeClientCertificateError(t *testing.T) {
279-
certificates := make(map[string]map[string][]byte)
281+
certificates := make(map[string]map[string]core.SecretBytes)
280282
certificates[CaCertificateSecretKey] = buildCaCertificateEntry(caCertificatePEM())
281283
certificates[ClientCertificateSecretKey] = buildClientCertificateEntry(clientKeyPEM(), invalidCertificatePEM())
282284

@@ -430,15 +432,15 @@ z/3KkMx4uqJXZyvQrmkolSg=
430432
`
431433
}
432434

433-
func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string][]byte {
434-
return map[string][]byte{
435-
certification.CertificateKey: []byte(certificatePEM),
436-
certification.CertificateKeyKey: []byte(keyPEM),
435+
func buildClientCertificateEntry(keyPEM, certificatePEM string) map[string]core.SecretBytes {
436+
return map[string]core.SecretBytes{
437+
certification.CertificateKey: core.SecretBytes([]byte(certificatePEM)),
438+
certification.CertificateKeyKey: core.SecretBytes([]byte(keyPEM)),
437439
}
438440
}
439441

440-
func buildCaCertificateEntry(certificatePEM string) map[string][]byte {
441-
return map[string][]byte{
442-
certification.CertificateKey: []byte(certificatePEM),
442+
func buildCaCertificateEntry(certificatePEM string) map[string]core.SecretBytes {
443+
return map[string]core.SecretBytes{
444+
certification.CertificateKey: core.SecretBytes([]byte(certificatePEM)),
443445
}
444446
}

‎internal/certification/certificates.go

Lines changed: 18 additions & 6 deletions
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,12 @@ const (
3033
)
3134

3235
type Certificates struct {
33-
Certificates map[string]map[string][]byte
36+
// {
37+
// ClientCertificateSecretKey: {
38+
// "tls.crt": []byte
39+
// }
40+
// }
41+
Certificates map[string]map[string]core.SecretBytes
3442

3543
// Context is the context used to control the application.
3644
Context context.Context
@@ -61,14 +69,14 @@ func NewCertificates(ctx context.Context, k8sClient kubernetes.Interface) *Certi
6169
}
6270

6371
// GetCACertificate returns the Certificate Authority certificate.
64-
func (c *Certificates) GetCACertificate() []byte {
72+
func (c *Certificates) GetCACertificate() core.SecretBytes {
6573
bytes := c.Certificates[c.CaCertificateSecretKey][CertificateKey]
6674

6775
return bytes
6876
}
6977

7078
// GetClientCertificate returns the Client certificate and key.
71-
func (c *Certificates) GetClientCertificate() ([]byte, []byte) {
79+
func (c *Certificates) GetClientCertificate() (core.SecretBytes, core.SecretBytes) {
7280
keyBytes := c.Certificates[c.ClientCertificateSecretKey][CertificateKeyKey]
7381
certificateBytes := c.Certificates[c.ClientCertificateSecretKey][CertificateKey]
7482

@@ -81,7 +89,7 @@ func (c *Certificates) Initialize() error {
8189

8290
var err error
8391

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

8694
informer, err := c.buildInformer()
8795
if err != nil {
@@ -151,10 +159,14 @@ func (c *Certificates) handleAddEvent(obj interface{}) {
151159
return
152160
}
153161

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

164+
// Input from the secret comes in the form
165+
// tls.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVCVEN...
166+
// tls.key: LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2Z0l...
167+
// Where the keys are `tls.crt` and `tls.key` and the values are []byte
156168
for k, v := range secret.Data {
157-
c.Certificates[secret.Name][k] = v
169+
c.Certificates[secret.Name][k] = core.SecretBytes(v)
158170
}
159171

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

‎internal/configuration/settings.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ func (s *Settings) Initialize() error {
179179

180180
certificates := certification.NewCertificates(s.Context, s.K8sClient)
181181

182+
// q. Why is this a separate step?
182183
err = certificates.Initialize()
183184
if err != nil {
184185
return fmt.Errorf(`error occurred initializing certificates: %w`, err)

‎internal/core/secret_bytes.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package core
2+
3+
import (
4+
"encoding/json"
5+
)
6+
7+
type SecretBytes []byte
8+
9+
func (sb SecretBytes) String() string {
10+
return "[REDACTED]"
11+
}
12+
13+
func (sb SecretBytes) MarshalJSON() ([]byte, error) {
14+
return json.Marshal("[REDACTED]")
15+
}

‎internal/core/secret_bytes_test.go

Lines changed: 31 additions & 0 deletions
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)