Skip to content

Commit

Permalink
[FAB-3497] Removing the hash from gossip dataMsg
Browse files Browse the repository at this point in the history 
The hash field isn't used anywhere but in the protos comparator function
to compare 2 ledger blocks disseminated.
This imposes a security vulnerability:
A malicious peer can send blocks with an arbitrary hash in the hash field
of the DataMsg, and then these messages would enter the in-memory stores
and fill up the memory.

Since we have no use of the block hash of our own message, I am removing it
entirely.

Change-Id: Ic22ed3d06f102795c8f2a74b27063848affc926a
Signed-off-by: Yacov Manevich <yacovm@il.ibm.com>
  • Loading branch information
@yacovm
yacovm committed May 1, 2017
1 parent 1195f2f commit 1acb65f
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 152 deletions.
10 changes: 8 additions & 2 deletions gossip/comm/mock/mock_comm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ func TestMockComm_PingPong(t *testing.T) {
peerA.Send((&proto.GossipMessage{
Content: &proto.GossipMessage_DataMsg{
&proto.DataMessage{
&proto.Payload{1, "", []byte("Ping")},
&proto.Payload{
SeqNum: 1,
Data: []byte("Ping"),
},
}},
}).NoopSign(), &comm.RemotePeer{"peerB", common.PKIidType("peerB")})

Expand All @@ -88,7 +91,10 @@ func TestMockComm_PingPong(t *testing.T) {
msg.Respond(&proto.GossipMessage{
Content: &proto.GossipMessage_DataMsg{
&proto.DataMessage{
&proto.Payload{1, "", []byte("Pong")},
&proto.Payload{
SeqNum: 1,
Data: []byte("Pong"),
},
}},
})

Expand Down
2 changes: 0 additions & 2 deletions gossip/gossip/channel/channel_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,6 @@ func dataMsgOfChannel(seqnum uint64, channel common.ChainID) *proto.SignedGossip
DataMsg: &proto.DataMessage{
Payload: &proto.Payload{
Data: []byte{},
Hash: "",
SeqNum: seqnum,
},
},
Expand Down Expand Up @@ -1441,7 +1440,6 @@ func createDataMsg(seqnum uint64, channel common.ChainID) *proto.SignedGossipMes
DataMsg: &proto.DataMessage{
Payload: &proto.Payload{
Data: []byte{},
Hash: "",
SeqNum: seqnum,
},
},
Expand Down
13 changes: 6 additions & 7 deletions gossip/gossip/gossip_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ func TestPull(t *testing.T) {
}

for i := 1; i <= msgsCount2Send; i++ {
boot.Gossip(createDataMsg(uint64(i), []byte{}, "", common.ChainID("A")))
boot.Gossip(createDataMsg(uint64(i), []byte{}, common.ChainID("A")))
}

waitUntilOrFail(t, knowAll)
Expand Down Expand Up @@ -564,7 +564,7 @@ func TestDissemination(t *testing.T) {
t.Log("Membership establishment took", time.Since(membershipTime))

for i := 1; i <= msgsCount2Send; i++ {
boot.Gossip(createDataMsg(uint64(i), []byte{}, "", common.ChainID("A")))
boot.Gossip(createDataMsg(uint64(i), []byte{}, common.ChainID("A")))
}

t2 := time.Now()
Expand Down Expand Up @@ -902,8 +902,8 @@ func TestDataLeakage(t *testing.T) {
}

t1 = time.Now()
peers[0].Gossip(createDataMsg(1, []byte{}, "", channels[0]))
peers[n/2].Gossip(createDataMsg(2, []byte{}, "", channels[1]))
peers[0].Gossip(createDataMsg(1, []byte{}, channels[0]))
peers[n/2].Gossip(createDataMsg(2, []byte{}, channels[1]))
waitUntilOrFailBlocking(t, gotMessages)
t.Log("Dissemination took", time.Since(t1))
stop := func() {
Expand Down Expand Up @@ -972,7 +972,7 @@ func TestDisseminateAll2All(t *testing.T) {
blockStartIndex := i * 10
for j := 0; j < 10; j++ {
blockSeq := uint64(j + blockStartIndex)
peers[i].Gossip(createDataMsg(blockSeq, []byte{}, "", common.ChainID("A")))
peers[i].Gossip(createDataMsg(blockSeq, []byte{}, common.ChainID("A")))
}
}(i)
}
Expand Down Expand Up @@ -1044,7 +1044,7 @@ func TestEndedGoroutines(t *testing.T) {
ensureGoroutineExit(t)
}

func createDataMsg(seqnum uint64, data []byte, hash string, channel common.ChainID) *proto.GossipMessage {
func createDataMsg(seqnum uint64, data []byte, channel common.ChainID) *proto.GossipMessage {
return &proto.GossipMessage{
Channel: []byte(channel),
Nonce: 0,
Expand All @@ -1053,7 +1053,6 @@ func createDataMsg(seqnum uint64, data []byte, hash string, channel common.Chain
DataMsg: &proto.DataMessage{
Payload: &proto.Payload{
Data: data,
Hash: hash,
SeqNum: seqnum,
},
},
Expand Down
1 change: 0 additions & 1 deletion gossip/gossip/pull/pullstore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ func dataMsg(seqNum int) *proto.SignedGossipMessage {
DataMsg: &proto.DataMessage{
Payload: &proto.Payload{
Data: []byte{},
Hash: "",
SeqNum: uint64(seqNum),
},
},
Expand Down
23 changes: 4 additions & 19 deletions gossip/state/payloads_buffer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package state

import (
"crypto/rand"
"fmt"
"sync"
"sync/atomic"
"testing"
Expand All @@ -33,30 +32,16 @@ func init() {
util.SetupTestLogging()
}

func uuid() (string, error) {
uuid := make([]byte, 16)
_, err := rand.Read(uuid)
if err != nil {
return "", err
}
uuid[8] = uuid[8]&^0xc0 | 0x80

uuid[6] = uuid[6]&^0xf0 | 0x40
return fmt.Sprintf("%x-%x-%x-%x-%x", uuid[0:4], uuid[4:6], uuid[6:8], uuid[8:10], uuid[10:]), nil
}

func randomPayloadWithSeqNum(seqNum uint64) (*proto.Payload, error) {
data := make([]byte, 64)
_, err := rand.Read(data)
if err != nil {
return nil, err
}
uuid, err := uuid()
if err != nil {
return nil, err
}

return &proto.Payload{seqNum, uuid, data}, nil
return &proto.Payload{
SeqNum: seqNum,
Data: data,
}, nil
}

func TestNewPayloadsBuffer(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion gossip/state/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ func (s *GossipStateProviderImpl) handleStateRequest(msg proto.ReceivedMessage)
response.Payloads = append(response.Payloads, &proto.Payload{
SeqNum: seqNum,
Data: blockBytes,
Hash: string(blocks[0].Header.Hash()),
})
}
// Sending back response with missing blocks
Expand Down
15 changes: 12 additions & 3 deletions gossip/state/state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,10 @@ func TestAccessControl(t *testing.T) {
for i := 1; i <= msgCount; i++ {
rawblock := pcomm.NewBlock(uint64(i), []byte{})
if b, err := pb.Marshal(rawblock); err == nil {
payload := &proto.Payload{uint64(i), "", b}
payload := &proto.Payload{
SeqNum: uint64(i),
Data: b,
}
bootstrapSet[0].s.AddPayload(payload)
} else {
t.Fail()
Expand Down Expand Up @@ -550,7 +553,10 @@ func TestNewGossipStateProvider_SendingManyMessages(t *testing.T) {
for i := 1; i <= msgCount; i++ {
rawblock := pcomm.NewBlock(uint64(i), []byte{})
if b, err := pb.Marshal(rawblock); err == nil {
payload := &proto.Payload{uint64(i), "", b}
payload := &proto.Payload{
SeqNum: uint64(i),
Data: b,
}
bootstrapSet[0].s.AddPayload(payload)
} else {
t.Fail()
Expand Down Expand Up @@ -682,7 +688,10 @@ func TestNewGossipStateProvider_BatchingOfStateRequest(t *testing.T) {
for i := 1; i <= msgCount; i++ {
rawblock := pcomm.NewBlock(uint64(i), []byte{})
if b, err := pb.Marshal(rawblock); err == nil {
payload := &proto.Payload{uint64(i), "", b}
payload := &proto.Payload{
SeqNum: uint64(i),
Data: b,
}
bootPeer.s.AddPayload(payload)
} else {
t.Fail()
Expand Down
5 changes: 1 addition & 4 deletions protos/gossip/extensions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,7 @@ func (mc *msgComparator) identityInvalidationPolicy(thisIdentityMsg *PeerIdentit

func (mc *msgComparator) dataInvalidationPolicy(thisDataMsg *DataMessage, thatDataMsg *DataMessage) common.InvalidationResult {
if thisDataMsg.Payload.SeqNum == thatDataMsg.Payload.SeqNum {
if thisDataMsg.Payload.Hash == thatDataMsg.Payload.Hash {
return common.MessageInvalidated
}
return common.MessageNoAction
return common.MessageInvalidated
}

diff := abs(thisDataMsg.Payload.SeqNum, thatDataMsg.Payload.SeqNum)
Expand Down
42 changes: 19 additions & 23 deletions protos/gossip/extensions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,17 +273,15 @@ func TestDataMessageInvalidation(t *testing.T) {
comparator := NewGossipMessageComparator(5)

data := []byte{1, 1, 1}
sMsg1 := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(1, "hash", data))
sMsg2 := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(1, "hash", data))
sMsg3 := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(1, "newHash", data))
sMsg4 := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(2, "newHash", data))
sMsg5 := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(7, "newHash", data))
sMsg1 := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(1, data))
sMsg1Clone := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(1, data))
sMsg3 := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(2, data))
sMsg4 := signedGossipMessage("testChannel", GossipMessage_EMPTY, dataMessage(7, data))

assert.Equal(t, comparator(sMsg1, sMsg2), common.MessageInvalidated)
assert.Equal(t, comparator(sMsg1, sMsg1Clone), common.MessageInvalidated)
assert.Equal(t, comparator(sMsg1, sMsg3), common.MessageNoAction)
assert.Equal(t, comparator(sMsg1, sMsg4), common.MessageNoAction)
assert.Equal(t, comparator(sMsg1, sMsg5), common.MessageInvalidated)
assert.Equal(t, comparator(sMsg5, sMsg1), common.MessageInvalidates)
assert.Equal(t, comparator(sMsg1, sMsg4), common.MessageInvalidated)
assert.Equal(t, comparator(sMsg4, sMsg1), common.MessageInvalidates)
}

func TestIdentityMessagesInvalidation(t *testing.T) {
Expand Down Expand Up @@ -376,7 +374,7 @@ func TestCheckGossipMessageTypes(t *testing.T) {
assert.True(t, msg.IsAliveMsg())

// Create gossip data message
msg = signedGossipMessage(channelID, GossipMessage_EMPTY, dataMessage(1, "hash", []byte{1, 2, 3, 4, 5}))
msg = signedGossipMessage(channelID, GossipMessage_EMPTY, dataMessage(1, []byte{1, 2, 3, 4, 5}))
assert.True(t, msg.IsDataMsg())

// Create data request message
Expand Down Expand Up @@ -436,7 +434,6 @@ func TestCheckGossipMessageTypes(t *testing.T) {
StateResponse: &RemoteStateResponse{
Payloads: []*Payload{&Payload{
SeqNum: 1,
Hash: "hash",
Data: []byte{1, 2, 3, 4, 5},
}},
},
Expand Down Expand Up @@ -497,7 +494,7 @@ func TestGossipPullMessageType(t *testing.T) {
assert.Equal(t, msg.GetPullMsgType(), PullMsgType_IDENTITY_MSG)

// Create gossip data message
msg = signedGossipMessage(channelID, GossipMessage_EMPTY, dataMessage(1, "hash", []byte{1, 2, 3, 4, 5}))
msg = signedGossipMessage(channelID, GossipMessage_EMPTY, dataMessage(1, []byte{1, 2, 3, 4, 5}))
assert.True(t, msg.IsDataMsg())
assert.Equal(t, msg.GetPullMsgType(), PullMsgType_UNDEFINED)
}
Expand All @@ -506,30 +503,30 @@ func TestGossipMessageDataMessageTagType(t *testing.T) {
var msg *SignedGossipMessage
channelID := "testID1"

msg = signedGossipMessage(channelID, GossipMessage_CHAN_AND_ORG, dataMessage(1, "hash", []byte{1}))
msg = signedGossipMessage(channelID, GossipMessage_CHAN_AND_ORG, dataMessage(1, []byte{1}))
assert.True(t, msg.IsChannelRestricted())
assert.True(t, msg.IsOrgRestricted())
assert.NoError(t, msg.IsTagLegal())

msg = signedGossipMessage(channelID, GossipMessage_EMPTY, dataMessage(1, "hash", []byte{1}))
msg = signedGossipMessage(channelID, GossipMessage_EMPTY, dataMessage(1, []byte{1}))
assert.Error(t, msg.IsTagLegal())

msg = signedGossipMessage(channelID, GossipMessage_UNDEFINED, dataMessage(1, "hash", []byte{1}))
msg = signedGossipMessage(channelID, GossipMessage_UNDEFINED, dataMessage(1, []byte{1}))
assert.Error(t, msg.IsTagLegal())

msg = signedGossipMessage(channelID, GossipMessage_ORG_ONLY, dataMessage(1, "hash", []byte{1}))
msg = signedGossipMessage(channelID, GossipMessage_ORG_ONLY, dataMessage(1, []byte{1}))
assert.False(t, msg.IsChannelRestricted())
assert.True(t, msg.IsOrgRestricted())

msg = signedGossipMessage(channelID, GossipMessage_CHAN_OR_ORG, dataMessage(1, "hash", []byte{1}))
msg = signedGossipMessage(channelID, GossipMessage_CHAN_OR_ORG, dataMessage(1, []byte{1}))
assert.True(t, msg.IsChannelRestricted())
assert.False(t, msg.IsOrgRestricted())

msg = signedGossipMessage(channelID, GossipMessage_EMPTY, dataMessage(1, "hash", []byte{1}))
msg = signedGossipMessage(channelID, GossipMessage_EMPTY, dataMessage(1, []byte{1}))
assert.False(t, msg.IsChannelRestricted())
assert.False(t, msg.IsOrgRestricted())

msg = signedGossipMessage(channelID, GossipMessage_UNDEFINED, dataMessage(1, "hash", []byte{1}))
msg = signedGossipMessage(channelID, GossipMessage_UNDEFINED, dataMessage(1, []byte{1}))
assert.False(t, msg.IsChannelRestricted())
assert.False(t, msg.IsOrgRestricted())
}
Expand Down Expand Up @@ -774,7 +771,7 @@ func TestSignedGossipMessage_Verify(t *testing.T) {

func TestEnvelope(t *testing.T) {
dataMsg := &GossipMessage{
Content: dataMessage(1, "hash", []byte("data")),
Content: dataMessage(1, []byte("data")),
}
bytes, err := proto.Marshal(dataMsg)
assert.NoError(t, err)
Expand All @@ -791,7 +788,7 @@ func TestEnvelope(t *testing.T) {

func TestEnvelope_SignSecret(t *testing.T) {
dataMsg := &GossipMessage{
Content: dataMessage(1, "hash", []byte("data")),
Content: dataMessage(1, []byte("data")),
}
bytes, err := proto.Marshal(dataMsg)
assert.NoError(t, err)
Expand Down Expand Up @@ -851,12 +848,11 @@ func stateInfoMessage(incNumber uint64, seqNum uint64, pkid []byte, mac []byte)
}
}

func dataMessage(seqNum uint64, hash string, data []byte) *GossipMessage_DataMsg {
func dataMessage(seqNum uint64, data []byte) *GossipMessage_DataMsg {
return &GossipMessage_DataMsg{
DataMsg: &DataMessage{
Payload: &Payload{
SeqNum: seqNum,
Hash: hash,
Data: data,
},
},
Expand Down
Loading

0 comments on commit 1acb65f

Please sign in to comment.