Skip to content

[ca] Refactor SecurityConfig to automatically create a watch.Queue #2274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diogomonica merged 1 commit into from
Jun 26, 2017
Merged

[ca] Refactor SecurityConfig to automatically create a watch.Queue #2274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ca/certificates_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
42 changes: 22 additions & 20 deletions ca/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -123,19 +124,19 @@ 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.
// We're using ManagerRole as remote serverName for TLS host verification because both workers
// 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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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,
})
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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{
Expand All @@ -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
Expand Down
45 changes: 23 additions & 22 deletions ca/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -184,25 +185,26 @@ 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))

// a cert that is expired is not valid if expiry is not allowed
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) {
Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -277,16 +279,17 @@ 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])
require.NoError(t, err)

// 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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -486,19 +491,15 @@ func TestSecurityConfigUpdateRootCAUpdateConsistentWithTLSCertificates(t *testin

}

func TestSecurityConfigSetWatch(t *testing.T) {
func TestSecurityConfigWatch(t *testing.T) {
tc := cautils.NewTestCA(t)
defer tc.Stop()

secConfig, err := tc.NewNodeConfig(ca.ManagerRole)
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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion ca/external_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading