Skip to content

Commit dd7d197

Browse files
author
Mengqi Yu
committed
validate cert
1 parent 6865d0a commit dd7d197

File tree

4 files changed

+137
-71
lines changed

4 files changed

+137
-71
lines changed

‎pkg/admission/cert/writer/certwriter.go

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@ package writer
1919
import (
2020
"bytes"
2121
"crypto/tls"
22+
"crypto/x509"
23+
"encoding/pem"
2224
"errors"
2325
"fmt"
2426
"log"
2527
"net/url"
28+
"time"
2629

2730
admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
2831
"k8s.io/apimachinery/pkg/runtime"
@@ -39,6 +42,8 @@ const (
3942
ServerCertName = "cert.pem"
4043
)
4144

45+
const sixMonths = 6 * 31 * 24 * time.Hour
46+
4247
// CertWriter provides method to handle webhooks.
4348
type CertWriter interface {
4449
// EnsureCert ensures that the webhooks have proper certificates.
@@ -91,8 +96,13 @@ func handleCommon(webhook *admissionregistrationv1beta1.Webhook, ch certReadWrit
9196
return err
9297
}
9398

99+
dnsName, err := dnsNameForWebhook(&webhook.ClientConfig)
100+
if err != nil {
101+
return err
102+
}
94103
// Recreate the cert if it's invalid.
95-
if !validCert(certs) {
104+
valid := validCert(certs, dnsName)
105+
if !valid {
96106
log.Printf("cert is invalid or expiring, regenerating a new one")
97107
certs, err = ch.overwrite(webhook.Name)
98108
if err != nil {
@@ -141,18 +151,36 @@ type certReadWriter interface {
141151
overwrite(webhookName string) (*generator.Artifacts, error)
142152
}
143153

144-
func validCert(certs *generator.Artifacts) bool {
145-
// TODO:
146-
// 1) validate the key and the cert are valid pair e.g. call crypto/tls.X509KeyPair()
147-
// 2) validate the cert with the CA cert
148-
// 3) validate the cert is for a certain DNSName
149-
// e.g.
150-
// c, err := tls.X509KeyPair(cert, key)
151-
// err := c.Verify(options)
154+
func validCert(certs *generator.Artifacts, dnsName string) bool {
152155
if certs == nil {
153156
return false
154157
}
158+
159+
// Verify key and cert are valid pair
155160
_, err := tls.X509KeyPair(certs.Cert, certs.Key)
161+
if err != nil {
162+
return false
163+
}
164+
165+
// Verify cert is good for desired DNS name and signed by CA and will be valid for desired period of time.
166+
pool := x509.NewCertPool()
167+
if !pool.AppendCertsFromPEM(certs.CACert) {
168+
return false
169+
}
170+
block, _ := pem.Decode([]byte(certs.Cert))
171+
if block == nil {
172+
return false
173+
}
174+
cert, err := x509.ParseCertificate(block.Bytes)
175+
if err != nil {
176+
return false
177+
}
178+
ops := x509.VerifyOptions{
179+
DNSName: dnsName,
180+
Roots: pool,
181+
CurrentTime: time.Now().Add(sixMonths),
182+
}
183+
_, err = cert.Verify(ops)
156184
return err == nil
157185
}
158186

‎pkg/admission/cert/writer/certwriter_test.go

Lines changed: 70 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ import (
3131
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3232
)
3333

34+
var certs1, certs2 *generator.Artifacts
35+
36+
func init() {
37+
cn1 := "example.com"
38+
cn2 := "test-service.test-svc-namespace.svc"
39+
cp := generator.SelfSignedCertGenerator{}
40+
certs1, _ = cp.Generate(cn1)
41+
certs2, _ = cp.Generate(cn2)
42+
}
43+
3444
var _ = Describe("NewProvider", func() {
3545
var cl client.Client
3646
var ops Options
@@ -73,33 +83,6 @@ var _ = Describe("NewProvider", func() {
7383

7484
})
7585

76-
var certPEM = `-----BEGIN CERTIFICATE-----
77-
MIICRzCCAfGgAwIBAgIJALMb7ecMIk3MMA0GCSqGSIb3DQEBCwUAMH4xCzAJBgNV
78-
BAYTAkdCMQ8wDQYDVQQIDAZMb25kb24xDzANBgNVBAcMBkxvbmRvbjEYMBYGA1UE
79-
CgwPR2xvYmFsIFNlY3VyaXR5MRYwFAYDVQQLDA1JVCBEZXBhcnRtZW50MRswGQYD
80-
VQQDDBJ0ZXN0LWNlcnRpZmljYXRlLTAwIBcNMTcwNDI2MjMyNjUyWhgPMjExNzA0
81-
MDIyMzI2NTJaMH4xCzAJBgNVBAYTAkdCMQ8wDQYDVQQIDAZMb25kb24xDzANBgNV
82-
BAcMBkxvbmRvbjEYMBYGA1UECgwPR2xvYmFsIFNlY3VyaXR5MRYwFAYDVQQLDA1J
83-
VCBEZXBhcnRtZW50MRswGQYDVQQDDBJ0ZXN0LWNlcnRpZmljYXRlLTAwXDANBgkq
84-
hkiG9w0BAQEFAANLADBIAkEAtBMa7NWpv3BVlKTCPGO/LEsguKqWHBtKzweMY2CV
85-
tAL1rQm913huhxF9w+ai76KQ3MHK5IVnLJjYYA5MzP2H5QIDAQABo1AwTjAdBgNV
86-
HQ4EFgQU22iy8aWkNSxv0nBxFxerfsvnZVMwHwYDVR0jBBgwFoAU22iy8aWkNSxv
87-
0nBxFxerfsvnZVMwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAANBAEOefGbV
88-
NcHxklaW06w6OBYJPwpIhCVozC1qdxGX1dg8VkEKzjOzjgqVD30m59OFmSlBmHsl
89-
nkVA6wyOSDYBf3o=
90-
-----END CERTIFICATE-----`
91-
92-
var keyPEM = `-----BEGIN RSA PRIVATE KEY-----
93-
MIIBUwIBADANBgkqhkiG9w0BAQEFAASCAT0wggE5AgEAAkEAtBMa7NWpv3BVlKTC
94-
PGO/LEsguKqWHBtKzweMY2CVtAL1rQm913huhxF9w+ai76KQ3MHK5IVnLJjYYA5M
95-
zP2H5QIDAQABAkAS9BfXab3OKpK3bIgNNyp+DQJKrZnTJ4Q+OjsqkpXvNltPJosf
96-
G8GsiKu/vAt4HGqI3eU77NvRI+mL4MnHRmXBAiEA3qM4FAtKSRBbcJzPxxLEUSwg
97-
XSCcosCktbkXvpYrS30CIQDPDxgqlwDEJQ0uKuHkZI38/SPWWqfUmkecwlbpXABK
98-
iQIgZX08DA8VfvcA5/Xj1Zjdey9FVY6POLXen6RPiabE97UCICp6eUW7ht+2jjar
99-
e35EltCRCjoejRHTuN9TC0uCoVipAiAXaJIx/Q47vGwiw6Y8KXsNU6y54gTbOSxX
100-
54LzHNk/+Q==
101-
-----END RSA PRIVATE KEY-----`
102-
10386
type fakeCertReadWriter struct {
10487
numReadCalled int
10588
readCertAndErr []certAndErr
@@ -154,11 +137,16 @@ var _ = Describe("handleCommon", func() {
154137
var invalidCert *generator.Artifacts
155138

156139
BeforeEach(func(done Done) {
157-
webhook = &admissionregistration.Webhook{}
140+
url := "https://example.com/admission"
141+
webhook = &admissionregistration.Webhook{
142+
ClientConfig: admissionregistration.WebhookClientConfig{
143+
URL: &url,
144+
},
145+
}
158146
cert = &generator.Artifacts{
159-
CACert: []byte(`CACertBytes`),
160-
Cert: []byte(certPEM),
161-
Key: []byte(keyPEM),
147+
CACert: []byte(certs1.CACert),
148+
Cert: []byte(certs1.Cert),
149+
Key: []byte(certs1.Key),
162150
}
163151
invalidCert = &generator.Artifacts{
164152
CACert: []byte(`CACertBytes`),
@@ -188,7 +176,11 @@ var _ = Describe("handleCommon", func() {
188176
certrw := &fakeCertReadWriter{
189177
readCertAndErr: []certAndErr{
190178
{
191-
err: notFoundError{errors.NewNotFound(schema.GroupResource{}, "foo")},
179+
err: notFoundError{errors.NewNotFound(schema.GroupResource{}, "foo")},
180+
},
181+
},
182+
writeCertAndErr: []certAndErr{
183+
{
192184
cert: cert,
193185
},
194186
},
@@ -198,6 +190,7 @@ var _ = Describe("handleCommon", func() {
198190
Expect(err).NotTo(HaveOccurred())
199191
Expect(certrw.numReadCalled).To(Equal(1))
200192
Expect(certrw.numWriteCalled).To(Equal(1))
193+
Expect(certrw.numOverwriteCalled).To(Equal(0))
201194
})
202195

203196
It("should return the error on failed write", func() {
@@ -218,6 +211,7 @@ var _ = Describe("handleCommon", func() {
218211
Expect(err).To(MatchError(goerrors.New("failed to write")))
219212
Expect(certrw.numReadCalled).To(Equal(1))
220213
Expect(certrw.numWriteCalled).To(Equal(1))
214+
Expect(certrw.numOverwriteCalled).To(Equal(0))
221215
})
222216
})
223217

@@ -234,6 +228,8 @@ var _ = Describe("handleCommon", func() {
234228
err := handleCommon(webhook, certrw)
235229
Expect(err).NotTo(HaveOccurred())
236230
Expect(certrw.numReadCalled).To(Equal(1))
231+
Expect(certrw.numWriteCalled).To(Equal(0))
232+
Expect(certrw.numOverwriteCalled).To(Equal(0))
237233
})
238234

239235
It("should return the error on failed read", func() {
@@ -248,6 +244,8 @@ var _ = Describe("handleCommon", func() {
248244
err := handleCommon(webhook, certrw)
249245
Expect(err).To(MatchError(goerrors.New("failed to read")))
250246
Expect(certrw.numReadCalled).To(Equal(1))
247+
Expect(certrw.numWriteCalled).To(Equal(0))
248+
Expect(certrw.numOverwriteCalled).To(Equal(0))
251249
})
252250
})
253251

@@ -269,6 +267,7 @@ var _ = Describe("handleCommon", func() {
269267
err := handleCommon(webhook, certrw)
270268
Expect(err).NotTo(HaveOccurred())
271269
Expect(certrw.numReadCalled).To(Equal(1))
270+
Expect(certrw.numWriteCalled).To(Equal(0))
272271
Expect(certrw.numOverwriteCalled).To(Equal(1))
273272
})
274273

@@ -289,6 +288,7 @@ var _ = Describe("handleCommon", func() {
289288
err := handleCommon(webhook, certrw)
290289
Expect(err).NotTo(HaveOccurred())
291290
Expect(certrw.numReadCalled).To(Equal(1))
291+
Expect(certrw.numWriteCalled).To(Equal(0))
292292
Expect(certrw.numOverwriteCalled).To(Equal(1))
293293
})
294294

@@ -413,3 +413,41 @@ var _ = Describe("dnsNameForWebhook", func() {
413413
})
414414
})
415415
})
416+
417+
var _ = Describe("validate cert", func() {
418+
Context("invalid pair", func() {
419+
It("should detect it", func() {
420+
certs := generator.Artifacts{
421+
CACert: certs1.CACert,
422+
Cert: certs1.Cert,
423+
Key: certs2.Key,
424+
}
425+
valid := validCert(&certs, "example.com")
426+
Expect(valid).To(BeFalse())
427+
})
428+
})
429+
430+
Context("CA not matching", func() {
431+
It("should detect it", func() {
432+
certs := generator.Artifacts{
433+
CACert: certs2.CACert,
434+
Cert: certs1.Cert,
435+
Key: certs1.Key,
436+
}
437+
valid := validCert(&certs, "example.com")
438+
Expect(valid).To(BeFalse())
439+
})
440+
})
441+
442+
Context("DNS name not matching", func() {
443+
It("should detect it", func() {
444+
certs := generator.Artifacts{
445+
CACert: certs1.CACert,
446+
Cert: certs1.Cert,
447+
Key: certs1.Key,
448+
}
449+
valid := validCert(&certs, "foo.com")
450+
Expect(valid).To(BeFalse())
451+
})
452+
})
453+
})

‎pkg/admission/cert/writer/fs_test.go

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,9 @@ var _ = Describe("FSCertWriter", func() {
152152
CertGenerator: &fakegenerator.CertGenerator{
153153
DNSNameToCertArtifacts: map[string]*generator.Artifacts{
154154
"test-service.test-svc-namespace.svc": {
155-
CACert: []byte(`CACertBytes`),
156-
Cert: []byte(certPEM),
157-
Key: []byte(keyPEM),
155+
CACert: []byte(certs2.CACert),
156+
Cert: []byte(certs2.Cert),
157+
Key: []byte(certs2.Key),
158158
},
159159
},
160160
},
@@ -187,13 +187,13 @@ var _ = Describe("FSCertWriter", func() {
187187
Expect(err).NotTo(HaveOccurred())
188188
caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName))
189189
Expect(err).NotTo(HaveOccurred())
190-
Expect(caBytes).To(Equal([]byte(`CACertBytes`)))
190+
Expect(caBytes).To(Equal([]byte(certs2.CACert)))
191191
certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName))
192192
Expect(err).NotTo(HaveOccurred())
193-
Expect(certBytes).To(Equal([]byte(certPEM)))
193+
Expect(certBytes).To(Equal([]byte(certs2.Cert)))
194194
keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName))
195195
Expect(err).NotTo(HaveOccurred())
196-
Expect(keyBytes).To(Equal([]byte(keyPEM)))
196+
Expect(keyBytes).To(Equal([]byte(certs2.Key)))
197197
})
198198
})
199199

@@ -212,13 +212,13 @@ var _ = Describe("FSCertWriter", func() {
212212
Expect(err).NotTo(HaveOccurred())
213213
caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName))
214214
Expect(err).NotTo(HaveOccurred())
215-
Expect(caBytes).To(Equal([]byte(`CACertBytes`)))
215+
Expect(caBytes).To(Equal([]byte(certs2.CACert)))
216216
certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName))
217217
Expect(err).NotTo(HaveOccurred())
218-
Expect(certBytes).To(Equal([]byte(certPEM)))
218+
Expect(certBytes).To(Equal([]byte(certs2.Cert)))
219219
keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName))
220220
Expect(err).NotTo(HaveOccurred())
221-
Expect(keyBytes).To(Equal([]byte(keyPEM)))
221+
Expect(keyBytes).To(Equal([]byte(certs2.Key)))
222222
})
223223
})
224224

