[ca]: Add RequestAndSaveNewCertificates test with intermediates

ca/certificates.go

@@ -284,9 +284,9 @@ func (rca *RootCA) RequestAndSaveNewCertificates(ctx context.Context, kw KeyWrit
 	return &tlsKeyPair, nil
 }
 
-func (rca *RootCA) getKEKUpdate(ctx context.Context, cert *x509.Certificate, keypair tls.Certificate, connBroker *connectionbroker.Broker) (*KEKData, error) {
+func (rca *RootCA) getKEKUpdate(ctx context.Context, leafCert *x509.Certificate, keypair tls.Certificate, connBroker *connectionbroker.Broker) (*KEKData, error) {
 	var managerRole bool
-	for _, ou := range cert.Subject.OrganizationalUnit {
+	for _, ou := range leafCert.Subject.OrganizationalUnit {
 		if ou == ManagerRole {
 			managerRole = true
 			break

ca/certificates_test.go

@@ -314,43 +314,83 @@ func TestGetRemoteCAInvalidHash(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestRequestAndSaveNewCertificates(t *testing.T) {
-	tc := testutils.NewTestCA(t)
+func testRequestAndSaveNewCertificates(t *testing.T, tc *testutils.TestCA, numIntermediates int) {
 	defer tc.Stop()
 
 	// Copy the current RootCA without the signer
 	rca := ca.RootCA{Certs: tc.RootCA.Certs, Pool: tc.RootCA.Pool}
-	cert, err := rca.RequestAndSaveNewCertificates(tc.Context, tc.KeyReadWriter,
+	tlsCert, err := rca.RequestAndSaveNewCertificates(tc.Context, tc.KeyReadWriter,
 		ca.CertificateRequestConfig{
 			Token:      tc.ManagerToken,
 			ConnBroker: tc.ConnBroker,
 		})
-	assert.NoError(t, err)
-	assert.NotNil(t, cert)
+	require.NoError(t, err)
+	require.NotNil(t, tlsCert)
 	perms, err := permbits.Stat(tc.Paths.Node.Cert)
-	assert.NoError(t, err)
-	assert.False(t, perms.GroupWrite())
-	assert.False(t, perms.OtherWrite())
+	require.NoError(t, err)
+	require.False(t, perms.GroupWrite())
+	require.False(t, perms.OtherWrite())
 
-	// there was no encryption config in the remote, so the key should be unencrypted
-	unencryptedKeyReader := ca.NewKeyReadWriter(tc.Paths.Node, nil, nil)
-	_, _, err = unencryptedKeyReader.Read()
+	certs, err := ioutil.ReadFile(tc.Paths.Node.Cert)
 	require.NoError(t, err)
 
-	// the worker token is also unencrypted
-	cert, err = rca.RequestAndSaveNewCertificates(tc.Context, tc.KeyReadWriter,
-		ca.CertificateRequestConfig{
-			Token:      tc.WorkerToken,
-			ConnBroker: tc.ConnBroker,
-		})
-	assert.NoError(t, err)
-	assert.NotNil(t, cert)
-	_, _, err = unencryptedKeyReader.Read()
+	// ensure that the same number of certs was written
+	parsedCerts, err := helpers.ParseCertificatesPEM(certs)
+	require.NoError(t, err)
+	require.Len(t, parsedCerts, 1+numIntermediates)
+}
+
+func TestRequestAndSaveNewCertificatesNoIntermediate(t *testing.T) {
+	t.Parallel()
+
+	tc := testutils.NewTestCA(t)
+	testRequestAndSaveNewCertificates(t, tc, 0)
+}
+
+func TestRequestAndSaveNewCertificatesWithIntermediates(t *testing.T) {
+	t.Parallel()
+
+	// use a RootCA with an intermediate
+	rca, err := ca.NewRootCA(testutils.ECDSACertChain[2], testutils.ECDSACertChain[1], testutils.ECDSACertChainKeys[1],
+		ca.DefaultNodeCertExpiration, testutils.ECDSACertChain[1])
 	require.NoError(t, err)
 
+	tempdir, err := ioutil.TempDir("", "test-request-and-save-new-certificates")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempdir)
+
+	tc := testutils.NewTestCAFromRootCA(t, tempdir, rca, nil)
+	testRequestAndSaveNewCertificates(t, tc, 1)
+}
+
+func TestRequestAndSaveNewCertificatesWithKEKUpdate(t *testing.T) {
+	t.Parallel()
+
+	tc := testutils.NewTestCA(t)
+	defer tc.Stop()
+
+	// Copy the current RootCA without the signer
+	rca := ca.RootCA{Certs: tc.RootCA.Certs, Pool: tc.RootCA.Pool}
+
+	unencryptedKeyReader := ca.NewKeyReadWriter(tc.Paths.Node, nil, nil)
+
+	// key for the manager and worker are both unencrypted
+	for _, token := range []string{tc.ManagerToken, tc.WorkerToken} {
+		_, err := rca.RequestAndSaveNewCertificates(tc.Context, tc.KeyReadWriter,
+			ca.CertificateRequestConfig{
+				Token:      token,
+				ConnBroker: tc.ConnBroker,
+			})
+		require.NoError(t, err)
+
+		// there was no encryption config in the remote, so the key should be unencrypted
+		_, _, err = unencryptedKeyReader.Read()
+		require.NoError(t, err)
+	}
+
 	// If there is a different kek in the remote store, when TLS certs are renewed the new key will
 	// be encrypted with that kek
-	assert.NoError(t, tc.MemoryStore.Update(func(tx store.Tx) error {
+	require.NoError(t, tc.MemoryStore.Update(func(tx store.Tx) error {
 		cluster := store.GetCluster(tx, tc.Organization)
 		cluster.Spec.EncryptionConfig.AutoLockManagers = true
 		cluster.UnlockKeys = []*api.EncryptionKey{{
@@ -359,37 +399,31 @@ func TestRequestAndSaveNewCertificates(t *testing.T) {
 		}}
 		return store.UpdateCluster(tx, cluster)
 	}))
-	assert.NoError(t, os.RemoveAll(tc.Paths.Node.Cert))
-	assert.NoError(t, os.RemoveAll(tc.Paths.Node.Key))
-
-	_, err = rca.RequestAndSaveNewCertificates(tc.Context, tc.KeyReadWriter,
-		ca.CertificateRequestConfig{
-			Token:      tc.ManagerToken,
-			ConnBroker: tc.ConnBroker,
-		})
-	assert.NoError(t, err)
+	require.NoError(t, os.RemoveAll(tc.Paths.Node.Cert))
+	require.NoError(t, os.RemoveAll(tc.Paths.Node.Key))
 
-	// key can no longer be read without a kek
-	_, _, err = unencryptedKeyReader.Read()
-	require.Error(t, err)
+	// key for the manager will be encrypted, but certs for the worker will not be
+	for _, token := range []string{tc.ManagerToken, tc.WorkerToken} {
+		_, err := rca.RequestAndSaveNewCertificates(tc.Context, tc.KeyReadWriter,
+			ca.CertificateRequestConfig{
+				Token:      token,
+				ConnBroker: tc.ConnBroker,
+			})
+		require.NoError(t, err)
 
-	_, _, err = ca.NewKeyReadWriter(tc.Paths.Node, []byte("kek!"), nil).Read()
-	require.NoError(t, err)
+		// there was no encryption config in the remote, so the key should be unencrypted
+		_, _, err = unencryptedKeyReader.Read()
 
-	// if it's a worker though, the key is always unencrypted, even though the manager key is encrypted
-	_, err = rca.RequestAndSaveNewCertificates(tc.Context, tc.KeyReadWriter,
-		ca.CertificateRequestConfig{
-			Token:      tc.WorkerToken,
-			ConnBroker: tc.ConnBroker,
-		})
-	assert.NoError(t, err)
-	_, _, err = unencryptedKeyReader.Read()
-	require.NoError(t, err)
+		if token == tc.ManagerToken {
+			require.Error(t, err)
+			_, _, err = ca.NewKeyReadWriter(tc.Paths.Node, []byte("kek!"), nil).Read()
+			require.NoError(t, err)
+		} else {
+			require.NoError(t, err)
+		}
+	}
 }
 
-// TODO(cyli):  add test for RequestAndSaveNewCertificates but with intermediates - this involves adding
-// support for appending intermediates on the CA server first
-
 func TestIssueAndSaveNewCertificates(t *testing.T) {
 	tc := testutils.NewTestCA(t)
 	defer tc.Stop()

ca/testutils/cautils.go

@@ -15,7 +15,6 @@ import (
 	cfcsr "github.com/cloudflare/cfssl/csr"
 	"github.com/cloudflare/cfssl/helpers"
 	"github.com/cloudflare/cfssl/initca"
-	cfsigner "github.com/cloudflare/cfssl/signer"
 	"github.com/docker/swarmkit/api"
 	"github.com/docker/swarmkit/ca"
 	"github.com/docker/swarmkit/connectionbroker"
@@ -284,31 +283,12 @@ func genSecurityConfig(s *store.MemoryStore, rootCA ca.RootCA, krw *ca.KeyReadWr
 
 	// Obtain a signed Certificate
 	nodeID := identity.NewID()
-	// All managers get added the subject-alt-name of CA, so they can be used for cert issuance
-	hosts := []string{role}
-	if role == ca.ManagerRole {
-		hosts = append(hosts, ca.CARole)
-	}
 
-	signer, err := rootCA.Signer()
-	if err != nil {
-		return nil, err
-	}
-	cert, err := signer.Sign(cfsigner.SignRequest{
-		Request: string(csr),
-		// OU is used for Authentication of the node type. The CN has the random
-		// node ID.
-		Subject: &cfsigner.Subject{CN: nodeID, Names: []cfcsr.Name{{OU: role, O: org}}},
-		// Adding ou as DNS alt name, so clients can connect to ManagerRole and CARole
-		Hosts: hosts,
-	})
+	certChain, err := rootCA.ParseValidateAndSignCSR(csr, nodeID, role, org)
 	if err != nil {
 		return nil, err
 	}
 
-	// Append the root CA Key to the certificate, to create a valid chain
-	certChain := append(cert, rootCA.Certs...)
-
 	// If we were instructed to persist the files
 	if tmpDir != "" {
 		paths := ca.NewConfigPaths(tmpDir)
@@ -336,16 +316,17 @@ func genSecurityConfig(s *store.MemoryStore, rootCA ca.RootCA, krw *ca.KeyReadWr
 		return nil, err
 	}
 
-	err = createNode(s, nodeID, role, csr, cert)
+	err = createNode(s, nodeID, role, csr, certChain)
 	if err != nil {
 		return nil, err
 	}
 
 	if nonSigningRoot {
 		rootCA = ca.RootCA{
-			Certs:  rootCA.Certs,
-			Digest: rootCA.Digest,
-			Pool:   rootCA.Pool,
+			Certs:         rootCA.Certs,
+			Digest:        rootCA.Digest,
+			Pool:          rootCA.Pool,
+			Intermediates: rootCA.Intermediates,
 		}
 	}