ca: set the correct SigningKeyID after config update with Vault provider

The test added in this commit shows the problem. Previously the
SigningKeyID was set to the RootCert not the local leaf signing cert.

This same bug was fixed in two other places back in 2019, but this last one was
missed.

While fixing this bug I noticed I had the same few lines of code in 3
places, so I extracted a new function for them.

There would be 4 places, but currently the InitializeCA flow sets this
SigningKeyID in a different way, so I've left that alone for now.

.changelog/11672.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ca: fixes a bug that caused the SigningKeyID to be wrong in the primary DC, when the Vault provider is used, after a CA config creates a new root.
+```

agent/consul/leader_connect_ca.go

@@ -1005,7 +1005,9 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C
 		return err
 	}
 	if intermediate != newRootPEM {
-		newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediate)
+		if err := setLeafSigningCert(newActiveRoot, intermediate); err != nil {
+			return err
+		}
 	}
 
 	// Update the roots and CA config in the state store at the same time
@@ -1060,16 +1062,10 @@ func (c *CAManager) primaryRenewIntermediate(provider ca.Provider, newActiveRoot
 		return fmt.Errorf("error generating new intermediate cert: %v", err)
 	}
 
-	intermediateCert, err := connect.ParseCert(intermediatePEM)
-	if err != nil {
-		return fmt.Errorf("error parsing intermediate cert: %v", err)
+	if err := setLeafSigningCert(newActiveRoot, intermediatePEM); err != nil {
+		return err
 	}
 
-	// Append the new intermediate to our local active root entry. This is
-	// where the root representations start to diverge.
-	newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediatePEM)
-	newActiveRoot.SigningKeyID = connect.EncodeSigningKeyID(intermediateCert.SubjectKeyId)
-
 	c.logger.Info("generated new intermediate certificate for primary datacenter")
 	return nil
 }
@@ -1093,20 +1089,28 @@ func (c *CAManager) secondaryRenewIntermediate(provider ca.Provider, newActiveRo
 		return fmt.Errorf("Failed to set the intermediate certificate with the CA provider: %v", err)
 	}
 
-	intermediateCert, err := connect.ParseCert(intermediatePEM)
-	if err != nil {
-		return fmt.Errorf("error parsing intermediate cert: %v", err)
+	if err := setLeafSigningCert(newActiveRoot, intermediatePEM); err != nil {
+		return err
 	}
 
-	// Append the new intermediate to our local active root entry. This is
-	// where the root representations start to diverge.
-	newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediatePEM)
-	newActiveRoot.SigningKeyID = connect.EncodeSigningKeyID(intermediateCert.SubjectKeyId)
-
 	c.logger.Info("received new intermediate certificate from primary datacenter")
 	return nil
 }
 
+// setLeafSigningCert updates the CARoot by appending the pem to the list of
+// intermediate certificates, and setting the SigningKeyID to the encoded
+// SubjectKeyId of the certificate.
+func setLeafSigningCert(caRoot *structs.CARoot, pem string) error {
+	cert, err := connect.ParseCert(pem)
+	if err != nil {
+		return fmt.Errorf("error parsing leaf signing cert: %w", err)
+	}
+
+	caRoot.IntermediateCerts = append(caRoot.IntermediateCerts, pem)
+	caRoot.SigningKeyID = connect.EncodeSigningKeyID(cert.SubjectKeyId)
+	return nil
+}
+
 // intermediateCertRenewalWatch periodically attempts to renew the intermediate cert.
 func (c *CAManager) intermediateCertRenewalWatch(ctx context.Context) error {
 	isPrimary := c.serverConf.Datacenter == c.serverConf.PrimaryDatacenter

agent/consul/leader_connect_ca_test.go

@@ -14,6 +14,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/consul/testrpc"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -24,7 +26,6 @@ import (
 	"github.com/hashicorp/consul/agent/token"
 	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
-	"github.com/hashicorp/consul/testrpc"
 )
 
 // TODO(kyhavlov): replace with t.Deadline()
@@ -495,3 +496,58 @@ func TestCAManager_Initialize_Logging(t *testing.T) {
 
 	require.Contains(t, buf.String(), "consul CA provider configured")
 }
+
+func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) {
+	ca.SkipIfVaultNotPresent(t)
+	vault := ca.NewTestVaultServer(t)
+
+	_, s1 := testServerWithConfig(t, func(c *Config) {
+		c.PrimaryDatacenter = "dc1"
+		c.CAConfig = &structs.CAConfiguration{
+			Provider: "vault",
+			Config: map[string]interface{}{
+				"Address":             vault.Addr,
+				"Token":               vault.RootToken,
+				"RootPKIPath":         "pki-root/",
+				"IntermediatePKIPath": "pki-intermediate/",
+			},
+		}
+	})
+	defer func() {
+		s1.Shutdown()
+		s1.leaderRoutineManager.Wait()
+	}()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+
+	_, origRoot, err := s1.fsm.State().CARootActive(nil)
+	require.NoError(t, err)
+	require.Len(t, origRoot.IntermediateCerts, 1)
+
+	cert, err := connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(origRoot))
+	require.NoError(t, err)
+	require.Equal(t, connect.HexString(cert.SubjectKeyId), origRoot.SigningKeyID)
+
+	err = s1.caManager.UpdateConfiguration(&structs.CARequest{
+		Config: &structs.CAConfiguration{
+			Provider: "vault",
+			Config: map[string]interface{}{
+				"Address":             vault.Addr,
+				"Token":               vault.RootToken,
+				"RootPKIPath":         "pki-root-2/",
+				"IntermediatePKIPath": "pki-intermediate-2/",
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	_, newRoot, err := s1.fsm.State().CARootActive(nil)
+	require.NoError(t, err)
+	require.Len(t, newRoot.IntermediateCerts, 2,
+		"expected one cross-sign cert and one local leaf sign cert")
+	require.NotEqual(t, origRoot.ID, newRoot.ID)
+
+	cert, err = connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(newRoot))
+	require.NoError(t, err)
+	require.Equal(t, connect.HexString(cert.SubjectKeyId), newRoot.SigningKeyID)
+}