@@ -241,13 +241,13 @@ var _ = Describe("FSCertWriter", func() {
241241
Expect(err).NotTo(HaveOccurred())
242242
caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName))
243243
Expect(err).NotTo(HaveOccurred())
244-
Expect(caBytes).To(Equal([]byte(`CACertBytes`)))
244+
Expect(caBytes).To(Equal([]byte(certs2.CACert)))
245245
certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName))
246246
Expect(err).NotTo(HaveOccurred())
247-
Expect(certBytes).To(Equal([]byte(certPEM)))
247+
Expect(certBytes).To(Equal([]byte(certs2.Cert)))
248248
keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName))
249249
Expect(err).NotTo(HaveOccurred())
250-
Expect(keyBytes).To(Equal([]byte(keyPEM)))
250+
Expect(keyBytes).To(Equal([]byte(certs2.Key)))
251251
})
252252
})
253253
})
@@ -266,13 +266,13 @@ var _ = Describe("FSCertWriter", func() {
266266
Expect(err).NotTo(HaveOccurred())
267267
caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName))
268268
Expect(err).NotTo(HaveOccurred())
269-
Expect(caBytes).To(Equal([]byte(`CACertBytes`)))
269+
Expect(caBytes).To(Equal([]byte(certs2.CACert)))
270270
certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName))
271271
Expect(err).NotTo(HaveOccurred())
272-
Expect(certBytes).To(Equal([]byte(certPEM)))
272+
Expect(certBytes).To(Equal([]byte(certs2.Cert)))
273273
keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName))
274274
Expect(err).NotTo(HaveOccurred())
275-
Expect(keyBytes).To(Equal([]byte(keyPEM)))
275+
Expect(keyBytes).To(Equal([]byte(certs2.Key)))
276276
})
277277
})
278278

