Skip to content

Commit 92d0362

Browse files
pleshakovpleshakov
committed
Validate TLS and CA secrets
1 parent 0704e5b commit 92d0362

File tree

3 files changed

+198
-19
lines changed

3 files changed

+198
-19
lines changed

‎internal/k8s/controller_test.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -1364,7 +1364,7 @@ func TestGetIngressMTLSSecret(t *testing.T) {
13641364
Namespace: "default",
13651365
},
13661366
Type: SecretTypeCA,
1367-
Data: map[string][]byte{"ca.crt": nil},
1367+
Data: map[string][]byte{"ca.crt": validCACert},
13681368
}
13691369

13701370
invalidSecret := &v1.Secret{
@@ -1497,15 +1497,15 @@ func TestAddEgressMTLSSecrets(t *testing.T) {
14971497
Namespace: "default",
14981498
},
14991499
Type: api_v1.SecretTypeTLS,
1500-
Data: map[string][]byte{"tls.key": nil, "tls.crt": nil},
1500+
Data: map[string][]byte{"tls.crt": validCert, "tls.key": validKey},
15011501
}
1502-
validSecret2 := &v1.Secret{
1502+
validCASecret := &v1.Secret{
15031503
ObjectMeta: meta_v1.ObjectMeta{
15041504
Name: "valid-egress-trusted-secret",
15051505
Namespace: "default",
15061506
},
15071507
Type: SecretTypeCA,
1508-
Data: map[string][]byte{"ca.crt": nil},
1508+
Data: map[string][]byte{"ca.crt": validCACert},
15091509
}
15101510

15111511
invalidSecret := &v1.Secret{
@@ -1557,7 +1557,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) {
15571557
},
15581558
},
15591559
expectedEgressMTLSSecrets: map[string]*v1.Secret{
1560-
"default/valid-egress-trusted-secret": validSecret2,
1560+
"default/valid-egress-trusted-secret": validCASecret,
15611561
},
15621562
wantErr: false,
15631563
msg: "test getting valid TrustedCA secret",
@@ -1579,7 +1579,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) {
15791579
},
15801580
expectedEgressMTLSSecrets: map[string]*v1.Secret{
15811581
"default/valid-egress-mtls-secret": validSecret,
1582-
"default/valid-egress-trusted-secret": validSecret2,
1582+
"default/valid-egress-trusted-secret": validCASecret,
15831583
},
15841584
wantErr: false,
15851585
msg: "test getting valid secrets",
@@ -1655,7 +1655,7 @@ func TestAddEgressMTLSSecrets(t *testing.T) {
16551655
case "default/valid-egress-mtls-secret":
16561656
return validSecret, true, nil
16571657
case "default/valid-egress-trusted-secret":
1658-
return validSecret2, true, nil
1658+
return validCASecret, true, nil
16591659
case "default/invalid-egress-mtls-secret":
16601660
return invalidSecret, true, errors.New("secret is missing egress-mtls key in data")
16611661
default:

‎internal/k8s/secret.go

+24-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package k8s
22

33
import (
4+
"crypto/tls"
5+
"crypto/x509"
6+
"encoding/pem"
47
"fmt"
58

69
v1 "k8s.io/api/core/v1"
@@ -25,7 +28,11 @@ func ValidateTLSSecret(secret *v1.Secret) error {
2528
}
2629

2730
// Kubernetes ensures that 'tls.crt' and 'tls.key' are present for secrets of v1.SecretTypeTLS type
28-
// no need to validate that
31+
32+
_, err := tls.X509KeyPair(secret.Data[v1.TLSCertKey], secret.Data[v1.TLSPrivateKeyKey])
33+
if err != nil {
34+
return fmt.Errorf("Failed to validate TLS cert and key: %v", err)
35+
}
2936

3037
return nil
3138
}
@@ -40,6 +47,9 @@ func ValidateJWKSecret(secret *v1.Secret) error {
4047
return fmt.Errorf("JWK secret must have the data field %v", JWTKeyKey)
4148
}
4249

50+
// we don't validate the contents of secret.Data[JWTKeyKey], because invalid contents will not make NGINX Plus
51+
// fail to reload: NGINX Plus will return 500 responses for the affected URLs.
52+
4353
return nil
4454
}
4555

@@ -53,6 +63,19 @@ func ValidateCASecret(secret *v1.Secret) error {
5363
return fmt.Errorf("CA secret must have the data field %v", CAKey)
5464
}
5565

