Merge pull request hashicorp#12038 from hashicorp/dnephin/backport-1.9

[1.9.x] Backport CA and test fixes

agent/connect/ca/provider_consul_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/agent/connect"
+	"github.com/hashicorp/consul/agent/consul/fsm"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
 )
@@ -22,7 +23,16 @@ func (c *consulCAMockDelegate) State() *state.Store {
 }
 
 func (c *consulCAMockDelegate) ApplyCARequest(req *structs.CARequest) (interface{}, error) {
-	return ApplyCARequestToStore(c.state, req)
+	idx, _, err := c.state.CAConfig(nil)
+	if err != nil {
+		return nil, err
+	}
+
+	result := fsm.ApplyConnectCAOperationFromRequest(c.state, req, idx+1)
+	if err, ok := result.(error); ok && err != nil {
+		return nil, err
+	}
+	return result, nil
 }
 
 func newMockDelegate(t *testing.T, conf *structs.CAConfiguration) *consulCAMockDelegate {
@@ -150,7 +160,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 				require.NoError(err)
 				require.Equal(spiffeService.URI(), parsed.URIs[0])
 				require.Equal(connect.ServiceCN("foo", "default", connect.TestClusterID), parsed.Subject.CommonName)
-				require.Equal(uint64(2), parsed.SerialNumber.Uint64())
+				require.Equal(uint64(3), parsed.SerialNumber.Uint64())
 				subjectKeyID, err := connect.KeyId(csr.PublicKey)
 				require.NoError(err)
 				require.Equal(subjectKeyID, parsed.SubjectKeyId)
@@ -179,7 +189,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 				require.NoError(err)
 				require.Equal(spiffeService.URI(), parsed.URIs[0])
 				require.Equal(connect.ServiceCN("bar", "default", connect.TestClusterID), parsed.Subject.CommonName)
-				require.Equal(parsed.SerialNumber.Uint64(), uint64(2))
+				require.Equal(uint64(4), parsed.SerialNumber.Uint64())
 				requireNotEncoded(t, parsed.SubjectKeyId)
 				requireNotEncoded(t, parsed.AuthorityKeyId)
 
@@ -207,7 +217,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 				require.NoError(err)
 				require.Equal(spiffeAgent.URI(), parsed.URIs[0])
 				require.Equal(connect.AgentCN("uuid", connect.TestClusterID), parsed.Subject.CommonName)
-				require.Equal(uint64(2), parsed.SerialNumber.Uint64())
+				require.Equal(uint64(5), parsed.SerialNumber.Uint64())
 				requireNotEncoded(t, parsed.SubjectKeyId)
 				requireNotEncoded(t, parsed.AuthorityKeyId)
 

agent/connect/ca/testing.go

@@ -1,20 +1,20 @@
 package ca
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
 	"sync"
 
-	"github.com/hashicorp/consul/agent/connect"
-	"github.com/hashicorp/consul/agent/consul/state"
-	"github.com/hashicorp/consul/agent/structs"
-	"github.com/hashicorp/consul/sdk/freeport"
-	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/go-hclog"
 	vaultapi "github.com/hashicorp/vault/api"
 	"github.com/mitchellh/go-testing-interface"
+
+	"github.com/hashicorp/consul/agent/connect"
+	"github.com/hashicorp/consul/sdk/freeport"
+	"github.com/hashicorp/consul/sdk/testutil/retry"
 )
 
 // KeyTestCases is a list of the important CA key types that we should test
@@ -171,7 +171,9 @@ func runTestVault(t testing.T) (*TestVaultServer, error) {
 		returnPortsFn: returnPortsFn,
 	}
 	t.Cleanup(func() {
-		testVault.Stop()
+		if err := testVault.Stop(); err != nil {
+			t.Logf("failed to stop vault server: %v", err)
+		}
 	})
 	return testVault, nil
 }