@@ -301,13 +301,13 @@ var _ = Describe("FSCertWriter", func() {
301301
Expect(err).NotTo(HaveOccurred())
302302
caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName))
303303
Expect(err).NotTo(HaveOccurred())
304-
Expect(caBytes).To(Equal([]byte(`CACertBytes`)))
304+
Expect(caBytes).To(Equal([]byte(certs2.CACert)))
305305
certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName))
306306
Expect(err).NotTo(HaveOccurred())
307-
Expect(certBytes).To(Equal([]byte(certPEM)))
307+
Expect(certBytes).To(Equal([]byte(certs2.Cert)))
308308
keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName))
309309
Expect(err).NotTo(HaveOccurred())
310-
Expect(keyBytes).To(Equal([]byte(keyPEM)))
310+
Expect(keyBytes).To(Equal([]byte(certs2.Key)))
311311
})
312312
})
313313
})
@@ -317,11 +317,11 @@ var _ = Describe("FSCertWriter", func() {
317317
Context("cert is valid", func() {
318318
Context("when not expiring", func() {
319319
BeforeEach(func(done Done) {
320-
err := ioutil.WriteFile(path.Join(testingDir, CACertName), []byte(`oldCACertBytes`), 0600)
320+
err := ioutil.WriteFile(path.Join(testingDir, CACertName), []byte(certs2.CACert), 0600)
321321
Expect(err).NotTo(HaveOccurred())
322-
err = ioutil.WriteFile(path.Join(testingDir, ServerCertName), []byte(certPEM), 0600)
322+
err = ioutil.WriteFile(path.Join(testingDir, ServerCertName), []byte(certs2.Cert), 0600)
323323
Expect(err).NotTo(HaveOccurred())
324-
err = ioutil.WriteFile(path.Join(testingDir, ServerKeyName), []byte(keyPEM), 0600)
324+
err = ioutil.WriteFile(path.Join(testingDir, ServerKeyName), []byte(certs2.Key), 0600)
325325
Expect(err).NotTo(HaveOccurred())
326326
close(done)
327327
})
@@ -330,13 +330,13 @@ var _ = Describe("FSCertWriter", func() {
330330
Expect(err).NotTo(HaveOccurred())
331331
caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName))
332332
Expect(err).NotTo(HaveOccurred())
333-
Expect(caBytes).To(Equal([]byte(`oldCACertBytes`)))
333+
Expect(caBytes).To(Equal([]byte(certs2.CACert)))
334334
certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName))
335335
Expect(err).NotTo(HaveOccurred())
336-
Expect(certBytes).To(Equal([]byte(certPEM)))
336+
Expect(certBytes).To(Equal([]byte(certs2.Cert)))
337337
keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName))
338338
Expect(err).NotTo(HaveOccurred())
339-
Expect(keyBytes).To(Equal([]byte(keyPEM)))
339+
Expect(keyBytes).To(Equal([]byte(certs2.Key)))
340340
})
341341
})
342342

0 commit comments

Comments
 (0)