66+
block, _ := pem.Decode(secret.Data[CAKey])
67+
if block == nil {
68+
return fmt.Errorf("The data field %s must hold a valid CERTIFICATE PEM block", CAKey)
69+
}
70+
if block.Type != "CERTIFICATE" {
71+
return fmt.Errorf("The data field %s must hold a valid CERTIFICATE PEM block, but got '%s'", CAKey, block.Type)
72+
}
73+
74+
_, err := x509.ParseCertificate(block.Bytes)
75+
if err != nil {
76+
return fmt.Errorf("Failed to validate certificate: %v", err)
77+
}
78+
5679
return nil
5780
}
5881

‎internal/k8s/secret_test.go

+167-11
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestValidateCASecret(t *testing.T) {
7171
},
7272
Type: SecretTypeCA,
7373
Data: map[string][]byte{
74-
"ca.crt": nil,
74+
"ca.crt": validCert,
7575
},
7676
}
7777

@@ -94,7 +94,7 @@ func TestValidateCASecretFails(t *testing.T) {
9494
},
9595
Type: "some-type",
9696
Data: map[string][]byte{
97-
"ca.crt": nil,
97+
"ca.crt": validCert,
9898
},
9999
},
100100
msg: "Incorrect type for CA secret",
@@ -109,6 +109,45 @@ func TestValidateCASecretFails(t *testing.T) {
109109
},
110110
msg: "Missing ca.crt for CA secret",
111111
},
112+
{
113+
secret: &v1.Secret{
114+
ObjectMeta: meta_v1.ObjectMeta{
115+
Name: "ingress-mtls-secret",
116+
Namespace: "default",
117+
},
118+
Type: SecretTypeCA,
119+
Data: map[string][]byte{
120+
"ca.crt": invalidCACertWithNoPEMBlock,
121+
},
122+
},
123+
msg: "Invalid cert with no PEM block",
124+
},
125+
{
126+
secret: &v1.Secret{
127+
ObjectMeta: meta_v1.ObjectMeta{
128+
Name: "ingress-mtls-secret",
129+
Namespace: "default",
130+
},
131+
Type: SecretTypeCA,
132+
Data: map[string][]byte{
133+
"ca.crt": invalidCACertWithWrongPEMBlock,
134+
},
135+
},
136+
msg: "Invalid cert with wrong PEM block",
137+
},
138+
{
139+
secret: &v1.Secret{
140+
ObjectMeta: meta_v1.ObjectMeta{
141+
Name: "ingress-mtls-secret",
142+
Namespace: "default",
143+
},
144+
Type: SecretTypeCA,
145+
Data: map[string][]byte{
146+
"ca.crt": invalidCACert,
147+
},
148+
},
149+
msg: "Invalid cert",
150+
},
112151
}
113152

114153
for _, test := range tests {
@@ -126,6 +165,10 @@ func TestValidateTLSSecret(t *testing.T) {
126165
Namespace: "default",
127166
},
128167
Type: v1.SecretTypeTLS,
168+
Data: map[string][]byte{
169+
"tls.crt": validCert,
170+
"tls.key": validKey,
171+
},
129172
}
130173

131174
err := ValidateTLSSecret(secret)
@@ -135,17 +178,55 @@ func TestValidateTLSSecret(t *testing.T) {
135178
}
136179

