Merge bitcoin/bitcoin#22387: Rate limit the processing of rumoured addresses

a4bcd687c934d47aa3922334e97e579caf5f8124 Improve tests using statistics (John Newbery)
f424d601e1b6870e20bc60f5ccba36d2e210377b Add logging and addr rate limiting statistics (Pieter Wuille)
b4ece8a1cda69cc268d39d21bba59c51fa2fb9ed Functional tests for addr rate limiting (Pieter Wuille)
5648138f5949013331c017c740646e2f4013bc24 Randomize the order of addr processing (Pieter Wuille)
0d64b8f709b4655d8702f810d4876cd8d96ded82 Rate limit the processing of incoming addr messages (Pieter Wuille)

Pull request description:

  The rate at which IP addresses are rumoured (through ADDR and ADDRV2 messages) on the network seems to vary from 0 for some non-participating nodes, to 0.005-0.025 addr/s for recent Bitcoin Core nodes. However, the current codebase will happily accept and process an effectively unbounded rate from attackers. There are measures to limit the influence attackers can have on the addrman database (bucket restrictions based on source IPs), but still - there is no need to permit them to feed us addresses at a rate that's orders of magnitude larger than what is common on the network today, especially as it will cause us to spam our peers too.

  This PR implements a [token bucket](https://en.wikipedia.org/wiki/Token_bucket) based rate limiter, allowing an average of 0.1 addr/s per connection, with bursts up to 1000 addresses at once. Whitelisted peers as well as responses to GETADDR requests are exempt from the limit. New connections start with 1 token, so as to not interfere with the common practice of peers' self-announcement.

ACKs for top commit:
  laanwj:
    ACK a4bcd687c934d47aa3922334e97e579caf5f8124
  vasild:
    ACK a4bcd687c934d47aa3922334e97e579caf5f8124
  jnewbery:
    ACK a4bcd687c934d47aa3922334e97e579caf5f8124
  jonatack:
    ACK a4bcd687c934d47aa3922334e97e579caf5f8124

Tree-SHA512: b757de76ad78a53035b622944c4213b29b3b55d3d98bf23585afa84bfba10808299d858649f92269a16abfa75eb4366ea047eae3216f7e2f6d3c455782a16bea

3 files changed, 99 insertions, 6 deletions

diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py
index 1a414959b9..ff1d85a9be 100755
--- a/test/functional/p2p_addr_relay.py
+++ b/test/functional/p2p_addr_relay.py
@@ -13,17 +13,20 @@ from test_framework.messages import (
     msg_addr,
     msg_getaddr
 )
-from test_framework.p2p import P2PInterface
-from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (
-    assert_equal,
+from test_framework.p2p import (
+    P2PInterface,
+    p2p_lock,
 )
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+import random
 import time
 
 
 class AddrReceiver(P2PInterface):
     num_ipv4_received = 0
     test_addr_contents = False
+    _tokens = 1
 
     def __init__(self, test_addr_contents=False):
         super().__init__()
@@ -40,6 +43,20 @@ class AddrReceiver(P2PInterface):
                     raise AssertionError("Invalid addr.port of {} (8333-8342 expected)".format(addr.port))
                 assert addr.ip.startswith('123.123.123.')
 
+    def on_getaddr(self, message):
+        # When the node sends us a getaddr, it increments the addr relay tokens for the connection by 1000
+        self._tokens += 1000
+
+    @property
+    def tokens(self):
+        with p2p_lock:
+            return self._tokens
+
+    def increment_tokens(self, n):
+        # When we move mocktime forward, the node increments the addr relay tokens for its peers
+        with p2p_lock:
+            self._tokens += n
+
     def addr_received(self):
         return self.num_ipv4_received != 0
 
@@ -53,12 +70,14 @@ class AddrTest(BitcoinTestFramework):
 
     def set_test_params(self):
         self.num_nodes = 1
+        self.extra_args = [["-whitelist=addr@127.0.0.1"]]
 
     def run_test(self):
         self.oversized_addr_test()
         self.relay_tests()
         self.getaddr_tests()
         self.blocksonly_mode_tests()
+        self.rate_limit_tests()
 
     def setup_addr_msg(self, num):
         addrs = []
@@ -75,6 +94,19 @@ class AddrTest(BitcoinTestFramework):
         msg.addrs = addrs
         return msg
 
+    def setup_rand_addr_msg(self, num):
+        addrs = []
+        for i in range(num):
+            addr = CAddress()
+            addr.time = self.mocktime + i
+            addr.nServices = NODE_NETWORK | NODE_WITNESS
+            addr.ip = f"{random.randrange(128,169)}.{random.randrange(1,255)}.{random.randrange(1,255)}.{random.randrange(1,255)}"
+            addr.port = 8333
+            addrs.append(addr)
+        msg = msg_addr()
+        msg.addrs = addrs
+        return msg
+
     def send_addr_msg(self, source, msg, receivers):
         source.send_and_ping(msg)
         # pop m_next_addr_send timer
@@ -191,7 +223,7 @@ class AddrTest(BitcoinTestFramework):
 
     def blocksonly_mode_tests(self):
         self.log.info('Test addr relay in -blocksonly mode')
-        self.restart_node(0, ["-blocksonly"])
+        self.restart_node(0, ["-blocksonly", "-whitelist=addr@127.0.0.1"])
         self.mocktime = int(time.time())
 
         self.log.info('Check that we send getaddr messages')
@@ -207,6 +239,63 @@ class AddrTest(BitcoinTestFramework):
 
         self.nodes[0].disconnect_p2ps()
 
+    def send_addrs_and_test_rate_limiting(self, peer, no_relay, new_addrs, total_addrs):
+        """Send an addr message and check that the number of addresses processed and rate-limited is as expected"""
+
+        peer.send_and_ping(self.setup_rand_addr_msg(new_addrs))
+
+        peerinfo = self.nodes[0].getpeerinfo()[0]
+        addrs_processed = peerinfo['addr_processed']
+        addrs_rate_limited = peerinfo['addr_rate_limited']
+        self.log.debug(f"addrs_processed = {addrs_processed}, addrs_rate_limited = {addrs_rate_limited}")
+
+        if no_relay:
+            assert_equal(addrs_processed, 0)
+            assert_equal(addrs_rate_limited, 0)
+        else:
+            assert_equal(addrs_processed, min(total_addrs, peer.tokens))
+            assert_equal(addrs_rate_limited, max(0, total_addrs - peer.tokens))
+
+    def rate_limit_tests(self):
+
+        self.mocktime = int(time.time())
+        self.restart_node(0, [])
+        self.nodes[0].setmocktime(self.mocktime)
+
+        for contype, no_relay in [("outbound-full-relay", False), ("block-relay-only", True), ("inbound", False)]:
+            self.log.info(f'Test rate limiting of addr processing for {contype} peers')
+            if contype == "inbound":
+                peer = self.nodes[0].add_p2p_connection(AddrReceiver())
+            else:
+                peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type=contype)
+
+            # Send 600 addresses. For all but the block-relay-only peer this should result in addresses being processed.
+            self.send_addrs_and_test_rate_limiting(peer, no_relay, 600, 600)
+
+            # Send 600 more addresses. For the outbound-full-relay peer (which we send a GETADDR, and thus will
+            # process up to 1001 incoming addresses), this means more addresses will be processed.
+            self.send_addrs_and_test_rate_limiting(peer, no_relay, 600, 1200)
+
+            # Send 10 more. As we reached the processing limit for all nodes, no more addresses should be procesesd.
+            self.send_addrs_and_test_rate_limiting(peer, no_relay, 10, 1210)
+
+            # Advance the time by 100 seconds, permitting the processing of 10 more addresses.
+            # Send 200 and verify that 10 are processed.
+            self.mocktime += 100
+            self.nodes[0].setmocktime(self.mocktime)
+            peer.increment_tokens(10)
+
+            self.send_addrs_and_test_rate_limiting(peer, no_relay, 200, 1410)
+
+            # Advance the time by 1000 seconds, permitting the processing of 100 more addresses.
+            # Send 200 and verify that 100 are processed.
+            self.mocktime += 1000
+            self.nodes[0].setmocktime(self.mocktime)
+            peer.increment_tokens(100)
+
+            self.send_addrs_and_test_rate_limiting(peer, no_relay, 200, 1610)
+
+            self.nodes[0].disconnect_p2ps()
 
 if __name__ == '__main__':
     AddrTest().main()
diff --git a/test/functional/p2p_addrv2_relay.py b/test/functional/p2p_addrv2_relay.py
index c006a6c24f..32c1d42b1c 100755
--- a/test/functional/p2p_addrv2_relay.py
+++ b/test/functional/p2p_addrv2_relay.py
@@ -42,7 +42,9 @@ class AddrReceiver(P2PInterface):
         super().__init__(support_addrv2 = True)
 
     def on_addrv2(self, message):
-        if ADDRS == message.addrs:
+        expected_set = set((addr.ip, addr.port) for addr in ADDRS)
+        received_set = set((addr.ip, addr.port) for addr in message.addrs)
+        if expected_set == received_set:
             self.addrv2_received_and_checked = True
 
     def wait_for_addrv2(self):
@@ -53,6 +55,7 @@ class AddrTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 1
+        self.extra_args = [["-whitelist=addr@127.0.0.1"]]
 
     def run_test(self):
         self.log.info('Create connection that sends addrv2 messages')
diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py
index 788a81d4af..9c34506320 100755
--- a/test/functional/p2p_invalid_messages.py
+++ b/test/functional/p2p_invalid_messages.py
@@ -58,6 +58,7 @@ class InvalidMessagesTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
         self.setup_clean_chain = True
+        self.extra_args = [["-whitelist=addr@127.0.0.1"]]
 
     def run_test(self):
         self.test_buffer()