@@ -219,7 +221,7 @@ func (v *TestVaultServer) Stop() error {
 	}
 
 	if v.cmd.Process != nil {
-		if err := v.cmd.Process.Signal(os.Interrupt); err != nil {
+		if err := v.cmd.Process.Signal(os.Interrupt); err != nil && !errors.Is(err, os.ErrProcessDone) {
 			return fmt.Errorf("failed to kill vault server: %v", err)
 		}
 	}
@@ -237,32 +239,6 @@ func (v *TestVaultServer) Stop() error {
 	return nil
 }
 
-func ApplyCARequestToStore(store *state.Store, req *structs.CARequest) (interface{}, error) {
-	idx, _, err := store.CAConfig(nil)
-	if err != nil {
-		return nil, err
-	}
-
-	switch req.Op {
-	case structs.CAOpSetProviderState:
-		_, err := store.CASetProviderState(idx+1, req.ProviderState)
-		if err != nil {
-			return nil, err
-		}
-
-		return true, nil
-	case structs.CAOpDeleteProviderState:
-		if err := store.CADeleteProviderState(idx+1, req.ProviderState.ID); err != nil {
-			return nil, err
-		}
-
-		return true, nil
-	case structs.CAOpIncrementProviderSerialNumber:
-		return uint64(2), nil
-	default:
-		return nil, fmt.Errorf("Invalid CA operation '%s'", req.Op)
-	}
-}
 func requireTrailingNewline(t testing.T, leafPEM string) {
 	t.Helper()
 	if len(leafPEM) == 0 {

agent/consul/connect_ca_endpoint_test.go

@@ -527,8 +527,8 @@ func TestConnectCAConfig_UpdateSecondary(t *testing.T) {
 	require.Len(rootList.Roots, 1)
 	rootCert := activeRoot
 
-	waitForActiveCARoot(t, s1, rootCert)
-	waitForActiveCARoot(t, s2, rootCert)
+	testrpc.WaitForActiveCARoot(t, s1.RPC, "primary", rootCert)
+	testrpc.WaitForActiveCARoot(t, s2.RPC, "secondary", rootCert)
 
 	// Capture the current intermediate
 	rootList, activeRoot, err = getTestRoots(s2, "secondary")

agent/consul/fsm/commands_oss.go

@@ -420,61 +420,69 @@ func (c *FSM) applyConnectCAOperation(buf []byte, index uint64) interface{} {
 		[]metrics.Label{{Name: "op", Value: string(req.Op)}})
 	defer metrics.MeasureSinceWithLabels([]string{"fsm", "ca"}, time.Now(),
 		[]metrics.Label{{Name: "op", Value: string(req.Op)}})
+
+	result := ApplyConnectCAOperationFromRequest(c.state, &req, index)
+	if err, ok := result.(error); ok && err != nil {
+		c.logger.Warn("Failed to apply CA operation", "operation", req.Op)
+	}
+	return result
+}
+
+func ApplyConnectCAOperationFromRequest(state *state.Store, req *structs.CARequest, index uint64) interface{} {
 	switch req.Op {
 	case structs.CAOpSetConfig:
 		if req.Config.ModifyIndex != 0 {
-			act, err := c.state.CACheckAndSetConfig(index, req.Config.ModifyIndex, req.Config)
+			act, err := state.CACheckAndSetConfig(index, req.Config.ModifyIndex, req.Config)
 			if err != nil {
 				return err
 			}
 
 			return act
 		}
 
-		return c.state.CASetConfig(index, req.Config)
+		return state.CASetConfig(index, req.Config)
 	case structs.CAOpSetRoots:
-		act, err := c.state.CARootSetCAS(index, req.Index, req.Roots)
+		act, err := state.CARootSetCAS(index, req.Index, req.Roots)
 		if err != nil {
 			return err
 		}
 
 		return act
 	case structs.CAOpSetProviderState:
-		act, err := c.state.CASetProviderState(index, req.ProviderState)
+		act, err := state.CASetProviderState(index, req.ProviderState)
 		if err != nil {
 			return err
 		}
 
 		return act
 	case structs.CAOpDeleteProviderState:
-		if err := c.state.CADeleteProviderState(index, req.ProviderState.ID); err != nil {
+		if err := state.CADeleteProviderState(index, req.ProviderState.ID); err != nil {
 			return err
 		}
 
 		return true
 	case structs.CAOpSetRootsAndConfig:
-		act, err := c.state.CARootSetCAS(index, req.Index, req.Roots)
+		act, err := state.CARootSetCAS(index, req.Index, req.Roots)
 		if err != nil {
 			return err
 		}
 		if !act {
 			return act
 		}
 
-		act, err = c.state.CACheckAndSetConfig(index, req.Config.ModifyIndex, req.Config)
+		act, err = state.CACheckAndSetConfig(index, req.Config.ModifyIndex, req.Config)
 		if err != nil {
 			return err
 		}
 		return act
 	case structs.CAOpIncrementProviderSerialNumber:
-		sn, err := c.state.CAIncrementProviderSerialNumber(index)
+		sn, err := state.CAIncrementProviderSerialNumber(index)
 		if err != nil {
 			return err
 		}
 
 		return sn
 	default:
-		c.logger.Warn("Invalid CA operation", "operation", req.Op)
 		return fmt.Errorf("Invalid CA operation '%s'", req.Op)
 	}
 }

agent/consul/leader_connect_ca.go

@@ -286,7 +286,7 @@ func (c *CAManager) startPostInitializeRoutines() {
 		c.leaderRoutineManager.Start(secondaryCARootWatchRoutineName, c.secondaryCARootWatch)
 	}
 
-	c.leaderRoutineManager.Start(intermediateCertRenewWatchRoutineName, c.intermediateCertRenewalWatch)
+	c.leaderRoutineManager.Start(intermediateCertRenewWatchRoutineName, c.runRenewIntermediate)
 }
 
 func (c *CAManager) backgroundCAInitialization(ctx context.Context) error {
@@ -1051,8 +1051,22 @@ func (c *CAManager) getIntermediateCASigned(provider ca.Provider, newActiveRoot
 	return nil
 }
 
-// intermediateCertRenewalWatch periodically attempts to renew the intermediate cert.
-func (c *CAManager) intermediateCertRenewalWatch(ctx context.Context) error {
+// 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
+}
+
+// runRenewIntermediate periodically attempts to renew the intermediate cert.
+func (c *CAManager) runRenewIntermediate(ctx context.Context) error {
 	isPrimary := c.serverConf.Datacenter == c.serverConf.PrimaryDatacenter
 
 	for {
@@ -1120,8 +1134,7 @@ func (c *CAManager) RenewIntermediate(ctx context.Context, isPrimary bool) error
 		return fmt.Errorf("error parsing active intermediate cert: %v", err)
 	}
 
-	if lessThanHalfTimePassed(time.Now(), intermediateCert.NotBefore.Add(ca.CertificateTimeDriftBuffer),
-		intermediateCert.NotAfter) {
+	if lessThanHalfTimePassed(time.Now(), intermediateCert.NotBefore, intermediateCert.NotAfter) {
 		return nil
 	}