From cde26407ddc2f89ae4d769584f5639184771cdc6 Mon Sep 17 00:00:00 2001 From: YACOVM Date: Mon, 27 Feb 2017 16:22:31 +0200 Subject: [PATCH] [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 --- gossip/discovery/discovery_impl.go | 2 - gossip/gossip/gossip_impl.go | 24 +++++++--- gossip/gossip/gossip_test.go | 70 ++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 9 deletions(-) diff --git a/gossip/discovery/discovery_impl.go b/gossip/discovery/discovery_impl.go index cc56541c074..683a495d122 100644 --- a/gossip/discovery/discovery_impl.go +++ b/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) { diff --git a/gossip/gossip/gossip_impl.go b/gossip/gossip/gossip_impl.go index 7e325007285..322babfa1d1 100644 --- a/gossip/gossip/gossip_impl.go +++ b/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 { diff --git a/gossip/gossip/gossip_test.go b/gossip/gossip/gossip_test.go index f05903dd2f6..d826ca25a4c 100644 --- a/gossip/gossip/gossip_test.go +++ b/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