adding checks to prevent malicious leader behaviors (#2418)

fixing golint

generalizing the error messages based on message type name

removing the additional check for onNewView

updating checks for new view message
pull/2490/head
Ganesha Upadhyaya 5 years ago committed by GitHub
parent 6ef7b0a2a4
commit 4c40ad3eb1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 104
      consensus/checks.go
  2. 6
      consensus/consensus_v2.go
  3. 52
      consensus/view_change.go

@ -7,6 +7,9 @@ import (
"github.com/harmony-one/harmony/shard"
)
// MaxBlockNumDiff limits the received block number to only 100 further from the current block number
const MaxBlockNumDiff = 100
func (consensus *Consensus) validatorSanityChecks(msg *msg_pb.Message) bool {
senderKey, err := consensus.verifySenderKey(msg)
if err != nil {
@ -21,7 +24,10 @@ func (consensus *Consensus) validatorSanityChecks(msg *msg_pb.Message) bool {
if !senderKey.IsEqual(consensus.LeaderPubKey) &&
consensus.current.Mode() == Normal && !consensus.ignoreViewIDCheck {
consensus.getLogger().Warn().Msg("[OnPrepared] SenderKey not match leader PubKey")
consensus.getLogger().Warn().Msgf(
"[%s] SenderKey not match leader PubKey",
msg.GetType().String(),
)
return false
}
@ -39,17 +45,22 @@ func (consensus *Consensus) leaderSanityChecks(msg *msg_pb.Message) bool {
senderKey, err := consensus.verifySenderKey(msg)
if err != nil {
if err == shard.ErrValidNotInCommittee {
consensus.getLogger().Info().Msg(
"[OnAnnounce] sender key not in this slot's subcommittee",
consensus.getLogger().Info().Msgf(
"[%s] sender key not in this slot's subcommittee",
msg.GetType().String(),
)
} else {
consensus.getLogger().Error().Err(err).Msg("[OnAnnounce] erifySenderKey failed")
consensus.getLogger().Error().Err(err).Msgf(
"[%s] verifySenderKey failed",
msg.GetType().String(),
)
}
return false
}
if err = verifyMessageSig(senderKey, msg); err != nil {
consensus.getLogger().Error().Err(err).Msg(
"[OnPrepare] Failed to verify sender's signature",
consensus.getLogger().Error().Err(err).Msgf(
"[%s] Failed to verify sender's signature",
msg.GetType().String(),
)
return false
}
@ -104,8 +115,9 @@ func (consensus *Consensus) onAnnounceSanityChecks(recvMsg *FBFTMessage) bool {
Str("LeaderKey", consensus.LeaderPubKey.SerializeToHexStr()).
Msg("[OnAnnounce] Leader is malicious")
if consensus.current.Mode() == ViewChanging {
viewID := consensus.current.ViewID()
consensus.startViewChange(viewID + 1)
consensus.getLogger().Debug().Msg(
"[OnAnnounce] Already in ViewChanging mode, conflicing announce, doing noop",
)
} else {
consensus.startViewChange(consensus.viewID + 1)
}
@ -123,6 +135,12 @@ func (consensus *Consensus) isRightBlockNumCheck(recvMsg *FBFTMessage) bool {
Uint64("MsgBlockNum", recvMsg.BlockNum).
Msg("Wrong BlockNum Received, ignoring!")
return false
} else if recvMsg.BlockNum-consensus.blockNum > MaxBlockNumDiff {
consensus.getLogger().Debug().
Uint64("MsgBlockNum", recvMsg.BlockNum).
Uint64("MaxBlockNumDiff", MaxBlockNumDiff).
Msg("Received blockNum that is MaxBlockNumDiff further from the current blockNum!")
return false
}
return true
}
@ -146,7 +164,7 @@ func (consensus *Consensus) onPreparedSanityChecks(
Msg("[OnPrepared] BlockHash not match")
return false
}
if consensus.current.Mode() == Normal {
if consensus.current.Mode() == Normal || consensus.current.Mode() == Syncing {
err := chain.Engine.VerifyHeader(consensus.ChainReader, blockObj.Header(), true)
if err != nil {
consensus.getLogger().Error().
@ -166,3 +184,71 @@ func (consensus *Consensus) onPreparedSanityChecks(
return true
}
func (consensus *Consensus) viewChangeSanityCheck(msg *msg_pb.Message) bool {
senderKey, err := consensus.verifyViewChangeSenderKey(msg)
if err != nil {
consensus.getLogger().Error().Err(err).Msgf(
"[%s] VerifySenderKey Failed",
msg.GetType().String(),
)
return false
}
if err := verifyMessageSig(senderKey, msg); err != nil {
consensus.getLogger().Error().Err(err).Msgf(
"[%s] Failed To Verify Sender's Signature",
msg.GetType().String(),
)
return false
}
return true
}
func (consensus *Consensus) onViewChangeSanityCheck(recvMsg *FBFTMessage) bool {
// TODO: if difference is only one, new leader can still propose the same committed block to avoid another view change
// TODO: new leader catchup without ignore view change message
if consensus.blockNum > recvMsg.BlockNum {
consensus.getLogger().Debug().
Uint64("MsgBlockNum", recvMsg.BlockNum).
Msg("[onViewChange] Message BlockNum Is Low")
return false
}
if consensus.blockNum < recvMsg.BlockNum {
consensus.getLogger().Warn().
Uint64("MsgBlockNum", recvMsg.BlockNum).
Msg("[onViewChange] New Leader Has Lower Blocknum")
return false
}
if consensus.current.Mode() == ViewChanging &&
consensus.current.ViewID() > recvMsg.ViewID {
consensus.getLogger().Warn().
Uint64("MyViewChangingID", consensus.current.ViewID()).
Uint64("MsgViewChangingID", recvMsg.ViewID).
Msg("[onViewChange] ViewChanging ID Is Low")
return false
}
if recvMsg.ViewID-consensus.current.ViewID() > MaxViewIDDiff {
consensus.getLogger().Debug().
Uint64("MsgViewID", recvMsg.ViewID).
Uint64("MaxViewIDDiff", MaxViewIDDiff).
Msg("Received viewID that is MaxViewIDDiff further from the current viewID!")
return false
}
return true
}
func (consensus *Consensus) onNewViewSanityCheck(recvMsg *FBFTMessage) bool {
if recvMsg.ViewID <= consensus.viewID {
consensus.getLogger().Warn().
Uint64("LastSuccessfulConsensusViewID", consensus.viewID).
Uint64("MsgViewChangingID", recvMsg.ViewID).
Msg("[onNewView] ViewID should be larger than the viewID of the last successful consensus")
return false
}
if consensus.current.Mode() != ViewChanging {
consensus.getLogger().Warn().
Msg("[onNewView] Not in ViewChanging mode, ignoring the new view message")
return false
}
return true
}

@ -97,9 +97,11 @@ func (consensus *Consensus) handleMessageUpdate(payload []byte) {
intendedForLeader &&
consensus.leaderSanityChecks(msg):
consensus.onCommit(msg)
case t == msg_pb.MessageType_VIEWCHANGE:
case t == msg_pb.MessageType_VIEWCHANGE &&
consensus.viewChangeSanityCheck(msg):
consensus.onViewChange(msg)
case t == msg_pb.MessageType_NEWVIEW:
case t == msg_pb.MessageType_NEWVIEW &&
consensus.viewChangeSanityCheck(msg):
consensus.onNewView(msg)
}
}

@ -16,6 +16,9 @@ import (
"github.com/harmony-one/harmony/p2p/host"
)
// MaxViewIDDiff limits the received view ID to only 100 further from the current view ID
const MaxViewIDDiff = 100
// State contains current mode and current viewID
type State struct {
mode Mode
@ -149,11 +152,6 @@ func (consensus *Consensus) onViewChange(msg *msg_pb.Message) {
consensus.getLogger().Warn().Msg("[onViewChange] Unable To Parse Viewchange Message")
return
}
newLeaderKey := recvMsg.LeaderPubkey
newLeaderPriKey, err := consensus.GetLeaderPrivateKey(newLeaderKey)
if err != nil {
return
}
if consensus.Decider.IsQuorumAchieved(quorum.ViewChange) {
consensus.getLogger().Debug().
@ -164,38 +162,14 @@ func (consensus *Consensus) onViewChange(msg *msg_pb.Message) {
return
}
senderKey, err := consensus.verifyViewChangeSenderKey(msg)
if err != nil {
consensus.getLogger().Debug().Err(err).Msg("[onViewChange] VerifySenderKey Failed")
return
}
// TODO: if difference is only one, new leader can still propose the same committed block to avoid another view change
// TODO: new leader catchup without ignore view change message
if consensus.blockNum > recvMsg.BlockNum {
consensus.getLogger().Debug().
Uint64("MsgBlockNum", recvMsg.BlockNum).
Msg("[onViewChange] Message BlockNum Is Low")
if !consensus.onViewChangeSanityCheck(recvMsg) {
return
}
if consensus.blockNum < recvMsg.BlockNum {
consensus.getLogger().Warn().
Uint64("MsgBlockNum", recvMsg.BlockNum).
Msg("[onViewChange] New Leader Has Lower Blocknum")
return
}
if consensus.current.Mode() == ViewChanging &&
consensus.current.ViewID() > recvMsg.ViewID {
consensus.getLogger().Warn().
Uint64("MyViewChangingID", consensus.current.ViewID()).
Uint64("MsgViewChangingID", recvMsg.ViewID).
Msg("[onViewChange] ViewChanging ID Is Low")
return
}
if err = verifyMessageSig(senderKey, msg); err != nil {
consensus.getLogger().Debug().Err(err).Msg("[onViewChange] Failed To Verify Sender's Signature")
senderKey := recvMsg.SenderPubkey
newLeaderKey := recvMsg.LeaderPubkey
newLeaderPriKey, err := consensus.GetLeaderPrivateKey(newLeaderKey)
if err != nil {
return
}
@ -438,21 +412,17 @@ func (consensus *Consensus) onViewChange(msg *msg_pb.Message) {
// TODO: move to consensus_leader.go later
func (consensus *Consensus) onNewView(msg *msg_pb.Message) {
consensus.getLogger().Debug().Msg("[onNewView] Received NewView Message")
senderKey, err := consensus.verifyViewChangeSenderKey(msg)
if err != nil {
consensus.getLogger().Warn().Err(err).Msg("[onNewView] VerifySenderKey Failed")
return
}
recvMsg, err := consensus.ParseNewViewMessage(msg)
if err != nil {
consensus.getLogger().Warn().Err(err).Msg("[onNewView] Unable to Parse NewView Message")
return
}
if err = verifyMessageSig(senderKey, msg); err != nil {
consensus.getLogger().Error().Err(err).Msg("[onNewView] Failed to Verify New Leader's Signature")
if !consensus.onNewViewSanityCheck(recvMsg) {
return
}
senderKey := recvMsg.SenderPubkey
consensus.vcLock.Lock()
defer consensus.vcLock.Unlock()

Loading…
Cancel
Save