Skip to content

Commit

Permalink
[release-v2.0] server: Consolidate ban reason logging.
Browse files Browse the repository at this point in the history
This consolidates the logging for peer banning and adds a reason
parameter to help enforce consistent logging as well as make it much
less likely that a peer is banned without logging why.
  • Loading branch information
davecgh committed May 24, 2024
1 parent dd26110 commit 03b0c14
Showing 1 changed file with 31 additions and 25 deletions.
56 changes: 31 additions & 25 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ func (sp *serverPeer) addBanScore(persistent, transient uint32, reason string) b
return false
}
if sp.isWhitelisted {
peerLog.Debugf("Misbehaving whitelisted peer %s: %s", sp, reason)
srvrLog.Debugf("Misbehaving whitelisted peer %s: %s", sp, reason)
return false
}

Expand All @@ -959,19 +959,18 @@ func (sp *serverPeer) addBanScore(persistent, transient uint32, reason string) b
// logged if the score is above the warn threshold.
score := sp.banScore.Int()
if score > warnThreshold {
peerLog.Warnf("Misbehaving peer %s: %s -- ban score is %d, "+
srvrLog.Warnf("Misbehaving peer %s: %s -- ban score is %d, "+
"it was not increased this time", sp, reason, score)
}
return false
}
score := sp.banScore.Increase(persistent, transient)
if score > warnThreshold {
peerLog.Warnf("Misbehaving peer %s: %s -- ban score increased to %d",
srvrLog.Warnf("Misbehaving peer %s: %s -- ban score increased to %d",
sp, reason, score)
if score > cfg.BanThreshold {
peerLog.Warnf("Misbehaving peer %s -- banning and disconnecting",
sp)
sp.server.BanPeer(sp)
const reason = "ban score exceeds threshold"
sp.server.BanPeer(sp, reason)
return true
}
}
Expand Down Expand Up @@ -1379,8 +1378,8 @@ func (sp *serverPeer) OnBlock(_ *peer.Peer, msg *wire.MsgBlock, buf []byte) {
func (sp *serverPeer) OnInv(_ *peer.Peer, msg *wire.MsgInv) {
// Ban peers sending empty inventory announcements.
if len(msg.InvList) == 0 {
peerLog.Warnf("%s sent an empty inventory announcement", sp)
sp.server.BanPeer(sp)
const reason = "sent empty inventory announcement"
sp.server.BanPeer(sp, reason)
return
}

Expand Down Expand Up @@ -1424,8 +1423,8 @@ func (sp *serverPeer) OnHeaders(_ *peer.Peer, msg *wire.MsgHeaders) {
func (sp *serverPeer) OnGetData(_ *peer.Peer, msg *wire.MsgGetData) {
// Ban peers sending empty getdata requests.
if len(msg.InvList) == 0 {
peerLog.Warnf("%s sent an empty getdata request", sp)
sp.server.BanPeer(sp)
const reason = "sent an empty getdata request"
sp.server.BanPeer(sp, reason)
return
}

Expand Down Expand Up @@ -1667,10 +1666,9 @@ func (sp *serverPeer) OnAddr(_ *peer.Peer, msg *wire.MsgAddr) {

// A message that has no addresses is invalid.
if len(msg.AddrList) == 0 {
peerLog.Warnf("%s sent an empty address list", sp)

// Ban peers sending empty address requests.
sp.server.BanPeer(sp)
const reason = "sent an empty address list"
sp.server.BanPeer(sp, reason)
return
}

Expand Down Expand Up @@ -1729,8 +1727,8 @@ func (sp *serverPeer) onMixMessage(msg mixing.Message) {
return
}
if mixpool.IsBannable(err) {
peerLog.Warnf("%s: %v", sp, err)
sp.server.BanPeer(sp)
reason := fmt.Sprintf("sent malformed mix message: %s", err)
sp.server.BanPeer(sp, reason)
}
}

Expand Down Expand Up @@ -1785,8 +1783,8 @@ func (sp *serverPeer) OnRead(_ *peer.Peer, bytesRead int, msg wire.Message, err
// Ban peers sending messages that do not conform to the wire protocol.
var errCode wire.ErrorCode
if errors.As(err, &errCode) {
peerLog.Errorf("Unable to read wire message from %s: %v", sp, err)
sp.server.BanPeer(sp)
reason := fmt.Sprintf("sent malformed wire message: %s", err)
sp.server.BanPeer(sp, reason)
}

sp.server.AddBytesReceived(uint64(bytesRead))
Expand Down Expand Up @@ -2461,25 +2459,33 @@ func (s *server) DonePeer(sp *serverPeer) {
}
}

// BanPeer bans a peer that has already been connected to the server by ip
// unless banning is disabled or the peer has been whitelisted.
func (s *server) BanPeer(sp *serverPeer) {
if cfg.DisableBanning || sp.isWhitelisted {
// BanPeer bans and disconnects a peer that has already been connected to the
// server by ip unless banning is disabled or the peer has been whitelisted.
func (s *server) BanPeer(sp *serverPeer, reason string) {
// No warning is logged when banning is disabled.
if cfg.DisableBanning {
return
}
if sp.isWhitelisted {
srvrLog.Debugf("Misbehaving whitelisted peer %s: %s", sp, reason)
return
}

host, _, err := net.SplitHostPort(sp.Addr())
if err != nil {
srvrLog.Debugf("can't split ban peer %s %v", sp.Addr(), err)
srvrLog.Debugf("can't split ban peer %s: %v", sp, err)
srvrLog.Warnf("Misbehaving peer %s: %s -- disconnecting", sp, reason)
sp.Disconnect()
return
}

direction := directionString(sp.Inbound())
srvrLog.Infof("Banned peer %s (%s) for %v", host, direction,
cfg.BanDuration)
srvrLog.Warnf("Misbehaving peer %s (%s): %s -- banned for %v", host,
direction, cfg.BanDuration)
bannedUntil := time.Now().Add(cfg.BanDuration)
s.peerState.Lock()
s.peerState.banned[host] = bannedUntil
s.peerState.Unlock()

sp.Disconnect()
}

Expand Down

0 comments on commit 03b0c14

Please sign in to comment.