From 09dc607bef96feafdd7cbce1194cc39614366b6d Mon Sep 17 00:00:00 2001 From: Ying Li Date: Fri, 23 Oct 2015 15:56:47 -0700 Subject: [PATCH] Read multiple CA certs from a single PEM file - thanks @mtrmac! Signed-off-by: Ying Li --- utils/tls_config.go | 50 +++++++------- utils/tls_config_test.go | 143 ++++++++++++++++++++++++++++----------- 2 files changed, 129 insertions(+), 64 deletions(-) diff --git a/utils/tls_config.go b/utils/tls_config.go index 0b1e70eac5..b457335181 100644 --- a/utils/tls_config.go +++ b/utils/tls_config.go @@ -5,9 +5,7 @@ import ( "crypto/tls" "crypto/x509" "fmt" - "os" - - "github.com/docker/notary/trustmanager" + "io/ioutil" ) // 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 != "" { - // Check to see if the given directory exists - fi, err := os.Stat(opts.ClientCADirectory) + if opts.ClientCAFile != "" { + pool, err := poolFromFile(opts.ClientCAFile) if err != nil { return nil, err } - if !fi.IsDir() { - 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() + tlsConfig.ClientCAs = pool } 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( - "Could not load root ca file. %s", err.Error()) + return nil, err } - rootPool := x509.NewCertPool() - rootPool.AddCert(rootCert) - tlsConfig.RootCAs = rootPool + tlsConfig.RootCAs = pool } if opts.ClientCertFile != "" || opts.ClientKeyFile != "" { diff --git a/utils/tls_config_test.go b/utils/tls_config_test.go index 328ebc49d9..f6b2e73a59 100644 --- a/utils/tls_config_test.go +++ b/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 -func makeTempCertDir(t *testing.T) string { - tempDir, err := ioutil.TempDir("/tmp", "cert-test") - assert.NoError(t, err, "couldn't open temp directory") +// generates a multiple-certificate file with both RSA and ECDSA certs and +// some garbage, returns filename. +func generateMultiCert(t *testing.T) string { + tempFile, err := ioutil.TempFile("/tmp", "cert-test") + defer tempFile.Close() + assert.NoError(t, err) - in, err := os.Open(RootCA) - assert.NoError(t, err, "cannot open %s", RootCA) - defer in.Close() - copiedCert := filepath.Join(tempDir, filepath.Base(RootCA)) - out, err := os.Create(copiedCert) - assert.NoError(t, err, "cannot open %s", copiedCert) - defer out.Close() - _, err = io.Copy(out, in) - assert.NoError(t, err, "cannot copy %s to %s", RootCA, copiedCert) + rsaKey, err := rsa.GenerateKey(rand.Reader, 2048) + assert.NoError(t, err) + ecKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + assert.NoError(t, err) + template, err := trustmanager.NewCertificate("gun") + assert.NoError(t, err) - 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) { - tempDir, err := ioutil.TempDir("/tmp", "cert-test") - assert.NoError(t, err, "couldn't open temp directory") +func TestConfigServerTLSWithEmptyCACertFile(t *testing.T) { + tempFile, err := ioutil.TempFile("/tmp", "cert-test") + assert.NoError(t, err) + defer os.RemoveAll(tempFile.Name()) + tempFile.Close() tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{ - ServerCertFile: ServerCert, - ServerKeyFile: ServerKey, - ClientCADirectory: tempDir, + ServerCertFile: ServerCert, + ServerKeyFile: ServerKey, + ClientCAFile: tempFile.Name(), }) assert.Nil(t, tlsConfig) assert.Error(t, err) } -// If server cert and key are provided, and client cert directory is provided, -// a valid tls.Config is returned with the clientCAs set to the certs in that -// directory. -func TestConfigServerTLSWithCACerts(t *testing.T) { - tempDir := makeTempCertDir(t) +// If server cert and key are provided, and client cert file is provided with +// one cert, a valid tls.Config is returned with the clientCAs set to that +// cert. +func TestConfigServerTLSWithOneCACert(t *testing.T) { keypair, err := tls.LoadX509KeyPair(ServerCert, ServerKey) assert.NoError(t, err) tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{ - ServerCertFile: ServerCert, - ServerKeyFile: ServerKey, - ClientCADirectory: tempDir, + ServerCertFile: ServerCert, + ServerKeyFile: ServerKey, + 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 -func TestConfigClientTLSValidRootCA(t *testing.T) { +// The RootCA is set if the file provided has a single CA cert. +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 -func TestConfigClientTLSInvalidRootCA(t *testing.T) { +// If the root CA file provided has multiple CA certs and garbage, only the +// 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)