From 2f6a9edfac05946da93069133c071956b4e35786 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Fri, 11 Jan 2019 16:04:57 -0500 Subject: [PATCH] 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 | 28 +++++++++++++++++------- agent/consul/connect_ca_endpoint_test.go | 12 ++++++++++ agent/consul/fsm/commands_oss.go | 21 ++++++++++++++++++ agent/consul/state/connect_ca.go | 8 +++++++ agent/structs/connect_ca.go | 27 +++++++++++++++++++++++ agent/structs/structs.go | 1 + 6 files changed, 89 insertions(+), 8 deletions(-) diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 7edcdc215..aa97dd481 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/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 - // 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 - // cert issued so in all cases this index will be higher than any previous - // sign response. This has to be reloaded after the provider.Sign call to - // observe the index update. - state = s.srv.fsm.State() - modIdx, _, err := state.CAConfig() + // that as the raft index to return in response. + // + // UPDATE(mkeeler): The original implementation relied on updating the CAConfig + // and using its index as the ModifyIndex for certs. This was buggy. The long + // term goal is still to insert some metadata into raft about the certificates + // and use that raft index for the ModifyIndex. This is a partial step in that + // direction except that we only are setting an index and not storing the + // 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 diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go index 8ded72e4f..4ed7281c6 100644 --- a/agent/consul/connect_ca_endpoint_test.go +++ b/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) diff --git a/agent/consul/fsm/commands_oss.go b/agent/consul/fsm/commands_oss.go index 14157d864..0e4b972f7 100644 --- a/agent/consul/fsm/commands_oss.go +++ b/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 { diff --git a/agent/consul/state/connect_ca.go b/agent/consul/state/connect_ca.go index 78bce1d54..da7f820c2 100644 --- a/agent/consul/state/connect_ca.go +++ b/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) +} diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 1511b47e2..39d555b5a 100644 --- a/agent/structs/connect_ca.go +++ b/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 { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 4e11e9733..a7757ebe4 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -53,6 +53,7 @@ const ( ACLTokenDeleteRequestType = 18 ACLPolicySetRequestType = 19 ACLPolicyDeleteRequestType = 20 + ConnectCALeafRequestType = 21 ) const (