Preserve CARoots when updating Vault CA configuration (#16592)

If a CA config update did not cause a root change, the codepath would return early and skip some steps which preserve its intermediate certificates and signing key ID. This commit re-orders some code and prevents updates from generating new intermediate certificates.

.changelog/16592.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ca: Fixes a bug where updating Vault CA Provider config would cause TLS issues in the service mesh
+```

agent/consul/leader_connect_ca.go

@@ -12,7 +12,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-hclog"
-	uuid "github.com/hashicorp/go-uuid"
+	"github.com/hashicorp/go-uuid"
 	"golang.org/x/time/rate"
 
 	"github.com/hashicorp/consul/acl"
@@ -272,7 +272,7 @@ func newCARoot(pemValue, provider, clusterID string) (*structs.CARoot, error) {
 		ExternalTrustDomain: clusterID,
 		NotBefore:           primaryCert.NotBefore,
 		NotAfter:            primaryCert.NotAfter,
-		RootCert:            pemValue,
+		RootCert:            lib.EnsureTrailingNewline(pemValue),
 		PrivateKeyType:      keyType,
 		PrivateKeyBits:      keyBits,
 		Active:              true,
@@ -887,6 +887,23 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C
 		return err
 	}
 
+	// TODO: https://github.com/hashicorp/consul/issues/12386
+	intermediate, err := newProvider.ActiveIntermediate()
+	if err != nil {
+		return fmt.Errorf("error fetching active intermediate: %w", err)
+	}
+	if intermediate == "" {
+		intermediate, err = newProvider.GenerateIntermediate()
+		if err != nil {
+			return fmt.Errorf("error generating intermediate: %w", err)
+		}
+	}
+	if intermediate != newRootPEM {
+		if err := setLeafSigningCert(newActiveRoot, intermediate); err != nil {
+			return err
+		}
+	}
+
 	// See if the provider needs to persist any state along with the config
 	pState, err := newProvider.State()
 	if err != nil {
@@ -970,19 +987,9 @@ func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.C
 			}
 
 			// Add the cross signed cert to the new CA's intermediates (to be attached
-			// to leaf certs).
+			// to leaf certs). We do not want it to be the last cert if there are any
-			newActiveRoot.IntermediateCerts = []string{xcCert}
+			// existing intermediate certs so we push to the front.
-		}
+			newActiveRoot.IntermediateCerts = append([]string{xcCert}, newActiveRoot.IntermediateCerts...)
-	}
-
-	// TODO: https://github.com/hashicorp/consul/issues/12386
-	intermediate, err := newProvider.GenerateIntermediate()
-	if err != nil {
-		return err
-	}
-	if intermediate != newRootPEM {
-		if err := setLeafSigningCert(newActiveRoot, intermediate); err != nil {
-			return err
 		}
 	}
 

agent/consul/leader_connect_ca_test.go

@@ -25,7 +25,7 @@ import (
 
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/connect"
-	ca "github.com/hashicorp/consul/agent/connect/ca"
+	"github.com/hashicorp/consul/agent/connect/ca"
 	"github.com/hashicorp/consul/agent/consul/fsm"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
@@ -612,11 +612,43 @@ func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) {
 	_, origRoot, err := s1.fsm.State().CARootActive(nil)
 	require.NoError(t, err)
 	require.Len(t, origRoot.IntermediateCerts, 1)
+	origRoot.CreateIndex = s1.caManager.providerRoot.CreateIndex
+	origRoot.ModifyIndex = s1.caManager.providerRoot.ModifyIndex
+	require.Equal(t, s1.caManager.providerRoot, origRoot)
 
 	cert, err := connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(origRoot))
 	require.NoError(t, err)
 	require.Equal(t, connect.HexString(cert.SubjectKeyId), origRoot.SigningKeyID)
 
+	t.Run("update config without changing root", func(t *testing.T) {
+		err = s1.caManager.UpdateConfiguration(&structs.CARequest{
+			Config: &structs.CAConfiguration{
+				Provider: "vault",
+				Config: map[string]interface{}{
+					"Address":             vault.Addr,
+					"Token":               vaultToken,
+					"RootPKIPath":         "pki-root/",
+					"IntermediatePKIPath": "pki-intermediate/",
+					"CSRMaxPerSecond":     100,
+				},
+			},
+		})
+		require.NoError(t, err)
+		_, sameRoot, err := s1.fsm.State().CARootActive(nil)
+		require.NoError(t, err)
+		require.Len(t, sameRoot.IntermediateCerts, 1)
+		sameRoot.CreateIndex = s1.caManager.providerRoot.CreateIndex
+		sameRoot.ModifyIndex = s1.caManager.providerRoot.ModifyIndex
+
+		cert, err := connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(sameRoot))
+		require.NoError(t, err)
+		require.Equal(t, connect.HexString(cert.SubjectKeyId), sameRoot.SigningKeyID)
+
+		require.Equal(t, origRoot, sameRoot)
+		require.Equal(t, sameRoot, s1.caManager.providerRoot)
+	})
+
+	t.Run("update config and change root", func(t *testing.T) {
 		vaultToken2 := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{
 			RootPath:         "pki-root-2",
 			IntermediatePath: "pki-intermediate-2",
@@ -645,6 +677,7 @@ func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) {
 		cert, err = connect.ParseCert(s1.caManager.getLeafSigningCertFromRoot(newRoot))
 		require.NoError(t, err)
 		require.Equal(t, connect.HexString(cert.SubjectKeyId), newRoot.SigningKeyID)
+	})
 }
 
 func TestCAManager_Initialize_Vault_WithIntermediateAsPrimaryCA(t *testing.T) {