From 02de4c8b7626a1a3ab54da997829c9254d1b7afe Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Tue, 18 Aug 2020 09:50:24 +0200 Subject: [PATCH] add primary keys to list keyring (#8522) During gossip encryption key rotation it would be nice to be able to see if all nodes are using the same key. This PR adds another field to the json response from `GET v1/operator/keyring` which lists the primary keys in use per dc. That way an operator can tell when a key was successfully setup as primary key. Based on https://github.com/hashicorp/serf/pull/611 to add primary key to list keyring output: ```json [ { "WAN": true, "Datacenter": "dc2", "Segment": "", "Keys": { "0OuM4oC3Os18OblWiBbZUaHA7Hk+tNs/6nhNYtaNduM=": 6, "SINm887hKTzmMWeBNKTJReaTLX3mBEJKriDyt88Ad+g=": 6 }, "PrimaryKeys": { "SINm887hKTzmMWeBNKTJReaTLX3mBEJKriDyt88Ad+g=": 6 }, "NumNodes": 6 }, { "WAN": false, "Datacenter": "dc2", "Segment": "", "Keys": { "0OuM4oC3Os18OblWiBbZUaHA7Hk+tNs/6nhNYtaNduM=": 8, "SINm887hKTzmMWeBNKTJReaTLX3mBEJKriDyt88Ad+g=": 8 }, "PrimaryKeys": { "SINm887hKTzmMWeBNKTJReaTLX3mBEJKriDyt88Ad+g=": 8 }, "NumNodes": 8 }, { "WAN": false, "Datacenter": "dc1", "Segment": "", "Keys": { "0OuM4oC3Os18OblWiBbZUaHA7Hk+tNs/6nhNYtaNduM=": 3, "SINm887hKTzmMWeBNKTJReaTLX3mBEJKriDyt88Ad+g=": 8 }, "PrimaryKeys": { "SINm887hKTzmMWeBNKTJReaTLX3mBEJKriDyt88Ad+g=": 8 }, "NumNodes": 8 } ] ``` I intentionally did not change the CLI output because I didn't find a good way of displaying this information. There are a couple of options that we could implement later: * add a flag to show the primary keys * add a flag to show json output Fixes #3393. --- .changelog/8522.txt | 7 +++++++ agent/consul/internal_endpoint.go | 9 +++++---- agent/structs/structs.go | 15 ++++++++------- api/operator_keyring.go | 3 +++ go.mod | 2 +- go.sum | 2 ++ .../hashicorp/serf/serf/internal_query.go | 8 +++++++- .../hashicorp/serf/serf/keymanager.go | 17 ++++++++++------- vendor/github.com/hashicorp/serf/serf/serf.go | 17 +++++++++++++++-- vendor/modules.txt | 2 +- 10 files changed, 59 insertions(+), 23 deletions(-) create mode 100644 .changelog/8522.txt diff --git a/.changelog/8522.txt b/.changelog/8522.txt new file mode 100644 index 000000000..97637ea68 --- /dev/null +++ b/.changelog/8522.txt @@ -0,0 +1,7 @@ +```release-note:improvement +serf: update to `v0.9.4` which supports primary keys in the ListKeys operation. +``` + +```release-note:improvement +api: `GET v1/operator/keyring` also lists primary keys. +``` diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index 578b3bf1c..63ae9ed29 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -414,10 +414,11 @@ func (m *Internal) executeKeyringOpWAN(args *structs.KeyringRequest) *structs.Ke func translateKeyResponseToKeyringResponse(keyresponse *serf.KeyResponse, datacenter string, err error) structs.KeyringResponse { resp := structs.KeyringResponse{ - Datacenter: datacenter, - Messages: keyresponse.Messages, - Keys: keyresponse.Keys, - NumNodes: keyresponse.NumNodes, + Datacenter: datacenter, + Messages: keyresponse.Messages, + Keys: keyresponse.Keys, + PrimaryKeys: keyresponse.PrimaryKeys, + NumNodes: keyresponse.NumNodes, } if err != nil { resp.Error = err.Error() diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 7b37e0d6c..48024b6ab 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -2349,13 +2349,14 @@ func (r *KeyringRequest) RequestDatacenter() string { // KeyringResponse is a unified key response and can be used for install, // remove, use, as well as listing key queries. type KeyringResponse struct { - WAN bool - Datacenter string - Segment string - Messages map[string]string `json:",omitempty"` - Keys map[string]int - NumNodes int - Error string `json:",omitempty"` + WAN bool + Datacenter string + Segment string + Messages map[string]string `json:",omitempty"` + Keys map[string]int + PrimaryKeys map[string]int + NumNodes int + Error string `json:",omitempty"` } // KeyringResponses holds multiple responses to keyring queries. Each diff --git a/api/operator_keyring.go b/api/operator_keyring.go index 038d5d5b0..5b80f9f91 100644 --- a/api/operator_keyring.go +++ b/api/operator_keyring.go @@ -22,6 +22,9 @@ type KeyringResponse struct { // A map of the encryption keys to the number of nodes they're installed on Keys map[string]int + // A map of the encryption primary keys to the number of nodes they're installed on + PrimaryKeys map[string]int + // The total number of nodes in this ring NumNodes int } diff --git a/go.mod b/go.mod index 45ab978a4..12f46bd67 100644 --- a/go.mod +++ b/go.mod @@ -53,7 +53,7 @@ require ( github.com/hashicorp/net-rpc-msgpackrpc v0.0.0-20151116020338-a14192a58a69 github.com/hashicorp/raft v1.1.2 github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea - github.com/hashicorp/serf v0.9.3 + github.com/hashicorp/serf v0.9.4 github.com/hashicorp/vault/api v1.0.4 github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d github.com/imdario/mergo v0.3.6 diff --git a/go.sum b/go.sum index c69e44b84..dde7494dc 100644 --- a/go.sum +++ b/go.sum @@ -288,6 +288,8 @@ github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea h1:xykPFhrBA github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea/go.mod h1:pNv7Wc3ycL6F5oOWn+tPGo2gWD4a5X+yp/ntwdKLjRk= github.com/hashicorp/serf v0.9.3 h1:AVF6JDQQens6nMHT9OGERBvK0f8rPrAGILnsKLr6lzM= github.com/hashicorp/serf v0.9.3/go.mod h1:UWDWwZeL5cuWDJdl0C6wrvrUwEqtQ4ZKBKKENpqIUyk= +github.com/hashicorp/serf v0.9.4 h1:xrZ4ZR0wT5Dz8oQHHdfOzr0ei1jMToWlFFz3hh/DI7I= +github.com/hashicorp/serf v0.9.4/go.mod h1:UWDWwZeL5cuWDJdl0C6wrvrUwEqtQ4ZKBKKENpqIUyk= github.com/hashicorp/vault/api v1.0.4 h1:j08Or/wryXT4AcHj1oCbMd7IijXcKzYUGw59LGu9onU= github.com/hashicorp/vault/api v1.0.4/go.mod h1:gDcqh3WGcR1cpF5AJz/B1UFheUEneMoIospckxBxk6Q= github.com/hashicorp/vault/sdk v0.1.13 h1:mOEPeOhT7jl0J4AMl1E705+BcmeRs1VmKNb9F0sMLy8= diff --git a/vendor/github.com/hashicorp/serf/serf/internal_query.go b/vendor/github.com/hashicorp/serf/serf/internal_query.go index a74ebf705..0e71290d4 100644 --- a/vendor/github.com/hashicorp/serf/serf/internal_query.go +++ b/vendor/github.com/hashicorp/serf/serf/internal_query.go @@ -64,6 +64,9 @@ type nodeKeyResponse struct { // Keys is used in listing queries to relay a list of installed keys Keys []string + + // PrimaryKey is used in listing queries to relay the primary key + PrimaryKey string } // newSerfQueries is used to create a new serfQueries. We return an event @@ -346,7 +349,7 @@ SEND: func (s *serfQueries) handleListKeys(q *Query) { response := nodeKeyResponse{Result: false} keyring := s.serf.config.MemberlistConfig.Keyring - + var primaryKeyBytes []byte if !s.serf.EncryptionEnabled() { response.Message = "Keyring is empty (encryption not enabled)" s.logger.Printf("[ERR] serf: Keyring is empty (encryption not enabled)") @@ -360,6 +363,9 @@ func (s *serfQueries) handleListKeys(q *Query) { key := base64.StdEncoding.EncodeToString(keyBytes) response.Keys = append(response.Keys, key) } + primaryKeyBytes = keyring.GetPrimaryKey() + response.PrimaryKey = base64.StdEncoding.EncodeToString(primaryKeyBytes) + response.Result = true SEND: diff --git a/vendor/github.com/hashicorp/serf/serf/keymanager.go b/vendor/github.com/hashicorp/serf/serf/keymanager.go index 11aaff2aa..106552c0f 100644 --- a/vendor/github.com/hashicorp/serf/serf/keymanager.go +++ b/vendor/github.com/hashicorp/serf/serf/keymanager.go @@ -31,6 +31,10 @@ type KeyResponse struct { // Keys is a mapping of the base64-encoded value of the key bytes to the // number of nodes that have the key installed. Keys map[string]int + + // PrimaryKeys is a mapping of the base64-encoded value of the primary + // key bytes to the number of nodes that have the key installed. + PrimaryKeys map[string]int } // KeyRequestOptions is used to contain optional parameters for a keyring operation @@ -76,13 +80,11 @@ func (k *KeyManager) streamKeyResp(resp *KeyResponse, ch <-chan NodeResponse) { // Currently only used for key list queries, this adds keys to a counter // and increments them for each node response which contains them. for _, key := range nodeResponse.Keys { - if _, ok := resp.Keys[key]; !ok { - resp.Keys[key] = 1 - } else { - resp.Keys[key]++ - } + resp.Keys[key]++ } + resp.PrimaryKeys[nodeResponse.PrimaryKey]++ + NEXT: // Return early if all nodes have responded. This allows us to avoid // waiting for the full timeout when there is nothing left to do. @@ -97,8 +99,9 @@ func (k *KeyManager) streamKeyResp(resp *KeyResponse, ch <-chan NodeResponse) { // KeyResponse for uniform response handling. func (k *KeyManager) handleKeyRequest(key, query string, opts *KeyRequestOptions) (*KeyResponse, error) { resp := &KeyResponse{ - Messages: make(map[string]string), - Keys: make(map[string]int), + Messages: make(map[string]string), + Keys: make(map[string]int), + PrimaryKeys: make(map[string]int), } qName := internalQueryName(query) diff --git a/vendor/github.com/hashicorp/serf/serf/serf.go b/vendor/github.com/hashicorp/serf/serf/serf.go index 1c1dda1dd..c04719082 100644 --- a/vendor/github.com/hashicorp/serf/serf/serf.go +++ b/vendor/github.com/hashicorp/serf/serf/serf.go @@ -1102,8 +1102,21 @@ func (s *Serf) handleNodeLeaveIntent(leaveMsg *messageLeave) bool { return false } - // Always set the lamport time so that if we retransmit below it won't echo - // around forever! + // Always update the lamport time even when the status does not change + // (despite the variable naming implying otherwise). + // + // By updating this statusLTime here we ensure that the earlier conditional + // on "leaveMsg.LTime <= member.statusLTime" will prevent an infinite + // rebroadcast when seeing two successive leave message for the same + // member. Without this fix a leave message that arrives after a member is + // already marked as leaving/left will cause it to be rebroadcast without + // marking it locally as witnessed. If more than one serf instance in the + // cluster experiences this series of events then they will rebroadcast + // each other's messages about the affected node indefinitely. + // + // This eventually leads to overflowing serf intent queues + // - https://github.com/hashicorp/consul/issues/8179 + // - https://github.com/hashicorp/consul/issues/7960 member.statusLTime = leaveMsg.LTime // State transition depends on current state diff --git a/vendor/modules.txt b/vendor/modules.txt index c3a349a7c..d32f9b51d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -271,7 +271,7 @@ github.com/hashicorp/net-rpc-msgpackrpc github.com/hashicorp/raft # github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea github.com/hashicorp/raft-boltdb -# github.com/hashicorp/serf v0.9.3 +# github.com/hashicorp/serf v0.9.4 github.com/hashicorp/serf/coordinate github.com/hashicorp/serf/serf # github.com/hashicorp/vault/api v1.0.4