[NOD-614] Add ban score (#760)

* [NOD-614] Copy bitcoin-core ban score policy

* [NOD-614] Add ban score to disconnects

* [NOD-614] Fix wrong branch of AddBanScore

* [NOD-614] Add ban score on sending too many addresses

* [NOD-614] Add comments

* [NOD-614] Remove redundant reject messages

* [NOD-614] Fix log message

* [NOD-614] Ban every node that sends invalid invs

* [NOD-614] Make constants for ban scores

addrmgr/addrmanager.go

@@ -162,10 +162,10 @@ const (
 	// to a getAddr. If we have less than this amount, we send everything.
 	getAddrMin = 50
 
-	// getAddrMax is the most addresses that we will send in response
+	// GetAddrMax is the most addresses that we will send in response
 	// to a getAddr (in practise the most addresses we will return from a
 	// call to AddressCache()).
-	getAddrMax = 2500
+	GetAddrMax = 2500
 
 	// getAddrPercent is the percentage of total addresses known that we
 	// will share with a call to AddressCache.
@@ -839,8 +839,8 @@ func (a *AddrManager) AddressCache(includeAllSubnetworks bool, subnetworkID *sub
 	}
 
 	numAddresses := len(allAddr) * getAddrPercent / 100
-	if numAddresses > getAddrMax {
-		numAddresses = getAddrMax
+	if numAddresses > GetAddrMax {
+		numAddresses = GetAddrMax
 	}
 	if len(allAddr) < getAddrMin {
 		numAddresses = len(allAddr)

blockdag/dag.go

@@ -165,7 +165,7 @@ type BlockDAG struct {
 //
 // This function is safe for concurrent access.
 func (dag *BlockDAG) IsKnownBlock(hash *daghash.Hash) bool {
-	return dag.IsInDAG(hash) || dag.IsKnownOrphan(hash) || dag.isKnownDelayedBlock(hash)
+	return dag.IsInDAG(hash) || dag.IsKnownOrphan(hash) || dag.isKnownDelayedBlock(hash) || dag.IsKnownInvalid(hash)
 }
 
 // AreKnownBlocks returns whether or not the DAG instances has all blocks represented

netsync/manager.go

@@ -376,9 +376,8 @@ func (sm *SyncManager) handleTxMsg(tmsg *txMsg) {
 	// If we didn't ask for this transaction then the peer is misbehaving.
 	txID := tmsg.tx.ID()
 	if _, exists = state.requestedTxns[*txID]; !exists {
-		log.Warnf("Got unrequested transaction %s from %s -- "+
-			"disconnecting", txID, peer.Addr())
-		peer.Disconnect()
+		peer.AddBanScoreAndPushRejectMsg(wire.CmdTx, wire.RejectNotRequested, (*daghash.Hash)(txID),
+			peerpkg.BanScoreUnrequestedTx, 0, fmt.Sprintf("got unrequested transaction %s", txID))
 		return
 	}
 
@@ -412,19 +411,25 @@ func (sm *SyncManager) handleTxMsg(tmsg *txMsg) {
 		// When the error is a rule error, it means the transaction was
 		// simply rejected as opposed to something actually going wrong,
 		// so log it as such. Otherwise, something really did go wrong,
-		// so log it as an actual error.
-		if errors.As(err, &mempool.RuleError{}) {
-			log.Debugf("Rejected transaction %s from %s: %s",
-				txID, peer, err)
-		} else {
-			log.Errorf("Failed to process transaction %s: %s",
-				txID, err)
+		// so panic.
+		ruleErr := &mempool.RuleError{}
+		if !errors.As(err, ruleErr) {
+			panic(errors.Wrapf(err, "failed to process transaction %s", txID))
 		}
 
-		// Convert the error into an appropriate reject message and
-		// send it.
-		code, reason := mempool.ErrToRejectErr(err)
-		peer.PushRejectMsg(wire.CmdTx, code, reason, (*daghash.Hash)(txID), false)
+		shouldIncreaseBanScore := false
+		if txRuleErr := (&mempool.TxRuleError{}); errors.As(ruleErr.Err, txRuleErr) {
+			if txRuleErr.RejectCode == wire.RejectInvalid {
+				shouldIncreaseBanScore = true
+			}
+		} else if dagRuleErr := (&blockdag.RuleError{}); errors.As(ruleErr.Err, dagRuleErr) {
+			shouldIncreaseBanScore = true
+		}
+
+		if shouldIncreaseBanScore {
+			peer.AddBanScoreAndPushRejectMsg(wire.CmdTx, wire.RejectInvalid, (*daghash.Hash)(txID),
+				peerpkg.BanScoreInvalidTx, 0, fmt.Sprintf("rejected transaction %s: %s", txID, err))
+		}
 		return
 	}
 
@@ -480,9 +485,8 @@ func (sm *SyncManager) handleBlockMsg(bmsg *blockMsg) {
 		// mode in this case so the DAG code is actually fed the
 		// duplicate blocks.
 		if sm.dagParams != &dagconfig.RegressionNetParams {
-			log.Warnf("Got unrequested block %s from %s -- "+
-				"disconnecting", blockHash, peer.Addr())
-			peer.Disconnect()
+			peer.AddBanScoreAndPushRejectMsg(wire.CmdBlock, wire.RejectNotRequested, blockHash,
+				peerpkg.BanScoreUnrequestedBlock, 0, fmt.Sprintf("got unrequested block %s", blockHash))
 			return
 		}
 	}
@@ -518,13 +522,8 @@ func (sm *SyncManager) handleBlockMsg(bmsg *blockMsg) {
 		log.Infof("Rejected block %s from %s: %s", blockHash,
 			peer, err)
 
-		// Convert the error into an appropriate reject message and
-		// send it.
-		code, reason := mempool.ErrToRejectErr(err)
-		peer.PushRejectMsg(wire.CmdBlock, code, reason, blockHash, false)
-
-		// Disconnect from the misbehaving peer.
-		peer.Disconnect()
+		peer.AddBanScoreAndPushRejectMsg(wire.CmdBlock, wire.RejectInvalid, blockHash,
+			peerpkg.BanScoreInvalidBlock, 0, fmt.Sprintf("got invalid block: %s", err))
 		return
 	}
 
@@ -718,6 +717,10 @@ func (sm *SyncManager) handleInvMsg(imsg *invMsg) {
 		}
 
 		if iv.IsBlockOrSyncBlock() {
+			if sm.dag.IsKnownInvalid(iv.Hash) {
+				peer.AddBanScoreAndPushRejectMsg(imsg.inv.Command(), wire.RejectInvalid, iv.Hash, peerpkg.BanScoreInvalidInvBlock, 0, fmt.Sprintf("sent inv of invalid block %s", iv.Hash))
+				return
+			}
 			// The block is an orphan block that we already have.
 			// When the existing orphan was processed, it requested
 			// the missing parent blocks. When this scenario
@@ -913,9 +916,9 @@ func (sm *SyncManager) handleSelectedTipMsg(msg *selectedTipMsg) {
 	selectedTipHash := msg.selectedTipHash
 	state := sm.peerStates[peer]
 	if !state.peerShouldSendSelectedTip {
-		log.Warnf("Got unrequested selected tip message from %s -- "+
-			"disconnecting", peer.Addr())
-		peer.Disconnect()
+		peer.AddBanScoreAndPushRejectMsg(wire.CmdSelectedTip, wire.RejectNotRequested, nil,
+			peerpkg.BanScoreUnrequestedSelectedTip, 0, "got unrequested selected tip message")
+		return
 	}
 	state.peerShouldSendSelectedTip = false
 	if selectedTipHash.IsEqual(peer.SelectedTipHash()) {

peer/banscores.go

@@ -0,0 +1,34 @@
+package peer
+
+// Ban scores for misbehaving nodes
+const (
+	BanScoreUnrequestedBlock = 100
+	BanScoreInvalidBlock     = 100
+	BanScoreInvalidInvBlock  = 100
+
+	BanScoreUnrequestedSelectedTip = 20
+	BanScoreUnrequestedTx          = 20
+	BanScoreInvalidTx              = 100
+
+	BanScoreMalformedMessage = 10
+
+	BanScoreNonVersionFirstMessage = 1
+	BanScoreDuplicateVersion       = 1
+	BanScoreDuplicateVerack        = 1
+
+	BanScoreSentTooManyAddresses         = 20
+	BanScoreMsgAddrWithInvalidSubnetwork = 10
+
+	BanScoreInvalidFeeFilter = 100
+	BanScoreNoFilterLoaded   = 5
+
+	BanScoreInvalidMsgGetBlockInvs = 10
+
+	BanScoreInvalidMsgBlockLocator = 100
+
+	BanScoreSentTxToBlocksOnly = 20
+
+	BanScoreNodeBloomFlagViolation = 100
+
+	BanScoreStallTimeout = 1
+)

peer/peer.go

@@ -199,6 +199,13 @@ type Config struct {
 	// the DAG.
 	IsInDAG func(*daghash.Hash) bool
 
+	// AddBanScore increases the persistent and decaying ban score fields by the
+	// values passed as parameters. If the resulting score exceeds half of the ban
+	// threshold, a warning is logged including the reason provided. Further, if
+	// the score is above the ban threshold, the peer will be banned and
+	// disconnected.
+	AddBanScore func(persistent, transient uint32, reason string)
+
 	// HostToNetAddress returns the netaddress for the given host. This can be
 	// nil in  which case the host will be parsed as an IP address.
 	HostToNetAddress HostToNetAddrFunc
@@ -646,6 +653,22 @@ func (p *Peer) IsSelectedTipKnown() bool {
 	return !p.cfg.IsInDAG(p.selectedTipHash)
 }
 
+// AddBanScore increases the persistent and decaying ban score fields by the
+// values passed as parameters. If the resulting score exceeds half of the ban
+// threshold, a warning is logged including the reason provided. Further, if
+// the score is above the ban threshold, the peer will be banned and
+// disconnected.
+func (p *Peer) AddBanScore(persistent, transient uint32, reason string) {
+	p.cfg.AddBanScore(persistent, transient, reason)
+}
+
+// AddBanScoreAndPushRejectMsg increases ban score and sends a
+// reject message to the misbehaving peer.
+func (p *Peer) AddBanScoreAndPushRejectMsg(command string, code wire.RejectCode, hash *daghash.Hash, persistent, transient uint32, reason string) {
+	p.PushRejectMsg(command, code, reason, hash, true)
+	p.cfg.AddBanScore(persistent, transient, reason)
+}
+
 // LastSend returns the last send time of the peer.
 //
 // This function is safe for concurrent access.
@@ -1239,9 +1262,7 @@ out:
 					continue
 				}
 
-				log.Debugf("Peer %s appears to be stalled or "+
-					"misbehaving, %s timeout -- "+
-					"disconnecting", p, command)
+				p.AddBanScore(BanScoreStallTimeout, 0, fmt.Sprintf("got timeout for command %s", command))
 				p.Disconnect()
 				break
 			}
@@ -1316,15 +1337,15 @@ out:
 					log.Errorf(errMsg)
 				}
 
-				// Push a reject message for the malformed message and wait for
-				// the message to be sent before disconnecting.
+				// Add ban score, push a reject message for the malformed message
+				// and wait for the message to be sent before disconnecting.
 				//
 				// NOTE: Ideally this would include the command in the header if
 				// at least that much of the message was valid, but that is not
 				// currently exposed by wire, so just used malformed for the
 				// command.
-				p.PushRejectMsg("malformed", wire.RejectMalformed, errMsg, nil,
-					true)
+				p.AddBanScoreAndPushRejectMsg("malformed", wire.RejectMalformed, nil,
+					BanScoreMalformedMessage, 0, errMsg)
 			}
 			break out
 		}
@@ -1336,18 +1357,18 @@ out:
 		switch msg := rmsg.(type) {
 		case *wire.MsgVersion:
 
-			p.PushRejectMsg(msg.Command(), wire.RejectDuplicate,
-				"duplicate version message", nil, true)
-			break out
+			reason := "duplicate version message"
+			p.AddBanScoreAndPushRejectMsg(msg.Command(), wire.RejectDuplicate, nil,
+				BanScoreDuplicateVersion, 0, reason)
 
 		case *wire.MsgVerAck:
 
 			// No read lock is necessary because verAckReceived is not written
 			// to in any other goroutine.
 			if p.verAckReceived {
-				log.Infof("Already received 'verack' from peer %s -- "+
-					"disconnecting", p)
-				break out
+				p.AddBanScoreAndPushRejectMsg(msg.Command(), wire.RejectDuplicate, nil,
+					BanScoreDuplicateVerack, 0, "verack sent twice")
+				log.Warnf("Already received 'verack' from peer %s", p)
 			}
 			p.markVerAckReceived()
 			if p.cfg.Listeners.OnVerAck != nil {
@@ -1867,6 +1888,8 @@ func (p *Peer) readRemoteVersionMsg() error {
 		errStr := "A version message must precede all others"
 		log.Errorf(errStr)
 
+		p.AddBanScore(BanScoreNonVersionFirstMessage, 0, errStr)
+
 		rejectMsg := wire.NewMsgReject(msg.Command(), wire.RejectMalformed,
 			errStr)
 		return p.writeMessage(rejectMsg)

server/p2p/on_addr.go

@@ -1,6 +1,8 @@
 package p2p
 
 import (
+	"fmt"
+	"github.com/kaspanet/kaspad/addrmgr"
 	"github.com/kaspanet/kaspad/config"
 	"github.com/kaspanet/kaspad/peer"
 	"github.com/kaspanet/kaspad/wire"
@@ -18,10 +20,16 @@ func (sp *Peer) OnAddr(_ *peer.Peer, msg *wire.MsgAddr) {
 		return
 	}
 
+	if len(msg.AddrList) > addrmgr.GetAddrMax {
+		sp.AddBanScoreAndPushRejectMsg(msg.Command(), wire.RejectInvalid, nil,
+			peer.BanScoreSentTooManyAddresses, 0, fmt.Sprintf("address count excceeded %d", addrmgr.GetAddrMax))
+		return
+	}
+
 	if msg.IncludeAllSubnetworks {
-		peerLog.Errorf("Got unexpected IncludeAllSubnetworks=true in [%s] command from %s",
-			msg.Command(), sp.Peer)
-		sp.Disconnect()
+		sp.AddBanScoreAndPushRejectMsg(msg.Command(), wire.RejectInvalid, nil,
+			peer.BanScoreMsgAddrWithInvalidSubnetwork, 0,
+			fmt.Sprintf("got unexpected IncludeAllSubnetworks=true in [%s] command", msg.Command()))
 		return
 	} else if !msg.SubnetworkID.IsEqual(config.ActiveConfig().SubnetworkID) && msg.SubnetworkID != nil {
 		peerLog.Errorf("Only full nodes and %s subnetwork IDs are allowed in [%s] command, but got subnetwork ID %s from %s",

server/p2p/on_fee_filter.go

@@ -1,6 +1,7 @@
 package p2p
 
 import (
+	"fmt"
 	"github.com/kaspanet/kaspad/peer"
 	"github.com/kaspanet/kaspad/util"
 	"github.com/kaspanet/kaspad/wire"
@@ -14,9 +15,8 @@ import (
 func (sp *Peer) OnFeeFilter(_ *peer.Peer, msg *wire.MsgFeeFilter) {
 	// Check that the passed minimum fee is a valid amount.
 	if msg.MinFee < 0 || msg.MinFee > util.MaxSompi {
-		peerLog.Debugf("Peer %s sent an invalid feefilter '%s' -- "+
-			"disconnecting", sp, util.Amount(msg.MinFee))
-		sp.Disconnect()
+		sp.AddBanScoreAndPushRejectMsg(msg.Command(), wire.RejectInvalid, nil,
+			peer.BanScoreInvalidFeeFilter, 0, fmt.Sprintf("sent an invalid feefilter '%s'", util.Amount(msg.MinFee)))
 		return
 	}
 

server/p2p/on_filter_add.go

@@ -17,9 +17,8 @@ func (sp *Peer) OnFilterAdd(_ *peer.Peer, msg *wire.MsgFilterAdd) {
 	}
 
 	if sp.filter.IsLoaded() {
-		peerLog.Debugf("%s sent a filteradd request with no filter "+
-			"loaded -- disconnecting", sp)
-		sp.Disconnect()
+		sp.AddBanScoreAndPushRejectMsg(wire.CmdFilterAdd, wire.RejectInvalid, nil,
+			peer.BanScoreNoFilterLoaded, 0, "sent a filteradd request with no filter loaded")
 		return
 	}
 

server/p2p/on_filter_clear.go

@@ -17,9 +17,8 @@ func (sp *Peer) OnFilterClear(_ *peer.Peer, msg *wire.MsgFilterClear) {
 	}
 
 	if !sp.filter.IsLoaded() {
-		peerLog.Debugf("%s sent a filterclear request with no "+
-			"filter loaded -- disconnecting", sp)
-		sp.Disconnect()
+		sp.AddBanScoreAndPushRejectMsg(wire.CmdFilterClear, wire.RejectInvalid, nil,
+			peer.BanScoreNoFilterLoaded, 0, "sent a filterclear request with no filter loaded")
 		return
 	}
 

server/p2p/on_get_block_invs.go

@@ -1,6 +1,7 @@
 package p2p
 
 import (
+	"fmt"
 	"github.com/kaspanet/kaspad/peer"
 	"github.com/kaspanet/kaspad/wire"
 )
@@ -23,8 +24,9 @@ func (sp *Peer) OnGetBlockInvs(_ *peer.Peer, msg *wire.MsgGetBlockInvs) {
 	hashList, err := dag.AntiPastHashesBetween(msg.LowHash, msg.HighHash,
 		wire.MaxInvPerMsg)
 	if err != nil {
-		peerLog.Warnf("Error getting antiPast hashes between %s and %s: %s", msg.LowHash, msg.HighHash, err)
-		sp.Disconnect()
+		sp.AddBanScoreAndPushRejectMsg(wire.CmdGetBlockInvs, wire.RejectInvalid, nil,
+			peer.BanScoreInvalidMsgGetBlockInvs, 0,
+			fmt.Sprintf("error getting antiPast hashes between %s and %s: %s", msg.LowHash, msg.HighHash, err))
 		return
 	}
 

server/p2p/on_get_block_locator.go

@@ -11,13 +11,13 @@ import (
 func (sp *Peer) OnGetBlockLocator(_ *peer.Peer, msg *wire.MsgGetBlockLocator) {
 	locator, err := sp.server.DAG.BlockLocatorFromHashes(msg.HighHash, msg.LowHash)
 	if err != nil || len(locator) == 0 {
-		warning := fmt.Sprintf("Couldn't build a block locator between blocks "+
-			"%s and %s that was requested from peer %s", msg.HighHash, msg.LowHash, sp)
 		if err != nil {
-			warning = fmt.Sprintf("%s: %s", warning, err)
+			peerLog.Warnf("Couldn't build a block locator between blocks "+
+				"%s and %s that was requested from peer %s: %s", msg.HighHash, msg.LowHash, sp, err)
 		}
-		peerLog.Warnf(warning)
-		sp.Disconnect()
+		sp.AddBanScoreAndPushRejectMsg(msg.Command(), wire.RejectInvalid, nil,
+			peer.BanScoreInvalidMsgBlockLocator, 0,
+			fmt.Sprintf("couldn't build a block locator between blocks %s and %s", msg.HighHash, msg.LowHash))
 		return
 	}
 

server/p2p/on_inv.go

@@ -23,9 +23,8 @@ func (sp *Peer) OnInv(_ *peer.Peer, msg *wire.MsgInv) {
 		if invVect.Type == wire.InvTypeTx {
 			peerLog.Tracef("Ignoring tx %s in inv from %s -- "+
 				"blocksonly enabled", invVect.Hash, sp)
-			peerLog.Infof("Peer %s is announcing "+
-				"transactions -- disconnecting", sp)
-			sp.Disconnect()
+			sp.AddBanScoreAndPushRejectMsg(msg.Command(), wire.RejectNotRequested, invVect.Hash,
+				peer.BanScoreSentTxToBlocksOnly, 0, "announced transactions when blocksonly is enabled")
 			return
 		}
 		err := newInv.AddInvVect(invVect)

server/p2p/p2p.go

@@ -8,6 +8,7 @@ package p2p
 import (
 	"crypto/rand"
 	"encoding/binary"
+	"fmt"
 	"math"
 	"net"
 	"runtime"
@@ -328,13 +329,8 @@ func (sp *Peer) pushAddrMsg(addresses []*wire.NetAddress, subnetworkID *subnetwo
 // the score is above the ban threshold, the peer will be banned and
 // disconnected.
 func (sp *Peer) addBanScore(persistent, transient uint32, reason string) {
-	// No warning is logged and no score is calculated if banning is disabled.
-	if config.ActiveConfig().DisableBanning {
-		return
-	}
 	if sp.isWhitelisted {
 		peerLog.Debugf("Misbehaving whitelisted peer %s: %s", sp, reason)
-		return
 	}
 
 	warnThreshold := config.ActiveConfig().BanThreshold >> 1
@@ -348,16 +344,22 @@ func (sp *Peer) addBanScore(persistent, transient uint32, reason string) {
 		}
 		return
 	}
+
 	score := sp.DynamicBanScore.Increase(persistent, transient)
+	logMsg := fmt.Sprintf("Misbehaving peer %s: %s -- ban score increased to %d",
+		sp, reason, score)
 	if score > warnThreshold {
-		peerLog.Warnf("Misbehaving peer %s: %s -- ban score increased to %d",
-			sp, reason, score)
-		if score > config.ActiveConfig().BanThreshold {
+		peerLog.Warn(logMsg)
+		if !config.ActiveConfig().DisableBanning && !sp.isWhitelisted && score > config.ActiveConfig().BanThreshold {
 			peerLog.Warnf("Misbehaving peer %s -- banning and disconnecting",
 				sp)
 			sp.server.BanPeer(sp)
 			sp.Disconnect()
 		}
+	} else if persistent != 0 {
+		peerLog.Warn(logMsg)
+	} else {
+		peerLog.Trace(logMsg)
 	}
 }
 
@@ -375,7 +377,7 @@ func (sp *Peer) enforceNodeBloomFlag(cmd string) bool {
 
 			// Disconnect the peer regardless of whether it was
 			// banned.
-			sp.addBanScore(100, 0, cmd)
+			sp.addBanScore(peer.BanScoreNodeBloomFlagViolation, 0, cmd)
 			sp.Disconnect()
 			return false
 		}
@@ -937,6 +939,7 @@ func newPeerConfig(sp *Peer) *peer.Config {
 		},
 		SelectedTipHash:   sp.selectedTipHash,
 		IsInDAG:           sp.blockExists,
+		AddBanScore:       sp.addBanScore,
 		HostToNetAddress:  sp.server.addrManager.HostToNetAddress,
 		Proxy:             config.ActiveConfig().Proxy,
 		UserAgentName:     userAgentName,

wire/msgreject.go

@@ -21,6 +21,7 @@ const (
 	RejectInvalid         RejectCode = 0x10
 	RejectObsolete        RejectCode = 0x11
 	RejectDuplicate       RejectCode = 0x12
+	RejectNotRequested    RejectCode = 0x13
 	RejectNonstandard     RejectCode = 0x40
 	RejectDust            RejectCode = 0x41
 	RejectInsufficientFee RejectCode = 0x42
@@ -39,6 +40,7 @@ var rejectCodeStrings = map[RejectCode]string{
 	RejectInsufficientFee: "REJECT_INSUFFICIENTFEE",
 	RejectFinality:        "REJECT_FINALITY",
 	RejectDifficulty:      "REJECT_DIFFICULTY",
+	RejectNotRequested:    "REJECT_NOTREQUESTED",
 }
 
 // String returns the RejectCode in human-readable form.