From 7bef8ec1d3e10c73d9659869055faaf02d90088a Mon Sep 17 00:00:00 2001 From: Ying Li Date: Thu, 1 Jun 2017 11:02:48 -0700 Subject: [PATCH] Fix the UpdateRootCA function to also update the ExternalCA's rootCA, since if it's not updated when getting a cert signed by an external CA, no intermediates are appended Signed-off-by: Ying Li --- ca/certificates_test.go | 21 ++++++++--- ca/config.go | 2 + ca/config_test.go | 40 +++++++++++++++++--- ca/external.go | 13 +++++-- ca/server_test.go | 11 +++++- ca/testutils/cautils.go | 81 +++++++++++++++++------------------------ 6 files changed, 103 insertions(+), 65 deletions(-) diff --git a/ca/certificates_test.go b/ca/certificates_test.go index 6620802bb7..dc34141dd7 100644 --- a/ca/certificates_test.go +++ b/ca/certificates_test.go @@ -383,15 +383,20 @@ func TestRequestAndSaveNewCertificatesWithIntermediates(t *testing.T) { t.Parallel() // use a RootCA with an intermediate - rca, err := ca.NewRootCA(cautils.ECDSACertChain[2], cautils.ECDSACertChain[1], cautils.ECDSACertChainKeys[1], - ca.DefaultNodeCertExpiration, append([]byte(" "), cautils.ECDSACertChain[1]...)) - require.NoError(t, err) - + apiRootCA := api.RootCA{ + CACert: cautils.ECDSACertChain[2], + CAKey: cautils.ECDSACertChainKeys[2], + RootRotation: &api.RootRotation{ + CACert: cautils.ECDSACertChain[1], + CAKey: cautils.ECDSACertChainKeys[1], + CrossSignedCACert: concat([]byte(" "), cautils.ECDSACertChain[1]), + }, + } tempdir, err := ioutil.TempDir("", "test-request-and-save-new-certificates") require.NoError(t, err) defer os.RemoveAll(tempdir) - tc := cautils.NewTestCAFromRootCA(t, tempdir, rca, nil) + tc := cautils.NewTestCAFromAPIRootCA(t, tempdir, apiRootCA, nil) issuerInfo, parsedCerts := testRequestAndSaveNewCertificates(t, tc) require.Len(t, parsedCerts, 2) @@ -1245,6 +1250,10 @@ func TestRootCAWithCrossSignedIntermediates(t *testing.T) { newRoot, err := ca.NewRootCA(fauxRootCert, fauxRootCert, cautils.ECDSACertChainKeys[1], ca.DefaultNodeCertExpiration, nil) require.NoError(t, err) + apiNewRoot := api.RootCA{ + CACert: fauxRootCert, + CAKey: cautils.ECDSACertChainKeys[1], + } checkValidateAgainstAllRoots := func(cert []byte) { for i, root := range []ca.RootCA{signWithIntermediate, oldRoot, newRoot} { @@ -1272,7 +1281,7 @@ func TestRootCAWithCrossSignedIntermediates(t *testing.T) { } // create an external signing server that generates leaf certs with the new root (but does not append the intermediate) - tc := cautils.NewTestCAFromRootCA(t, tempdir, newRoot, nil) + tc := cautils.NewTestCAFromAPIRootCA(t, tempdir, apiNewRoot, nil) defer tc.Stop() // we need creds that trust both the old and new root in order to connect to the test CA, and we want this root CA to diff --git a/ca/config.go b/ca/config.go index 2aaaf78aba..a853fa6c16 100644 --- a/ca/config.go +++ b/ca/config.go @@ -157,6 +157,8 @@ func (s *SecurityConfig) UpdateRootCA(rootCA *RootCA, externalCARootPool *x509.C s.rootCA = rootCA s.externalCAClientRootPool = externalCARootPool + s.externalCA.UpdateRootCA(rootCA) + return s.updateTLSCredentials(s.certificate, s.issuerInfo) } diff --git a/ca/config_test.go b/ca/config_test.go index 9294735777..67b4d626b5 100644 --- a/ca/config_test.go +++ b/ca/config_test.go @@ -133,7 +133,10 @@ func TestCreateSecurityConfigNoCerts(t *testing.T) { assert.NotNil(t, nodeConfig) assert.NotNil(t, nodeConfig.ClientTLSCreds) assert.NotNil(t, nodeConfig.ServerTLSCreds) - assert.Equal(t, tc.RootCA, *nodeConfig.RootCA()) + // tc.RootCA can maybe sign, and the node root CA can also maybe sign, so we want to just compare the root + // certs and intermediates + assert.Equal(t, tc.RootCA.Certs, nodeConfig.RootCA().Certs) + assert.Equal(t, tc.RootCA.Intermediates, nodeConfig.RootCA().Intermediates) issuerInfo := nodeConfig.IssuerInfo() assert.NotNil(t, issuerInfo) @@ -299,6 +302,8 @@ func TestLoadSecurityConfigIntermediates(t *testing.T) { conn.Close() } +// When the root CA is updated on the security config, the root pools are updated +// and the external CA's rootCA is also updated. func TestSecurityConfigUpdateRootCA(t *testing.T) { tc := testutils.NewTestCA(t) defer tc.Stop() @@ -308,6 +313,7 @@ func TestSecurityConfigUpdateRootCA(t *testing.T) { // 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, cert, key, ca.DefaultNodeCertExpiration, nil) require.NoError(t, err) @@ -370,11 +376,17 @@ func TestSecurityConfigUpdateRootCA(t *testing.T) { 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) + // update the root CA on the "original security config to support both the old root + // and the "new root" (the testing CA root). Also make sure this root CA has an + // intermediate; we won't use it for anything, just make sure that newly generated TLS + // certs have the intermediate appended. + someOtherRootCA, err := ca.CreateRootCA("someOtherRootCA") + require.NoError(t, err) + intermediate, err := someOtherRootCA.CrossSignCACertificate(cert) + require.NoError(t, err) rSigner, err := rootCA.Signer() require.NoError(t, err) - updatedRootCA, err := ca.NewRootCA(append(rootCA.Certs, tc.RootCA.Certs...), rSigner.Cert, rSigner.Key, ca.DefaultNodeCertExpiration, nil) + updatedRootCA, err := ca.NewRootCA(concat(rootCA.Certs, tc.RootCA.Certs, someOtherRootCA.Certs), rSigner.Cert, rSigner.Key, ca.DefaultNodeCertExpiration, intermediate) require.NoError(t, err) err = secConfig.UpdateRootCA(&updatedRootCA, updatedRootCA.Pool) require.NoError(t, err) @@ -387,15 +399,31 @@ func TestSecurityConfigUpdateRootCA(t *testing.T) { 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 + // make sure any generated certs after updating contain the intermediate + var generatedCert []byte if testutils.External { + // we can also now connect to the test CA's external signing server secConfig.ExternalCA().UpdateURLs(externalServer.URL) - _, err := secConfig.ExternalCA().Sign(context.Background(), req) + generatedCert, err = secConfig.ExternalCA().Sign(context.Background(), req) + require.NoError(t, err) + } else { + krw := ca.NewKeyReadWriter(configPaths.Node, nil, nil) + _, _, err := secConfig.RootCA().IssueAndSaveNewCertificates(krw, "cn", "ou", "org") + require.NoError(t, err) + generatedCert, _, err = krw.Read() require.NoError(t, err) } + + parsedCerts, err := helpers.ParseCertificatesPEM(generatedCert) + require.NoError(t, err) + require.Len(t, parsedCerts, 2) + parsedIntermediate, err := helpers.ParseCertificatePEM(intermediate) + require.NoError(t, err) + require.Equal(t, parsedIntermediate, parsedCerts[1]) } func TestSecurityConfigSetWatch(t *testing.T) { diff --git a/ca/external.go b/ca/external.go index c4b26a6c77..11a6f87558 100644 --- a/ca/external.go +++ b/ca/external.go @@ -82,8 +82,7 @@ func (eca *ExternalCA) UpdateTLSConfig(tlsConfig *tls.Config) { } } -// UpdateURLs updates the list of CSR API endpoints by setting it to the given -// urls. +// UpdateURLs updates the list of CSR API endpoints by setting it to the given urls. func (eca *ExternalCA) UpdateURLs(urls ...string) { eca.mu.Lock() defer eca.mu.Unlock() @@ -91,6 +90,13 @@ func (eca *ExternalCA) UpdateURLs(urls ...string) { eca.urls = urls } +// UpdateRootCA changes the root CA used to append intermediates +func (eca *ExternalCA) UpdateRootCA(rca *RootCA) { + eca.mu.Lock() + eca.rootCA = rca + eca.mu.Unlock() +} + // Sign signs a new certificate by proxying the given certificate signing // request to an external CFSSL API server. func (eca *ExternalCA) Sign(ctx context.Context, req signer.SignRequest) (cert []byte, err error) { @@ -99,6 +105,7 @@ func (eca *ExternalCA) Sign(ctx context.Context, req signer.SignRequest) (cert [ eca.mu.Lock() urls := eca.urls client := eca.client + intermediates := eca.rootCA.Intermediates eca.mu.Unlock() if len(urls) == 0 { @@ -117,7 +124,7 @@ func (eca *ExternalCA) Sign(ctx context.Context, req signer.SignRequest) (cert [ cert, err = makeExternalSignRequest(requestCtx, client, url, csrJSON) cancel() if err == nil { - return append(cert, eca.rootCA.Intermediates...), err + return append(cert, intermediates...), err } logrus.Debugf("unable to proxy certificate signing request to %s: %s", url, err) } diff --git a/ca/server_test.go b/ca/server_test.go index 1ed48f9e2c..f0d25ede84 100644 --- a/ca/server_test.go +++ b/ca/server_test.go @@ -538,11 +538,18 @@ func TestCAServerUpdateRootCA(t *testing.T) { if testCase.externalCertSignedBy != nil { require.NoError(t, err) - parsed, err := helpers.ParseCertificatePEM(signedCert) + parsed, err := helpers.ParseCertificatesPEM(signedCert) require.NoError(t, err) rootPool := x509.NewCertPool() rootPool.AppendCertsFromPEM(testCase.externalCertSignedBy) - _, err = parsed.Verify(x509.VerifyOptions{Roots: rootPool}) + var intermediatePool *x509.CertPool + if len(parsed) > 1 { + intermediatePool = x509.NewCertPool() + for _, cert := range parsed[1:] { + intermediatePool.AddCert(cert) + } + } + _, err = parsed[0].Verify(x509.VerifyOptions{Roots: rootPool, Intermediates: intermediatePool}) require.NoError(t, err) } else { require.Equal(t, ca.ErrNoExternalCAURLs, err) diff --git a/ca/testutils/cautils.go b/ca/testutils/cautils.go index 532943181c..a1cce37f04 100644 --- a/ca/testutils/cautils.go +++ b/ca/testutils/cautils.go @@ -9,7 +9,6 @@ import ( "io/ioutil" "net" "os" - "path/filepath" "testing" "time" @@ -103,17 +102,21 @@ var External bool func NewTestCA(t *testing.T, krwGenerators ...func(ca.CertPaths) *ca.KeyReadWriter) *TestCA { tempdir, err := ioutil.TempDir("", "swarm-ca-test-") require.NoError(t, err) - paths := ca.NewConfigPaths(tempdir) - rootCA, err := createAndWriteRootCA("swarm-test-CA", paths.RootCA, ca.DefaultNodeCertExpiration) + cert, key, err := CreateRootCertAndKey("swarm-test-CA") require.NoError(t, err) + apiRootCA := api.RootCA{ + CACert: cert, + CAKey: key, + } - return NewTestCAFromRootCA(t, tempdir, rootCA, krwGenerators) + return NewTestCAFromAPIRootCA(t, tempdir, apiRootCA, krwGenerators) } -// NewTestCAFromRootCA is a helper method that creates a TestCA and a bunch of default -// connections and security configs, given a temp directory and a RootCA to use for signing. -func NewTestCAFromRootCA(t *testing.T, tempBaseDir string, rootCA ca.RootCA, krwGenerators []func(ca.CertPaths) *ca.KeyReadWriter) *TestCA { +// NewTestCAFromAPIRootCA is a helper method that creates a TestCA and a bunch of default +// connections and security configs, given a temp directory and an api.RootCA to use for creating +// a cluster and for signing. +func NewTestCAFromAPIRootCA(t *testing.T, tempBaseDir string, apiRootCA api.RootCA, krwGenerators []func(ca.CertPaths) *ca.KeyReadWriter) *TestCA { s := store.NewMemoryStore(&stateutils.MockProposer{}) paths := ca.NewConfigPaths(tempBaseDir) @@ -123,10 +126,24 @@ func NewTestCAFromRootCA(t *testing.T, tempBaseDir string, rootCA ca.RootCA, krw externalSigningServer *ExternalSigningServer externalCAs []*api.ExternalCA err error + rootCA ca.RootCA ) + if apiRootCA.RootRotation != nil { + rootCA, err = ca.NewRootCA( + apiRootCA.CACert, apiRootCA.RootRotation.CACert, apiRootCA.RootRotation.CAKey, ca.DefaultNodeCertExpiration, apiRootCA.RootRotation.CrossSignedCACert) + } else { + rootCA, err = ca.NewRootCA( + apiRootCA.CACert, apiRootCA.CACert, apiRootCA.CAKey, ca.DefaultNodeCertExpiration, nil) + + } + require.NoError(t, err) + + // Write the root certificate to disk, using decent permissions + require.NoError(t, ioutils.AtomicWriteFile(paths.RootCA.Cert, apiRootCA.CACert, 0644)) + if External { - // Start the CA API server. + // Start the CA API server - ensure that the external server doesn't have any intermediates externalSigningServer, err = NewExternalSigningServer(rootCA, tempBaseDir) assert.NoError(t, err) externalCAs = []*api.ExternalCA{ @@ -175,7 +192,7 @@ func NewTestCAFromRootCA(t *testing.T, tempBaseDir string, rootCA ca.RootCA, krw serverOpts := []grpc.ServerOption{grpc.Creds(managerConfig.ServerTLSCreds)} grpcServer := grpc.NewServer(serverOpts...) - clusterObj := createClusterObject(t, s, organization, &rootCA, externalCAs...) + clusterObj := createClusterObject(t, s, organization, apiRootCA, &rootCA, externalCAs...) caServer := ca.NewServer(s, managerConfig, paths.RootCA) caServer.SetReconciliationRetryInterval(50 * time.Millisecond) @@ -338,7 +355,7 @@ func genSecurityConfig(s *store.MemoryStore, rootCA ca.RootCA, krw *ca.KeyReadWr }) } -func createClusterObject(t *testing.T, s *store.MemoryStore, clusterID string, rootCA *ca.RootCA, externalCAs ...*api.ExternalCA) *api.Cluster { +func createClusterObject(t *testing.T, s *store.MemoryStore, clusterID string, apiRootCA api.RootCA, caRootCA *ca.RootCA, externalCAs ...*api.ExternalCA) *api.Cluster { cluster := &api.Cluster{ ID: clusterID, Spec: api.ClusterSpec{ @@ -349,16 +366,13 @@ func createClusterObject(t *testing.T, s *store.MemoryStore, clusterID string, r ExternalCAs: externalCAs, }, }, - RootCA: api.RootCA{ - CACert: rootCA.Certs, - JoinTokens: api.JoinTokens{ - Worker: ca.GenerateJoinToken(rootCA), - Manager: ca.GenerateJoinToken(rootCA), - }, - }, + RootCA: apiRootCA, + } + if cluster.RootCA.JoinTokens.Worker == "" { + cluster.RootCA.JoinTokens.Worker = ca.GenerateJoinToken(caRootCA) } - if s, err := rootCA.Signer(); err == nil && !External { - cluster.RootCA.CAKey = s.Key + if cluster.RootCA.JoinTokens.Manager == "" { + cluster.RootCA.JoinTokens.Manager = ca.GenerateJoinToken(caRootCA) } assert.NoError(t, s.Update(func(tx store.Tx) error { store.CreateCluster(tx, cluster) @@ -381,35 +395,6 @@ func CreateRootCertAndKey(rootCN string) ([]byte, []byte, error) { return cert, key, err } -// createAndWriteRootCA creates a Certificate authority for a new Swarm Cluster. -// We're copying ca.CreateRootCA, so we can have smaller key-sizes for tests -func createAndWriteRootCA(rootCN string, paths ca.CertPaths, expiry time.Duration) (ca.RootCA, error) { - cert, key, err := CreateRootCertAndKey(rootCN) - if err != nil { - return ca.RootCA{}, err - } - - rootCA, err := ca.NewRootCA(cert, cert, key, ca.DefaultNodeCertExpiration, nil) - if err != nil { - return ca.RootCA{}, err - } - - // Ensure directory exists - err = os.MkdirAll(filepath.Dir(paths.Cert), 0755) - if err != nil { - return ca.RootCA{}, err - } - - // Write the Private Key and Certificate to disk, using decent permissions - if err := ioutils.AtomicWriteFile(paths.Cert, cert, 0644); err != nil { - return ca.RootCA{}, err - } - if err := ioutils.AtomicWriteFile(paths.Key, key, 0600); err != nil { - return ca.RootCA{}, err - } - return rootCA, nil -} - // ReDateCert takes an existing cert and changes the not before and not after date, to make it easier // to test expiry func ReDateCert(t *testing.T, cert, signerCert, signerKey []byte, notBefore, notAfter time.Time) []byte {