Read multiple CA certs from a single PEM file - thanks @mtrmac!

Signed-off-by: Ying Li <ying.li@docker.com>

utils/tls_config.go

@@ -5,9 +5,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
-	"os"
+	"io/ioutil"
-
-	"github.com/docker/notary/trustmanager"
 )
 
 // Client TLS cipher suites (dropping CBC ciphers for client preferred suite set)
@@ -26,13 +24,30 @@ var serverCipherSuites = append(clientCipherSuites, []uint16{
 	tls.TLS_RSA_WITH_AES_128_CBC_SHA,
 }...)
 
+func poolFromFile(filename string) (*x509.CertPool, error) {
+	pemBytes, err := ioutil.ReadFile(filename)
+	if err != nil {
+		return nil, err
+	}
+	pool := x509.NewCertPool()
+	if ok := pool.AppendCertsFromPEM(pemBytes); !ok {
+		return nil, fmt.Errorf(
+			"Unable to parse certificates from %s", filename)
+	}
+	if len(pool.Subjects()) == 0 {
+		return nil, fmt.Errorf(
+			"No certificates parsed from %s", filename)
+	}
+	return pool, nil
+}
+
 // ServerTLSOpts generates a tls configuration for servers using the
 // provided parameters.
 type ServerTLSOpts struct {
 	ServerCertFile    string
 	ServerKeyFile     string
 	RequireClientAuth bool
-	ClientCADirectory string
+	ClientCAFile      string
 }
 
 // ConfigureServerTLS specifies a set of ciphersuites, the server cert and key,
@@ -58,24 +73,12 @@ func ConfigureServerTLS(opts *ServerTLSOpts) (*tls.Config, error) {
 		tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert
 	}
 
