connect: Add logic for updating secondary DC intermediate on config set

agent/connect/testing_spiffe.go

@@ -12,10 +12,16 @@ func TestSpiffeIDService(t testing.T, service string) *SpiffeIDService {
 // TestSpiffeIDServiceWithHost returns a SPIFFE ID representing a service with
 // the specified trust domain.
 func TestSpiffeIDServiceWithHost(t testing.T, service, host string) *SpiffeIDService {
+	return TestSpiffeIDServiceWithHostDC(t, service, host, "dc1")
+}
+
+// TestSpiffeIDServiceWithHostDC returns a SPIFFE ID representing a service with
+// the specified trust domain for the given datacenter.
+func TestSpiffeIDServiceWithHostDC(t testing.T, service, host, datacenter string) *SpiffeIDService {
 	return &SpiffeIDService{
 		Host:       host,
 		Namespace:  "default",
-		Datacenter: "dc1",
+		Datacenter: datacenter,
 		Service:    service,
 	}
 }

agent/consul/connect_ca_endpoint.go

@@ -163,6 +163,27 @@ func (s *ConnectCA) ConfigurationSet(
 		}
 	}()
 
+	// If this is a secondary, check if the intermediate needs to be regenerated.
+	if s.srv.config.Datacenter != s.srv.config.PrimaryDatacenter {
+		// Get the current root certs from the primary DC.
+		var roots structs.IndexedCARoots
+		rootArgs := structs.DCSpecificRequest{
+			Datacenter: s.srv.config.PrimaryDatacenter,
+		}
+		if err := s.srv.forwardDC("ConnectCA.Roots", s.srv.config.PrimaryDatacenter, &rootArgs, &roots); err != nil {
+			return fmt.Errorf("Error retrieving the primary datacenter's roots: %v", err)
+		}
+
+		s.srv.caProviderReconfigurationLock.Lock()
+		defer s.srv.caProviderReconfigurationLock.Unlock()
+		if err := s.srv.initializeSecondaryCA(newProvider, roots, args.Config); err != nil {
+			return fmt.Errorf("Error updating secondary datacenter CA config: %v", err)
+		}
+		cleanupNewProvider = false
+		s.logger.Info("Secondary CA provider config updated")
+		return nil
+	}
+
 	if err := newProvider.GenerateRoot(); err != nil {
 		return fmt.Errorf("error generating CA root certificate: %v", err)
 	}
@@ -192,10 +213,8 @@ func (s *ConnectCA) ConfigurationSet(
 		return err
 	}
 
