Merge pull request #11672 from hashicorp/dnephin/ca-fix-signing-key-i…

…d-post-update

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

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