Skip to content

Commit 1500cd2

Browse files
cylicyli
committed
Add a boolean to the config that determines whether custom CA certs are added
to a system root cert pool or a new empty root cert pool. Signed-off-by: cyli <[email protected]>
1 parent 7da10c8 commit 1500cd2

File tree

2 files changed

+242
-15
lines changed

2 files changed

+242
-15
lines changed

‎tlsconfig/config.go

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ type Options struct {
2929
InsecureSkipVerify bool
3030
// server-only option
3131
ClientAuth tls.ClientAuthType
32+
33+
// If ExclusiveRootPools is set, then if a CA file is provided, the root pool used for TLS
34+
// creds will include exclusively the roots in that CA file. If no CA file is provided,
35+
// the system pool will be used.
36+
ExclusiveRootPools bool
3237
}
3338

3439
// Extra (server-side) accepted CBC cipher suites - will phase out in the future
@@ -66,11 +71,19 @@ func ClientDefault() *tls.Config {
6671
}
6772

6873
// certPool returns an X.509 certificate pool from `caFile`, the certificate file.
69-
func certPool(caFile string) (*x509.CertPool, error) {
74+
func certPool(caFile string, exclusivePool bool) (*x509.CertPool, error) {
7075
// If we should verify the server, we need to load a trusted ca
71-
certPool, err := SystemCertPool()
72-
if err != nil {
73-
return nil, fmt.Errorf("failed to read system certificates: %v", err)
76+
var (
77+
certPool *x509.CertPool
78+
err error
79+
)
80+
if exclusivePool {
81+
certPool = x509.NewCertPool()
82+
} else {
83+
certPool, err = SystemCertPool()
84+
if err != nil {
85+
return nil, fmt.Errorf("failed to read system certificates: %v", err)
86+
}
7487
}
7588
pem, err := ioutil.ReadFile(caFile)
7689
if err != nil {
@@ -88,7 +101,7 @@ func Client(options Options) (*tls.Config, error) {
88101
tlsConfig := ClientDefault()
89102
tlsConfig.InsecureSkipVerify = options.InsecureSkipVerify
90103
if !options.InsecureSkipVerify && options.CAFile != "" {
91-
CAs, err := certPool(options.CAFile)
104+
CAs, err := certPool(options.CAFile, options.ExclusiveRootPools)
92105
if err != nil {
93106
return nil, err
94107
}
@@ -119,7 +132,7 @@ func Server(options Options) (*tls.Config, error) {
119132
}
120133
tlsConfig.Certificates = []tls.Certificate{tlsCert}
121134
if options.ClientAuth >= tls.VerifyClientCertIfGiven && options.CAFile != "" {
122-
CAs, err := certPool(options.CAFile)
135+
CAs, err := certPool(options.CAFile, options.ExclusiveRootPools)
123136
if err != nil {
124137
return nil, err
125138
}

‎tlsconfig/config_test.go

Lines changed: 223 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,37 @@ import (
2121
"time"
2222
)
2323

24+
// This is the currently active LetsEncrypt IdenTrust cross-signed CA cert. It expires Mar 17, 2021.
25+
const systemRootTrustedCert = `
26+
-----BEGIN CERTIFICATE-----
27+
MIIEkjCCA3qgAwIBAgIQCgFBQgAAAVOFc2oLheynCDANBgkqhkiG9w0BAQsFADA/
28+
MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT
29+
DkRTVCBSb290IENBIFgzMB4XDTE2MDMxNzE2NDA0NloXDTIxMDMxNzE2NDA0Nlow
30+
SjELMAkGA1UEBhMCVVMxFjAUBgNVBAoTDUxldCdzIEVuY3J5cHQxIzAhBgNVBAMT
31+
GkxldCdzIEVuY3J5cHQgQXV0aG9yaXR5IFgzMIIBIjANBgkqhkiG9w0BAQEFAAOC
32+
AQ8AMIIBCgKCAQEAnNMM8FrlLke3cl03g7NoYzDq1zUmGSXhvb418XCSL7e4S0EF
33+
q6meNQhY7LEqxGiHC6PjdeTm86dicbp5gWAf15Gan/PQeGdxyGkOlZHP/uaZ6WA8
34+
SMx+yk13EiSdRxta67nsHjcAHJyse6cF6s5K671B5TaYucv9bTyWaN8jKkKQDIZ0
35+
Z8h/pZq4UmEUEz9l6YKHy9v6Dlb2honzhT+Xhq+w3Brvaw2VFn3EK6BlspkENnWA
36+
a6xK8xuQSXgvopZPKiAlKQTGdMDQMc2PMTiVFrqoM7hD8bEfwzB/onkxEz0tNvjj
37+
/PIzark5McWvxI0NHWQWM6r6hCm21AvA2H3DkwIDAQABo4IBfTCCAXkwEgYDVR0T
38+
AQH/BAgwBgEB/wIBADAOBgNVHQ8BAf8EBAMCAYYwfwYIKwYBBQUHAQEEczBxMDIG
39+
CCsGAQUFBzABhiZodHRwOi8vaXNyZy50cnVzdGlkLm9jc3AuaWRlbnRydXN0LmNv
40+
bTA7BggrBgEFBQcwAoYvaHR0cDovL2FwcHMuaWRlbnRydXN0LmNvbS9yb290cy9k
41+
c3Ryb290Y2F4My5wN2MwHwYDVR0jBBgwFoAUxKexpHsscfrb4UuQdf/EFWCFiRAw
42+
VAYDVR0gBE0wSzAIBgZngQwBAgEwPwYLKwYBBAGC3xMBAQEwMDAuBggrBgEFBQcC
43+
ARYiaHR0cDovL2Nwcy5yb290LXgxLmxldHNlbmNyeXB0Lm9yZzA8BgNVHR8ENTAz
44+
MDGgL6AthitodHRwOi8vY3JsLmlkZW50cnVzdC5jb20vRFNUUk9PVENBWDNDUkwu
45+
Y3JsMB0GA1UdDgQWBBSoSmpjBH3duubRObemRWXv86jsoTANBgkqhkiG9w0BAQsF
46+
AAOCAQEA3TPXEfNjWDjdGBX7CVW+dla5cEilaUcne8IkCJLxWh9KEik3JHRRHGJo
47+
uM2VcGfl96S8TihRzZvoroed6ti6WqEBmtzw3Wodatg+VyOeph4EYpr/1wXKtx8/
48+
wApIvJSwtmVi4MFU5aMqrSDE6ea73Mj2tcMyo5jMd6jmeWUHK8so/joWUoHOUgwu
49+
X4Po1QYz+3dszkDqMp4fklxBwXRsW10KXzPMTZ+sOPAveyxindmjkW8lGy+QsRlG
50+
PfZ+G6Z6h7mjem0Y+iWlkYcV4PIWL1iwBi8saCbGS5jN2p8M+X+Q7UNKEkROb3N6
51+
KOqkqm57TH2H3eDJAkSnh6/DNFu0Qg==
52+
-----END CERTIFICATE-----
53+
`
54+
2455
var certTemplate = x509.Certificate{
2556
SerialNumber: big.NewInt(199999),
2657
Subject: pkix.Name{
@@ -30,13 +61,19 @@ var certTemplate = x509.Certificate{
3061
NotAfter: time.Now().AddDate(1, 1, 1),
3162

3263
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
33-
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageCodeSigning},
64+
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageAny},
3465

3566
BasicConstraintsValid: true,
3667
}
3768

38-
func generateCertificate(t *testing.T, signer crypto.Signer, out io.Writer) {
39-
derBytes, err := x509.CreateCertificate(rand.Reader, &certTemplate, &certTemplate, signer.Public(), signer)
69+
func generateCertificate(t *testing.T, signer crypto.Signer, out io.Writer, isCA bool) {
70+
template := certTemplate
71+
template.IsCA = isCA
72+
if isCA {
73+
template.KeyUsage = template.KeyUsage | x509.KeyUsageCertSign
74+
template.MaxPathLen = 1
75+
}
76+
derBytes, err := x509.CreateCertificate(rand.Reader, &template, &certTemplate, signer.Public(), signer)
4077
if err != nil {
4178
t.Fatal("Unable to generate a certificate", err.Error())
4279
}
@@ -46,7 +83,7 @@ func generateCertificate(t *testing.T, signer crypto.Signer, out io.Writer) {
4683
}
4784
}
4885

49-
// generates a multiple-certificate file with both RSA and ECDSA certs and
86+
// generates a multiple-certificate CA file with both RSA and ECDSA certs and
5087
// returns the filename so that cleanup can be deferred.
5188
func generateMultiCert(t *testing.T, tempDir string) string {
5289
certOut, err := os.Create(filepath.Join(tempDir, "multi"))
@@ -65,7 +102,7 @@ func generateMultiCert(t *testing.T, tempDir string) string {
65102
}
66103

67104
for _, signer := range []crypto.Signer{rsaKey, ecKey} {
68-
generateCertificate(t, signer, certOut)
105+
generateCertificate(t, signer, certOut, true)
69106
}
70107

71108
return certOut.Name()
@@ -96,7 +133,7 @@ func generateCertAndKey(t *testing.T, tempDir string) (string, string) {
96133
}
97134
defer certOut.Close()
98135

99-
generateCertificate(t, rsaKey, certOut)
136+
generateCertificate(t, rsaKey, certOut, false)
100137

101138
return keyOut.Name(), certOut.Name()
102139
}
@@ -240,11 +277,106 @@ func TestConfigServerTLSClientCASet(t *testing.T) {
240277
if tlsConfig.ClientAuth != tls.VerifyClientCertIfGiven {
241278
t.Fatal("ClientAuth was not set to what was in the options")
242279
}
243-
if tlsConfig.ClientCAs == nil || len(tlsConfig.ClientCAs.Subjects()) != 2 {
280+
basePool, err := SystemCertPool()
281+
if err != nil {
282+
basePool = x509.NewCertPool()
283+
}
284+
// because we are not enabling `ExclusiveRootPools`, any root pool will also contain the system roots
285+
if tlsConfig.ClientCAs == nil || len(tlsConfig.ClientCAs.Subjects()) != len(basePool.Subjects())+2 {
244286
t.Fatalf("Client CAs were never set correctly")
245287
}
246288
}
247289

290+
// Exclusive root pools determines whether the CA pool will be a union of the system
291+
// certificate pool and custom certs, or an exclusive or of the custom certs and system pool
292+
func TestConfigServerExclusiveRootPools(t *testing.T) {
293+
tempDir := makeTempDir(t)
294+
defer os.RemoveAll(tempDir)
295+
key, cert := generateCertAndKey(t, tempDir)
296+
ca := generateMultiCert(t, tempDir)
297+
298+
caBytes, err := ioutil.ReadFile(ca)
299+
if err != nil {
300+
t.Fatal("Unable to read CA certs", err)
301+
}
302+
303+
var testCerts []*x509.Certificate
304+
for _, pemBytes := range [][]byte{caBytes, []byte(systemRootTrustedCert)} {
305+
pemBlock, _ := pem.Decode(pemBytes)
306+
if pemBlock == nil {
307+
t.Fatal("Malformed certificate")
308+
}
309+
cert, err := x509.ParseCertificate(pemBlock.Bytes)
310+
if err != nil {
311+
t.Fatal("Unable to parse certificate")
312+
}
313+
testCerts = append(testCerts, cert)
314+
}
315+
316+
// ExclusiveRootPools not set, so should be able to verify both system-signed certs
317+
// and custom CA-signed certs
318+
tlsConfig, err := Server(Options{
319+
CertFile: cert,
320+
KeyFile: key,
321+
ClientAuth: tls.VerifyClientCertIfGiven,
322+
CAFile: ca,
323+
})
324+
325+
if err != nil || tlsConfig == nil {
326+
t.Fatal("Unable to configure server TLS", err)
327+
}
328+
329+
for i, cert := range testCerts {
330+
if _, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.ClientCAs}); err != nil {
331+
t.Fatalf("Unable to verify certificate %d: %v", i, err)
332+
}
333+
}
334+
335+
// ExclusiveRootPools set and custom CA provided, so system certs should not be verifiable
336+
// and custom CA-signed certs should be verifiable
337+
tlsConfig, err = Server(Options{
338+
CertFile: cert,
339+
KeyFile: key,
340+
ClientAuth: tls.VerifyClientCertIfGiven,
341+
CAFile: ca,
342+
ExclusiveRootPools: true,
343+
})
344+
345+
if err != nil || tlsConfig == nil {
346+
t.Fatal("Unable to configure server TLS", err)
347+
}
348+
349+
for i, cert := range testCerts {
350+
_, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.ClientCAs})
351+
switch {
352+
case i == 0 && err != nil:
353+
t.Fatal("Unable to verify custom certificate, even though the root pool should have only the custom CA", err)
354+
case i == 1 && err == nil:
355+
t.Fatal("Successfully verified system root-signed certificate though the root pool should have only the cusotm CA", err)
356+
}
357+
}
358+
359+
// No CA file provided, system cert should be verifiable only
360+
tlsConfig, err = Server(Options{
361+
CertFile: cert,
362+
KeyFile: key,
363+
})
364+
365+
if err != nil || tlsConfig == nil {
366+
t.Fatal("Unable to configure server TLS", err)
367+
}
368+
369+
for i, cert := range testCerts {
370+
_, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.ClientCAs})
371+
switch {
372+
case i == 1 && err != nil:
373+
t.Fatal("Unable to verify system root-signed certificate, even though the root pool should be the system pool only", err)
374+
case i == 0 && err == nil:
375+
t.Fatal("Successfully verified custom certificate though the root pool should be the system pool only", err)
376+
}
377+
}
378+
}
379+
248380
// The root CA is never set if InsecureSkipBoolean is set to true, but the
249381
// default client options are set
250382
func TestConfigClientTLSNoVerify(t *testing.T) {
@@ -310,8 +442,12 @@ func TestConfigClientTLSRootCAFileWithOneCert(t *testing.T) {
310442
if err != nil || tlsConfig == nil {
311443
t.Fatal("Unable to configure client TLS", err)
312444
}
313-
314-
if tlsConfig.RootCAs == nil || len(tlsConfig.RootCAs.Subjects()) != 2 {
445+
basePool, err := SystemCertPool()
446+
if err != nil {
447+
basePool = x509.NewCertPool()
448+
}
449+
// because we are not enabling `ExclusiveRootPools`, any root pool will also contain the system roots
450+
if tlsConfig.RootCAs == nil || len(tlsConfig.RootCAs.Subjects()) != len(basePool.Subjects())+2 {
315451
t.Fatal("Root CAs not set properly", err)
316452
}
317453
if tlsConfig.Certificates != nil {
@@ -389,3 +525,81 @@ func TestConfigClientTLSValidClientCertAndKey(t *testing.T) {
389525
t.Fatal("Root CAs should not have been set", err)
390526
}
391527
}
528+
529+
// Exclusive root pools determines whether the CA pool will be a union of the system
530+
// certificate pool and custom certs, or an exclusive or of the custom certs and system pool
531+
func TestConfigClientExclusiveRootPools(t *testing.T) {
532+
tempDir := makeTempDir(t)
533+
defer os.RemoveAll(tempDir)
534+
ca := generateMultiCert(t, tempDir)
535+
536+
caBytes, err := ioutil.ReadFile(ca)
537+
if err != nil {
538+
t.Fatal("Unable to read CA certs", err)
539+
}
540+
541+
var testCerts []*x509.Certificate
542+
for _, pemBytes := range [][]byte{caBytes, []byte(systemRootTrustedCert)} {
543+
pemBlock, _ := pem.Decode(pemBytes)
544+
if pemBlock == nil {
545+
t.Fatal("Malformed certificate")
546+
}
547+
cert, err := x509.ParseCertificate(pemBlock.Bytes)
548+
if err != nil {
549+
t.Fatal("Unable to parse certificate")
550+
}
551+
testCerts = append(testCerts, cert)
552+
}
553+
554+
// ExclusiveRootPools not set, so should be able to verify both system-signed certs
555+
// and custom CA-signed certs
556+
tlsConfig, err := Client(Options{CAFile: ca})
557+
558+
if err != nil || tlsConfig == nil {
559+
t.Fatal("Unable to configure client TLS", err)
560+
}
561+
562+
for i, cert := range testCerts {
563+
if _, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.RootCAs}); err != nil {
564+
t.Fatalf("Unable to verify certificate %d: %v", i, err)
565+
}
566+
}
567+
568+
// ExclusiveRootPools set and custom CA provided, so system certs should not be verifiable
569+
// and custom CA-signed certs should be verifiable
570+
tlsConfig, err = Client(Options{
571+
CAFile: ca,
572+
ExclusiveRootPools: true,
573+
})
574+
575+
if err != nil || tlsConfig == nil {
576+
t.Fatal("Unable to configure client TLS", err)
577+
}
578+
579+
for i, cert := range testCerts {
580+
_, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.RootCAs})
581+
switch {
582+
case i == 0 && err != nil:
583+
t.Fatal("Unable to verify custom certificate, even though the root pool should have only the custom CA", err)
584+
case i == 1 && err == nil:
585+
t.Fatal("Successfully verified system root-signed certificate though the root pool should have only the cusotm CA", err)
586+
}
587+
}
588+
589+
// No CA file provided, system cert should be verifiable only
590+
tlsConfig, err = Client(Options{})
591+
592+
if err != nil || tlsConfig == nil {
593+
t.Fatal("Unable to configure client TLS", err)
594+
}
595+
596+
for i, cert := range testCerts {
597+
_, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.RootCAs})
598+
switch {
599+
case i == 1 && err != nil:
600+
t.Fatal("Unable to verify system root-signed certificate, even though the root pool should be the system pool only", err)
601+
case i == 0 && err == nil:
602+
t.Fatal("Successfully verified custom certificate though the root pool should be the system pool only", err)
603+
}
604+
}
605+
}

0 commit comments

Comments
 (0)