-	if opts.ClientCADirectory != "" {
+	if opts.ClientCAFile != "" {
-		// Check to see if the given directory exists
+		pool, err := poolFromFile(opts.ClientCAFile)
-		fi, err := os.Stat(opts.ClientCADirectory)
 		if err != nil {
 			return nil, err
 		}
-		if !fi.IsDir() {
+		tlsConfig.ClientCAs = pool
-			return nil, fmt.Errorf("No such directory: %s", opts.ClientCADirectory)
-		}
-
-		certStore, err := trustmanager.NewX509FileStore(opts.ClientCADirectory)
-		if err != nil {
-			return nil, err
-		}
-		if certStore.Empty() {
-			return nil, fmt.Errorf("No certificates in %s", opts.ClientCADirectory)
-		}
-		tlsConfig.ClientCAs = certStore.GetCertificatePool()
 	}
 
 	return tlsConfig, nil
@@ -102,14 +105,11 @@ func ConfigureClientTLS(opts *ClientTLSOpts) (*tls.Config, error) {
 	}
 
 	if opts.RootCAFile != "" {
-		rootCert, err := trustmanager.LoadCertFromFile(opts.RootCAFile)
+		pool, err := poolFromFile(opts.RootCAFile)
 		if err != nil {
-			return nil, fmt.Errorf(
+			return nil, err
-				"Could not load root ca file. %s", err.Error())
 		}
-		rootPool := x509.NewCertPool()
+		tlsConfig.RootCAs = pool
-		rootPool.AddCert(rootCert)
-		tlsConfig.RootCAs = rootPool
 	}
 
 	if opts.ClientCertFile != "" || opts.ClientKeyFile != "" {

utils/tls_config_test.go

@@ -1,13 +1,18 @@
 package utils
 
 import (
+	"crypto"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/rsa"
 	"crypto/tls"
-	"io"
+	"crypto/x509"
 	"io/ioutil"
 	"os"
-	"path/filepath"
 	"testing"
 
+	"github.com/docker/notary/trustmanager"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -17,38 +22,57 @@ const (
 	RootCA     = "../fixtures/root-ca.crt"
 )
 
-// copies the provided certificate into a temporary directory
+// generates a multiple-certificate file with both RSA and ECDSA certs and
-func makeTempCertDir(t *testing.T) string {
+// some garbage, returns filename.
-	tempDir, err := ioutil.TempDir("/tmp", "cert-test")
+func generateMultiCert(t *testing.T) string {
-	assert.NoError(t, err, "couldn't open temp directory")
+	tempFile, err := ioutil.TempFile("/tmp", "cert-test")
+	defer tempFile.Close()
+	assert.NoError(t, err)
 
-	in, err := os.Open(RootCA)
+	rsaKey, err := rsa.GenerateKey(rand.Reader, 2048)
-	assert.NoError(t, err, "cannot open %s", RootCA)
+	assert.NoError(t, err)
-	defer in.Close()
+	ecKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader)
-	copiedCert := filepath.Join(tempDir, filepath.Base(RootCA))
+	assert.NoError(t, err)
-	out, err := os.Create(copiedCert)
+	template, err := trustmanager.NewCertificate("gun")
-	assert.NoError(t, err, "cannot open %s", copiedCert)
+	assert.NoError(t, err)
-	defer out.Close()
-	_, err = io.Copy(out, in)
-	assert.NoError(t, err, "cannot copy %s to %s", RootCA, copiedCert)
 
-	return tempDir
+	for _, key := range []crypto.Signer{rsaKey, ecKey} {
+		derBytes, err := x509.CreateCertificate(
+			rand.Reader, template, template, key.Public(), key)
+		assert.NoError(t, err)
+
+		cert, err := x509.ParseCertificate(derBytes)
+		assert.NoError(t, err)
+
+		pemBytes := trustmanager.CertToPEM(cert)
+		nBytes, err := tempFile.Write(pemBytes)
+		assert.NoError(t, err)
+		assert.Equal(t, nBytes, len(pemBytes))
+
+		assert.NoError(t, err)
+	}
+
+	_, err = tempFile.WriteString(`\n
+    -----BEGIN CERTIFICATE-----
+    This is some garbage that isnt a cert
+    -----END CERTIFICATE-----
+    `)
+
+	return tempFile.Name()
 }
 
 // If the cert files and directory are provided but are invalid, an error is
 // returned.
 func TestConfigServerTLSFailsIfUnableToLoadCerts(t *testing.T) {
-	tempDir := makeTempCertDir(t)
-
 	for i := 0; i < 3; i++ {
-		files := []string{ServerCert, ServerKey, tempDir}
+		files := []string{ServerCert, ServerKey, RootCA}
 		files[i] = "not-real-file"
 
 		result, err := ConfigureServerTLS(&ServerTLSOpts{
 			ServerCertFile:    files[0],
 			ServerKeyFile:     files[1],
 			RequireClientAuth: true,
-			ClientCADirectory: files[2],
+			ClientCAFile:      files[2],
 		})
 		assert.Nil(t, result)
 		assert.Error(t, err)
@@ -72,33 +96,34 @@ func TestConfigServerTLSServerCertsOnly(t *testing.T) {
 	assert.Nil(t, tlsConfig.ClientCAs)
 }
 
-// If a valid client cert directory is provided, but it contains no client
+// If a valid client cert file is provided, but it contains no client
 // certs, an error is returned.
-func TestConfigServerTLSWithEmptyCACertDir(t *testing.T) {
+func TestConfigServerTLSWithEmptyCACertFile(t *testing.T) {
-	tempDir, err := ioutil.TempDir("/tmp", "cert-test")
+	tempFile, err := ioutil.TempFile("/tmp", "cert-test")
-	assert.NoError(t, err, "couldn't open temp directory")
+	assert.NoError(t, err)
+	defer os.RemoveAll(tempFile.Name())
+	tempFile.Close()
 
 	tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{
 		ServerCertFile: ServerCert,
 		ServerKeyFile:  ServerKey,
-		ClientCADirectory: tempDir,
+		ClientCAFile:   tempFile.Name(),
 	})
 	assert.Nil(t, tlsConfig)
 	assert.Error(t, err)
 }
 
-// If server cert and key are provided, and client cert directory is provided,
+// If server cert and key are provided, and client cert file is provided with
-// a valid tls.Config is returned with the clientCAs set to the certs in that
+// one cert, a valid tls.Config is returned with the clientCAs set to that
-// directory.
+// cert.
-func TestConfigServerTLSWithCACerts(t *testing.T) {
+func TestConfigServerTLSWithOneCACert(t *testing.T) {
-	tempDir := makeTempCertDir(t)
 	keypair, err := tls.LoadX509KeyPair(ServerCert, ServerKey)
 	assert.NoError(t, err)
 
 	tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{
 		ServerCertFile: ServerCert,
 		ServerKeyFile:  ServerKey,
-		ClientCADirectory: tempDir,
+		ClientCAFile:   RootCA,
 	})
 	assert.NoError(t, err)
 	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
@@ -107,6 +132,30 @@ func TestConfigServerTLSWithCACerts(t *testing.T) {
 	assert.Len(t, tlsConfig.ClientCAs.Subjects(), 1)
 }
 
+// If server cert and key are provided, and client cert file is provided with
+// multiple certs (and garbage), a valid tls.Config is returned with the
+// clientCAs set to the valid cert and the garbage is ignored (but only
+// because the garbage is at the end - actually CertPool.AppendCertsFromPEM
+// aborts as soon as it finds an invalid cert)
+func TestConfigServerTLSWithMultipleCACertsAndGarbage(t *testing.T) {
+	tempFilename := generateMultiCert(t)
+	defer os.RemoveAll(tempFilename)
+
+	keypair, err := tls.LoadX509KeyPair(ServerCert, ServerKey)
+	assert.NoError(t, err)
+
+	tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{
+		ServerCertFile: ServerCert,
+		ServerKeyFile:  ServerKey,
+		ClientCAFile:   tempFilename,
+	})
+	assert.NoError(t, err)
+	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
+	assert.True(t, tlsConfig.PreferServerCipherSuites)
+	assert.Equal(t, tls.NoClientCert, tlsConfig.ClientAuth)
+	assert.Len(t, tlsConfig.ClientCAs.Subjects(), 2)
+}
+
 // If server cert and key are provided, and client auth is disabled, then
 // a valid tls.Config is returned with ClientAuth set to
 // RequireAndVerifyClientCert
@@ -151,8 +200,8 @@ func TestConfigClientServerName(t *testing.T) {
 	}
 }
 
-// The RootCA is set if it is provided and valid
+// The RootCA is set if the file provided has a single CA cert.
-func TestConfigClientTLSValidRootCA(t *testing.T) {
+func TestConfigClientTLSRootCAFileWithOneCert(t *testing.T) {
 	tlsConfig, err := ConfigureClientTLS(&ClientTLSOpts{RootCAFile: RootCA})
 	assert.NoError(t, err)
 	assert.Nil(t, tlsConfig.Certificates)
@@ -161,8 +210,24 @@ func TestConfigClientTLSValidRootCA(t *testing.T) {
 	assert.Len(t, tlsConfig.RootCAs.Subjects(), 1)
 }
 
-// An error is returned if a root CA is provided but not valid
+// If the root CA file provided has multiple CA certs and garbage, only the
-func TestConfigClientTLSInvalidRootCA(t *testing.T) {
+// valid certs are read (but only because the garbage is at the end - actually
+// CertPool.AppendCertsFromPEM aborts as soon as it finds an invalid cert)
+func TestConfigClientTLSRootCAFileMultipleCertsAndGarbage(t *testing.T) {
+	tempFilename := generateMultiCert(t)
+	defer os.RemoveAll(tempFilename)
+
+	tlsConfig, err := ConfigureClientTLS(
+		&ClientTLSOpts{RootCAFile: tempFilename})
+	assert.NoError(t, err)
+	assert.Nil(t, tlsConfig.Certificates)
+	assert.Equal(t, false, tlsConfig.InsecureSkipVerify)
+	assert.Equal(t, "", tlsConfig.ServerName)
+	assert.Len(t, tlsConfig.RootCAs.Subjects(), 2)
+}
+
+// An error is returned if a root CA is provided but the file doesn't exist.
+func TestConfigClientTLSNonexistentRootCAFile(t *testing.T) {
 	tlsConfig, err := ConfigureClientTLS(
 		&ClientTLSOpts{RootCAFile: "not-a-file"})
 	assert.Error(t, err)