Store leaf cert indexes in raft and use for the ModifyIndex on the returned certs (#5211)

* Store leaf cert indexes in raft and use for the ModifyIndex on the returned certs

This ensures that future certificate signings will have a strictly greater ModifyIndex than any previous certs signed.

agent/consul/connect_ca_endpoint.go

@@ -396,18 +396,30 @@ func (s *ConnectCA) Sign(
 	}
 
 	// TODO(banks): when we implement IssuedCerts table we can use the insert to
-	// that as the raft index to return in response. Right now we can rely on only
+	// that as the raft index to return in response.
-	// the built-in provider being supported and the implementation detail that we
+	//
-	// have to write a SerialIndex update to the provider config table for every
+	// UPDATE(mkeeler): The original implementation relied on updating the CAConfig
-	// cert issued so in all cases this index will be higher than any previous
+	// and using its index as the ModifyIndex for certs. This was buggy. The long
-	// sign response. This has to be reloaded after the provider.Sign call to
+	// term goal is still to insert some metadata into raft about the certificates
-	// observe the index update.
+	// and use that raft index for the ModifyIndex. This is a partial step in that
-	state = s.srv.fsm.State()
+	// direction except that we only are setting an index and not storing the
-	modIdx, _, err := state.CAConfig()
+	// metadata.
+	req := structs.CALeafRequest{
+		Op:           structs.CALeafOpIncrementIndex,
+		Datacenter:   s.srv.config.Datacenter,
+		WriteRequest: structs.WriteRequest{Token: args.Token},
+	}
+
+	resp, err := s.srv.raftApply(structs.ConnectCALeafRequestType|structs.IgnoreUnknownTypeFlag, &req)
 	if err != nil {
 		return err
 	}
 
+	modIdx, ok := resp.(uint64)
+	if !ok {
+		return fmt.Errorf("Invalid response from updating the leaf cert index")
+	}
+
 	cert, err := connect.ParseCert(pem)
 	if err != nil {
 		return err

agent/consul/connect_ca_endpoint_test.go

@@ -316,6 +316,18 @@ func TestConnectCASign(t *testing.T) {
 	var reply structs.IssuedCert
 	require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", args, &reply))
 
+	// Generate a second CSR and request signing
+	spiffeId2 := connect.TestSpiffeIDService(t, "web2")
+	csr, _ = connect.TestCSR(t, spiffeId2)
+	args = &structs.CASignRequest{
+		Datacenter: "dc1",
+		CSR:        csr,
+	}
+
+	var reply2 structs.IssuedCert
+	require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.Sign", args, &reply2))
+	require.True(reply2.ModifyIndex > reply.ModifyIndex)
+
 	// Get the current CA
 	state := s1.fsm.State()
 	_, ca, err := state.CARootActive(nil)

agent/consul/fsm/commands_oss.go

@@ -28,6 +28,7 @@ func init() {
 	registerCommand(structs.ACLBootstrapRequestType, (*FSM).applyACLTokenBootstrap)
 	registerCommand(structs.ACLPolicySetRequestType, (*FSM).applyACLPolicySetOperation)
 	registerCommand(structs.ACLPolicyDeleteRequestType, (*FSM).applyACLPolicyDeleteOperation)
+	registerCommand(structs.ConnectCALeafRequestType, (*FSM).applyConnectCALeafOperation)
 }
 
 func (c *FSM) applyRegister(buf []byte, index uint64) interface{} {
@@ -354,6 +355,26 @@ func (c *FSM) applyConnectCAOperation(buf []byte, index uint64) interface{} {
 	}
 }
 
+func (c *FSM) applyConnectCALeafOperation(buf []byte, index uint64) interface{} {
+	var req structs.CALeafRequest
+	if err := structs.Decode(buf, &req); err != nil {
+		panic(fmt.Errorf("failed to decode request: %v", err))
+	}
+
+	defer metrics.MeasureSinceWithLabels([]string{"fsm", "ca", "leaf"}, time.Now(),
+		[]metrics.Label{{Name: "op", Value: string(req.Op)}})
+	switch req.Op {
+	case structs.CALeafOpIncrementIndex:
+		if err := c.state.CALeafSetIndex(index); err != nil {
+			return err
+		}
+		return index
+	default:
+		c.logger.Printf("[WARN consul.fsm: Invalid CA Leaf operation '%s'", req.Op)
+		return fmt.Errorf("Invalid CA operation '%s'", req.Op)
+	}
+}
+
 func (c *FSM) applyACLTokenSetOperation(buf []byte, index uint64) interface{} {
 	var req structs.ACLTokenBatchSetRequest
 	if err := structs.Decode(buf, &req); err != nil {

agent/consul/state/connect_ca.go

@@ -11,6 +11,7 @@ const (
 	caBuiltinProviderTableName = "connect-ca-builtin"
 	caConfigTableName          = "connect-ca-config"
 	caRootTableName            = "connect-ca-roots"
+	caLeafIndexName            = "connect-ca-leaf-certs"
 )
 
 // caBuiltinProviderTableSchema returns a new table schema used for storing
@@ -443,3 +444,10 @@ func (s *Store) CADeleteProviderState(id string) error {
 
 	return nil
 }
+
+func (s *Store) CALeafSetIndex(index uint64) error {
+	tx := s.db.Txn(true)
+	defer tx.Abort()
+
+	return indexUpdateMaxTxn(tx, index, caLeafIndexName)
+}

agent/structs/connect_ca.go

@@ -296,6 +296,33 @@ type VaultCAProviderConfig struct {
 	TLSSkipVerify bool
 }
 
+// CALeafOp is the operation for a request related to leaf certificates.
+type CALeafOp string
+
+const (
+	CALeafOpIncrementIndex CALeafOp = "increment-index"
+)
+
+// CALeafRequest is used to modify connect CA leaf data. This is used by the
+// FSM (agent/consul/fsm) to apply changes.
+type CALeafRequest struct {
+	// Op is the type of operation being requested. This determines what
+	// other fields are required.
+	Op CALeafOp
+
+	// Datacenter is the target for this request.
+	Datacenter string
+
+	// WriteRequest is a common struct containing ACL tokens and other
+	// write-related common elements for requests.
+	WriteRequest
+}
+
+// RequestDatacenter returns the datacenter for a given request.
+func (q *CALeafRequest) RequestDatacenter() string {
+	return q.Datacenter
+}
+
 // ParseDurationFunc is a mapstructure hook for decoding a string or
 // []uint8 into a time.Duration value.
 func ParseDurationFunc() mapstructure.DecodeHookFunc {

agent/structs/structs.go

@@ -53,6 +53,7 @@ const (
 	ACLTokenDeleteRequestType              = 18
 	ACLPolicySetRequestType                = 19
 	ACLPolicyDeleteRequestType             = 20
+	ConnectCALeafRequestType               = 21
 )
 
 const (