Fix bug with Vault CA provider (#18112)

Updating RootPKIPath but not IntermediatePKIPath would not update 
leaf signing certs with the new root. Unsure if this happens in practice 
but manual testing showed it is a bug that would break mesh and agent 
connections once the old root is pruned.

.changelog/18112.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+ca: Fixes a Vault CA provider bug where updating RootPKIPath but not IntermediatePKIPath would not renew leaf signing certificates
+```

agent/connect/ca/provider.go

@@ -135,9 +135,12 @@ type PrimaryProvider interface {
 	// provider.
 	//
 	// Depending on the provider and its configuration, GenerateCAChain may return
-	// a single root certificate or a chain of certs. The provider should return an
-	// existing CA chain if one exists or generate a new one and return it.
-	GenerateCAChain() (CAChainResult, error)
+	// a single root certificate or a chain of certs.
+	// The first certificate must be the primary CA used to sign intermediates for
+	// secondary datacenters, and the last certificate must be the trusted CA.
+	// The provider should return an existing CA chain if one exists or generate a
+	// new one and return it.
+	GenerateCAChain() (string, error)
 
 	// SignIntermediate will validate the CSR to ensure the trust domain in the
 	// URI SAN matches the local one and that basic constraints for a CA

agent/connect/ca/provider_aws.go

@@ -140,19 +140,19 @@ func (a *AWSProvider) State() (map[string]string, error) {
 }
 
 // GenerateCAChain implements Provider
-func (a *AWSProvider) GenerateCAChain() (CAChainResult, error) {
+func (a *AWSProvider) GenerateCAChain() (string, error) {
 	if !a.isPrimary {
-		return CAChainResult{}, fmt.Errorf("provider is not the root certificate authority")
+		return "", fmt.Errorf("provider is not the root certificate authority")
 	}
 
 	if err := a.ensureCA(); err != nil {
-		return CAChainResult{}, err
+		return "", err
 	}
 
 	if a.rootPEM == "" {
-		return CAChainResult{}, fmt.Errorf("AWS CA provider not fully Initialized")
+		return "", fmt.Errorf("AWS CA provider not fully Initialized")
 	}
-	return CAChainResult{PEM: a.rootPEM}, nil
+	return a.rootPEM, nil
 }
 
 // ensureCA loads the CA resource to check it exists if configured by User or in

agent/connect/ca/provider_aws_test.go

@@ -49,9 +49,8 @@ func TestAWSBootstrapAndSignPrimary(t *testing.T) {
 			provider := testAWSProvider(t, testProviderConfigPrimary(t, cfg))
 			defer provider.Cleanup(true, nil)
 
-			root, err := provider.GenerateCAChain()
+			rootPEM, err := provider.GenerateCAChain()
 			require.NoError(t, err)
-			rootPEM := root.PEM
 
 			// Ensure they use the right key type
 			rootCert, err := connect.ParseCert(rootPEM)
@@ -76,9 +75,8 @@ func TestAWSBootstrapAndSignPrimary(t *testing.T) {
 		provider := testAWSProvider(t, testProviderConfigPrimary(t, nil))
 		defer provider.Cleanup(true, nil)
 
-		root, err := provider.GenerateCAChain()
+		rootPEM, err := provider.GenerateCAChain()
 		require.NoError(t, err)
-		rootPEM := root.PEM
 
 		// Ensure they use the right key type
 		rootCert, err := connect.ParseCert(rootPEM)
@@ -111,9 +109,8 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
 
 	p1 := testAWSProvider(t, testProviderConfigPrimary(t, nil))
 	defer p1.Cleanup(true, nil)
-	root, err := p1.GenerateCAChain()
+	rootPEM, err := p1.GenerateCAChain()
 	require.NoError(t, err)
-	rootPEM := root.PEM
 
 	p2 := testAWSProvider(t, testProviderConfigSecondary(t, nil))
 	defer p2.Cleanup(true, nil)
@@ -140,9 +137,8 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
 		cfg1 := testProviderConfigPrimary(t, nil)
 		cfg1.State = p1State
 		p1 = testAWSProvider(t, cfg1)
-		root, err := p1.GenerateCAChain()
+		newRootPEM, err := p1.GenerateCAChain()
 		require.NoError(t, err)
-		newRootPEM := root.PEM
 
 		cfg2 := testProviderConfigPrimary(t, nil)
 		cfg2.State = p2State
@@ -174,9 +170,8 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
 			"ExistingARN": p1State[AWSStateCAARNKey],
 		})
 		p1 = testAWSProvider(t, cfg1)
-		root, err := p1.GenerateCAChain()
+		newRootPEM, err := p1.GenerateCAChain()
 		require.NoError(t, err)
-		newRootPEM := root.PEM
 
 		cfg2 := testProviderConfigPrimary(t, map[string]interface{}{
 			"ExistingARN": p2State[AWSStateCAARNKey],
@@ -213,9 +208,8 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
 		p2 = testAWSProvider(t, cfg2)
 		require.NoError(t, p2.SetIntermediate(newIntPEM, newRootPEM, ""))
 
-		root, err = p1.GenerateCAChain()
+		newRootPEM, err = p1.GenerateCAChain()
 		require.NoError(t, err)
-		newRootPEM = root.PEM
 		newIntPEM, err = p2.ActiveLeafSigningCert()
 		require.NoError(t, err)
 

agent/connect/ca/provider_consul.go

@@ -155,25 +155,25 @@ func (c *ConsulProvider) State() (map[string]string, error) {
 }
 
 // GenerateCAChain initializes a new root certificate and private key if needed.
-func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
+func (c *ConsulProvider) GenerateCAChain() (string, error) {
 	providerState, err := c.getState()
 	if err != nil {
-		return CAChainResult{}, err
+		return "", err
 	}
 
 	if !c.isPrimary {
-		return CAChainResult{}, fmt.Errorf("provider is not the root certificate authority")
+		return "", fmt.Errorf("provider is not the root certificate authority")
 	}
 	if providerState.RootCert != "" {
-		return CAChainResult{PEM: providerState.RootCert}, nil
+		return providerState.RootCert, nil
 	}
 
 	// Generate a private key if needed
 	newState := *providerState
 	if c.config.PrivateKey == "" {
 		_, pk, err := connect.GeneratePrivateKeyWithConfig(c.config.PrivateKeyType, c.config.PrivateKeyBits)
 		if err != nil {
-			return CAChainResult{}, err
+			return "", err
 		}
 		newState.PrivateKey = pk
 	} else {
@@ -184,12 +184,12 @@ func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
 	if c.config.RootCert == "" {
 		nextSerial, err := c.incrementAndGetNextSerialNumber()
 		if err != nil {
-			return CAChainResult{}, fmt.Errorf("error computing next serial number: %v", err)
+			return "", fmt.Errorf("error computing next serial number: %v", err)
 		}
 
 		ca, err := c.generateCA(newState.PrivateKey, nextSerial, c.config.RootCertTTL)
 		if err != nil {
-			return CAChainResult{}, fmt.Errorf("error generating CA: %v", err)
+			return "", fmt.Errorf("error generating CA: %v", err)
 		}
 		newState.RootCert = ca
 	} else {
@@ -202,10 +202,10 @@ func (c *ConsulProvider) GenerateCAChain() (CAChainResult, error) {
 		ProviderState: &newState,
 	}
 	if _, err := c.Delegate.ApplyCARequest(args); err != nil {
-		return CAChainResult{}, err
+		return "", err
 	}
 
-	return CAChainResult{PEM: newState.RootCert}, nil
+	return newState.RootCert, nil
 }
 
 // GenerateIntermediateCSR creates a private key and generates a CSR

agent/connect/ca/provider_consul_test.go

@@ -93,10 +93,10 @@ func TestConsulCAProvider_Bootstrap(t *testing.T) {
 	// Intermediate should be the same cert.
 	inter, err := provider.ActiveLeafSigningCert()
 	require.NoError(t, err)
-	require.Equal(t, root.PEM, inter)
+	require.Equal(t, root, inter)
 
 	// Should be a valid cert
-	parsed, err := connect.ParseCert(root.PEM)
+	parsed, err := connect.ParseCert(root)
 	require.NoError(t, err)
 	require.Equal(t, parsed.URIs[0].String(), fmt.Sprintf("spiffe://%s.consul", conf.ClusterID))
 	requireNotEncoded(t, parsed.SubjectKeyId)
@@ -128,10 +128,10 @@ func TestConsulCAProvider_Bootstrap_WithCert(t *testing.T) {
 
 	root, err := provider.GenerateCAChain()
 	require.NoError(t, err)
-	require.Equal(t, root.PEM, rootCA.RootCert)
+	require.Equal(t, root, rootCA.RootCert)
 
 	// Should be a valid cert
-	parsed, err := connect.ParseCert(root.PEM)
+	parsed, err := connect.ParseCert(root)
 	require.NoError(t, err)
 
 	// test that the default root cert ttl was not applied to the provided cert
@@ -298,7 +298,7 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) {
 	root, err := provider2.GenerateCAChain()
 	require.NoError(t, err)
 
-	newRoot, err := connect.ParseCert(root.PEM)
+	newRoot, err := connect.ParseCert(root)
 	require.NoError(t, err)
 	oldSubject := newRoot.Subject.CommonName
 	requireNotEncoded(t, newRoot.SubjectKeyId)
@@ -321,7 +321,7 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) {
 
 	p1Root, err := provider1.GenerateCAChain()
 	require.NoError(t, err)
-	oldRoot, err := connect.ParseCert(p1Root.PEM)
+	oldRoot, err := connect.ParseCert(p1Root)
 	require.NoError(t, err)
 	requireNotEncoded(t, oldRoot.SubjectKeyId)
 	requireNotEncoded(t, oldRoot.AuthorityKeyId)
@@ -385,7 +385,7 @@ func testCrossSignProvidersShouldFail(t *testing.T, provider1, provider2 Provide
 	root, err := provider2.GenerateCAChain()
 	require.NoError(t, err)
 
-	newRoot, err := connect.ParseCert(root.PEM)
+	newRoot, err := connect.ParseCert(root)
 	require.NoError(t, err)
 	requireNotEncoded(t, newRoot.SubjectKeyId)
 	requireNotEncoded(t, newRoot.AuthorityKeyId)
@@ -454,7 +454,7 @@ func testSignIntermediateCrossDC(t *testing.T, provider1, provider2 Provider) {
 	require.NoError(t, err)
 	root, err := provider1.GenerateCAChain()
 	require.NoError(t, err)
-	rootPEM := root.PEM
+	rootPEM := root
 
 	// Give the new intermediate to provider2 to use.
 	require.NoError(t, provider2.SetIntermediate(intermediatePEM, rootPEM, opaque))