diff --git a/gossip/comm/comm_impl.go b/gossip/comm/comm_impl.go index c482a3199c8..2073dd52510 100644 --- a/gossip/comm/comm_impl.go +++ b/gossip/comm/comm_impl.go @@ -433,7 +433,10 @@ func (c *commImpl) authenticateRemotePeer(stream stream) (*proto.ConnectionInfo, return nil, errors.New("No TLS certificate") } - cMsg = c.createConnectionMsg(c.PKIID, c.selfCertHash, c.peerIdentity, signer) + cMsg, err = c.createConnectionMsg(c.PKIID, c.selfCertHash, c.peerIdentity, signer) + if err != nil { + return nil, err + } c.logger.Debug("Sending", cMsg, "to", remoteAddress) stream.Send(cMsg.Envelope) @@ -580,7 +583,7 @@ func readWithTimeout(stream interface{}, timeout time.Duration, address string) } } -func (c *commImpl) createConnectionMsg(pkiID common.PKIidType, certHash []byte, cert api.PeerIdentityType, signer proto.Signer) *proto.SignedGossipMessage { +func (c *commImpl) createConnectionMsg(pkiID common.PKIidType, certHash []byte, cert api.PeerIdentityType, signer proto.Signer) (*proto.SignedGossipMessage, error) { m := &proto.GossipMessage{ Tag: proto.GossipMessage_EMPTY, Nonce: 0, @@ -595,8 +598,8 @@ func (c *commImpl) createConnectionMsg(pkiID common.PKIidType, certHash []byte, sMsg := &proto.SignedGossipMessage{ GossipMessage: m, } - sMsg.Sign(signer) - return sMsg + _, err := sMsg.Sign(signer) + return sMsg, err } type stream interface { diff --git a/gossip/comm/comm_test.go b/gossip/comm/comm_test.go index 1d3883ac631..b37014fe311 100644 --- a/gossip/comm/comm_test.go +++ b/gossip/comm/comm_test.go @@ -156,7 +156,7 @@ func handshaker(endpoint string, comm Comm, t *testing.T, connMutator msgMutator pkiID := common.PKIidType(endpoint) assert.NoError(t, err, "%v", err) - msg := c.createConnectionMsg(pkiID, clientCertHash, []byte(endpoint), func(msg []byte) ([]byte, error) { + msg, _ := c.createConnectionMsg(pkiID, clientCertHash, []byte(endpoint), func(msg []byte) ([]byte, error) { mac := hmac.New(sha256.New, hmacKey) mac.Write(msg) return mac.Sum(nil), nil @@ -423,7 +423,7 @@ func TestCloseConn(t *testing.T) { assert.NoError(t, err, "%v", err) c := &commImpl{} tlsCertHash := certHashFromRawCert(tlsCfg.Certificates[0].Certificate[0]) - connMsg := c.createConnectionMsg(common.PKIidType("pkiID"), tlsCertHash, api.PeerIdentityType("pkiID"), func(msg []byte) ([]byte, error) { + connMsg, _ := c.createConnectionMsg(common.PKIidType("pkiID"), tlsCertHash, api.PeerIdentityType("pkiID"), func(msg []byte) ([]byte, error) { mac := hmac.New(sha256.New, hmacKey) mac.Write(msg) return mac.Sum(nil), nil @@ -711,13 +711,14 @@ func TestPresumedDead(t *testing.T) { } func createGossipMsg() *proto.SignedGossipMessage { - return (&proto.GossipMessage{ + msg, _ := (&proto.GossipMessage{ Tag: proto.GossipMessage_EMPTY, Nonce: uint64(rand.Int()), Content: &proto.GossipMessage_DataMsg{ DataMsg: &proto.DataMessage{}, }, }).NoopSign() + return msg } func remotePeer(port int) *RemotePeer { diff --git a/gossip/comm/mock/mock_comm.go b/gossip/comm/mock/mock_comm.go index c1467efff69..3f4d7b6dbaa 100644 --- a/gossip/comm/mock/mock_comm.go +++ b/gossip/comm/mock/mock_comm.go @@ -85,10 +85,11 @@ func NewCommMock(id string, members map[string]*socketMock) comm.Comm { // Respond sends a GossipMessage to the origin from which this ReceivedMessage was sent from func (packet *packetMock) Respond(msg *proto.GossipMessage) { + sMsg, _ := msg.NoopSign() packet.src.socket <- &packetMock{ src: packet.dst, dst: packet.src, - msg: msg.NoopSign(), + msg: sMsg, } } diff --git a/gossip/comm/mock/mock_comm_test.go b/gossip/comm/mock/mock_comm_test.go index 6d8f7ef8402..61456c3279e 100644 --- a/gossip/comm/mock/mock_comm_test.go +++ b/gossip/comm/mock/mock_comm_test.go @@ -44,12 +44,13 @@ func TestMockComm(t *testing.T) { comm2 := NewCommMock(second.endpoint, members) defer comm2.Stop() - comm2.Send((&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Content: &proto.GossipMessage_StateRequest{&proto.RemoteStateRequest{ StartSeqNum: 1, EndSeqNum: 3, }}, - }).NoopSign(), &comm.RemotePeer{"first", common.PKIidType("first")}) + }).NoopSign() + comm2.Send(sMsg, &comm.RemotePeer{"first", common.PKIidType("first")}) msg := <-msgCh @@ -73,7 +74,7 @@ func TestMockComm_PingPong(t *testing.T) { rcvChA := peerA.Accept(all) rcvChB := peerB.Accept(all) - peerA.Send((&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Content: &proto.GossipMessage_DataMsg{ &proto.DataMessage{ &proto.Payload{ @@ -81,7 +82,8 @@ func TestMockComm_PingPong(t *testing.T) { Data: []byte("Ping"), }, }}, - }).NoopSign(), &comm.RemotePeer{"peerB", common.PKIidType("peerB")}) + }).NoopSign() + peerA.Send(sMsg, &comm.RemotePeer{"peerB", common.PKIidType("peerB")}) msg := <-rcvChB dataMsg := msg.GetGossipMessage().GetDataMsg() diff --git a/gossip/comm/msg.go b/gossip/comm/msg.go index a34155988c1..4b3d4cc6cb5 100644 --- a/gossip/comm/msg.go +++ b/gossip/comm/msg.go @@ -38,7 +38,12 @@ func (m *ReceivedMessageImpl) GetSourceEnvelope() *proto.Envelope { // Respond sends a msg to the source that sent the ReceivedMessageImpl func (m *ReceivedMessageImpl) Respond(msg *proto.GossipMessage) { - m.conn.send(msg.NoopSign(), func(e error) {}) + sMsg, err := msg.NoopSign() + if err != nil { + m.conn.logger.Error("Failed creating SignedGossipMessage:", err) + return + } + m.conn.send(sMsg, func(e error) {}) } // GetGossipMessage returns the inner GossipMessage diff --git a/gossip/discovery/discovery_impl.go b/gossip/discovery/discovery_impl.go index cf0d15356d2..57f3dce939a 100644 --- a/gossip/discovery/discovery_impl.go +++ b/gossip/discovery/discovery_impl.go @@ -172,9 +172,17 @@ func (d *gossipDiscoveryImpl) Connect(member NetworkMember, id identifier) { Endpoint: member.Endpoint, PKIid: id.ID, } - req := d.createMembershipRequest(id.SelfOrg).NoopSign() + req, err := d.createMembershipRequest(id.SelfOrg).NoopSign() + if err != nil { + d.logger.Warning("Failed creating SignedGossipMessage:", err) + continue + } req.Nonce = util.RandomUInt64() - req.NoopSign() + req, err = req.NoopSign() + if err != nil { + d.logger.Warning("Failed adding NONCE to SignedGossipMessage", err) + continue + } go d.sendUntilAcked(peer, req) return } @@ -221,19 +229,16 @@ func (d *gossipDiscoveryImpl) sendUntilAcked(peer *NetworkMember, message *proto } } -func (d *gossipDiscoveryImpl) somePeerIsKnown() bool { - d.lock.RLock() - defer d.lock.RUnlock() - return len(d.aliveLastTS) != 0 -} - func (d *gossipDiscoveryImpl) InitiateSync(peerNum int) { if d.toDie() { return } var peers2SendTo []*NetworkMember - memReq := d.createMembershipRequest(true).NoopSign() - + memReq, err := d.createMembershipRequest(true).NoopSign() + if err != nil { + d.logger.Warning("Failed creating SignedGossipMessage:", err) + return + } d.lock.RLock() n := d.aliveMembership.Size() @@ -404,7 +409,11 @@ func (d *gossipDiscoveryImpl) sendMemResponse(targetMember *proto.Member, intern InternalEndpoint: internalEndpoint, } - memResp := d.createMembershipResponse(targetPeer) + aliveMsg := d.createAliveMessage(true) + if aliveMsg == nil { + return + } + memResp := d.createMembershipResponse(aliveMsg, targetPeer) if memResp == nil { errMsg := `Got a membership request from a peer that shouldn't have sent one: %v, closing connection to the peer as a result.` d.logger.Warningf(errMsg, targetMember) @@ -414,18 +423,22 @@ func (d *gossipDiscoveryImpl) sendMemResponse(targetMember *proto.Member, intern defer d.logger.Debug("Exiting, replying with", memResp) - d.comm.SendToPeer(targetPeer, (&proto.GossipMessage{ + msg, err := (&proto.GossipMessage{ Tag: proto.GossipMessage_EMPTY, Nonce: nonce, Content: &proto.GossipMessage_MemRes{ MemRes: memResp, }, - }).NoopSign()) + }).NoopSign() + if err != nil { + d.logger.Warning("Failed creating SignedGossipMessage:", err) + return + } + d.comm.SendToPeer(targetPeer, msg) } -func (d *gossipDiscoveryImpl) createMembershipResponse(targetMember *NetworkMember) *proto.MembershipResponse { +func (d *gossipDiscoveryImpl) createMembershipResponse(aliveMsg *proto.SignedGossipMessage, targetMember *NetworkMember) *proto.MembershipResponse { shouldBeDisclosed, omitConcealedFields := d.disclosurePolicy(targetMember) - aliveMsg := d.createAliveMessage(true) if !shouldBeDisclosed(aliveMsg) { return nil @@ -594,10 +607,15 @@ func (d *gossipDiscoveryImpl) periodicalReconnectToDead() { } func (d *gossipDiscoveryImpl) sendMembershipRequest(member *NetworkMember, includeInternalEndpoint bool) { - d.comm.SendToPeer(member, d.createMembershipRequest(includeInternalEndpoint)) + req, err := d.createMembershipRequest(includeInternalEndpoint).NoopSign() + if err != nil { + d.logger.Error("Failed creating SignedGossipMessage:", err) + return + } + d.comm.SendToPeer(member, req) } -func (d *gossipDiscoveryImpl) createMembershipRequest(includeInternalEndpoint bool) *proto.SignedGossipMessage { +func (d *gossipDiscoveryImpl) createMembershipRequest(includeInternalEndpoint bool) *proto.GossipMessage { req := &proto.MembershipRequest{ SelfInformation: d.createAliveMessage(includeInternalEndpoint).Envelope, // TODO: sending the known peers is not secure because the remote peer might shouldn't know @@ -605,13 +623,13 @@ func (d *gossipDiscoveryImpl) createMembershipRequest(includeInternalEndpoint bo // TODO: See FAB-2570 for tracking this issue. Known: [][]byte{}, } - return (&proto.GossipMessage{ + return &proto.GossipMessage{ Tag: proto.GossipMessage_EMPTY, Nonce: uint64(0), Content: &proto.GossipMessage_MemReq{ MemReq: req, }, - }).NoopSign() + } } func (d *gossipDiscoveryImpl) copyLastSeen(lastSeenMap map[string]*timestamp) []NetworkMember { @@ -693,7 +711,11 @@ func (d *gossipDiscoveryImpl) periodicalSendAlive() { for !d.toDie() { d.logger.Debug("Sleeping", getAliveTimeInterval()) time.Sleep(getAliveTimeInterval()) - d.comm.Gossip(d.createAliveMessage(true)) + msg := d.createAliveMessage(true) + if msg == nil { + return + } + d.comm.Gossip(msg) } } @@ -726,9 +748,13 @@ func (d *gossipDiscoveryImpl) createAliveMessage(includeInternalEndpoint bool) * }, } + envp := d.crypt.SignMessage(msg2Gossip, internalEndpoint) + if envp == nil { + return nil + } signedMsg := &proto.SignedGossipMessage{ GossipMessage: msg2Gossip, - Envelope: d.crypt.SignMessage(msg2Gossip, internalEndpoint), + Envelope: envp, } if !includeInternalEndpoint { diff --git a/gossip/discovery/discovery_test.go b/gossip/discovery/discovery_test.go index 72efefc01ce..ac2ad7ce855 100644 --- a/gossip/discovery/discovery_test.go +++ b/gossip/discovery/discovery_test.go @@ -44,6 +44,8 @@ func init() { } type dummyCommModule struct { + msgsReceived uint32 + msgsSent uint32 id string presumeDead chan common.PKIidType detectedDead chan string @@ -82,7 +84,8 @@ func (comm *dummyCommModule) SignMessage(am *proto.GossipMessage, internalEndpoi signer := func(msg []byte) ([]byte, error) { return nil, nil } - env := am.NoopSign().Envelope + s, _ := am.NoopSign() + env := s.Envelope env.SignSecret(signer, secret) return env } @@ -115,8 +118,10 @@ func (comm *dummyCommModule) SendToPeer(peer *NetworkMember, msg *proto.SignedGo } } comm.lock.Lock() - comm.streams[peer.Endpoint].Send(msg.NoopSign().Envelope) + s, _ := msg.NoopSign() + comm.streams[peer.Endpoint].Send(s.Envelope) comm.lock.Unlock() + atomic.AddUint32(&comm.msgsSent, 1) } func (comm *dummyCommModule) Ping(peer *NetworkMember) bool { @@ -167,6 +172,14 @@ func (comm *dummyCommModule) CloseConn(peer *NetworkMember) { comm.conns[peer.Endpoint].Close() } +func (g *gossipInstance) receivedMsgCount() int { + return int(atomic.LoadUint32(&g.comm.msgsReceived)) +} + +func (g *gossipInstance) sentMsgCount() int { + return int(atomic.LoadUint32(&g.comm.msgsSent)) +} + func (g *gossipInstance) discoveryImpl() *gossipDiscoveryImpl { return g.Discovery.(*gossipDiscoveryImpl) } @@ -205,6 +218,7 @@ func (g *gossipInstance) GossipStream(stream proto.Gossip_GossipStreamServer) er lgr.Debug(g.Discovery.Self().Endpoint, "Got message:", gMsg) g.comm.incMsgs <- gMsg + atomic.AddUint32(&g.comm.msgsReceived, 1) if aliveMsg := gMsg.GetAliveMsg(); aliveMsg != nil { g.tryForwardMessage(gMsg) @@ -345,11 +359,12 @@ func TestToString(t *testing.T) { func TestBadInput(t *testing.T) { inst := createDiscoveryInstance(2048, fmt.Sprintf("d%d", 0), []string{}) inst.Discovery.(*gossipDiscoveryImpl).handleMsgFromComm(nil) - inst.Discovery.(*gossipDiscoveryImpl).handleMsgFromComm((&proto.GossipMessage{ + s, _ := (&proto.GossipMessage{ Content: &proto.GossipMessage_DataMsg{ DataMsg: &proto.DataMessage{}, }, - }).NoopSign()) + }).NoopSign() + inst.Discovery.(*gossipDiscoveryImpl).handleMsgFromComm(s) } func TestConnect(t *testing.T) { @@ -906,7 +921,8 @@ func TestMsgStoreExpirationWithMembershipMessages(t *testing.T) { // Creating MembershipRequest messages for i := 0; i < peersNum; i++ { memReqMsg := instances[i].discoveryImpl().createMembershipRequest(true) - memReqMsgs = append(memReqMsgs, memReqMsg) + sMsg, _ := memReqMsg.NoopSign() + memReqMsgs = append(memReqMsgs, sMsg) } // Creating Alive messages for i := 0; i < peersNum; i++ { @@ -973,7 +989,8 @@ func TestMsgStoreExpirationWithMembershipMessages(t *testing.T) { return k == i }, func(k int) { - memResp := instances[k].discoveryImpl().createMembershipResponse(peerToResponse) + aliveMsg := instances[k].discoveryImpl().createAliveMessage(true) + memResp := instances[k].discoveryImpl().createMembershipResponse(aliveMsg, peerToResponse) memRespMsgs[i] = append(memRespMsgs[i], memResp) }) } @@ -1048,13 +1065,14 @@ func TestMsgStoreExpirationWithMembershipMessages(t *testing.T) { for i := 0; i < peersNum; i++ { respForPeer := memRespMsgs[i] for _, msg := range respForPeer { - instances[i].discoveryImpl().handleMsgFromComm((&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Tag: proto.GossipMessage_EMPTY, Nonce: uint64(0), Content: &proto.GossipMessage_MemRes{ MemRes: msg, }, - }).NoopSign()) + }).NoopSign() + instances[i].discoveryImpl().handleMsgFromComm(sMsg) } } @@ -1087,7 +1105,8 @@ func TestAliveMsgStore(t *testing.T) { // Creating MembershipRequest messages for i := 0; i < peersNum; i++ { memReqMsg := instances[i].discoveryImpl().createMembershipRequest(true) - memReqMsgs = append(memReqMsgs, memReqMsg) + sMsg, _ := memReqMsg.NoopSign() + memReqMsgs = append(memReqMsgs, sMsg) } // Creating Alive messages for i := 0; i < peersNum; i++ { @@ -1117,6 +1136,29 @@ func TestAliveMsgStore(t *testing.T) { } } +func TestMemRespDisclosurePol(t *testing.T) { + t.Parallel() + pol := func(remotePeer *NetworkMember) (Sieve, EnvelopeFilter) { + return func(_ *proto.SignedGossipMessage) bool { + return remotePeer.Endpoint == "localhost:7880" + }, func(m *proto.SignedGossipMessage) *proto.Envelope { + return m.Envelope + } + } + d1 := createDiscoveryInstanceThatGossips(7878, "d1", []string{}, true, pol) + defer d1.Stop() + d2 := createDiscoveryInstanceThatGossips(7879, "d2", []string{"localhost:7878"}, true, noopPolicy) + defer d2.Stop() + d3 := createDiscoveryInstanceThatGossips(7880, "d3", []string{"localhost:7878"}, true, noopPolicy) + defer d3.Stop() + // Both d1 and d3 know each other, and also about d2 + assertMembership(t, []*gossipInstance{d1, d3}, 2) + // d2 doesn't know about any one because the bootstrap peer is ignoring it due to custom policy + assertMembership(t, []*gossipInstance{d2}, 0) + assert.Zero(t, d2.receivedMsgCount()) + assert.NotZero(t, d2.sentMsgCount()) +} + func waitUntilOrFail(t *testing.T, pred func() bool) { waitUntilTimeoutOrFail(t, pred, timeout) } diff --git a/gossip/gossip/anchor_test.go b/gossip/gossip/anchor_test.go index 9817e6fcbf6..7cdcfef3d9f 100644 --- a/gossip/gossip/anchor_test.go +++ b/gossip/gossip/anchor_test.go @@ -169,19 +169,20 @@ func memResp(nonce uint64, endpoint string) *proto.SignedGossipMessage { }, } - gMsg := &proto.SignedGossipMessage{ + m, _ := fakePeerAliveMsg.Sign((&configurableCryptoService{}).Sign) + sMsg, _ := (&proto.SignedGossipMessage{ GossipMessage: &proto.GossipMessage{ Tag: proto.GossipMessage_EMPTY, Nonce: nonce, Content: &proto.GossipMessage_MemRes{ MemRes: &proto.MembershipResponse{ - Alive: []*proto.Envelope{fakePeerAliveMsg.Sign((&configurableCryptoService{}).Sign)}, + Alive: []*proto.Envelope{m}, Dead: []*proto.Envelope{}, }, }, }, - } - return gMsg.NoopSign() + }).NoopSign() + return sMsg } type msgInspection func(t *testing.T, index int, m *receivedMsg) diff --git a/gossip/gossip/certstore.go b/gossip/gossip/certstore.go index 1c39c865f89..75020781003 100644 --- a/gossip/gossip/certstore.go +++ b/gossip/gossip/certstore.go @@ -56,7 +56,11 @@ func newCertStore(puller pull.Mediator, idMapper identity.Mapper, selfIdentity a certStore.logger.Panic("Failed associating self PKIID to cert:", err) } - puller.Add(certStore.createIdentityMessage()) + selfIdMsg, err := certStore.createIdentityMessage() + if err != nil { + logger.Panic("Failed creating self identity message:", err) + } + puller.Add(selfIdMsg) puller.RegisterMsgHook(pull.RequestMsgType, func(_ []string, msgs []*proto.SignedGossipMessage, _ proto.ReceivedMessage) { for _, msg := range msgs { pkiID := common.PKIidType(msg.GetPeerIdentity().PkiId) @@ -115,7 +119,7 @@ func (cs *certStore) validateIdentityMsg(msg *proto.SignedGossipMessage) error { return cs.mcs.ValidateIdentity(api.PeerIdentityType(idMsg.Cert)) } -func (cs *certStore) createIdentityMessage() *proto.SignedGossipMessage { +func (cs *certStore) createIdentityMessage() (*proto.SignedGossipMessage, error) { identity := &proto.PeerIdentity{ Cert: cs.selfIdentity, Metadata: nil, @@ -135,8 +139,8 @@ func (cs *certStore) createIdentityMessage() *proto.SignedGossipMessage { sMsg := &proto.SignedGossipMessage{ GossipMessage: m, } - sMsg.Sign(signer) - return sMsg + _, err := sMsg.Sign(signer) + return sMsg, err } func (cs *certStore) listRevokedPeers(isSuspected api.PeerSuspector) []common.PKIidType { diff --git a/gossip/gossip/certstore_test.go b/gossip/gossip/certstore_test.go index a1f5a64274a..9e49b5e10d8 100644 --- a/gossip/gossip/certstore_test.go +++ b/gossip/gossip/certstore_test.go @@ -169,7 +169,8 @@ func TestCertRevocation(t *testing.T) { }, }, } - go cStore.handleMessage(&sentMsg{msg: dig.NoopSign()}) + sMsg, _ := dig.NoopSign() + go cStore.handleMessage(&sentMsg{msg: sMsg}) } if dataReq := msg.GetDataReq(); dataReq != nil { @@ -263,18 +264,19 @@ func TestCertExpiration(t *testing.T) { } func testCertificateUpdate(t *testing.T, shouldSucceed bool, certStore *certStore) { - hello := &sentMsg{ - msg: (&proto.GossipMessage{ - Channel: []byte(""), - Tag: proto.GossipMessage_EMPTY, - Content: &proto.GossipMessage_Hello{ - Hello: &proto.GossipHello{ - Nonce: 0, - Metadata: nil, - MsgType: proto.PullMsgType_IDENTITY_MSG, - }, + msg, _ := (&proto.GossipMessage{ + Channel: []byte(""), + Tag: proto.GossipMessage_EMPTY, + Content: &proto.GossipMessage_Hello{ + Hello: &proto.GossipHello{ + Nonce: 0, + Metadata: nil, + MsgType: proto.PullMsgType_IDENTITY_MSG, }, - }).NoopSign(), + }, + }).NoopSign() + hello := &sentMsg{ + msg: msg, } responseChan := make(chan *proto.GossipMessage, 1) hello.On("Respond", mock.Anything).Run(func(arg mock.Arguments) { @@ -387,7 +389,8 @@ func createUpdateMessage(nonce uint64, idMsg *proto.SignedGossipMessage) proto.R }, }, } - return &sentMsg{msg: update.NoopSign()} + sMsg, _ := update.NoopSign() + return &sentMsg{msg: sMsg} } func createDigest(nonce uint64) proto.ReceivedMessage { @@ -401,7 +404,8 @@ func createDigest(nonce uint64) proto.ReceivedMessage { }, }, } - return &sentMsg{msg: digest.NoopSign()} + sMsg, _ := digest.NoopSign() + return &sentMsg{msg: sMsg} } func createObjects(updateFactory func(uint64) proto.ReceivedMessage, msgCons proto.MsgConsumer) (pull.Mediator, *certStore, *senderMock) { diff --git a/gossip/gossip/channel/channel.go b/gossip/gossip/channel/channel.go index da705430ce4..ae1383e79f2 100644 --- a/gossip/gossip/channel/channel.go +++ b/gossip/gossip/channel/channel.go @@ -249,7 +249,11 @@ func (gc *gossipChannel) GetPeers() []discovery.NetworkMember { } func (gc *gossipChannel) requestStateInfo() { - req := gc.createStateInfoRequest().NoopSign() + req, err := gc.createStateInfoRequest() + if err != nil { + gc.logger.Warning("Failed creating SignedGossipMessage:", err) + return + } endpoints := filter.SelectPeers(gc.GetConf().PullPeerNum, gc.GetMembership(), gc.IsMemberInChan) gc.Send(req, endpoints...) } @@ -636,7 +640,7 @@ func (gc *gossipChannel) verifyMsg(msg proto.ReceivedMessage) bool { return true } -func (gc *gossipChannel) createStateInfoRequest() *proto.SignedGossipMessage { +func (gc *gossipChannel) createStateInfoRequest() (*proto.SignedGossipMessage, error) { return (&proto.GossipMessage{ Tag: proto.GossipMessage_CHAN_OR_ORG, Nonce: 0, diff --git a/gossip/gossip/channel/channel_test.go b/gossip/gossip/channel/channel_test.go index 7f0a1714b5d..b5cf2c79578 100644 --- a/gossip/gossip/channel/channel_test.go +++ b/gossip/gossip/channel/channel_test.go @@ -268,9 +268,11 @@ func TestBadInput(t *testing.T) { assert.False(t, gc.verifyMsg(nil)) assert.False(t, gc.verifyMsg(&receivedMsg{msg: nil, PKIID: nil})) - gc.UpdateStateInfo(createDataMsg(0, channelA).NoopSign()) + s, _ := createDataMsg(0, channelA).NoopSign() + gc.UpdateStateInfo(s) gc.IsMemberInChan(discovery.NetworkMember{PKIid: pkiIDnilOrg}) - gc.HandleMessage(&receivedMsg{msg: (&proto.GossipMessage{}).NoopSign()}) + s, _ = (&proto.GossipMessage{}).NoopSign() + gc.HandleMessage(&receivedMsg{msg: s}) gc.HandleMessage(&receivedMsg{msg: createDataUpdateMsg(0), PKIID: pkiIDnilOrg}) } @@ -313,16 +315,17 @@ func TestMsgStoreNotExpire(t *testing.T) { simulateStateInfoRequest := func(pkiID []byte, outChan chan *proto.SignedGossipMessage) { sentMessages := make(chan *proto.GossipMessage, 1) // Ensure we respond to stateInfoSnapshot requests with valid MAC + s, _ := (&proto.GossipMessage{ + Tag: proto.GossipMessage_CHAN_OR_ORG, + Content: &proto.GossipMessage_StateInfoPullReq{ + StateInfoPullReq: &proto.StateInfoPullRequest{ + Channel_MAC: GenerateMAC(pkiID, channelA), + }, + }, + }).NoopSign() snapshotReq := &receivedMsg{ PKIID: pkiID, - msg: (&proto.GossipMessage{ - Tag: proto.GossipMessage_CHAN_OR_ORG, - Content: &proto.GossipMessage_StateInfoPullReq{ - StateInfoPullReq: &proto.StateInfoPullRequest{ - Channel_MAC: GenerateMAC(pkiID, channelA), - }, - }, - }).NoopSign(), + msg: s, } snapshotReq.On("Respond", mock.Anything).Run(func(args mock.Arguments) { sentMessages <- args.Get(0).(*proto.GossipMessage) @@ -699,7 +702,7 @@ func TestChannelPeerNotInChannel(t *testing.T) { cs.Mock = mock.Mock{} // Ensure we respond to a valid StateInfoRequest - req := gc.(*gossipChannel).createStateInfoRequest() + req, _ := gc.(*gossipChannel).createStateInfoRequest() validReceivedMsg := &receivedMsg{ msg: req, PKIID: pkiIDInOrg1, @@ -726,7 +729,7 @@ func TestChannelPeerNotInChannel(t *testing.T) { } // Ensure we don't respond to a StateInfoRequest in the wrong channel from a peer in the right org - req2 := gc.(*gossipChannel).createStateInfoRequest() + req2, _ := gc.(*gossipChannel).createStateInfoRequest() req2.GetStateInfoPullReq().Channel_MAC = GenerateMAC(pkiIDInOrg1, common.ChainID("B")) invalidReceivedMsg2 := &receivedMsg{ msg: req2, @@ -974,7 +977,8 @@ func TestChannelPulledBadBlocks(t *testing.T) { changeChan := func(env *proto.Envelope) { sMsg, _ := env.ToGossipMessage() sMsg.Channel = []byte("B") - env.Payload = sMsg.NoopSign().Payload + sMsg, _ = sMsg.NoopSign() + env.Payload = sMsg.Payload } pullPhase1 := simulatePullPhase(gc, t, &wg, changeChan, 10, 11) @@ -1019,7 +1023,8 @@ func TestChannelPulledBadBlocks(t *testing.T) { emptyBlock := func(env *proto.Envelope) { sMsg, _ := env.ToGossipMessage() sMsg.GossipMessage.GetDataMsg().Payload = nil - env.Payload = sMsg.NoopSign().Payload + sMsg, _ = sMsg.NoopSign() + env.Payload = sMsg.Payload } pullPhase3 := simulatePullPhase(gc, t, &wg3, emptyBlock, 10, 11) adapter.On("Send", mock.Anything, mock.Anything).Run(pullPhase3) @@ -1042,7 +1047,8 @@ func TestChannelPulledBadBlocks(t *testing.T) { nonBlockMsg := func(env *proto.Envelope) { sMsg, _ := env.ToGossipMessage() sMsg.Content = createHelloMsg(pkiIDInOrg1).GetGossipMessage().Content - env.Payload = sMsg.NoopSign().Payload + sMsg, _ = sMsg.NoopSign() + env.Payload = sMsg.Payload } pullPhase4 := simulatePullPhase(gc, t, &wg4, nonBlockMsg, 10, 11) adapter.On("Send", mock.Anything, mock.Anything).Run(pullPhase4) @@ -1082,7 +1088,7 @@ func TestChannelStateInfoSnapshot(t *testing.T) { // Ensure we ignore stateInfo snapshots with StateInfo messages with wrong MACs sim := createStateInfoMsg(4, pkiIDInOrg1, channelA) sim.GetStateInfo().Channel_MAC = append(sim.GetStateInfo().Channel_MAC, 1) - sim = sim.NoopSign() + sim, _ = sim.NoopSign() gc.HandleMessage(&receivedMsg{PKIID: pkiIDInOrg1, msg: stateInfoSnapshotForChannel(channelA, sim)}) assert.Empty(t, gc.GetPeers()) @@ -1096,16 +1102,17 @@ func TestChannelStateInfoSnapshot(t *testing.T) { assert.Equal(t, "4", string(gc.GetPeers()[0].Metadata)) // Check we don't respond to stateInfoSnapshot requests with wrong MAC + sMsg, _ := (&proto.GossipMessage{ + Tag: proto.GossipMessage_CHAN_OR_ORG, + Content: &proto.GossipMessage_StateInfoPullReq{ + StateInfoPullReq: &proto.StateInfoPullRequest{ + Channel_MAC: append(GenerateMAC(pkiIDInOrg1, channelA), 1), + }, + }, + }).NoopSign() snapshotReq := &receivedMsg{ PKIID: pkiIDInOrg1, - msg: (&proto.GossipMessage{ - Tag: proto.GossipMessage_CHAN_OR_ORG, - Content: &proto.GossipMessage_StateInfoPullReq{ - StateInfoPullReq: &proto.StateInfoPullRequest{ - Channel_MAC: append(GenerateMAC(pkiIDInOrg1, channelA), 1), - }, - }, - }).NoopSign(), + msg: sMsg, } snapshotReq.On("Respond", mock.Anything).Run(func(args mock.Arguments) { sentMessages <- args.Get(0).(*proto.GossipMessage) @@ -1119,16 +1126,17 @@ func TestChannelStateInfoSnapshot(t *testing.T) { } // Ensure we respond to stateInfoSnapshot requests with valid MAC + sMsg, _ = (&proto.GossipMessage{ + Tag: proto.GossipMessage_CHAN_OR_ORG, + Content: &proto.GossipMessage_StateInfoPullReq{ + StateInfoPullReq: &proto.StateInfoPullRequest{ + Channel_MAC: GenerateMAC(pkiIDInOrg1, channelA), + }, + }, + }).NoopSign() snapshotReq = &receivedMsg{ PKIID: pkiIDInOrg1, - msg: (&proto.GossipMessage{ - Tag: proto.GossipMessage_CHAN_OR_ORG, - Content: &proto.GossipMessage_StateInfoPullReq{ - StateInfoPullReq: &proto.StateInfoPullRequest{ - Channel_MAC: GenerateMAC(pkiIDInOrg1, channelA), - }, - }, - }).NoopSign(), + msg: sMsg, } snapshotReq.On("Respond", mock.Anything).Run(func(args mock.Arguments) { sentMessages <- args.Get(0).(*proto.GossipMessage) @@ -1187,16 +1195,17 @@ func TestInterOrgExternalEndpointDisclosure(t *testing.T) { // Check that we only return StateInfo messages of peers with external endpoints // to peers of other orgs + sMsg, _ := (&proto.GossipMessage{ + Tag: proto.GossipMessage_CHAN_OR_ORG, + Content: &proto.GossipMessage_StateInfoPullReq{ + StateInfoPullReq: &proto.StateInfoPullRequest{ + Channel_MAC: GenerateMAC(pkiID3, channelA), + }, + }, + }).NoopSign() snapshotReq := &receivedMsg{ PKIID: pkiID3, - msg: (&proto.GossipMessage{ - Tag: proto.GossipMessage_CHAN_OR_ORG, - Content: &proto.GossipMessage_StateInfoPullReq{ - StateInfoPullReq: &proto.StateInfoPullRequest{ - Channel_MAC: GenerateMAC(pkiID3, channelA), - }, - }, - }).NoopSign(), + msg: sMsg, } snapshotReq.On("Respond", mock.Anything).Run(func(args mock.Arguments) { sentMessages <- args.Get(0).(*proto.GossipMessage) @@ -1218,16 +1227,17 @@ func TestInterOrgExternalEndpointDisclosure(t *testing.T) { // Check that we return all StateInfo messages to peers in our organization, regardless // if the peers from foreign organizations have external endpoints or not + sMsg, _ = (&proto.GossipMessage{ + Tag: proto.GossipMessage_CHAN_OR_ORG, + Content: &proto.GossipMessage_StateInfoPullReq{ + StateInfoPullReq: &proto.StateInfoPullRequest{ + Channel_MAC: GenerateMAC(pkiID2, channelA), + }, + }, + }).NoopSign() snapshotReq = &receivedMsg{ PKIID: pkiID2, - msg: (&proto.GossipMessage{ - Tag: proto.GossipMessage_CHAN_OR_ORG, - Content: &proto.GossipMessage_StateInfoPullReq{ - StateInfoPullReq: &proto.StateInfoPullRequest{ - Channel_MAC: GenerateMAC(pkiID2, channelA), - }, - }, - }).NoopSign(), + msg: sMsg, } snapshotReq.On("Respond", mock.Anything).Run(func(args mock.Arguments) { sentMessages <- args.Get(0).(*proto.GossipMessage) @@ -1355,8 +1365,9 @@ func TestChannelReconfigureChannel(t *testing.T) { assert.True(t, gc.IsMemberInChan(discovery.NetworkMember{PKIid: pkiIDinOrg2})) // Ensure we don't respond to a StateInfoRequest from a peer in the wrong org + sMsg, _ := gc.(*gossipChannel).createStateInfoRequest() invalidReceivedMsg := &receivedMsg{ - msg: gc.(*gossipChannel).createStateInfoRequest(), + msg: sMsg, PKIID: pkiIDInOrg1, } gossipMessagesSentFromChannel := make(chan *proto.GossipMessage, 1) @@ -1516,7 +1527,8 @@ func createDataUpdateMsg(nonce uint64, seqs ...uint64) *proto.SignedGossipMessag for _, seq := range seqs { msg.GetDataUpdate().Data = append(msg.GetDataUpdate().Data, createDataMsg(seq, channelA).Envelope) } - return (msg).NoopSign() + sMsg, _ := msg.NoopSign() + return sMsg } func createHelloMsg(PKIID common.PKIidType) *receivedMsg { @@ -1531,11 +1543,12 @@ func createHelloMsg(PKIID common.PKIidType) *receivedMsg { }, }, } - return &receivedMsg{msg: msg.NoopSign(), PKIID: PKIID} + sMsg, _ := msg.NoopSign() + return &receivedMsg{msg: sMsg, PKIID: PKIID} } func dataMsgOfChannel(seqnum uint64, channel common.ChainID) *proto.SignedGossipMessage { - return (&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Channel: []byte(channel), Nonce: 0, Tag: proto.GossipMessage_CHAN_AND_ORG, @@ -1548,10 +1561,11 @@ func dataMsgOfChannel(seqnum uint64, channel common.ChainID) *proto.SignedGossip }, }, }).NoopSign() + return sMsg } func createStateInfoMsg(ledgerHeight int, pkiID common.PKIidType, channel common.ChainID) *proto.SignedGossipMessage { - return (&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Tag: proto.GossipMessage_CHAN_OR_ORG, Content: &proto.GossipMessage_StateInfo{ StateInfo: &proto.StateInfo{ @@ -1562,6 +1576,7 @@ func createStateInfoMsg(ledgerHeight int, pkiID common.PKIidType, channel common }, }, }).NoopSign() + return sMsg } func stateInfoSnapshotForChannel(chainID common.ChainID, stateInfoMsgs ...*proto.SignedGossipMessage) *proto.SignedGossipMessage { @@ -1569,7 +1584,7 @@ func stateInfoSnapshotForChannel(chainID common.ChainID, stateInfoMsgs ...*proto for i, sim := range stateInfoMsgs { envelopes[i] = sim.Envelope } - return (&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Channel: chainID, Tag: proto.GossipMessage_CHAN_OR_ORG, Nonce: 0, @@ -1579,10 +1594,11 @@ func stateInfoSnapshotForChannel(chainID common.ChainID, stateInfoMsgs ...*proto }, }, }).NoopSign() + return sMsg } func createDataMsg(seqnum uint64, channel common.ChainID) *proto.SignedGossipMessage { - return (&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Nonce: 0, Tag: proto.GossipMessage_CHAN_AND_ORG, Channel: []byte(channel), @@ -1595,6 +1611,7 @@ func createDataMsg(seqnum uint64, channel common.ChainID) *proto.SignedGossipMes }, }, }).NoopSign() + return sMsg } func simulatePullPhase(gc GossipChannel, t *testing.T, wg *sync.WaitGroup, mutator msgMutator, seqs ...uint64) func(args mock.Arguments) { @@ -1608,19 +1625,20 @@ func simulatePullPhase(gc GossipChannel, t *testing.T, wg *sync.WaitGroup, mutat if msg.IsHelloMsg() && !sentHello { sentHello = true // Simulate a digest message an imaginary peer responds to the hello message sent + sMsg, _ := (&proto.GossipMessage{ + Tag: proto.GossipMessage_CHAN_AND_ORG, + Channel: []byte(channelA), + Content: &proto.GossipMessage_DataDig{ + DataDig: &proto.DataDigest{ + MsgType: proto.PullMsgType_BLOCK_MSG, + Digests: []string{"10", "11"}, + Nonce: msg.GetHello().Nonce, + }, + }, + }).NoopSign() digestMsg := &receivedMsg{ PKIID: pkiIDInOrg1, - msg: (&proto.GossipMessage{ - Tag: proto.GossipMessage_CHAN_AND_ORG, - Channel: []byte(channelA), - Content: &proto.GossipMessage_DataDig{ - DataDig: &proto.DataDigest{ - MsgType: proto.PullMsgType_BLOCK_MSG, - Digests: []string{"10", "11"}, - Nonce: msg.GetHello().Nonce, - }, - }, - }).NoopSign(), + msg: sMsg, } go gc.HandleMessage(digestMsg) } diff --git a/gossip/gossip/gossip_impl.go b/gossip/gossip/gossip_impl.go index b908668b93a..157449c6d34 100644 --- a/gossip/gossip/gossip_impl.go +++ b/gossip/gossip/gossip_impl.go @@ -628,9 +628,14 @@ func (g *gossipServiceImpl) Gossip(msg *proto.GossipMessage) { sMsg := &proto.SignedGossipMessage{ GossipMessage: msg, } - sMsg.Sign(func(msg []byte) ([]byte, error) { + + _, err := sMsg.Sign(func(msg []byte) ([]byte, error) { return g.mcs.Sign(msg) }) + if err != nil { + g.logger.Warning("Failed signing message:", err) + return + } if msg.IsChannelRestricted() { gc := g.chanState.getGossipChannelByChainID(msg.Channel) @@ -651,7 +656,12 @@ func (g *gossipServiceImpl) Gossip(msg *proto.GossipMessage) { // Send sends a message to remote peers func (g *gossipServiceImpl) Send(msg *proto.GossipMessage, peers ...*comm.RemotePeer) { - g.comm.Send(msg.NoopSign(), peers...) + m, err := msg.NoopSign() + if err != nil { + g.logger.Warning("Failed creating SignedGossipMessage:", err) + return + } + g.comm.Send(m, peers...) } // GetPeers returns a mapping of endpoint --> []discovery.NetworkMember @@ -845,7 +855,10 @@ func (da *discoveryAdapter) SendToPeer(peer *discovery.NetworkMember, msg *proto MemReq: memReq, } // Update the envelope of the outer message, no need to sign (point2point) - msg = msg.NoopSign() + msg, err = msg.NoopSign() + if err != nil { + return + } } da.c.Send(msg, &comm.RemotePeer{PKIID: peer.PKIid, Endpoint: peer.PreferredEndpoint()}) } @@ -934,7 +947,12 @@ func (sa *discoverySecurityAdapter) SignMessage(m *proto.GossipMessage, internal sMsg := &proto.SignedGossipMessage{ GossipMessage: m, } - e := sMsg.Sign(signer) + e, err := sMsg.Sign(signer) + if err != nil { + sa.logger.Warning("Failed signing message:", err) + return nil + } + if internalEndpoint == "" { return e } @@ -1084,8 +1102,8 @@ func (g *gossipServiceImpl) createStateInfoMsg(metadata []byte, chainID common.C signer := func(msg []byte) ([]byte, error) { return g.mcs.Sign(msg) } - sMsg.Sign(signer) - return sMsg, nil + _, err := sMsg.Sign(signer) + return sMsg, err } func (g *gossipServiceImpl) hasExternalEndpoint(PKIID common.PKIidType) bool { diff --git a/gossip/gossip/gossip_test.go b/gossip/gossip/gossip_test.go index a9aaf19b9bb..81a1d325f7f 100644 --- a/gossip/gossip/gossip_test.go +++ b/gossip/gossip/gossip_test.go @@ -761,7 +761,7 @@ func TestMembershipRequestSpoofing(t *testing.T) { // Now, create a membership request message memRequestSpoofFactory := func(aliveMsgEnv *proto.Envelope) *proto.SignedGossipMessage { - return (&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Tag: proto.GossipMessage_EMPTY, Nonce: uint64(0), Content: &proto.GossipMessage_MemReq{ @@ -771,6 +771,7 @@ func TestMembershipRequestSpoofing(t *testing.T) { }, }, }).NoopSign() + return sMsg } spoofedMemReq := memRequestSpoofFactory(aliveMsg.GetSourceEnvelope()) g2.Send(spoofedMemReq.GossipMessage, &comm.RemotePeer{Endpoint: "localhost:2000", PKIID: common.PKIidType("localhost:2000")}) diff --git a/gossip/gossip/pull/pullstore.go b/gossip/gossip/pull/pullstore.go index 93674f35801..0dc98c58159 100644 --- a/gossip/gossip/pull/pullstore.go +++ b/gossip/gossip/pull/pullstore.go @@ -262,7 +262,12 @@ func (p *pullMediatorImpl) Hello(dest string, nonce uint64) { } p.logger.Debug("Sending", p.config.MsgType, "hello to", dest) - p.Sndr.Send(helloMsg.NoopSign(), p.peersWithEndpoints(dest)...) + sMsg, err := helloMsg.NoopSign() + if err != nil { + p.logger.Error("Failed creating SignedGossipMessage:", err) + return + } + p.Sndr.Send(sMsg, p.peersWithEndpoints(dest)...) } // SendDigest sends a digest to a remote PullEngine. @@ -301,7 +306,12 @@ func (p *pullMediatorImpl) SendReq(dest string, items []string, nonce uint64) { }, } p.logger.Debug("Sending", req, "to", dest) - p.Sndr.Send(req.NoopSign(), p.peersWithEndpoints(dest)...) + sMsg, err := req.NoopSign() + if err != nil { + p.logger.Warning("Failed creating SignedGossipMessage:", err) + return + } + p.Sndr.Send(sMsg, p.peersWithEndpoints(dest)...) } // SendRes sends an array of items to a remote PullEngine identified by a context. @@ -314,7 +324,6 @@ func (p *pullMediatorImpl) SendRes(items []string, context interface{}, nonce ui items2return = append(items2return, msg.Envelope) } } - returnedUpdate := &proto.GossipMessage{ Channel: p.config.Channel, Tag: p.config.Tag, diff --git a/gossip/gossip/pull/pullstore_test.go b/gossip/gossip/pull/pullstore_test.go index ea96a879be6..754f0861fcf 100644 --- a/gossip/gossip/pull/pullstore_test.go +++ b/gossip/gossip/pull/pullstore_test.go @@ -54,8 +54,9 @@ func (pm *pullMsg) GetSourceEnvelope() *proto.Envelope { } func (pm *pullMsg) Respond(msg *proto.GossipMessage) { + sMsg, _ := msg.NoopSign() pm.respondChan <- &pullMsg{ - msg: msg.NoopSign(), + msg: sMsg, respondChan: pm.respondChan, } } @@ -296,13 +297,15 @@ func TestHandleMessage(t *testing.T) { }) // inst1 sends hello to inst2 - inst2.mediator.HandleMessage(inst1.wrapPullMsg(helloMsg().NoopSign())) + sMsg, _ := helloMsg().NoopSign() + inst2.mediator.HandleMessage(inst1.wrapPullMsg(sMsg)) // inst2 is expected to send digest to inst1 waitUntilOrFail(t, func() bool { return atomic.LoadInt32(&inst1ReceivedDigest) == int32(1) }) // inst1 sends request to inst2 - inst2.mediator.HandleMessage(inst1.wrapPullMsg(reqMsg("0", "1", "2").NoopSign())) + sMsg, _ = reqMsg("0", "1", "2").NoopSign() + inst2.mediator.HandleMessage(inst1.wrapPullMsg(sMsg)) // inst2 is expected to send response to inst1 waitUntilOrFail(t, func() bool { return atomic.LoadInt32(&inst1ReceivedResponse) == int32(1) }) @@ -325,7 +328,7 @@ func waitUntilOrFail(t *testing.T, pred func() bool) { } func dataMsg(seqNum int) *proto.SignedGossipMessage { - return (&proto.GossipMessage{ + sMsg, _ := (&proto.GossipMessage{ Nonce: 0, Tag: proto.GossipMessage_EMPTY, Content: &proto.GossipMessage_DataMsg{ @@ -337,6 +340,7 @@ func dataMsg(seqNum int) *proto.SignedGossipMessage { }, }, }).NoopSign() + return sMsg } func helloMsg() *proto.GossipMessage { diff --git a/gossip/state/state_test.go b/gossip/state/state_test.go index 138bcbb000f..fc4eeefbe4e 100644 --- a/gossip/state/state_test.go +++ b/gossip/state/state_test.go @@ -270,8 +270,9 @@ func TestNilDirectMsg(t *testing.T) { defer p.shutdown() p.s.(*GossipStateProviderImpl).handleStateRequest(nil) p.s.(*GossipStateProviderImpl).directMessage(nil) + sMsg, _ := p.s.(*GossipStateProviderImpl).stateRequestMessage(uint64(10), uint64(8)).NoopSign() req := &comm.ReceivedMessageImpl{ - SignedGossipMessage: p.s.(*GossipStateProviderImpl).stateRequestMessage(uint64(10), uint64(8)).NoopSign(), + SignedGossipMessage: sMsg, } p.s.(*GossipStateProviderImpl).directMessage(req) } diff --git a/protos/gossip/extensions.go b/protos/gossip/extensions.go index e83c1103aa9..210081de4d8 100644 --- a/protos/gossip/extensions.go +++ b/protos/gossip/extensions.go @@ -361,7 +361,7 @@ type AuthInfo struct { // Sign signs a GossipMessage with given Signer. // Returns an Envelope on success, // panics on failure. -func (m *SignedGossipMessage) Sign(signer Signer) *Envelope { +func (m *SignedGossipMessage) Sign(signer Signer) (*Envelope, error) { // If we have a secretEnvelope, don't override it. // Back it up, and restore it later var secretEnvelope *SecretEnvelope @@ -371,11 +371,11 @@ func (m *SignedGossipMessage) Sign(signer Signer) *Envelope { m.Envelope = nil payload, err := proto.Marshal(m.GossipMessage) if err != nil { - panic(err) + return nil, err } sig, err := signer(payload) if err != nil { - panic(err) + return nil, err } e := &Envelope{ @@ -384,19 +384,19 @@ func (m *SignedGossipMessage) Sign(signer Signer) *Envelope { SecretEnvelope: secretEnvelope, } m.Envelope = e - return e + return e, nil } // NoopSign creates a SignedGossipMessage with a nil signature -func (m *GossipMessage) NoopSign() *SignedGossipMessage { +func (m *GossipMessage) NoopSign() (*SignedGossipMessage, error) { signer := func(msg []byte) ([]byte, error) { return nil, nil } sMsg := &SignedGossipMessage{ GossipMessage: m, } - sMsg.Sign(signer) - return sMsg + _, err := sMsg.Sign(signer) + return sMsg, err } // Verify verifies a signed GossipMessage with a given Verifier. @@ -452,19 +452,20 @@ func (e *Envelope) ToGossipMessage() (*SignedGossipMessage, error) { // SignSecret signs the secret payload and creates // a secret envelope out of it. -func (e *Envelope) SignSecret(signer Signer, secret *Secret) { +func (e *Envelope) SignSecret(signer Signer, secret *Secret) error { payload, err := proto.Marshal(secret) if err != nil { - panic(err) + return err } sig, err := signer(payload) if err != nil { - panic(err) + return err } e.SecretEnvelope = &SecretEnvelope{ Payload: payload, Signature: sig, } + return nil } // InternalEndpoint returns the internal endpoint diff --git a/protos/gossip/extensions_test.go b/protos/gossip/extensions_test.go index c2310caad8c..9a8bccf84b4 100644 --- a/protos/gossip/extensions_test.go +++ b/protos/gossip/extensions_test.go @@ -697,18 +697,14 @@ func TestGossipMessageSign(t *testing.T) { DataMsg: &DataMessage{}, }) - signedMsg := msg.Sign(idSigner) + signedMsg, _ := msg.Sign(idSigner) // Since checking the identity signer, signature will be same as the payload assert.Equal(t, signedMsg.Payload, signedMsg.Signature) - defer func() { - if r := recover(); r == nil { - t.Error("Using error signer should lead to the panic") - } - }() - - _ = msg.Sign(errSigner) + env, err := msg.Sign(errSigner) + assert.Error(t, err) + assert.Nil(t, env) } func TestEnvelope_NoopSign(t *testing.T) { @@ -717,10 +713,11 @@ func TestEnvelope_NoopSign(t *testing.T) { DataMsg: &DataMessage{}, }) - signedMsg := msg.NoopSign() + signedMsg, err := msg.NoopSign() // Since checking the identity signer, signature will be same as the payload assert.Nil(t, signedMsg.Signature) + assert.NoError(t, err) } func TestSignedGossipMessage_Verify(t *testing.T) {