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 <ying.li@docker.com>

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

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)
 }
 

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) {

ca/external.go

@@ -82,15 +82,21 @@ 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()
 
 	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)
 	}

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)

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 {