Skip to content

Commit b115e42

Browse files
vvolandvvoland
authored
Merge pull request docker#154 from thaJeztah/fix_non_linux_tests
tlsconfig: make root pool tests deterministic across platforms
2 parents 894d811 + b4454a6 commit b115e42

File tree

3 files changed

+233
-108
lines changed

3 files changed

+233
-108
lines changed

‎tlsconfig/config.go‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ type Options struct {
3434
// the system pool will be used.
3535
ExclusiveRootPools bool
3636
MinVersion uint16
37+
38+
// systemCertPool allows mocking the system cert-pool for testing.
39+
systemCertPool func() (*x509.CertPool, error)
3740
}
3841

3942
// DefaultServerAcceptedCiphers should be uses by code which already has a crypto/tls
@@ -77,26 +80,33 @@ func defaultConfig(ops ...func(*tls.Config)) *tls.Config {
7780
}
7881

7982
// certPool returns an X.509 certificate pool from `caFile`, the certificate file.
80-
func certPool(caFile string, exclusivePool bool) (*x509.CertPool, error) {
83+
func certPool(opts Options) (*x509.CertPool, error) {
8184
// If we should verify the server, we need to load a trusted ca
8285
var (
8386
pool *x509.CertPool
8487
err error
8588
)
86-
if exclusivePool {
89+
if opts.ExclusiveRootPools {
8790
pool = x509.NewCertPool()
8891
} else {
89-
pool, err = x509.SystemCertPool()
92+
if opts.systemCertPool != nil {
93+
pool, err = opts.systemCertPool()
94+
} else {
95+
pool, err = x509.SystemCertPool()
96+
}
9097
if err != nil {
9198
return nil, fmt.Errorf("failed to read system certificates: %v", err)
9299
}
93100
}
94-
pemData, err := os.ReadFile(caFile)
101+
if opts.CAFile == "" {
102+
return pool, nil
103+
}
104+
pemData, err := os.ReadFile(opts.CAFile)
95105
if err != nil {
96-
return nil, fmt.Errorf("could not read CA certificate %q: %v", caFile, err)
106+
return nil, fmt.Errorf("could not read CA certificate %q: %v", opts.CAFile, err)
97107
}
98108
if !pool.AppendCertsFromPEM(pemData) {
99-
return nil, fmt.Errorf("failed to append certificates from PEM file: %q", caFile)
109+
return nil, fmt.Errorf("failed to append certificates from PEM file: %q", opts.CAFile)
100110
}
101111
return pool, nil
102112
}
@@ -199,7 +209,7 @@ func Client(options Options) (*tls.Config, error) {
199209
tlsConfig := defaultConfig()
200210
tlsConfig.InsecureSkipVerify = options.InsecureSkipVerify
201211
if !options.InsecureSkipVerify && options.CAFile != "" {
202-
CAs, err := certPool(options.CAFile, options.ExclusiveRootPools)
212+
CAs, err := certPool(options)
203213
if err != nil {
204214
return nil, err
205215
}
@@ -232,7 +242,7 @@ func Server(options Options) (*tls.Config, error) {
232242
}
233243
tlsConfig.Certificates = []tls.Certificate{tlsCert}
234244
if options.ClientAuth >= tls.VerifyClientCertIfGiven && options.CAFile != "" {
235-
CAs, err := certPool(options.CAFile, options.ExclusiveRootPools)
245+
CAs, err := certPool(options)
236246
if err != nil {
237247
return nil, err
238248
}

‎tlsconfig/config_test.go‎

Lines changed: 69 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -4,41 +4,14 @@ import (
44
"bytes"
55
"crypto/tls"
66
"crypto/x509"
7-
"encoding/pem"
87
"errors"
98
"os"
9+
"path/filepath"
1010
"reflect"
11-
"runtime"
1211
"testing"
1312
)
1413

15-
// This is the currently active Amazon Root CA 1 (CN=Amazon Root CA 1,O=Amazon,C=US),
16-
// downloaded from: https://www.amazontrust.com/repository/AmazonRootCA1.pem
17-
// It's valid since May 26 00:00:00 2015 GMT and expires on Jan 17 00:00:00 2038 GMT.
18-
// Download updated versions from https://www.amazontrust.com/repository/
1914
const (
20-
systemRootTrustedCert = `
21-
-----BEGIN CERTIFICATE-----
22-
MIIDQTCCAimgAwIBAgITBmyfz5m/jAo54vB4ikPmljZbyjANBgkqhkiG9w0BAQsF
23-
ADA5MQswCQYDVQQGEwJVUzEPMA0GA1UEChMGQW1hem9uMRkwFwYDVQQDExBBbWF6
24-
b24gUm9vdCBDQSAxMB4XDTE1MDUyNjAwMDAwMFoXDTM4MDExNzAwMDAwMFowOTEL
25-
MAkGA1UEBhMCVVMxDzANBgNVBAoTBkFtYXpvbjEZMBcGA1UEAxMQQW1hem9uIFJv
26-
b3QgQ0EgMTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALJ4gHHKeNXj
27-
ca9HgFB0fW7Y14h29Jlo91ghYPl0hAEvrAIthtOgQ3pOsqTQNroBvo3bSMgHFzZM
28-
9O6II8c+6zf1tRn4SWiw3te5djgdYZ6k/oI2peVKVuRF4fn9tBb6dNqcmzU5L/qw
29-
IFAGbHrQgLKm+a/sRxmPUDgH3KKHOVj4utWp+UhnMJbulHheb4mjUcAwhmahRWa6
30-
VOujw5H5SNz/0egwLX0tdHA114gk957EWW67c4cX8jJGKLhD+rcdqsq08p8kDi1L
31-
93FcXmn/6pUCyziKrlA4b9v7LWIbxcceVOF34GfID5yHI9Y/QCB/IIDEgEw+OyQm
32-
jgSubJrIqg0CAwEAAaNCMEAwDwYDVR0TAQH/BAUwAwEB/zAOBgNVHQ8BAf8EBAMC
33-
AYYwHQYDVR0OBBYEFIQYzIU07LwMlJQuCFmcx7IQTgoIMA0GCSqGSIb3DQEBCwUA
34-
A4IBAQCY8jdaQZChGsV2USggNiMOruYou6r4lK5IpDB/G/wkjUu0yKGX9rbxenDI
35-
U5PMCCjjmCXPI6T53iHTfIUJrU6adTrCC2qJeHZERxhlbI1Bjjt/msv0tadQ1wUs
36-
N+gDS63pYaACbvXy8MWy7Vu33PqUXHeeE6V/Uq2V8viTO96LXFvKWlJbYK8U90vv
37-
o/ufQJVtMVT8QtPHRh8jrdkPSHCa2XV4cdFyQzR1bldZwgJcJmApzyMZFo6IQ6XU
38-
5MsI+yMRQ+hDKXJioaldXgjUkK642M4UwtBV8ob2xJNDd2ZhwLnoQdeXeGADbkpy
39-
rqXRfboQnoZsG4q5WTP468SQvvG5
40-
-----END CERTIFICATE-----
41-
`
4215
rsaPrivateKeyFile = "fixtures/key.pem"
4316
certificateFile = "fixtures/cert.pem"
4417
multiCertificateFile = "fixtures/multi.pem"
@@ -118,8 +91,8 @@ func TestConfigServerTLSServerCertsOnly(t *testing.T) {
11891
if len(tlsConfig.Certificates[0].Certificate) != len(keypair.Certificate) {
11992
t.Fatal("Unexpected server certificates")
12093
}
121-
for i, cert := range tlsConfig.Certificates[0].Certificate {
122-
if !bytes.Equal(cert, keypair.Certificate[i]) {
94+
for i, crt := range tlsConfig.Certificates[0].Certificate {
95+
if !bytes.Equal(crt, keypair.Certificate[i]) {
12396
t.Fatal("Unexpected server certificates")
12497
}
12598
}
@@ -195,47 +168,47 @@ func TestConfigServerTLSClientCASet(t *testing.T) {
195168

196169
// Exclusive root pools determines whether the CA pool will be a union of the system
197170
// certificate pool and custom certs, or an exclusive or of the custom certs and system pool
171+
//
172+
// NOTE: In reality this behavior depends on the platform trust store and would
173+
// be best validated via integration tests. Here we inject a fake system pool so
174+
// the unit test can deterministically verify the intended semantics.
198175
func TestConfigServerExclusiveRootPools(t *testing.T) {
199-
if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
200-
// FIXME: see https://github.com/docker/go-connections/issues/105.
201-
t.Skip("FIXME: failing on Windows and darwin")
202-
}
203176
key, cert := getCertAndKey()
204-
ca := getMultiCert()
205177

206-
caBytes, err := os.ReadFile(ca)
178+
customRoot, err := newTestTrustRoot()
207179
if err != nil {
208-
t.Fatal("Unable to read CA certs", err)
180+
t.Fatal("Unable to generate fake custom CA", err)
209181
}
210182

211-
var testCerts []*x509.Certificate
212-
for _, pemBytes := range [][]byte{caBytes, []byte(systemRootTrustedCert)} {
213-
pemBlock, _ := pem.Decode(pemBytes)
214-
if pemBlock == nil {
215-
t.Fatal("Malformed certificate")
216-
}
217-
cert, err := x509.ParseCertificate(pemBlock.Bytes)
218-
if err != nil {
219-
t.Fatal("Unable to parse certificate")
220-
}
221-
testCerts = append(testCerts, cert)
183+
ca := filepath.Join(t.TempDir(), "custom-ca.pem")
184+
if err := os.WriteFile(ca, customRoot.RootPEM, 0o644); err != nil {
185+
t.Fatal("Unable to write custom CA file", err)
222186
}
223187

188+
systemLeaf, err := systemRootTrustedX509()
189+
if err != nil {
190+
t.Fatal("Unable to load fake system certificate", err)
191+
}
192+
193+
testCerts := []*x509.Certificate{customRoot.LeafCert, systemLeaf}
194+
224195
// ExclusiveRootPools not set, so should be able to verify both system-signed certs
225196
// and custom CA-signed certs
226197
tlsConfig, err := Server(Options{
227198
CertFile: cert,
228199
KeyFile: key,
229200
ClientAuth: tls.VerifyClientCertIfGiven,
230201
CAFile: ca,
202+
203+
systemCertPool: fakeSystemCertPool(),
231204
})
232205

233206
if err != nil || tlsConfig == nil {
234207
t.Fatal("Unable to configure server TLS", err)
235208
}
236209

237-
for i, cert := range testCerts {
238-
if _, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.ClientCAs}); err != nil {
210+
for i, crt := range testCerts {
211+
if _, err := crt.Verify(x509.VerifyOptions{Roots: tlsConfig.ClientCAs}); err != nil {
239212
t.Fatalf("Unable to verify certificate %d: %v", i, err)
240213
}
241214
}
@@ -248,40 +221,36 @@ func TestConfigServerExclusiveRootPools(t *testing.T) {
248221
ClientAuth: tls.VerifyClientCertIfGiven,
249222
CAFile: ca,
250223
ExclusiveRootPools: true,
224+
systemCertPool: fakeSystemCertPool(),
251225
})
252226

253227
if err != nil || tlsConfig == nil {
254228
t.Fatal("Unable to configure server TLS", err)
255229
}
256230

257-
for i, cert := range testCerts {
258-
_, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.ClientCAs})
231+
for i, crt := range testCerts {
232+
_, err := crt.Verify(x509.VerifyOptions{Roots: tlsConfig.ClientCAs})
259233
switch {
260234
case i == 0 && err != nil:
261235
t.Fatal("Unable to verify custom certificate, even though the root pool should have only the custom CA", err)
262236
case i == 1 && err == nil:
263-
t.Fatal("Successfully verified system root-signed certificate though the root pool should have only the cusotm CA", err)
237+
t.Fatal("Successfully verified system root-signed certificate though the root pool should have only the custom CA", err)
264238
}
265239
}
266240

267-
// No CA file provided, system cert should be verifiable only
241+
// No CA file provided means the server does not configure ClientCAs.
268242
tlsConfig, err = Server(Options{
269243
CertFile: cert,
270244
KeyFile: key,
245+
246+
systemCertPool: fakeSystemCertPool(),
271247
})
272248

273249
if err != nil || tlsConfig == nil {
274250
t.Fatal("Unable to configure server TLS", err)
275251
}
276-
277-
for i, cert := range testCerts {
278-
_, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.ClientCAs})
279-
switch {
280-
case i == 1 && err != nil:
281-
t.Fatal("Unable to verify system root-signed certificate, even though the root pool should be the system pool only", err)
282-
case i == 0 && err == nil:
283-
t.Fatal("Successfully verified custom certificate though the root pool should be the system pool only", err)
284-
}
252+
if tlsConfig.ClientCAs != nil {
253+
t.Fatal("Expected ClientCAs to be nil when no CA file is provided")
285254
}
286255
}
287256

@@ -510,8 +479,8 @@ func TestConfigClientTLSValidClientCertAndKey(t *testing.T) {
510479
if len(tlsConfig.Certificates[0].Certificate) != len(keypair.Certificate) {
511480
t.Fatal("Unexpected client certificates")
512481
}
513-
for i, cert := range tlsConfig.Certificates[0].Certificate {
514-
if !bytes.Equal(cert, keypair.Certificate[i]) {
482+
for i, crt := range tlsConfig.Certificates[0].Certificate {
483+
if !bytes.Equal(crt, keypair.Certificate[i]) {
515484
t.Fatal("Unexpected client certificates")
516485
}
517486
}
@@ -538,41 +507,43 @@ func TestConfigClientTLSEncryptedKey(t *testing.T) {
538507

539508
// Exclusive root pools determines whether the CA pool will be a union of the system
540509
// certificate pool and custom certs, or an exclusive or of the custom certs and system pool
510+
//
511+
// NOTE: In reality this behavior depends on the platform trust store and would
512+
// be best validated via integration tests. Here we inject a fake system pool so
513+
// the unit test can deterministically verify the intended semantics.
541514
func TestConfigClientExclusiveRootPools(t *testing.T) {
542-
if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
543-
// FIXME: see https://github.com/docker/go-connections/issues/105.
544-
t.Skip("FIXME: failing on Windows and darwin")
545-
}
546-
ca := getMultiCert()
515+
key, cert := getCertAndKey()
547516

548-
caBytes, err := os.ReadFile(ca)
517+
customRoot, err := newTestTrustRoot()
549518
if err != nil {
550-
t.Fatal("Unable to read CA certs", err)
519+
t.Fatal("Unable to generate fake custom CA", err)
551520
}
552521

553-
var testCerts []*x509.Certificate
554-
for _, pemBytes := range [][]byte{caBytes, []byte(systemRootTrustedCert)} {
555-
pemBlock, _ := pem.Decode(pemBytes)
556-
if pemBlock == nil {
557-
t.Fatal("Malformed certificate")
558-
}
559-
cert, err := x509.ParseCertificate(pemBlock.Bytes)
560-
if err != nil {
561-
t.Fatal("Unable to parse certificate")
562-
}
563-
testCerts = append(testCerts, cert)
522+
ca := filepath.Join(t.TempDir(), "custom-ca.pem")
523+
if err := os.WriteFile(ca, customRoot.RootPEM, 0o644); err != nil {
524+
t.Fatal("Unable to write custom CA file", err)
525+
}
526+
527+
systemLeaf, err := systemRootTrustedX509()
528+
if err != nil {
529+
t.Fatal("Unable to load fake system certificate", err)
564530
}
565531

532+
testCerts := []*x509.Certificate{customRoot.LeafCert, systemLeaf}
533+
566534
// ExclusiveRootPools not set, so should be able to verify both system-signed certs
567535
// and custom CA-signed certs
568-
tlsConfig, err := Client(Options{CAFile: ca})
536+
tlsConfig, err := Client(Options{
537+
CAFile: ca,
538+
systemCertPool: fakeSystemCertPool(),
539+
})
569540

570541
if err != nil || tlsConfig == nil {
571542
t.Fatal("Unable to configure client TLS", err)
572543
}
573544

574-
for i, cert := range testCerts {
575-
if _, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.RootCAs}); err != nil {
545+
for i, crt := range testCerts {
546+
if _, err := crt.Verify(x509.VerifyOptions{Roots: tlsConfig.RootCAs}); err != nil {
576547
t.Fatalf("Unable to verify certificate %d: %v", i, err)
577548
}
578549
}
@@ -582,37 +553,35 @@ func TestConfigClientExclusiveRootPools(t *testing.T) {
582553
tlsConfig, err = Client(Options{
583554
CAFile: ca,
584555
ExclusiveRootPools: true,
556+
systemCertPool: fakeSystemCertPool(),
585557
})
586558

587559
if err != nil || tlsConfig == nil {
588560
t.Fatal("Unable to configure client TLS", err)
589561
}
590562

591-
for i, cert := range testCerts {
592-
_, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.RootCAs})
563+
for i, crt := range testCerts {
564+
_, err := crt.Verify(x509.VerifyOptions{Roots: tlsConfig.RootCAs})
593565
switch {
594566
case i == 0 && err != nil:
595567
t.Fatal("Unable to verify custom certificate, even though the root pool should have only the custom CA", err)
596568
case i == 1 && err == nil:
597-
t.Fatal("Successfully verified system root-signed certificate though the root pool should have only the cusotm CA", err)
569+
t.Fatal("Successfully verified system root-signed certificate though the root pool should have only the custom CA", err)
598570
}
599571
}
600572

601-
// No CA file provided, system cert should be verifiable only
602-
tlsConfig, err = Client(Options{})
573+
// No CA file provided means the client leaves RootCAs unset.
574+
tlsConfig, err = Client(Options{
575+
CertFile: cert,
576+
KeyFile: key,
577+
systemCertPool: fakeSystemCertPool(),
578+
})
603579

604580
if err != nil || tlsConfig == nil {
605581
t.Fatal("Unable to configure client TLS", err)
606582
}
607-
608-
for i, cert := range testCerts {
609-
_, err := cert.Verify(x509.VerifyOptions{Roots: tlsConfig.RootCAs})
610-
switch {
611-
case i == 1 && err != nil:
612-
t.Fatal("Unable to verify system root-signed certificate, even though the root pool should be the system pool only", err)
613-
case i == 0 && err == nil:
614-
t.Fatal("Successfully verified custom certificate though the root pool should be the system pool only", err)
615-
}
583+
if tlsConfig.RootCAs != nil {
584+
t.Fatal("Expected RootCAs to be nil when no CA file is provided")
616585
}
617586
}
618587

0 commit comments

Comments
 (0)