Skip to content

Commit

Permalink
[FAB-11608] Gossip: Optimize alive message verification
Browse files Browse the repository at this point in the history
Alive messages are currently verified in 2 places:
1) Gossip routing layer, before forwarded to the gossip membership layer
2) Gossip membership layer

This causes redundant message verification which induces CPU overhead due
to un-needed ECDSA signature verification.

This change set:

1) Removes alive message verification in the routing layer
2) Solidifies the logic of the verification in the membership layer
3) Adds a unit test that ensures that:
   - Alive messages are verified "once"
   - Alive messages from membership responses are verified
     if they are fresher than the messages received directly
     by gossip so far.
   - Alive messages from membership requests are always verified
     since they trigger sending membership responses based on
     the membership request's alive message.

Change-Id: I70f8c0f1acfdf52e2107fd71ad55a3d002a392ec
Signed-off-by: yacovm <yacovm@il.ibm.com>
(cherry picked from commit f4067cb)
  • Loading branch information
yacovm committed Dec 12, 2018
1 parent 681dcfc commit ccf843c
Show file tree
Hide file tree
Showing 3 changed files with 260 additions and 47 deletions.
60 changes: 34 additions & 26 deletions gossip/discovery/discovery_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,10 @@ func (d *gossipDiscoveryImpl) handleMsgFromComm(msg proto.ReceivedMessage) {
return
}

if !d.crypt.ValidateAliveMsg(selfInfoGossipMsg) {
return
}

if d.msgStore.CheckValid(selfInfoGossipMsg) {
d.handleAliveMessage(selfInfoGossipMsg)
}
Expand All @@ -354,11 +358,16 @@ func (d *gossipDiscoveryImpl) handleMsgFromComm(msg proto.ReceivedMessage) {
}

if m.IsAliveMsg() {
if !d.msgStore.Add(m) {
if !d.msgStore.CheckValid(m) || !d.crypt.ValidateAliveMsg(m) {
return
}
// If the message was sent by me, ignore it and don't forward it further
if d.isSentByMe(m) {
return
}
d.handleAliveMessage(m)

d.msgStore.Add(m)
d.handleAliveMessage(m)
d.comm.Forward(msg)
return
}
Expand All @@ -376,7 +385,7 @@ func (d *gossipDiscoveryImpl) handleMsgFromComm(msg proto.ReceivedMessage) {
return
}

if d.msgStore.CheckValid(am) {
if d.msgStore.CheckValid(am) && d.crypt.ValidateAliveMsg(am) {
d.handleAliveMessage(am)
}
}
Expand All @@ -387,14 +396,10 @@ func (d *gossipDiscoveryImpl) handleMsgFromComm(msg proto.ReceivedMessage) {
d.logger.Warningf("Membership response contains an invalid message from an offline peer %+v", errors.WithStack(err))
return
}
if !d.crypt.ValidateAliveMsg(dm) {
d.logger.Debugf("Alive message isn't authentic, someone spoofed %s's identity", dm.GetAliveMsg().Membership)
continue
}

if !d.msgStore.CheckValid(dm) {
//Newer alive message exist
return
// Newer alive message exists or the message isn't authentic
if !d.msgStore.CheckValid(dm) || !d.crypt.ValidateAliveMsg(dm) {
continue
}

newDeadMembers := []*proto.SignedGossipMessage{}
Expand Down Expand Up @@ -493,26 +498,11 @@ func (d *gossipDiscoveryImpl) handleAliveMessage(m *proto.SignedGossipMessage) {
d.logger.Debug("Entering", m)
defer d.logger.Debug("Exiting")

if !d.crypt.ValidateAliveMsg(m) {
d.logger.Debugf("Alive message isn't authentic, someone must be spoofing %s's identity", m.GetAliveMsg())
if d.isSentByMe(m) {
return
}

pkiID := m.GetAliveMsg().Membership.PkiId
if equalPKIid(pkiID, d.self.PKIid) {
d.logger.Debug("Got alive message about ourselves,", m)
diffExternalEndpoint := d.self.Endpoint != m.GetAliveMsg().Membership.Endpoint
var diffInternalEndpoint bool
secretEnvelope := m.GetSecretEnvelope()
if secretEnvelope != nil && secretEnvelope.InternalEndpoint() != "" {
diffInternalEndpoint = secretEnvelope.InternalEndpoint() != d.self.InternalEndpoint
}
if diffInternalEndpoint || diffExternalEndpoint {
d.logger.Error("Bad configuration detected: Received AliveMessage from a peer with the same PKI-ID as myself:", m.GossipMessage)
}

return
}

ts := m.GetAliveMsg().Timestamp

Expand Down Expand Up @@ -565,6 +555,24 @@ func (d *gossipDiscoveryImpl) handleAliveMessage(m *proto.SignedGossipMessage) {
// else, ignore the message because it is too old
}

func (d *gossipDiscoveryImpl) isSentByMe(m *proto.SignedGossipMessage) bool {
pkiID := m.GetAliveMsg().Membership.PkiId
if !equalPKIid(pkiID, d.self.PKIid) {
return false
}
d.logger.Debug("Got alive message about ourselves,", m)
diffExternalEndpoint := d.self.Endpoint != m.GetAliveMsg().Membership.Endpoint
var diffInternalEndpoint bool
secretEnvelope := m.GetSecretEnvelope()
if secretEnvelope != nil && secretEnvelope.InternalEndpoint() != "" {
diffInternalEndpoint = secretEnvelope.InternalEndpoint() != d.self.InternalEndpoint
}
if diffInternalEndpoint || diffExternalEndpoint {
d.logger.Error("Bad configuration detected: Received AliveMessage from a peer with the same PKI-ID as myself:", m.GossipMessage)
}
return true
}

func (d *gossipDiscoveryImpl) resurrectMember(am *proto.SignedGossipMessage, t proto.PeerTime) {
d.logger.Debug("Entering, AliveMessage:", am, "t:", t)
defer d.logger.Debug("Exiting")
Expand Down

0 comments on commit ccf843c

Please sign in to comment.