peer: Strictly enforce bloom filter service bit.

This makes the enforcement of the bloom filter service bit much more
strict.  In particular, it does the following:

- Moves the enforcement of the bloom filter service bit out of the peer
  package and into the server so the server can ban as necessary
- Disconnect peers that send filter commands when the server is
  configured to disable them regardless of the protocol version
- Bans peers that are a high enough protocol version that they are
  supposed to observe the service bit is disabled, but ignore it and
  send filter commands regardless.

As an added bonus, this fixes the old logic which had a bug in that it
was examining the *remote* peer's supported services in order to choose
whether or not to disconnect instead of the *local* server's supported
services.

peer/peer.go

@@ -141,26 +141,14 @@ type MessageListeners struct {
 	OnGetHeaders func(p *Peer, msg *wire.MsgGetHeaders)
 
 	// OnFilterAdd is invoked when a peer receives a filteradd bitcoin message.
-	// Peers that do not advertise support for bloom filters and negotiate to a
-	// protocol version before BIP0111 will simply ignore the message while
-	// those that negotiate to the BIP0111 protocol version or higher will be
-	// immediately disconnected.
 	OnFilterAdd func(p *Peer, msg *wire.MsgFilterAdd)
 
 	// OnFilterClear is invoked when a peer receives a filterclear bitcoin
 	// message.
-	// Peers that do not advertise support for bloom filters and negotiate to a
-	// protocol version before BIP0111 will simply ignore the message while
-	// those that negotiate to the BIP0111 protocol version or higher will be
-	// immediately disconnected.
 	OnFilterClear func(p *Peer, msg *wire.MsgFilterClear)
 
 	// OnFilterLoad is invoked when a peer receives a filterload bitcoin
 	// message.
-	// Peers that do not advertise support for bloom filters and negotiate to a
-	// protocol version before BIP0111 will simply ignore the message while
-	// those that negotiate to the BIP0111 protocol version or higher will be
-	// immediately disconnected.
 	OnFilterLoad func(p *Peer, msg *wire.MsgFilterLoad)
 
 	// OnMerkleBlock  is invoked when a peer receives a merkleblock bitcoin
@@ -1021,24 +1009,6 @@ func (p *Peer) handleRemoteVersionMsg(msg *wire.MsgVersion) error {
 	return nil
 }
 
-// isValidBIP0111 is a helper function for the bloom filter commands to check
-// BIP0111 compliance.
-func (p *Peer) isValidBIP0111(cmd string) bool {
-	if p.Services()&wire.SFNodeBloom != wire.SFNodeBloom {
-		if p.ProtocolVersion() >= wire.BIP0111Version {
-			log.Debugf("%s sent an unsupported %s "+
-				"request -- disconnecting", p, cmd)
-			p.Disconnect()
-		} else {
-			log.Debugf("Ignoring %s request from %s -- bloom "+
-				"support is disabled", cmd, p)
-		}
-		return false
-	}
-
-	return true
-}
-
 // handlePingMsg is invoked when a peer receives a ping bitcoin message.  For
 // recent clients (protocol version > BIP0031Version), it replies with a pong
 // message.  For older clients, it does nothing and anything other than failure
@@ -1531,17 +1501,17 @@ out:
 			}
 
 		case *wire.MsgFilterAdd:
