Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 2 additions & 8 deletions ca/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 30 additions & 3 deletions ca/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ func (s *SecurityConfig) RootCA() *RootCA {
return s.rootCA
}

// ExternalCA returns the external CA.
func (s *SecurityConfig) ExternalCA() *ExternalCA {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need locking?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return s.externalCA
}

// KeyWriter returns the object that can write keys to disk
func (s *SecurityConfig) KeyWriter() KeyWriter {
return s.keyReadWriter
Expand All @@ -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
Expand Down
98 changes: 98 additions & 0 deletions ca/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()

Expand Down
33 changes: 17 additions & 16 deletions ca/testutils/cautils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 11 additions & 0 deletions ca/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to clone instead of directly modifying it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that originally, but it's not really safe to modify a tls.Config while it's being used I think. I ended up with a data race in the tests, since a tls.Config stores some handshake/session state. The comment for tls.Config contains:

After one has been passed to a TLS function it must not be modified.

config.RootCAs = rootCAs
config.ClientCAs = clientCAs
c.config = config
}

// Config returns the current underlying TLS config.
func (c *MutableTLSCreds) Config() *tls.Config {
c.Lock()
Expand Down