diff --git a/ca/certificates_test.go b/ca/certificates_test.go index 842ced23d3..ee229156cb 100644 --- a/ca/certificates_test.go +++ b/ca/certificates_test.go @@ -1290,8 +1290,9 @@ func TestRootCAWithCrossSignedIntermediates(t *testing.T) { connectToExternalRootCA, err := ca.NewRootCA(append(cautils.ECDSACertChain[2], fauxRootCert...), cautils.ECDSACertChain[1], cautils.ECDSACertChainKeys[1], ca.DefaultNodeCertExpiration, cautils.ECDSACertChain[1]) require.NoError(t, err) - secConfig, err := connectToExternalRootCA.CreateSecurityConfig(tc.Context, krw, ca.CertificateRequestConfig{}) + secConfig, cancel, err := connectToExternalRootCA.CreateSecurityConfig(tc.Context, krw, ca.CertificateRequestConfig{}) require.NoError(t, err) + cancel() externalCA := secConfig.ExternalCA() externalCA.UpdateURLs(tc.ExternalSigningServer.URL) diff --git a/ca/config.go b/ca/config.go index 9dd84ecb36..cfaccd00b9 100644 --- a/ca/config.go +++ b/ca/config.go @@ -14,6 +14,7 @@ import ( "github.com/Sirupsen/logrus" cfconfig "github.com/cloudflare/cfssl/config" + events "github.com/docker/go-events" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/connectionbroker" "github.com/docker/swarmkit/identity" @@ -123,11 +124,11 @@ func validateRootCAAndTLSCert(rootCA *RootCA, externalCARootPool *x509.CertPool, } // NewSecurityConfig initializes and returns a new SecurityConfig. -func NewSecurityConfig(rootCA *RootCA, krw *KeyReadWriter, tlsKeyPair *tls.Certificate, issuerInfo *IssuerInfo) (*SecurityConfig, error) { +func NewSecurityConfig(rootCA *RootCA, krw *KeyReadWriter, tlsKeyPair *tls.Certificate, issuerInfo *IssuerInfo) (*SecurityConfig, func() error, error) { // Create the Server TLS Credentials for this node. These will not be used by workers. serverTLSCreds, err := rootCA.NewServerTLSCredentials(tlsKeyPair) if err != nil { - return nil, err + return nil, nil, err } // Create a TLSConfig to be used when this node connects as a client to another remote node. @@ -135,7 +136,7 @@ func NewSecurityConfig(rootCA *RootCA, krw *KeyReadWriter, tlsKeyPair *tls.Certi // and managers always connect to remote managers. clientTLSCreds, err := rootCA.NewClientTLSCredentials(tlsKeyPair, ManagerRole) if err != nil { - return nil, err + return nil, nil, err } // Make a new TLS config for the external CA client without a @@ -146,18 +147,21 @@ func NewSecurityConfig(rootCA *RootCA, krw *KeyReadWriter, tlsKeyPair *tls.Certi MinVersion: tls.VersionTLS12, } + q := watch.NewQueue() + return &SecurityConfig{ rootCA: rootCA, keyReadWriter: krw, certificate: tlsKeyPair, issuerInfo: issuerInfo, + queue: q, externalCA: NewExternalCA(rootCA, externalCATLSConfig), ClientTLSCreds: clientTLSCreds, ServerTLSCreds: serverTLSCreds, externalCAClientRootPool: rootCA.Pool, - }, nil + }, q.Close, nil } // RootCA returns the root CA. @@ -200,11 +204,9 @@ func (s *SecurityConfig) UpdateRootCA(rootCA *RootCA, externalCARootPool *x509.C return s.updateTLSCredentials(s.certificate, s.issuerInfo) } -// SetWatch allows you to set a watch on the security config, in order to be notified of any changes -func (s *SecurityConfig) SetWatch(q *watch.Queue) { - s.mu.Lock() - defer s.mu.Unlock() - s.queue = q +// Watch allows you to set a watch on the security config, in order to be notified of any changes +func (s *SecurityConfig) Watch() (chan events.Event, func()) { + return s.queue.Watch() } // IssuerInfo returns the issuer subject and issuer public key @@ -382,7 +384,7 @@ func DownloadRootCA(ctx context.Context, paths CertPaths, token string, connBrok // LoadSecurityConfig loads TLS credentials from disk, or returns an error if // these credentials do not exist or are unusable. -func LoadSecurityConfig(ctx context.Context, rootCA RootCA, krw *KeyReadWriter, allowExpired bool) (*SecurityConfig, error) { +func LoadSecurityConfig(ctx context.Context, rootCA RootCA, krw *KeyReadWriter, allowExpired bool) (*SecurityConfig, func() error, error) { ctx = log.WithModule(ctx, "tls") // At this point we've successfully loaded the CA details from disk, or @@ -392,13 +394,13 @@ func LoadSecurityConfig(ctx context.Context, rootCA RootCA, krw *KeyReadWriter, // Read both the Cert and Key from disk cert, key, err := krw.Read() if err != nil { - return nil, err + return nil, nil, err } // Check to see if this certificate was signed by our CA, and isn't expired _, chains, err := ValidateCertChain(rootCA.Pool, cert, allowExpired) if err != nil { - return nil, err + return nil, nil, err } // ValidateChain, if successful, will always return at least 1 chain containing // at least 2 certificates: the leaf and the root. @@ -408,10 +410,10 @@ func LoadSecurityConfig(ctx context.Context, rootCA RootCA, krw *KeyReadWriter, // credentials keyPair, err := tls.X509KeyPair(cert, key) if err != nil { - return nil, err + return nil, nil, err } - secConfig, err := NewSecurityConfig(&rootCA, krw, &keyPair, &IssuerInfo{ + secConfig, cleanup, err := NewSecurityConfig(&rootCA, krw, &keyPair, &IssuerInfo{ Subject: issuer.RawSubject, PublicKey: issuer.RawSubjectPublicKeyInfo, }) @@ -421,7 +423,7 @@ func LoadSecurityConfig(ctx context.Context, rootCA RootCA, krw *KeyReadWriter, "node.role": secConfig.ClientTLSCreds.Role(), }).Debug("loaded node credentials") } - return secConfig, err + return secConfig, cleanup, err } // CertificateRequestConfig contains the information needed to request a @@ -450,7 +452,7 @@ type CertificateRequestConfig struct { // CreateSecurityConfig creates a new key and cert for this node, either locally // or via a remote CA. -func (rootCA RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWriter, config CertificateRequestConfig) (*SecurityConfig, error) { +func (rootCA RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWriter, config CertificateRequestConfig) (*SecurityConfig, func() error, error) { ctx = log.WithModule(ctx, "tls") // Create a new random ID for this certificate @@ -467,7 +469,7 @@ func (rootCA RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWrite tlsKeyPair, issuerInfo, err = rootCA.RequestAndSaveNewCertificates(ctx, krw, config) if err != nil { log.G(ctx).WithError(err).Error("failed to request and save new certificate") - return nil, err + return nil, nil, err } case nil: log.G(ctx).WithFields(logrus.Fields{ @@ -479,17 +481,17 @@ func (rootCA RootCA) CreateSecurityConfig(ctx context.Context, krw *KeyReadWrite "node.id": cn, "node.role": proposedRole, }).WithError(err).Errorf("failed to issue and save new certificate") - return nil, err + return nil, nil, err } - secConfig, err := NewSecurityConfig(&rootCA, krw, tlsKeyPair, issuerInfo) + secConfig, cleanup, err := NewSecurityConfig(&rootCA, krw, tlsKeyPair, issuerInfo) if err == nil { log.G(ctx).WithFields(logrus.Fields{ "node.id": secConfig.ClientTLSCreds.NodeID(), "node.role": secConfig.ClientTLSCreds.Role(), }).Debugf("new node credentials generated: %s", krw.Target()) } - return secConfig, err + return secConfig, cleanup, err } // TODO(cyli): currently we have to only update if it's a worker role - if we have a single root CA update path for diff --git a/ca/config_test.go b/ca/config_test.go index 5c28f70dac..5e3bb66b01 100644 --- a/ca/config_test.go +++ b/ca/config_test.go @@ -27,7 +27,6 @@ import ( "github.com/docker/swarmkit/manager/state" "github.com/docker/swarmkit/manager/state/store" "github.com/docker/swarmkit/testutils" - "github.com/docker/swarmkit/watch" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -101,12 +100,13 @@ func TestCreateSecurityConfigEmptyDir(t *testing.T) { // Remove all the contents from the temp dir and try again with a new node os.RemoveAll(tc.TempDir) krw := ca.NewKeyReadWriter(tc.Paths.Node, nil, nil) - nodeConfig, err := tc.RootCA.CreateSecurityConfig(tc.Context, krw, + nodeConfig, cancel, err := tc.RootCA.CreateSecurityConfig(tc.Context, krw, ca.CertificateRequestConfig{ Token: tc.WorkerToken, ConnBroker: tc.ConnBroker, }) assert.NoError(t, err) + cancel() assert.NotNil(t, nodeConfig) assert.NotNil(t, nodeConfig.ClientTLSCreds) assert.NotNil(t, nodeConfig.ServerTLSCreds) @@ -130,12 +130,13 @@ func TestCreateSecurityConfigNoCerts(t *testing.T) { assert.NoError(t, err) validateNodeConfig := func(rootCA *ca.RootCA) { - nodeConfig, err := rootCA.CreateSecurityConfig(tc.Context, krw, + nodeConfig, cancel, err := rootCA.CreateSecurityConfig(tc.Context, krw, ca.CertificateRequestConfig{ Token: tc.WorkerToken, ConnBroker: tc.ConnBroker, }) assert.NoError(t, err) + cancel() assert.NotNil(t, nodeConfig) assert.NotNil(t, nodeConfig.ClientTLSCreds) assert.NotNil(t, nodeConfig.ServerTLSCreds) @@ -184,11 +185,11 @@ func TestLoadSecurityConfigExpiredCert(t *testing.T) { invalidCert := cautils.ReDateCert(t, certBytes, tc.RootCA.Certs, s.Key, now.Add(time.Hour), now.Add(time.Hour*2)) require.NoError(t, ioutil.WriteFile(tc.Paths.Node.Cert, invalidCert, 0700)) - _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false) + _, _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false) require.Error(t, err) require.IsType(t, x509.CertificateInvalidError{}, errors.Cause(err)) - _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, true) + _, _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, true) require.Error(t, err) require.IsType(t, x509.CertificateInvalidError{}, errors.Cause(err)) @@ -196,13 +197,14 @@ func TestLoadSecurityConfigExpiredCert(t *testing.T) { invalidCert = cautils.ReDateCert(t, certBytes, tc.RootCA.Certs, s.Key, now.Add(-2*time.Minute), now.Add(-1*time.Minute)) require.NoError(t, ioutil.WriteFile(tc.Paths.Node.Cert, invalidCert, 0700)) - _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false) + _, _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false) require.Error(t, err) require.IsType(t, x509.CertificateInvalidError{}, errors.Cause(err)) // but it is valid if expiry is allowed - _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, true) + _, cancel, err := ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, true) require.NoError(t, err) + cancel() } func TestLoadSecurityConfigInvalidCert(t *testing.T) { @@ -219,7 +221,7 @@ some random garbage\n krw := ca.NewKeyReadWriter(tc.Paths.Node, nil, nil) - _, err := ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false) + _, _, err := ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false) assert.Error(t, err) } @@ -237,7 +239,7 @@ some random garbage\n krw := ca.NewKeyReadWriter(tc.Paths.Node, nil, nil) - _, err := ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false) + _, _, err := ca.LoadSecurityConfig(tc.Context, tc.RootCA, krw, false) assert.Error(t, err) } @@ -253,7 +255,7 @@ func TestLoadSecurityConfigIncorrectPassphrase(t *testing.T) { "nodeID", ca.WorkerRole, tc.Organization) require.NoError(t, err) - _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, ca.NewKeyReadWriter(paths.Node, nil, nil), false) + _, _, err = ca.LoadSecurityConfig(tc.Context, tc.RootCA, ca.NewKeyReadWriter(paths.Node, nil, nil), false) require.IsType(t, ca.ErrInvalidKEK{}, err) } @@ -277,7 +279,7 @@ func TestLoadSecurityConfigIntermediates(t *testing.T) { // loading the incomplete chain fails require.NoError(t, krw.Write(cautils.ECDSACertChain[0], cautils.ECDSACertChainKeys[0], nil)) - _, err = ca.LoadSecurityConfig(ctx, rootCA, krw, false) + _, _, err = ca.LoadSecurityConfig(ctx, rootCA, krw, false) require.Error(t, err) intermediate, err := helpers.ParseCertificatePEM(cautils.ECDSACertChain[1]) @@ -285,8 +287,9 @@ func TestLoadSecurityConfigIntermediates(t *testing.T) { // loading the complete chain succeeds require.NoError(t, krw.Write(append(cautils.ECDSACertChain[0], cautils.ECDSACertChain[1]...), cautils.ECDSACertChainKeys[0], nil)) - secConfig, err := ca.LoadSecurityConfig(ctx, rootCA, krw, false) + secConfig, cancel, err := ca.LoadSecurityConfig(ctx, rootCA, krw, false) require.NoError(t, err) + defer cancel() require.NotNil(t, secConfig) issuerInfo := secConfig.IssuerInfo() require.NotNil(t, issuerInfo) @@ -333,9 +336,10 @@ func TestSecurityConfigUpdateRootCA(t *testing.T) { defer os.RemoveAll(tempdir) configPaths := ca.NewConfigPaths(tempdir) - secConfig, err := rootCA.CreateSecurityConfig(tc.Context, + secConfig, cancel, err := rootCA.CreateSecurityConfig(tc.Context, ca.NewKeyReadWriter(configPaths.Node, nil, nil), ca.CertificateRequestConfig{}) require.NoError(t, err) + cancel() // 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 @@ -467,8 +471,9 @@ func TestSecurityConfigUpdateRootCAUpdateConsistentWithTLSCertificates(t *testin // that something else does the validation when loading the security config for the first // time and when getting new TLS credentials - secConfig, err := ca.NewSecurityConfig(&rootCA, krw, tlsKeyPair, issuerInfo) + secConfig, cancel, err := ca.NewSecurityConfig(&rootCA, krw, tlsKeyPair, issuerInfo) require.NoError(t, err) + cancel() // can't update the root CA or external pool to one that doesn't match the tls certs require.Error(t, secConfig.UpdateRootCA(&otherRootCA, rootCA.Pool)) @@ -486,7 +491,7 @@ func TestSecurityConfigUpdateRootCAUpdateConsistentWithTLSCertificates(t *testin } -func TestSecurityConfigSetWatch(t *testing.T) { +func TestSecurityConfigWatch(t *testing.T) { tc := cautils.NewTestCA(t) defer tc.Stop() @@ -494,11 +499,7 @@ func TestSecurityConfigSetWatch(t *testing.T) { require.NoError(t, err) issuer := secConfig.IssuerInfo() - w := watch.NewQueue() - defer w.Close() - secConfig.SetWatch(w) - - configWatch, configCancel := w.Watch() + configWatch, configCancel := secConfig.Watch() defer configCancel() require.NoError(t, ca.RenewTLSConfigNow(tc.Context, secConfig, tc.ConnBroker, tc.Paths.RootCA)) @@ -530,7 +531,6 @@ func TestSecurityConfigSetWatch(t *testing.T) { } configCancel() - w.Close() // ensure that we can still update tls certs and roots without error even though the watch is closed require.NoError(t, secConfig.UpdateRootCA(&tc.RootCA, tc.RootCA.Pool)) @@ -648,8 +648,9 @@ func TestRenewTLSConfigUpdatesRootOnUnknownAuthError(t *testing.T) { }, }) })) - secConfig, err := ca.NewSecurityConfig(testCase.initialRootCA, krw, tlsKeyPair, issuerInfo) + secConfig, qClose, err := ca.NewSecurityConfig(testCase.initialRootCA, krw, tlsKeyPair, issuerInfo) require.NoError(t, err) + defer qClose() paths := ca.NewConfigPaths(filepath.Join(tempdir, nodeID)) err = ca.RenewTLSConfigNow(tc.Context, secConfig, tc.ConnBroker, paths.RootCA) diff --git a/ca/external_test.go b/ca/external_test.go index 952f1e56cb..5736a172f3 100644 --- a/ca/external_test.go +++ b/ca/external_test.go @@ -29,9 +29,10 @@ func TestExternalCACrossSign(t *testing.T) { defer tc.Stop() paths := ca.NewConfigPaths(tc.TempDir) - secConfig, err := tc.RootCA.CreateSecurityConfig(tc.Context, + secConfig, cancel, err := tc.RootCA.CreateSecurityConfig(tc.Context, ca.NewKeyReadWriter(paths.Node, nil, nil), ca.CertificateRequestConfig{}) require.NoError(t, err) + cancel() externalCA := secConfig.ExternalCA() externalCA.UpdateURLs(tc.ExternalSigningServer.URL) diff --git a/ca/testutils/cautils.go b/ca/testutils/cautils.go index e4fd9f5708..b176245da3 100644 --- a/ca/testutils/cautils.go +++ b/ca/testutils/cautils.go @@ -51,12 +51,16 @@ type TestCA struct { ConnBroker *connectionbroker.Broker KeyReadWriter *ca.KeyReadWriter ctxCancel, watchCancel func() + securityConfigCleanups []func() error } // Stop cleans up after TestCA func (tc *TestCA) Stop() { tc.watchCancel() tc.ctxCancel() + for _, qClose := range tc.securityConfigCleanups { + qClose() + } os.RemoveAll(tc.TempDir) for _, conn := range tc.Conns { conn.Close() @@ -71,28 +75,23 @@ func (tc *TestCA) Stop() { // NewNodeConfig returns security config for a new node, given a role func (tc *TestCA) NewNodeConfig(role string) (*ca.SecurityConfig, error) { - withNonSigningRoot := tc.ExternalSigningServer != nil - return genSecurityConfig(tc.MemoryStore, tc.RootCA, tc.KeyReadWriter, role, tc.Organization, tc.TempDir, withNonSigningRoot) + return tc.NewNodeConfigOrg(role, tc.Organization) } // WriteNewNodeConfig returns security config for a new node, given a role // saving the generated key and certificates to disk func (tc *TestCA) WriteNewNodeConfig(role string) (*ca.SecurityConfig, error) { - withNonSigningRoot := tc.ExternalSigningServer != nil - return genSecurityConfig(tc.MemoryStore, tc.RootCA, tc.KeyReadWriter, role, tc.Organization, tc.TempDir, withNonSigningRoot) + return tc.NewNodeConfigOrg(role, tc.Organization) } // NewNodeConfigOrg returns security config for a new node, given a role and an org func (tc *TestCA) NewNodeConfigOrg(role, org string) (*ca.SecurityConfig, error) { withNonSigningRoot := tc.ExternalSigningServer != nil - return genSecurityConfig(tc.MemoryStore, tc.RootCA, tc.KeyReadWriter, role, org, tc.TempDir, withNonSigningRoot) -} - -// WriteNewNodeConfigOrg returns security config for a new node, given a role and an org -// saving the generated key and certificates to disk -func (tc *TestCA) WriteNewNodeConfigOrg(role, org string) (*ca.SecurityConfig, error) { - withNonSigningRoot := tc.ExternalSigningServer != nil - return genSecurityConfig(tc.MemoryStore, tc.RootCA, tc.KeyReadWriter, role, org, tc.TempDir, withNonSigningRoot) + s, qClose, err := genSecurityConfig(tc.MemoryStore, tc.RootCA, tc.KeyReadWriter, role, org, tc.TempDir, withNonSigningRoot) + if err != nil { + tc.securityConfigCleanups = append(tc.securityConfigCleanups, qClose) + } + return s, err } // External controls whether or not NewTestCA() will create a TestCA server @@ -177,13 +176,13 @@ func NewTestCAFromAPIRootCA(t *testing.T, tempBaseDir string, apiRootCA api.Root krw = krwGenerators[0](paths.Node) } - managerConfig, err := genSecurityConfig(s, rootCA, krw, ca.ManagerRole, organization, "", External) + managerConfig, qClose1, err := genSecurityConfig(s, rootCA, krw, ca.ManagerRole, organization, "", External) assert.NoError(t, err) - managerDiffOrgConfig, err := genSecurityConfig(s, rootCA, krw, ca.ManagerRole, "swarm-test-org-2", "", External) + managerDiffOrgConfig, qClose2, err := genSecurityConfig(s, rootCA, krw, ca.ManagerRole, "swarm-test-org-2", "", External) assert.NoError(t, err) - workerConfig, err := genSecurityConfig(s, rootCA, krw, ca.WorkerRole, organization, "", External) + workerConfig, qClose3, err := genSecurityConfig(s, rootCA, krw, ca.WorkerRole, organization, "", External) assert.NoError(t, err) l, err := net.Listen("tcp", "127.0.0.1:0") @@ -263,26 +262,27 @@ func NewTestCAFromAPIRootCA(t *testing.T, tempBaseDir string, apiRootCA api.Root conns := []*grpc.ClientConn{conn1, conn2, conn3, conn4} return &TestCA{ - RootCA: rootCA, - ExternalSigningServer: externalSigningServer, - MemoryStore: s, - TempDir: tempBaseDir, - Organization: organization, - Paths: paths, - Context: ctx, - CAClients: caClients, - NodeCAClients: nodeCAClients, - Conns: conns, - Addr: l.Addr().String(), - Server: grpcServer, - ServingSecurityConfig: managerConfig, - CAServer: caServer, - WorkerToken: clusterObj.RootCA.JoinTokens.Worker, - ManagerToken: clusterObj.RootCA.JoinTokens.Manager, - ConnBroker: connectionbroker.New(remotes), - KeyReadWriter: krw, - watchCancel: clusterWatchCancel, - ctxCancel: ctxCancel, + RootCA: rootCA, + ExternalSigningServer: externalSigningServer, + MemoryStore: s, + TempDir: tempBaseDir, + Organization: organization, + Paths: paths, + Context: ctx, + CAClients: caClients, + NodeCAClients: nodeCAClients, + Conns: conns, + Addr: l.Addr().String(), + Server: grpcServer, + ServingSecurityConfig: managerConfig, + CAServer: caServer, + WorkerToken: clusterObj.RootCA.JoinTokens.Worker, + ManagerToken: clusterObj.RootCA.JoinTokens.Manager, + ConnBroker: connectionbroker.New(remotes), + KeyReadWriter: krw, + watchCancel: clusterWatchCancel, + ctxCancel: ctxCancel, + securityConfigCleanups: []func() error{qClose1, qClose2, qClose3}, } } @@ -314,14 +314,14 @@ func createNode(s *store.MemoryStore, nodeID, role string, csr, cert []byte) err return err } -func genSecurityConfig(s *store.MemoryStore, rootCA ca.RootCA, krw *ca.KeyReadWriter, role, org, tmpDir string, nonSigningRoot bool) (*ca.SecurityConfig, error) { +func genSecurityConfig(s *store.MemoryStore, rootCA ca.RootCA, krw *ca.KeyReadWriter, role, org, tmpDir string, nonSigningRoot bool) (*ca.SecurityConfig, func() error, error) { req := &cfcsr.CertificateRequest{ KeyRequest: cfcsr.NewBasicKeyRequest(), } csr, key, err := cfcsr.ParseRequest(req) if err != nil { - return nil, err + return nil, nil, err } // Obtain a signed Certificate @@ -329,29 +329,29 @@ func genSecurityConfig(s *store.MemoryStore, rootCA ca.RootCA, krw *ca.KeyReadWr certChain, err := rootCA.ParseValidateAndSignCSR(csr, nodeID, role, org) if err != nil { - return nil, err + return nil, nil, err } // If we were instructed to persist the files if tmpDir != "" { paths := ca.NewConfigPaths(tmpDir) if err := ioutil.WriteFile(paths.Node.Cert, certChain, 0644); err != nil { - return nil, err + return nil, nil, err } if err := ioutil.WriteFile(paths.Node.Key, key, 0600); err != nil { - return nil, err + return nil, nil, err } } // Load a valid tls.Certificate from the chain and the key nodeCert, err := tls.X509KeyPair(certChain, key) if err != nil { - return nil, err + return nil, nil, err } err = createNode(s, nodeID, role, csr, certChain) if err != nil { - return nil, err + return nil, nil, err } signingCert := rootCA.Certs @@ -360,7 +360,7 @@ func genSecurityConfig(s *store.MemoryStore, rootCA ca.RootCA, krw *ca.KeyReadWr } parsedCert, err := helpers.ParseCertificatePEM(signingCert) if err != nil { - return nil, err + return nil, nil, err } if nonSigningRoot { diff --git a/manager/controlapi/ca_rotation_test.go b/manager/controlapi/ca_rotation_test.go index 9187ffb89e..6b029074c5 100644 --- a/manager/controlapi/ca_rotation_test.go +++ b/manager/controlapi/ca_rotation_test.go @@ -63,8 +63,9 @@ func getSecurityConfig(t *testing.T, localRootCA *ca.RootCA, cluster *api.Cluste require.NoError(t, err) defer os.RemoveAll(tempdir) paths := ca.NewConfigPaths(tempdir) - secConfig, err := localRootCA.CreateSecurityConfig(context.Background(), ca.NewKeyReadWriter(paths.Node, nil, nil), ca.CertificateRequestConfig{}) + secConfig, cancel, err := localRootCA.CreateSecurityConfig(context.Background(), ca.NewKeyReadWriter(paths.Node, nil, nil), ca.CertificateRequestConfig{}) require.NoError(t, err) + cancel() require.NoError(t, ca.NewServer(store.NewMemoryStore(nil), secConfig, paths.RootCA).UpdateRootCA(context.Background(), cluster)) return secConfig diff --git a/node/node.go b/node/node.go index 29badd06ed..77fe5b3d75 100644 --- a/node/node.go +++ b/node/node.go @@ -28,7 +28,6 @@ import ( "github.com/docker/swarmkit/manager" "github.com/docker/swarmkit/manager/encryption" "github.com/docker/swarmkit/remotes" - "github.com/docker/swarmkit/watch" "github.com/docker/swarmkit/xnet" grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" "github.com/pkg/errors" @@ -276,10 +275,11 @@ func (n *Node) run(ctx context.Context) (err error) { }(ctx) paths := ca.NewConfigPaths(filepath.Join(n.config.StateDir, certDirectory)) - securityConfig, err := n.loadSecurityConfig(ctx, paths) + securityConfig, secConfigCancel, err := n.loadSecurityConfig(ctx, paths) if err != nil { return err } + defer secConfigCancel() renewer := ca.NewTLSRenewer(securityConfig, n.connBroker, paths.RootCA) @@ -509,11 +509,8 @@ waitPeer: default: } - secChangeQueue := watch.NewQueue() - defer secChangeQueue.Close() - secChangesCh, secChangesCancel := secChangeQueue.Watch() + secChangesCh, secChangesCancel := securityConfig.Watch() defer secChangesCancel() - securityConfig.SetWatch(secChangeQueue) rootCA := securityConfig.RootCA() issuer := securityConfig.IssuerInfo() @@ -668,28 +665,31 @@ func (n *Node) Remotes() []api.Peer { return remotes } -func (n *Node) loadSecurityConfig(ctx context.Context, paths *ca.SecurityConfigPaths) (*ca.SecurityConfig, error) { - var securityConfig *ca.SecurityConfig +func (n *Node) loadSecurityConfig(ctx context.Context, paths *ca.SecurityConfigPaths) (*ca.SecurityConfig, func() error, error) { + var ( + securityConfig *ca.SecurityConfig + cancel func() error + ) krw := ca.NewKeyReadWriter(paths.Node, n.unlockKey, &manager.RaftDEKData{}) if err := krw.Migrate(); err != nil { - return nil, err + return nil, nil, err } // Check if we already have a valid certificates on disk. rootCA, err := ca.GetLocalRootCA(paths.RootCA) if err != nil && err != ca.ErrNoLocalRootCA { - return nil, err + return nil, nil, err } if err == nil { // if forcing a new cluster, we allow the certificates to be expired - a new set will be generated - securityConfig, err = ca.LoadSecurityConfig(ctx, rootCA, krw, n.config.ForceNewCluster) + securityConfig, cancel, err = ca.LoadSecurityConfig(ctx, rootCA, krw, n.config.ForceNewCluster) if err != nil { _, isInvalidKEK := errors.Cause(err).(ca.ErrInvalidKEK) if isInvalidKEK { - return nil, ErrInvalidUnlockKey + return nil, nil, ErrInvalidUnlockKey } else if !os.IsNotExist(err) { - return nil, errors.Wrapf(err, "error while loading TLS certificate in %s", paths.Node.Cert) + return nil, nil, errors.Wrapf(err, "error while loading TLS certificate in %s", paths.Node.Cert) } } } @@ -704,16 +704,16 @@ func (n *Node) loadSecurityConfig(ctx context.Context, paths *ca.SecurityConfigP krw = ca.NewKeyReadWriter(paths.Node, n.unlockKey, &manager.RaftDEKData{}) rootCA, err = ca.CreateRootCA(ca.DefaultRootCN) if err != nil { - return nil, err + return nil, nil, err } if err := ca.SaveRootCA(rootCA, paths.RootCA); err != nil { - return nil, err + return nil, nil, err } log.G(ctx).Debug("generated CA key and certificate") } else if err == ca.ErrNoLocalRootCA { // from previous error loading the root CA from disk rootCA, err = ca.DownloadRootCA(ctx, paths.RootCA, n.config.JoinToken, n.connBroker) if err != nil { - return nil, err + return nil, nil, err } log.G(ctx).Debug("downloaded CA certificate") } @@ -724,25 +724,25 @@ func (n *Node) loadSecurityConfig(ctx context.Context, paths *ca.SecurityConfigP // - We wait for CreateSecurityConfig to finish since we need a certificate to operate. // Attempt to load certificate from disk - securityConfig, err = ca.LoadSecurityConfig(ctx, rootCA, krw, n.config.ForceNewCluster) + securityConfig, cancel, err = ca.LoadSecurityConfig(ctx, rootCA, krw, n.config.ForceNewCluster) if err == nil { log.G(ctx).WithFields(logrus.Fields{ "node.id": securityConfig.ClientTLSCreds.NodeID(), }).Debugf("loaded TLS certificate") } else { if _, ok := errors.Cause(err).(ca.ErrInvalidKEK); ok { - return nil, ErrInvalidUnlockKey + return nil, nil, ErrInvalidUnlockKey } log.G(ctx).WithError(err).Debugf("no node credentials found in: %s", krw.Target()) - securityConfig, err = rootCA.CreateSecurityConfig(ctx, krw, ca.CertificateRequestConfig{ + securityConfig, cancel, err = rootCA.CreateSecurityConfig(ctx, krw, ca.CertificateRequestConfig{ Token: n.config.JoinToken, Availability: n.config.Availability, ConnBroker: n.connBroker, }) if err != nil { - return nil, err + return nil, nil, err } } } @@ -753,7 +753,7 @@ func (n *Node) loadSecurityConfig(ctx context.Context, paths *ca.SecurityConfigP n.roleCond.Broadcast() n.Unlock() - return securityConfig, nil + return securityConfig, cancel, nil } func (n *Node) initManagerConnection(ctx context.Context, ready chan<- struct{}) error { diff --git a/node/node_test.go b/node/node_test.go index 5bfa5b7252..8f10f14eea 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -40,8 +40,9 @@ func TestLoadSecurityConfigNewNode(t *testing.T) { AutoLockManagers: autoLockManagers, }) require.NoError(t, err) - securityConfig, err := node.loadSecurityConfig(context.Background(), paths) + securityConfig, cancel, err := node.loadSecurityConfig(context.Background(), paths) require.NoError(t, err) + defer cancel() require.NotNil(t, securityConfig) unencryptedReader := ca.NewKeyReadWriter(paths.Node, nil, nil) @@ -70,8 +71,9 @@ func TestLoadSecurityConfigPartialCertsOnDisk(t *testing.T) { StateDir: tempdir, }) require.NoError(t, err) - securityConfig, err := node.loadSecurityConfig(context.Background(), paths) + securityConfig, cancel, err := node.loadSecurityConfig(context.Background(), paths) require.NoError(t, err) + defer cancel() require.NotNil(t, securityConfig) cert, key, err := securityConfig.KeyReader().Read() @@ -88,8 +90,9 @@ func TestLoadSecurityConfigPartialCertsOnDisk(t *testing.T) { StateDir: tempdir, }) require.NoError(t, err) - securityConfig, err = node.loadSecurityConfig(context.Background(), paths) + securityConfig, cancel, err = node.loadSecurityConfig(context.Background(), paths) require.NoError(t, err) + defer cancel() require.NotNil(t, securityConfig) newCert, newKey, err := securityConfig.KeyReader().Read() @@ -129,8 +132,9 @@ func TestLoadSecurityConfigLoadFromDisk(t *testing.T) { UnlockKey: []byte("passphrase"), }) require.NoError(t, err) - securityConfig, err := node.loadSecurityConfig(context.Background(), paths) + securityConfig, cancel, err := node.loadSecurityConfig(context.Background(), paths) require.NoError(t, err) + defer cancel() require.NotNil(t, securityConfig) // Invalid passphrase @@ -140,7 +144,7 @@ func TestLoadSecurityConfigLoadFromDisk(t *testing.T) { JoinToken: tc.ManagerToken, }) require.NoError(t, err) - _, err = node.loadSecurityConfig(context.Background(), paths) + _, _, err = node.loadSecurityConfig(context.Background(), paths) require.Equal(t, ErrInvalidUnlockKey, err) // Invalid CA @@ -154,7 +158,7 @@ func TestLoadSecurityConfigLoadFromDisk(t *testing.T) { UnlockKey: []byte("passphrase"), }) require.NoError(t, err) - _, err = node.loadSecurityConfig(context.Background(), paths) + _, _, err = node.loadSecurityConfig(context.Background(), paths) require.IsType(t, x509.UnknownAuthorityError{}, errors.Cause(err)) } @@ -174,7 +178,7 @@ func TestLoadSecurityConfigDownloadAllCerts(t *testing.T) { JoinAddr: "127.0.0.1:12", }) require.NoError(t, err) - _, err = node.loadSecurityConfig(context.Background(), paths) + _, _, err = node.loadSecurityConfig(context.Background(), paths) require.Error(t, err) tc := cautils.NewTestCA(t) @@ -189,8 +193,9 @@ func TestLoadSecurityConfigDownloadAllCerts(t *testing.T) { JoinToken: tc.ManagerToken, }) require.NoError(t, err) - _, err = node.loadSecurityConfig(context.Background(), paths) + _, cancel, err := node.loadSecurityConfig(context.Background(), paths) require.NoError(t, err) + cancel() // the TLS key and cert were written to disk unencrypted _, _, err = ca.NewKeyReadWriter(paths.Node, nil, nil).Read() @@ -233,8 +238,9 @@ func TestLoadSecurityConfigDownloadAllCerts(t *testing.T) { JoinToken: tc.ManagerToken, }) require.NoError(t, err) - _, err = node.loadSecurityConfig(context.Background(), paths) + _, cancel, err = node.loadSecurityConfig(context.Background(), paths) require.NoError(t, err) + cancel() // make sure the CA cert has not been replaced readCertBytes, err := ioutil.ReadFile(paths.RootCA.Cert) @@ -303,9 +309,10 @@ func TestAgentRespectsDispatcherRootCAUpdate(t *testing.T) { rootCA, err := ca.CreateRootCA("rootCN") require.NoError(t, err) require.NoError(t, ca.SaveRootCA(rootCA, paths.RootCA)) - managerSecConfig, err := rootCA.CreateSecurityConfig(context.Background(), + managerSecConfig, cancel, err := rootCA.CreateSecurityConfig(context.Background(), ca.NewKeyReadWriter(paths.Node, nil, nil), ca.CertificateRequestConfig{}) require.NoError(t, err) + defer cancel() _, _, err = rootCA.IssueAndSaveNewCertificates(ca.NewKeyReadWriter(paths.Node, nil, nil), "workerNode", ca.WorkerRole, managerSecConfig.ServerTLSCreds.Organization())