137180
func TestValidateTLSSecretFails(t *testing.T) {
138-
secret := &v1.Secret{
139-
ObjectMeta: meta_v1.ObjectMeta{
140-
Name: "tls-secret",
141-
Namespace: "default",
181+
tests := []struct {
182+
secret *v1.Secret
183+
msg string
184+
}{
185+
{
186+
secret: &v1.Secret{
187+
ObjectMeta: meta_v1.ObjectMeta{
188+
Name: "tls-secret",
189+
Namespace: "default",
190+
},
191+
Type: "some type",
192+
},
193+
msg: "Wrong type",
194+
},
195+
{
196+
secret: &v1.Secret{
197+
ObjectMeta: meta_v1.ObjectMeta{
198+
Name: "tls-secret",
199+
Namespace: "default",
200+
},
201+
Type: v1.SecretTypeTLS,
202+
Data: map[string][]byte{
203+
"tls.crt": invalidCert,
204+
"tls.key": validKey,
205+
},
206+
},
207+
msg: "Invalid cert",
208+
},
209+
{
210+
secret: &v1.Secret{
211+
ObjectMeta: meta_v1.ObjectMeta{
212+
Name: "tls-secret",
213+
Namespace: "default",
214+
},
215+
Type: v1.SecretTypeTLS,
216+
Data: map[string][]byte{
217+
"tls.crt": validCert,
218+
"tls.key": invalidKey,
219+
},
220+
},
221+
msg: "Invalid key",
142222
},
143-
Type: "some type",
144223
}
145224

146-
err := ValidateTLSSecret(secret)
147-
if err == nil {
148-
t.Errorf("ValidateTLSSecret() returned no error")
225+
for _, test := range tests {
226+
err := ValidateTLSSecret(test.secret)
227+
if err == nil {
228+
t.Errorf("ValidateTLSSecret() returned no error for the case of %s", test.msg)
229+
}
149230
}
150231
}
151232

@@ -161,6 +242,10 @@ func TestValidateSecret(t *testing.T) {
161242
Namespace: "default",
162243
},
163244
Type: v1.SecretTypeTLS,
245+
Data: map[string][]byte{
246+
"tls.crt": validCert,
247+
"tls.key": validKey,
248+
},
164249
},
165250
msg: "Valid TLS secret",
166251
},
@@ -172,7 +257,7 @@ func TestValidateSecret(t *testing.T) {
172257
},
173258
Type: SecretTypeCA,
174259
Data: map[string][]byte{
175-
"ca.crt": nil,
260+
"ca.crt": validCACert,
176261
},
177262
},
178263
msg: "Valid CA secret",
@@ -210,6 +295,10 @@ func TestValidateSecretFails(t *testing.T) {
210295
Name: "tls-secret",
211296
Namespace: "default",
212297
},
298+
Data: map[string][]byte{
299+
"tls.crt": validCert,
300+
"tls.key": validKey,
301+
},
213302
},
214303
msg: "Missing type for TLS secret",
215304
},
@@ -272,3 +361,70 @@ func TestHasCorrectSecretType(t *testing.T) {
272361
}
273362
}
274363
}
364+
365+
var (
366+
validCert = []byte(`-----BEGIN CERTIFICATE-----
367+
MIIDLjCCAhYCCQDAOF9tLsaXWjANBgkqhkiG9w0BAQsFADBaMQswCQYDVQQGEwJV
368+
UzELMAkGA1UECAwCQ0ExITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0
369+
ZDEbMBkGA1UEAwwSY2FmZS5leGFtcGxlLmNvbSAgMB4XDTE4MDkxMjE2MTUzNVoX
370+
DTIzMDkxMTE2MTUzNVowWDELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMSEwHwYD
371+
VQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQxGTAXBgNVBAMMEGNhZmUuZXhh
372+
bXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCp6Kn7sy81
373+
p0juJ/cyk+vCAmlsfjtFM2muZNK0KtecqG2fjWQb55xQ1YFA2XOSwHAYvSdwI2jZ
374+
ruW8qXXCL2rb4CZCFxwpVECrcxdjm3teViRXVsYImmJHPPSyQgpiobs9x7DlLc6I
375+
BA0ZjUOyl0PqG9SJexMV73WIIa5rDVSF2r4kSkbAj4Dcj7LXeFlVXH2I5XwXCptC
376+
n67JCg42f+k8wgzcRVp8XZkZWZVjwq9RUKDXmFB2YyN1XEWdZ0ewRuKYUJlsm692
377+
skOrKQj0vkoPn41EE/+TaVEpqLTRoUY3rzg7DkdzfdBizFO2dsPNFx2CW0jXkNLv
378+
Ko25CZrOhXAHAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAKHFCcyOjZvoHswUBMdL
379+
RdHIb383pWFynZq/LuUovsVA58B0Cg7BEfy5vWVVrq5RIkv4lZ81N29x21d1JH6r
380+
jSnQx+DXCO/TJEV5lSCUpIGzEUYaUPgRyjsM/NUdCJ8uHVhZJ+S6FA+CnOD9rn2i
381+
ZBePCI5rHwEXwnnl8ywij3vvQ5zHIuyBglWr/Qyui9fjPpwWUvUm4nv5SMG9zCV7
382+
PpuwvuatqjO1208BjfE/cZHIg8Hw9mvW9x9C+IQMIMDE7b/g6OcK7LGTLwlFxvA8
383+
7WjEequnayIphMhKRXVf1N349eN98Ez38fOTHTPbdJjFA/PcC+Gyme+iGt5OQdFh
384+
yRE=
385+
-----END CERTIFICATE-----`)
386+
387+
validKey = []byte(`-----BEGIN RSA PRIVATE KEY-----
388+
MIIEowIBAAKCAQEAqeip+7MvNadI7if3MpPrwgJpbH47RTNprmTStCrXnKhtn41k
389+
G+ecUNWBQNlzksBwGL0ncCNo2a7lvKl1wi9q2+AmQhccKVRAq3MXY5t7XlYkV1bG
390+
CJpiRzz0skIKYqG7Pcew5S3OiAQNGY1DspdD6hvUiXsTFe91iCGuaw1Uhdq+JEpG
391+
wI+A3I+y13hZVVx9iOV8FwqbQp+uyQoONn/pPMIM3EVafF2ZGVmVY8KvUVCg15hQ
392+
dmMjdVxFnWdHsEbimFCZbJuvdrJDqykI9L5KD5+NRBP/k2lRKai00aFGN684Ow5H
393+
c33QYsxTtnbDzRcdgltI15DS7yqNuQmazoVwBwIDAQABAoIBAQCPSdSYnQtSPyql
394+
FfVFpTOsoOYRhf8sI+ibFxIOuRauWehhJxdm5RORpAzmCLyL5VhjtJme223gLrw2
395+
N99EjUKb/VOmZuDsBc6oCF6QNR58dz8cnORTewcotsJR1pn1hhlnR5HqJJBJask1
396+
ZEnUQfcXZrL94lo9JH3E+Uqjo1FFs8xxE8woPBqjZsV7pRUZgC3LhxnwLSExyFo4
397+
cxb9SOG5OmAJozStFoQ2GJOes8rJ5qfdvytgg9xbLaQL/x0kpQ62BoFMBDdqOePW
398+
KfP5zZ6/07/vpj48yA1Q32PzobubsBLd3Kcn32jfm1E7prtWl+JeOFiOznBQFJbN
399+
4qPVRz5hAoGBANtWyxhNCSLu4P+XgKyckljJ6F5668fNj5CzgFRqJ09zn0TlsNro
400+
FTLZcxDqnR3HPYM42JERh2J/qDFZynRQo3cg3oeivUdBVGY8+FI1W0qdub/L9+yu
401+
edOZTQ5XmGGp6r6jexymcJim/OsB3ZnYOpOrlD7SPmBvzNLk4MF6gxbXAoGBAMZO
402+
0p6HbBmcP0tjFXfcKE77ImLm0sAG4uHoUx0ePj/2qrnTnOBBNE4MvgDuTJzy+caU
403+
k8RqmdHCbHzTe6fzYq/9it8sZ77KVN1qkbIcuc+RTxA9nNh1TjsRne74Z0j1FCLk
404+
hHcqH0ri7PYSKHTE8FvFCxZYdbuB84CmZihvxbpRAoGAIbjqaMYPTYuklCda5S79
405+
YSFJ1JzZe1Kja//tDw1zFcgVCKa31jAwciz0f/lSRq3HS1GGGmezhPVTiqLfeZqc
406+
R0iKbhgbOcVVkJJ3K0yAyKwPTumxKHZ6zImZS0c0am+RY9YGq5T7YrzpzcfvpiOU
407+
ffe3RyFT7cfCmfoOhDCtzukCgYB30oLC1RLFOrqn43vCS51zc5zoY44uBzspwwYN
408+
TwvP/ExWMf3VJrDjBCH+T/6sysePbJEImlzM+IwytFpANfiIXEt/48Xf60Nx8gWM
409+
uHyxZZx/NKtDw0V8vX1POnq2A5eiKa+8jRARYKJLYNdfDuwolxvG6bZhkPi/4EtT
410+
3Y18sQKBgHtKbk+7lNJVeswXE5cUG6EDUsDe/2Ua7fXp7FcjqBEoap1LSw+6TXp0
411+
ZgrmKE8ARzM47+EJHUviiq/nupE15g0kJW3syhpU9zZLO7ltB0KIkO9ZRcmUjo8Q
412+
cpLlHMAqbLJ8WYGJCkhiWxyal6hYTyWY4cVkC0xtTl/hUE9IeNKo
413+
-----END RSA PRIVATE KEY-----`)
414+
415+
invalidCert = []byte(`-----BEGIN CERTIFICATE-----
416+
-----END CERTIFICATE-----`)
417+
418+
invalidKey = []byte(`-----BEGIN RSA PRIVATE KEY-----
419+
-----END RSA PRIVATE KEY-----`)
420+
421+
validCACert = validCert
422+
423+
invalidCACertWithNoPEMBlock []byte
424+
425+
invalidCACertWithWrongPEMBlock = []byte(`-----BEGIN PRIVATE KEY-----
426+
-----END PRIVATE KEY-----`)
427+
428+
invalidCACert = []byte(`-----BEGIN CERTIFICATE-----
429+
-----END CERTIFICATE-----`)
430+
)

0 commit comments

Comments
 (0)