From 598a7ed688f051543ecb11c9c8093672eb547c12 Mon Sep 17 00:00:00 2001 From: cyli Date: Thu, 23 Feb 2017 17:13:47 -0800 Subject: [PATCH] When the root CA is updated in the security config, ensure that the server, client, and external CA TLS credentials all also use the new root pool. Signed-off-by: cyli --- ca/certificates.go | 10 +---- ca/config.go | 33 ++++++++++++-- ca/config_test.go | 98 +++++++++++++++++++++++++++++++++++++++++ ca/testutils/cautils.go | 33 +++++++------- ca/transport.go | 11 +++++ 5 files changed, 158 insertions(+), 27 deletions(-) diff --git a/ca/certificates.go b/ca/certificates.go index 8af117816e..5af07717eb 100644 --- a/ca/certificates.go +++ b/ca/certificates.go @@ -666,17 +666,11 @@ func saveRootCA(rootCA RootCA, paths CertPaths) error { } // GenerateNewCSR returns a newly generated key and CSR signed with said key -func GenerateNewCSR() (csr, key []byte, err error) { +func GenerateNewCSR() ([]byte, []byte, error) { req := &cfcsr.CertificateRequest{ KeyRequest: cfcsr.NewBasicKeyRequest(), } - - csr, key, err = cfcsr.ParseRequest(req) - if err != nil { - return - } - - return + return cfcsr.ParseRequest(req) } // EncryptECPrivateKey receives a PEM encoded private key and returns an encrypted diff --git a/ca/config.go b/ca/config.go index 652bc455d0..56eb12b20c 100644 --- a/ca/config.go +++ b/ca/config.go @@ -112,6 +112,11 @@ func (s *SecurityConfig) RootCA() *RootCA { return s.rootCA } +// ExternalCA returns the external CA. +func (s *SecurityConfig) ExternalCA() *ExternalCA { + return s.externalCA +} + // KeyWriter returns the object that can write keys to disk func (s *SecurityConfig) KeyWriter() KeyWriter { return s.keyReadWriter @@ -129,11 +134,33 @@ func (s *SecurityConfig) UpdateRootCA(cert, key []byte, certExpiry time.Duration defer s.mu.Unlock() rootCA, err := NewRootCA(cert, key, certExpiry) - if err == nil { - s.rootCA = &rootCA + if err != nil { + return err + } + + // the RootCA pool should validate against the TLS certificate in the credentials + if s.ClientTLSCreds != nil { + s.ClientTLSCreds.UpdateCAs(rootCA.Pool, nil) + } + + if s.ServerTLSCreds != nil { + s.ServerTLSCreds.UpdateCAs(rootCA.Pool, rootCA.Pool) } - return err + if s.externalCA != nil { + clientTLSConfig := s.ClientTLSCreds.Config() + + externalCATLSConfig := &tls.Config{ + Certificates: clientTLSConfig.Certificates, + RootCAs: rootCA.Pool, + MinVersion: tls.VersionTLS12, + } + + s.externalCA.UpdateTLSConfig(externalCATLSConfig) + } + + s.rootCA = &rootCA + return nil } // SigningPolicy creates a policy used by the signer to ensure that the only fields diff --git a/ca/config_test.go b/ca/config_test.go index 8d6f3d674f..93aec80318 100644 --- a/ca/config_test.go +++ b/ca/config_test.go @@ -7,13 +7,18 @@ import ( "encoding/pem" "io/ioutil" "math/big" + "net" "os" "strings" "testing" "time" + "google.golang.org/grpc" + "golang.org/x/net/context" + "crypto/tls" + cfconfig "github.com/cloudflare/cfssl/config" "github.com/cloudflare/cfssl/helpers" "github.com/docker/swarmkit/ca" @@ -265,6 +270,99 @@ func TestLoadSecurityConfigIncorrectPassphrase(t *testing.T) { require.IsType(t, ca.ErrInvalidKEK{}, err) } +func TestSecurityConfigUpdateRootCA(t *testing.T) { + tc := testutils.NewTestCA(t) + defer tc.Stop() + tcConfig, err := tc.NewNodeConfig("worker") + require.NoError(t, err) + + // create the "original" security config, and we'll update it to trust the test server's + cert, key, err := testutils.CreateRootCertAndKey("root1") + require.NoError(t, err) + rootCA, err := ca.NewRootCA(cert, key, ca.DefaultNodeCertExpiration) + require.NoError(t, err) + + tempdir, err := ioutil.TempDir("", "test-security-config-update") + require.NoError(t, err) + defer os.RemoveAll(tempdir) + configPaths := ca.NewConfigPaths(tempdir) + + secConfig, err := rootCA.CreateSecurityConfig(context.Background(), + ca.NewKeyReadWriter(configPaths.Node, nil, nil), ca.CertificateRequestConfig{}) + require.NoError(t, err) + // update the server TLS to require certificates, otherwise this will all pass + // even if the root pools aren't updated + secConfig.ServerTLSCreds.Config().ClientAuth = tls.RequireAndVerifyClientCert + + // set up a GRPC server using these credentials + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + serverOpts := []grpc.ServerOption{grpc.Creds(secConfig.ServerTLSCreds)} + grpcServer := grpc.NewServer(serverOpts...) + go grpcServer.Serve(l) + defer grpcServer.Stop() + + // we should not be able to connect to the test CA server using the original security config, and should not + // be able to connect to new server using the test CA's client credentials + dialOptsBase := []grpc.DialOption{ + grpc.WithBlock(), + grpc.WithTimeout(10 * time.Second), + } + dialOpts := append(dialOptsBase, grpc.WithTransportCredentials(secConfig.ClientTLSCreds)) + _, err = grpc.Dial(tc.Addr, dialOpts...) + require.Error(t, err) + require.IsType(t, x509.UnknownAuthorityError{}, err) + + dialOpts = append(dialOptsBase, grpc.WithTransportCredentials(tcConfig.ClientTLSCreds)) + _, err = grpc.Dial(l.Addr().String(), dialOpts...) + require.Error(t, err) + require.IsType(t, x509.UnknownAuthorityError{}, err) + + // we can't connect to the test CA's external server either + csr, _, err := ca.GenerateNewCSR() + require.NoError(t, err) + req := ca.PrepareCSR(csr, "cn", ca.ManagerRole, secConfig.ClientTLSCreds.Organization()) + + externalServer := tc.ExternalSigningServer + if testutils.External { + // stop the external server and create a new one because the external server actually has to trust our client certs as well. + updatedRoot, err := ca.NewRootCA(append(tc.RootCA.Cert, cert...), tc.RootCA.Key, ca.DefaultNodeCertExpiration) + require.NoError(t, err) + externalServer, err = testutils.NewExternalSigningServer(updatedRoot, tc.TempDir) + require.NoError(t, err) + defer externalServer.Stop() + + secConfig.ExternalCA().UpdateURLs(externalServer.URL) + _, err = secConfig.ExternalCA().Sign(context.Background(), req) + require.Error(t, err) + // the type is weird (it's wrapped in a bunch of other things in ctxhttp), so just compare strings + require.Contains(t, err.Error(), x509.UnknownAuthorityError{}.Error()) + } + + // update the root CA on the "original"" security config to support both the old root + // and the "new root" (the testing CA root) + err = secConfig.UpdateRootCA(append(rootCA.Cert, tc.RootCA.Cert...), rootCA.Key, ca.DefaultNodeCertExpiration) + require.NoError(t, err) + + // can now connect to the test CA using our modified security config, and can cannect to our server using + // the test CA config + conn, err := grpc.Dial(tc.Addr, dialOpts...) + require.NoError(t, err) + conn.Close() + + dialOpts = append(dialOptsBase, grpc.WithTransportCredentials(secConfig.ClientTLSCreds)) + conn, err = grpc.Dial(tc.Addr, dialOpts...) + require.NoError(t, err) + conn.Close() + + // we can also now connect to the test CA's external signing server + if testutils.External { + secConfig.ExternalCA().UpdateURLs(externalServer.URL) + _, err := secConfig.ExternalCA().Sign(context.Background(), req) + require.NoError(t, err) + } +} + func TestRenewTLSConfigWorker(t *testing.T) { t.Parallel() diff --git a/ca/testutils/cautils.go b/ca/testutils/cautils.go index 881fce6e6a..cd66d60870 100644 --- a/ca/testutils/cautils.go +++ b/ca/testutils/cautils.go @@ -34,22 +34,22 @@ import ( // TestCA is a structure that encapsulates everything needed to test a CA Server type TestCA struct { - RootCA ca.RootCA - ExternalSigningServer *ExternalSigningServer - MemoryStore *store.MemoryStore - TempDir, Organization string - Paths *ca.SecurityConfigPaths - Server *grpc.Server - CAServer *ca.Server - Context context.Context - NodeCAClients []api.NodeCAClient - CAClients []api.CAClient - Conns []*grpc.ClientConn - WorkerToken string - ManagerToken string - ConnBroker *connectionbroker.Broker - KeyReadWriter *ca.KeyReadWriter - watchCancel func() + RootCA ca.RootCA + ExternalSigningServer *ExternalSigningServer + MemoryStore *store.MemoryStore + Addr, TempDir, Organization string + Paths *ca.SecurityConfigPaths + Server *grpc.Server + CAServer *ca.Server + Context context.Context + NodeCAClients []api.NodeCAClient + CAClients []api.CAClient + Conns []*grpc.ClientConn + WorkerToken string + ManagerToken string + ConnBroker *connectionbroker.Broker + KeyReadWriter *ca.KeyReadWriter + watchCancel func() } // Stop cleans up after TestCA @@ -224,6 +224,7 @@ func NewTestCA(t *testing.T, krwGenerators ...func(ca.CertPaths) *ca.KeyReadWrit CAClients: caClients, NodeCAClients: nodeCAClients, Conns: conns, + Addr: l.Addr().String(), Server: grpcServer, CAServer: caServer, WorkerToken: workerToken, diff --git a/ca/transport.go b/ca/transport.go index 834f2c1170..5f4497996a 100644 --- a/ca/transport.go +++ b/ca/transport.go @@ -8,6 +8,7 @@ import ( "strings" "sync" + "github.com/docker/docker/pkg/tlsconfig" "github.com/pkg/errors" "golang.org/x/net/context" "google.golang.org/grpc/credentials" @@ -135,6 +136,16 @@ func (c *MutableTLSCreds) LoadNewTLSConfig(newConfig *tls.Config) error { return nil } +// UpdateCAs updates the root CAs and client CAs of the existing TLS config in place +func (c *MutableTLSCreds) UpdateCAs(rootCAs, clientCAs *x509.CertPool) { + c.Lock() + defer c.Unlock() + config := tlsconfig.Clone(c.config) + config.RootCAs = rootCAs + config.ClientCAs = clientCAs + c.config = config +} + // Config returns the current underlying TLS config. func (c *MutableTLSCreds) Config() *tls.Config { c.Lock()