Skip to content

Commit 5eb459a

Browse files
committed
[FAB-2007] Gossip/Comm deep probing
When a peer receives a join-channel event, it doesn't know whether the anchor peers in the genesis block indeed match their said organizations. A malicious attacker may create a channel and add an anchor peer of its own in each organization, and this way- make peers expose their internal endpoints which is not wanted. This commit introduces another method - Handkshake() that performs a handshake to the remote peer, and returns its identity, or an error if either a communication or authentication error occures. The certificate then can be used to extract the remote peers' organization. Change-Id: I0a0e71a10cb5831f07c72be28308cadb486c1d3a Signed-off-by: Yacov Manevich <[email protected]>
1 parent 821c9d8 commit 5eb459a

File tree

5 files changed

+67
-3
lines changed

5 files changed

+67
-3
lines changed

gossip/comm/comm.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package comm
1919
import (
2020
"fmt"
2121

22+
"github.com/hyperledger/fabric/gossip/api"
2223
"github.com/hyperledger/fabric/gossip/common"
2324
proto "github.com/hyperledger/fabric/protos/gossip"
2425
)
@@ -33,9 +34,14 @@ type Comm interface {
3334
// Send sends a message to remote peers
3435
Send(msg *proto.SignedGossipMessage, peers ...*RemotePeer)
3536

36-
// Probe probes a remote node and returns nil if its responsive
37+
// Probe probes a remote node and returns nil if its responsive,
38+
// and an error if it's not.
3739
Probe(peer *RemotePeer) error
3840

41+
// Handshake authenticates a remote peer and returns
42+
// (its identity, nil) on success and (nil, error)
43+
Handshake(peer *RemotePeer) (api.PeerIdentityType, error)
44+
3945
// Accept returns a dedicated read-only channel for messages sent by other nodes that match a certain predicate.
4046
// Each message from the channel can be used to send a reply back to the sender
4147
Accept(common.MessageAcceptor) <-chan proto.ReceivedMessage

gossip/comm/comm_impl.go

+26
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,32 @@ func (c *commImpl) Probe(remotePeer *RemotePeer) error {
296296
return err
297297
}
298298

299+
func (c *commImpl) Handshake(remotePeer *RemotePeer) (api.PeerIdentityType, error) {
300+
cc, err := grpc.Dial(remotePeer.Endpoint, append(c.opts, grpc.WithBlock())...)
301+
if err != nil {
302+
return nil, err
303+
}
304+
defer cc.Close()
305+
306+
cl := proto.NewGossipClient(cc)
307+
if _, err = cl.Ping(context.Background(), &proto.Empty{}); err != nil {
308+
return nil, err
309+
}
310+
311+
stream, err := cl.GossipStream(context.Background())
312+
if err != nil {
313+
return nil, err
314+
}
315+
connInfo, err := c.authenticateRemotePeer(stream)
316+
if err != nil {
317+
return nil, err
318+
}
319+
if len(remotePeer.PKIID) > 0 && !bytes.Equal(connInfo.ID, remotePeer.PKIID) {
320+
return nil, errors.New("PKI-ID of remote peer doesn't match expected PKI-ID")
321+
}
322+
return connInfo.Identity, nil
323+
}
324+
299325
func (c *commImpl) Accept(acceptor common.MessageAcceptor) <-chan proto.ReceivedMessage {
300326
genericChan := c.msgPublisher.AddChannel(acceptor)
301327
specificChan := make(chan proto.ReceivedMessage, 10)

gossip/comm/comm_test.go

+23
Original file line numberDiff line numberDiff line change
@@ -487,15 +487,38 @@ func TestProbe(t *testing.T) {
487487
comm2, _ := newCommInstance(6612, naiveSec)
488488
time.Sleep(time.Duration(1) * time.Second)
489489
assert.NoError(t, comm1.Probe(remotePeer(6612)))
490+
_, err := comm1.Handshake(remotePeer(6612))
491+
assert.NoError(t, err)
490492
assert.Error(t, comm1.Probe(remotePeer(9012)))
493+
_, err = comm1.Handshake(remotePeer(9012))
494+
assert.Error(t, err)
491495
comm2.Stop()
492496
time.Sleep(time.Second)
493497
assert.Error(t, comm1.Probe(remotePeer(6612)))
498+
_, err = comm1.Handshake(remotePeer(6612))
499+
assert.Error(t, err)
494500
comm2, _ = newCommInstance(6612, naiveSec)
495501
defer comm2.Stop()
496502
time.Sleep(time.Duration(1) * time.Second)
497503
assert.NoError(t, comm2.Probe(remotePeer(6611)))
504+
_, err = comm2.Handshake(remotePeer(6611))
505+
assert.NoError(t, err)
498506
assert.NoError(t, comm1.Probe(remotePeer(6612)))
507+
_, err = comm1.Handshake(remotePeer(6612))
508+
assert.NoError(t, err)
509+
// Now try a deep probe with an expected PKI-ID that doesn't match
510+
wrongRemotePeer := remotePeer(6612)
511+
if wrongRemotePeer.PKIID[0] == 0 {
512+
wrongRemotePeer.PKIID[0] = 1
513+
} else {
514+
wrongRemotePeer.PKIID[0] = 0
515+
}
516+
_, err = comm1.Handshake(wrongRemotePeer)
517+
assert.Error(t, err)
518+
// Try a deep probe with a nil PKI-ID
519+
id, err := comm1.Handshake(&RemotePeer{Endpoint: "localhost:6612"})
520+
assert.NoError(t, err)
521+
assert.Equal(t, api.PeerIdentityType("localhost:6612"), id)
499522
}
500523

501524
func TestPresumedDead(t *testing.T) {

gossip/comm/mock/mock_comm.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package mock
1818

1919
import (
20+
"github.com/hyperledger/fabric/gossip/api"
2021
"github.com/hyperledger/fabric/gossip/comm"
2122
"github.com/hyperledger/fabric/gossip/common"
2223
"github.com/hyperledger/fabric/gossip/util"
@@ -152,11 +153,18 @@ func (mock *commMock) Send(msg *proto.SignedGossipMessage, peers ...*comm.Remote
152153
}
153154
}
154155

155-
// Probe probes a remote node and returns nil if its responsive
156+
// Probe probes a remote node and returns nil if its responsive,
157+
// and an error if it's not.
156158
func (mock *commMock) Probe(peer *comm.RemotePeer) error {
157159
return nil
158160
}
159161

162+
// Handshake authenticates a remote peer and returns
163+
// (its identity, nil) on success and (nil, error)
164+
func (mock *commMock) Handshake(peer *comm.RemotePeer) (api.PeerIdentityType, error) {
165+
return nil, nil
166+
}
167+
160168
// Accept returns a dedicated read-only channel for messages sent by other nodes that match a certain predicate.
161169
// Each message from the channel can be used to send a reply back to the sender
162170
func (mock *commMock) Accept(accept common.MessageAcceptor) <-chan proto.ReceivedMessage {

gossip/gossip/gossip_impl.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,8 @@ func (da *discoveryAdapter) SendToPeer(peer *discovery.NetworkMember, msg *proto
768768
}
769769

770770
func (da *discoveryAdapter) Ping(peer *discovery.NetworkMember) bool {
771-
return da.c.Probe(&comm.RemotePeer{Endpoint: peer.PreferredEndpoint(), PKIID: peer.PKIid}) == nil
771+
err := da.c.Probe(&comm.RemotePeer{Endpoint: peer.PreferredEndpoint(), PKIID: peer.PKIid})
772+
return err == nil
772773
}
773774

774775
func (da *discoveryAdapter) Accept() <-chan *proto.SignedGossipMessage {

0 commit comments

Comments
 (0)