-			if p.isValidBIP0111(msg.Command()) && p.cfg.Listeners.OnFilterAdd != nil {
+			if p.cfg.Listeners.OnFilterAdd != nil {
 				p.cfg.Listeners.OnFilterAdd(p, msg)
 			}
 
 		case *wire.MsgFilterClear:
-			if p.isValidBIP0111(msg.Command()) && p.cfg.Listeners.OnFilterClear != nil {
+			if p.cfg.Listeners.OnFilterClear != nil {
 				p.cfg.Listeners.OnFilterClear(p, msg)
 			}
 
 		case *wire.MsgFilterLoad:
-			if p.isValidBIP0111(msg.Command()) && p.cfg.Listeners.OnFilterLoad != nil {
+			if p.cfg.Listeners.OnFilterLoad != nil {
 				p.cfg.Listeners.OnFilterLoad(p, msg)
 			}
 

server.go

@@ -789,11 +789,52 @@ func (sp *serverPeer) OnGetHeaders(p *peer.Peer, msg *wire.MsgGetHeaders) {
 	p.QueueMessage(headersMsg, nil)
 }
 
+// enforceNodeBloomFlag disconnects the peer if the server is not configured to
+// allow bloom filters.  Additionally, if the peer has negotiated to a protocol
+// version  that is high enough to observe the bloom filter service support bit,
+// it will be banned since it is intentionally violating the protocol.
+func (sp *serverPeer) enforceNodeBloomFlag(cmd string) bool {
+	if sp.server.services&wire.SFNodeBloom != wire.SFNodeBloom {
+		// Ban the peer if the protocol version is high enough that the
+		// peer is knowingly violating the protocol and banning is
+		// enabled.
+		//
+		// NOTE: Even though the addBanScore function already examines
+		// whether or not banning is enabled, it is checked here as well
+		// to ensure the violation is logged and the peer is
+		// disconnected regardless.
+		if sp.ProtocolVersion() >= wire.BIP0111Version &&
+			!cfg.DisableBanning {
+
+			// Disonnect the peer regardless of whether it was
+			// banned.
+			sp.addBanScore(100, 0, cmd)
+			sp.Disconnect()
+			return false
+		}
+
+		// Disconnect the peer regardless of protocol version or banning
+		// state.
+		peerLog.Debugf("%s sent an unsupported %s request -- "+
+			"disconnecting", sp, cmd)
+		sp.Disconnect()
+		return false
+	}
+
+	return true
+}
+
 // OnFilterAdd is invoked when a peer receives a filteradd bitcoin
 // message and is used by remote peers to add data to an already loaded bloom
 // filter.  The peer will be disconnected if a filter is not loaded when this
-// message is received.
+// message is received or the server is not configured to allow bloom filters.
 func (sp *serverPeer) OnFilterAdd(p *peer.Peer, msg *wire.MsgFilterAdd) {
+	// Disconnect and/or ban depending on the node bloom services flag and
+	// negotiated protocol version.
+	if !sp.enforceNodeBloomFlag(msg.Command()) {
+		return
+	}
+
 	if sp.filter.IsLoaded() {
 		peerLog.Debugf("%s sent a filteradd request with no filter "+
 			"loaded -- disconnecting", p)
@@ -807,8 +848,14 @@ func (sp *serverPeer) OnFilterAdd(p *peer.Peer, msg *wire.MsgFilterAdd) {
 // OnFilterClear is invoked when a peer receives a filterclear bitcoin
 // message and is used by remote peers to clear an already loaded bloom filter.
 // The peer will be disconnected if a filter is not loaded when this message is
-// received.
+// received  or the server is not configured to allow bloom filters.
 func (sp *serverPeer) OnFilterClear(p *peer.Peer, msg *wire.MsgFilterClear) {
+	// Disconnect and/or ban depending on the node bloom services flag and
+	// negotiated protocol version.
+	if !sp.enforceNodeBloomFlag(msg.Command()) {
+		return
+	}
+
 	if !sp.filter.IsLoaded() {
 		peerLog.Debugf("%s sent a filterclear request with no "+
 			"filter loaded -- disconnecting", p)
@@ -822,7 +869,15 @@ func (sp *serverPeer) OnFilterClear(p *peer.Peer, msg *wire.MsgFilterClear) {
 // OnFilterLoad is invoked when a peer receives a filterload bitcoin
 // message and it used to load a bloom filter that should be used for
 // delivering merkle blocks and associated transactions that match the filter.
+// The peer will be disconnected if the server is not configured to allow bloom
+// filters.
 func (sp *serverPeer) OnFilterLoad(p *peer.Peer, msg *wire.MsgFilterLoad) {
+	// Disconnect and/or ban depending on the node bloom services flag and
+	// negotiated protocol version.
+	if !sp.enforceNodeBloomFlag(msg.Command()) {
+		return
+	}
+
 	sp.setDisableRelayTx(false)
 
 	sp.filter.Reload(msg)