-	// If the root didn't change or if this is a secondary DC, just update the
-	// config and return.
-	if (s.srv.config.Datacenter != s.srv.config.PrimaryDatacenter) ||
-		root != nil && root.ID == newActiveRoot.ID {
+	// If the root didn't change, just update the config and return.
+	if root != nil && root.ID == newActiveRoot.ID {
 		args.Op = structs.CAOpSetConfig
 		resp, err := s.srv.raftApply(structs.ConnectCARequestType, args)
 		if err != nil {

agent/consul/connect_ca_endpoint_test.go

@@ -405,6 +405,140 @@ func TestConnectCAConfig_TriggerRotation(t *testing.T) {
 	}
 }
 
+func TestConnectCAConfig_UpdateSecondary(t *testing.T) {
+	t.Parallel()
+
+	assert := assert.New(t)
+	require := require.New(t)
+
+	// Initialize primary as the primary DC
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.Datacenter = "primary"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+
+	testrpc.WaitForLeader(t, s1.RPC, "primary")
+
+	// secondary as a secondary DC
+	dir2, s2 := testServerWithConfig(t, func(c *Config) {
+		c.Datacenter = "secondary"
+		c.PrimaryDatacenter = "primary"
+	})
+	defer os.RemoveAll(dir2)
+	defer s2.Shutdown()
+	codec := rpcClient(t, s2)
+	defer codec.Close()
+
+	// Create the WAN link
+	joinWAN(t, s2, s1)
+	testrpc.WaitForLeader(t, s2.RPC, "secondary")
+
+	// Capture the current root
+	rootList, activeRoot, err := getTestRoots(s1, "primary")
+	require.NoError(err)
+	require.Len(rootList.Roots, 1)
+	rootCert := activeRoot
+
+	waitForActiveCARoot(t, s1, rootCert)
+	waitForActiveCARoot(t, s2, rootCert)
+
+	// Capture the current intermediate
+	rootList, activeRoot, err = getTestRoots(s2, "secondary")
+	require.NoError(err)
+	require.Len(rootList.Roots, 1)
+	require.Len(activeRoot.IntermediateCerts, 1)
+	oldIntermediatePEM := activeRoot.IntermediateCerts[0]
+
+	// Update the secondary CA config to use a new private key, which should
+	// cause a re-signing with a new intermediate.
+	_, newKey, err := connect.GeneratePrivateKey()
+	assert.NoError(err)
+	newConfig := &structs.CAConfiguration{
+		Provider: "consul",
+		Config: map[string]interface{}{
+			"PrivateKey":     newKey,
+			"RootCert":       "",
+			"RotationPeriod": 90 * 24 * time.Hour,
+		},
+	}
+	{
+		args := &structs.CARequest{
+			Datacenter: "secondary",
+			Config:     newConfig,
+		}
+		var reply interface{}
+
+		require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply))
+	}
+
+	// Make sure the new intermediate has replaced the old one in the active root,
+	// and that the root itself hasn't changed.
+	var newIntermediatePEM string
+	{
+		args := &structs.DCSpecificRequest{
+			Datacenter: "secondary",
+		}
+		var reply structs.IndexedCARoots
+		require.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", args, &reply))
+		require.Len(reply.Roots, 1)
+		require.Len(reply.Roots[0].IntermediateCerts, 1)
+		newIntermediatePEM = reply.Roots[0].IntermediateCerts[0]
+		require.NotEqual(oldIntermediatePEM, newIntermediatePEM)
+		require.Equal(reply.Roots[0].RootCert, rootCert.RootCert)
+	}
+
+	// Verify the new config was set.
+	{
+		args := &structs.DCSpecificRequest{
+			Datacenter: "secondary",
+		}
+		var reply structs.CAConfiguration
+		require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", args, &reply))
+
+		actual, err := ca.ParseConsulCAConfig(reply.Config)
+		require.NoError(err)
+		expected, err := ca.ParseConsulCAConfig(newConfig.Config)
+		require.NoError(err)
+		assert.Equal(reply.Provider, newConfig.Provider)
+		assert.Equal(actual, expected)
+	}
+
+	// Verify that new leaf certs get the new intermediate bundled
+	{
+		// Generate a CSR and request signing
+		spiffeId := connect.TestSpiffeIDServiceWithHostDC(t, "web", connect.TestClusterID+".consul", "secondary")
+		csr, _ := connect.TestCSR(t, spiffeId)
+		args := &structs.CASignRequest{
+			Datacenter: "secondary",
+			CSR:        csr,
+		}
+		var reply structs.IssuedCert
+		require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", args, &reply))
+
+		// Verify the leaf cert has the new intermediate.
+		{
+			roots := x509.NewCertPool()
+			assert.True(roots.AppendCertsFromPEM([]byte(rootCert.RootCert)))
+			leaf, err := connect.ParseCert(reply.CertPEM)
+			require.NoError(err)
+
+			intermediates := x509.NewCertPool()
+			require.True(intermediates.AppendCertsFromPEM([]byte(newIntermediatePEM)))
+
+			_, err = leaf.Verify(x509.VerifyOptions{
+				Roots:         roots,
+				Intermediates: intermediates,
+			})
+			require.NoError(err)
+		}
+
+		// Verify other fields
+		assert.Equal("web", reply.Service)
+		assert.Equal(spiffeId.URI().String(), reply.ServiceURI)
+	}
+}
+
 // Test CA signing
 func TestConnectCASign(t *testing.T) {
 	t.Parallel()

agent/consul/leader_connect.go

@@ -214,7 +214,7 @@ func (s *Server) initializeCA() error {
 		if err := s.initializeSecondaryProvider(provider, roots); err != nil {
 			return fmt.Errorf("error configuring provider: %v", err)
 		}
-		if err := s.initializeSecondaryCA(provider, roots); err != nil {
+		if err := s.initializeSecondaryCA(provider, roots, nil); err != nil {
 			return err
 		}
 
@@ -337,7 +337,7 @@ func (s *Server) initializeRootCA(provider ca.Provider, conf *structs.CAConfigur
 // intermediate.
 // It is being called while holding caProviderReconfigurationLock
 // which means it must never take that lock itself or call anything that does.
-func (s *Server) initializeSecondaryCA(provider ca.Provider, primaryRoots structs.IndexedCARoots) error {
+func (s *Server) initializeSecondaryCA(provider ca.Provider, primaryRoots structs.IndexedCARoots, config *structs.CAConfiguration) error {
 	activeIntermediate, err := provider.ActiveIntermediate()
 	if err != nil {
 		return err
@@ -432,19 +432,25 @@ func (s *Server) initializeSecondaryCA(provider ca.Provider, primaryRoots struct
 	if err != nil {
 		return err
 	}
+
+	// Determine whether a root update is needed, and persist the roots/config accordingly.
+	var newRoot *structs.CARoot
 	if activeRoot == nil || activeRoot.ID != newActiveRoot.ID || newIntermediate {
-		if err := s.persistNewRoot(provider, newActiveRoot); err != nil {
-			return err
-		}
+		newRoot = newActiveRoot
+	}
+	if err := s.persistNewRootAndConfig(provider, newRoot, config); err != nil {
+		return err
 	}
 
 	s.setCAProvider(provider, newActiveRoot)
 	return nil
 }
 
-// persistNewRoot is being called while holding caProviderReconfigurationLock
+// persistNewRootAndConfig is being called while holding caProviderReconfigurationLock
 // which means it must never take that lock itself or call anything that does.
-func (s *Server) persistNewRoot(provider ca.Provider, newActiveRoot *structs.CARoot) error {
+// If newActiveRoot is non-nil, it will be appended to the current roots list.
+// If config is non-nil, it will be used to overwrite the existing config.
+func (s *Server) persistNewRootAndConfig(provider ca.Provider, newActiveRoot *structs.CARoot, config *structs.CAConfiguration) error {
 	connectLogger := s.loggers.Named(logging.Connect)
 	state := s.fsm.State()
 	idx, oldRoots, err := state.CARoots(nil)
@@ -452,15 +458,23 @@ func (s *Server) persistNewRoot(provider ca.Provider, newActiveRoot *structs.CAR
 		return err
 	}
 
-	_, config, err := state.CAConfig(nil)
+	var newConf structs.CAConfiguration
+	_, storedConfig, err := state.CAConfig(nil)
 	if err != nil {
 		return err
 	}
-	if config == nil {
+	if storedConfig == nil {
 		return fmt.Errorf("local CA not initialized yet")
 	}
-	newConf := *config
-	newConf.ClusterID = newActiveRoot.ExternalTrustDomain
+	if config != nil {
+		newConf = *config
+	} else {
+		newConf = *storedConfig
+	}
+	newConf.ModifyIndex = storedConfig.ModifyIndex
+	if newActiveRoot != nil {
+		newConf.ClusterID = newActiveRoot.ExternalTrustDomain
+	}
 
 	// Persist any state the provider needs us to
 	newConf.State, err = provider.State()
@@ -468,21 +482,25 @@ func (s *Server) persistNewRoot(provider ca.Provider, newActiveRoot *structs.CAR
 		return fmt.Errorf("error getting provider state: %v", err)
 	}
 
-	// Copy the root list and append the new active root, updating the old root
-	// with the time it was rotated out.
+	// If there's a new active root, copy the root list and append it, updating
+	// the old root with the time it was rotated out.
 	var newRoots structs.CARoots
-	for _, r := range oldRoots {
-		newRoot := *r
-		if newRoot.Active {
-			newRoot.Active = false
-			newRoot.RotatedOutAt = time.Now()
+	if newActiveRoot != nil {
+		for _, r := range oldRoots {
+			newRoot := *r
+			if newRoot.Active {
+				newRoot.Active = false
+				newRoot.RotatedOutAt = time.Now()
+			}
+			if newRoot.ExternalTrustDomain == "" {
+				newRoot.ExternalTrustDomain = config.ClusterID
+			}
+			newRoots = append(newRoots, &newRoot)
 		}
-		if newRoot.ExternalTrustDomain == "" {
-			newRoot.ExternalTrustDomain = config.ClusterID
-		}
-		newRoots = append(newRoots, &newRoot)
+		newRoots = append(newRoots, newActiveRoot)
+	} else {
+		newRoots = oldRoots
 	}
-	newRoots = append(newRoots, newActiveRoot)
 
 	args := &structs.CARequest{
 		Op:     structs.CAOpSetRootsAndConfig,
@@ -748,7 +766,7 @@ func (s *Server) intermediateCertRenewalWatch(ctx context.Context) error {
 					return err
 				}
 
-				if err := s.persistNewRoot(provider, activeRoot); err != nil {
+				if err := s.persistNewRootAndConfig(provider, activeRoot, nil); err != nil {
 					return err
 				}
 
@@ -808,7 +826,7 @@ func (s *Server) secondaryCARootWatch(ctx context.Context) error {
 		// Run the secondary CA init routine to see if we need to request a new
 		// intermediate.
 		if s.configuredSecondaryCA() {
-			if err := s.initializeSecondaryCA(provider, roots); err != nil {
+			if err := s.initializeSecondaryCA(provider, roots, nil); err != nil {
 				return fmt.Errorf("Failed to initialize the secondary CA: %v", err)
 			}
 		}