From f10e80b6e4fbc151abbf1c20fbdcc3581d3688f0 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 12 Apr 2022 13:46:59 +0200 Subject: [PATCH 1/3] [net] Use ConnectedThroughNetwork() instead of GetNetwork() to seed addr cache randomizer --- src/net.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index 46d7020c5e6..e8c131b5b0f 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2799,7 +2799,7 @@ std::vector CConnman::GetAddresses(CNode& requestor, size_t max_addres { auto local_socket_bytes = requestor.addrBind.GetAddrBytes(); uint64_t cache_id = GetDeterministicRandomizer(RANDOMIZER_ID_ADDRCACHE) - .Write(requestor.addr.GetNetwork()) + .Write(requestor.ConnectedThroughNetwork()) .Write(local_socket_bytes.data(), local_socket_bytes.size()) .Finalize(); const auto current_time = GetTime(); From 3382905befd23364989d941038bf7b1530fea0dc Mon Sep 17 00:00:00 2001 From: dergoegge Date: Tue, 12 Apr 2022 13:47:48 +0200 Subject: [PATCH 2/3] [net] Seed addr cache randomizer with port from binding address --- src/net.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/net.cpp b/src/net.cpp index e8c131b5b0f..3820ee1fd03 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2801,6 +2801,9 @@ std::vector CConnman::GetAddresses(CNode& requestor, size_t max_addres uint64_t cache_id = GetDeterministicRandomizer(RANDOMIZER_ID_ADDRCACHE) .Write(requestor.ConnectedThroughNetwork()) .Write(local_socket_bytes.data(), local_socket_bytes.size()) + // For outbound connections, the port of the bound address is randomly + // assigned by the OS and would therefore not be useful for seeding. + .Write(requestor.IsInboundConn() ? requestor.addrBind.GetPort() : 0) .Finalize(); const auto current_time = GetTime(); auto r = m_addr_response_caches.emplace(cache_id, CachedAddrResponse{}); From 292828cd7744ec7eadede4ad54aa2117087c5435 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 9 May 2022 17:25:16 +0200 Subject: [PATCH 3/3] [test] Test addr cache for multiple onion binds --- test/functional/p2p_getaddr_caching.py | 66 +++++++++++++++++++++----- 1 file changed, 53 insertions(+), 13 deletions(-) diff --git a/test/functional/p2p_getaddr_caching.py b/test/functional/p2p_getaddr_caching.py index d375af6fe1c..c934a977294 100755 --- a/test/functional/p2p_getaddr_caching.py +++ b/test/functional/p2p_getaddr_caching.py @@ -14,6 +14,8 @@ from test_framework.p2p import ( from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + PORT_MIN, + PORT_RANGE, ) # As defined in net_processing. @@ -42,6 +44,13 @@ class AddrReceiver(P2PInterface): class AddrTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 + # Start onion ports after p2p and rpc ports. + port = PORT_MIN + 2 * PORT_RANGE + self.onion_port1 = port + self.onion_port2 = port + 1 + self.extra_args = [ + [f"-bind=127.0.0.1:{self.onion_port1}=onion", f"-bind=127.0.0.1:{self.onion_port2}=onion"], + ] def run_test(self): self.log.info('Fill peer AddrMan with a lot of records') @@ -55,35 +64,66 @@ class AddrTest(BitcoinTestFramework): # only a fraction of all known addresses can be cached and returned. assert(len(self.nodes[0].getnodeaddresses(0)) > int(MAX_ADDR_TO_SEND / (MAX_PCT_ADDR_TO_SEND / 100))) - responses = [] + last_response_on_local_bind = None + last_response_on_onion_bind1 = None + last_response_on_onion_bind2 = None self.log.info('Send many addr requests within short time to receive same response') N = 5 cur_mock_time = int(time.time()) for i in range(N): - addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) - addr_receiver.send_and_ping(msg_getaddr()) + addr_receiver_local = self.nodes[0].add_p2p_connection(AddrReceiver()) + addr_receiver_local.send_and_ping(msg_getaddr()) + addr_receiver_onion1 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port1) + addr_receiver_onion1.send_and_ping(msg_getaddr()) + addr_receiver_onion2 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port2) + addr_receiver_onion2.send_and_ping(msg_getaddr()) + # Trigger response cur_mock_time += 5 * 60 self.nodes[0].setmocktime(cur_mock_time) - addr_receiver.wait_until(addr_receiver.addr_received) - responses.append(addr_receiver.get_received_addrs()) - for response in responses[1:]: - assert_equal(response, responses[0]) - assert(len(response) == MAX_ADDR_TO_SEND) + addr_receiver_local.wait_until(addr_receiver_local.addr_received) + addr_receiver_onion1.wait_until(addr_receiver_onion1.addr_received) + addr_receiver_onion2.wait_until(addr_receiver_onion2.addr_received) + + if i > 0: + # Responses from different binds should be unique + assert(last_response_on_local_bind != addr_receiver_onion1.get_received_addrs()) + assert(last_response_on_local_bind != addr_receiver_onion2.get_received_addrs()) + assert(last_response_on_onion_bind1 != addr_receiver_onion2.get_received_addrs()) + # Responses on from the same bind should be the same + assert_equal(last_response_on_local_bind, addr_receiver_local.get_received_addrs()) + assert_equal(last_response_on_onion_bind1, addr_receiver_onion1.get_received_addrs()) + assert_equal(last_response_on_onion_bind2, addr_receiver_onion2.get_received_addrs()) + + last_response_on_local_bind = addr_receiver_local.get_received_addrs() + last_response_on_onion_bind1 = addr_receiver_onion1.get_received_addrs() + last_response_on_onion_bind2 = addr_receiver_onion2.get_received_addrs() + + for response in [last_response_on_local_bind, last_response_on_onion_bind1, last_response_on_onion_bind2]: + assert_equal(len(response), MAX_ADDR_TO_SEND) cur_mock_time += 3 * 24 * 60 * 60 self.nodes[0].setmocktime(cur_mock_time) self.log.info('After time passed, see a new response to addr request') - last_addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) - last_addr_receiver.send_and_ping(msg_getaddr()) + addr_receiver_local = self.nodes[0].add_p2p_connection(AddrReceiver()) + addr_receiver_local.send_and_ping(msg_getaddr()) + addr_receiver_onion1 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port1) + addr_receiver_onion1.send_and_ping(msg_getaddr()) + addr_receiver_onion2 = self.nodes[0].add_p2p_connection(AddrReceiver(), dstport=self.onion_port2) + addr_receiver_onion2.send_and_ping(msg_getaddr()) + # Trigger response cur_mock_time += 5 * 60 self.nodes[0].setmocktime(cur_mock_time) - last_addr_receiver.wait_until(last_addr_receiver.addr_received) - # new response is different - assert(set(responses[0]) != set(last_addr_receiver.get_received_addrs())) + addr_receiver_local.wait_until(addr_receiver_local.addr_received) + addr_receiver_onion1.wait_until(addr_receiver_onion1.addr_received) + addr_receiver_onion2.wait_until(addr_receiver_onion2.addr_received) + # new response is different + assert(set(last_response_on_local_bind) != set(addr_receiver_local.get_received_addrs())) + assert(set(last_response_on_onion_bind1) != set(addr_receiver_onion1.get_received_addrs())) + assert(set(last_response_on_onion_bind2) != set(addr_receiver_onion2.get_received_addrs())) if __name__ == '__main__': AddrTest().main()