[FAB-2007] Gossip: External and internal endpoints II

Background:
------------
The previous commit added support in the discovery layer
for policies that effect the membership response handling logic.
That logic is based on the selfInformation field which is a signed
GossipMessage that contains an AliveMessage and is supposed to represent
the remote peer that sent the membership request.

Even though the message is validated, nothing prevents an attacker
to replay a signedMessage he recorded.

There is a TODO in the code that says:
https://github.com/hyperledger/fabric/blob/master/gossip/discovery/discovery_impl.go#L293
// TODO: make sure somehow that the membership request is "fresh"

This is to prevent replay attacks, and:
1) This needs to be addressed for FAB-2007
2) A replay attack can happen anyway if a malicious peer gets hold
   of such a message early enough befor the attacked peer got the message

What's in this commit?
----------------------
This commit leverages the fact that a membership request is point-to-point,
and checks that the sender of the membership request is the same peer
that is on the other side of the connection.
Also removed the TODO since now it's not needed anymore.

How is this tested?
--------------------
I added a test that creates such a replay attack and spoofs
a membership request, and compares it to a valid membership
request to demonstrate that the attack prevention works.

Change-Id: I8c994b5627189a1d0fb3f6a7d9edbd9a9c021b2c
Signed-off-by: Yacov Manevich <[email protected]>

Author: yacovm

gossip/discovery/discovery_impl.go

@@ -292,7 +292,6 @@ func (d *gossipDiscoveryImpl) handleMsgFromComm(m *proto.SignedGossipMessage) {
 	d.logger.Debug("Got message:", m)
 	defer d.logger.Debug("Exiting")
 
-	// TODO: make sure somehow that the membership request is "fresh"
 	if memReq := m.GetMemReq(); memReq != nil {
 		selfInfoGossipMsg, err := memReq.SelfInformation.ToGossipMessage()
 		if err != nil {
@@ -395,7 +394,6 @@ func (d *gossipDiscoveryImpl) createMembershipResponse(targetMember *NetworkMemb
 	defer d.lock.RUnlock()
 
 	deadPeers := []*proto.Envelope{}
-
 	for _, dm := range d.deadMembership.ToSlice() {
 
 		if !shouldBeDisclosed(dm) {

gossip/gossip/gossip_impl.go

@@ -329,6 +329,23 @@ func (g *gossipServiceImpl) handleMessage(m proto.ReceivedMessage) {
 	}
 
 	if selectOnlyDiscoveryMessages(m) {
+		// It's a membership request, check its self information
+		// matches the sender
+		if m.GetGossipMessage().GetMemReq() != nil {
+			sMsg, err := m.GetGossipMessage().GetMemReq().SelfInformation.ToGossipMessage()
+			if err != nil {
+				g.logger.Warning("Got membership request with invalid selfInfo:", err)
+				return
+			}
+			if !sMsg.IsAliveMsg() {
+				g.logger.Warning("Got membership request with selfInfo that isn't an AliveMessage")
+				return
+			}
+			if !bytes.Equal(sMsg.GetAliveMsg().Membership.PkiID, m.GetConnectionInfo().ID) {
+				g.logger.Warning("Got membership request with selfInfo that doesn't match the handshake")
+				return
+			}
+		}
 		g.forwardDiscoveryMsg(m)
 	}
 
@@ -385,13 +402,6 @@ func (g *gossipServiceImpl) validateMsg(msg proto.ReceivedMessage) bool {
 	return true
 }
 
-func (g *gossipServiceImpl) forwardToDiscoveryLayer(msg proto.ReceivedMessage) {
-	defer func() { // can be closed while shutting down
-		recover()
-	}()
-	g.discAdapter.incChan <- msg.GetGossipMessage()
-}
-
 func (g *gossipServiceImpl) sendGossipBatch(a []interface{}) {
 	msgs2Gossip := make([]*proto.SignedGossipMessage, len(a))
 	for i, e := range a {

gossip/gossip/gossip_test.go

@@ -29,6 +29,7 @@ import (
 	"time"
 
 	"github.com/hyperledger/fabric/gossip/api"
+	"github.com/hyperledger/fabric/gossip/comm"
 	"github.com/hyperledger/fabric/gossip/common"
 	"github.com/hyperledger/fabric/gossip/discovery"
 	"github.com/hyperledger/fabric/gossip/gossip/algo"
@@ -665,6 +666,75 @@ func TestMembershipConvergence(t *testing.T) {
 	testWG.Done()
 }
 
+func TestMembershipRequestSpoofing(t *testing.T) {
+	t.Parallel()
+	// Scenario: g1, g2, g3 are peers, and g2 is malicious, and wants
+	// to impersonate g3 when sending a membership request to g1.
+	// Expected output: g1 should *NOT* respond to g2,
+	// However, g1 should respond to g3 when it sends the message itself.
+
+	portPrefix := 2000
+	g1 := newGossipInstance(portPrefix, 0, 100)
+	g2 := newGossipInstance(portPrefix, 1, 100, 2)
+	g3 := newGossipInstance(portPrefix, 2, 100, 1)
+	defer g1.Stop()
+	defer g2.Stop()
+	defer g3.Stop()
+
+	// Wait for g2 and g3 to know about each other
+	waitUntilOrFail(t, checkPeersMembership(t, []Gossip{g2, g3}, 1))
+	// Obtain an alive message from p3
+	_, aliveMsgChan := g2.Accept(func(o interface{}) bool {
+		msg := o.(proto.ReceivedMessage).GetGossipMessage()
+		// Make sure we get an AliveMessage and it's about g3
+		return msg.IsAliveMsg() && bytes.Equal(msg.GetAliveMsg().Membership.PkiID, []byte("localhost:2002"))
+	}, true)
+	aliveMsg := <-aliveMsgChan
+
+	// Obtain channel for messages from g1 to g2
+	_, g1ToG2 := g2.Accept(func(o interface{}) bool {
+		connInfo := o.(proto.ReceivedMessage).GetConnectionInfo()
+		return bytes.Equal([]byte("localhost:2000"), connInfo.ID)
+	}, true)
+
+	// Obtain channel for messages from g1 to g3
+	_, g1ToG3 := g3.Accept(func(o interface{}) bool {
+		connInfo := o.(proto.ReceivedMessage).GetConnectionInfo()
+		return bytes.Equal([]byte("localhost:2000"), connInfo.ID)
+	}, true)
+
+	// Now, create a membership request message
+	memRequestSpoofFactory := func(aliveMsgEnv *proto.Envelope) *proto.SignedGossipMessage {
+		return (&proto.GossipMessage{
+			Tag:   proto.GossipMessage_EMPTY,
+			Nonce: uint64(0),
+			Content: &proto.GossipMessage_MemReq{
+				MemReq: &proto.MembershipRequest{
+					SelfInformation: aliveMsgEnv,
+					Known:           [][]byte{},
+				},
+			},
+		}).NoopSign()
+	}
+	spoofedMemReq := memRequestSpoofFactory(aliveMsg.GetSourceEnvelope())
+	g2.Send(spoofedMemReq.GossipMessage, &comm.RemotePeer{Endpoint: "localhost:2000", PKIID: common.PKIidType("localhost:2000")})
+	select {
+	case <-time.After(time.Second):
+		break
+	case <-g1ToG2:
+		assert.Fail(t, "Received response from g1 but shouldn't have")
+	}
+
+	// Now send the same message from g3 to g1
+	g3.Send(spoofedMemReq.GossipMessage, &comm.RemotePeer{Endpoint: "localhost:2000", PKIID: common.PKIidType("localhost:2000")})
+	select {
+	case <-time.After(time.Second):
+		assert.Fail(t, "Didn't receive a message back from g1 on time")
+	case <-g1ToG3:
+		break
+	}
+}
+
 func TestDataLeakage(t *testing.T) {
 	t.Parallel()
 	portPrefix := 1610