Skip to content

Commit a33bd76

Browse files
fanquakefanquake
committed
Merge bitcoin/bitcoin#33464: p2p: Use network-dependent timers for inbound inv scheduling
0f7d4ee p2p: Use different inbound inv timer per network (Martin Zumsande) 94db966 net: use generic network key for addrcache (Martin Zumsande) Pull request description: Currently, `NextInvToInbounds` schedules each round of `inv` at the same time for all inbound peers. It's being done this way because with a separate timer per peer (like it's done for outbounds), an attacker could do multiple connections to learn about the time a transaction arrived. (#13298). However, having a single timer for inbounds of all networks is also an obvious fingerprinting vector: Connecting to a suspected pair of privacy-network and clearnet addresses and observing the `inv` pattern makes it trivial to confirm or refute that they are the same node. This PR changes it such that a separate timer is used for each network. It uses the existing method from `getaddr` caching and generalizes it to be saved in a new field `m_network_key` in `CNode` which will be used for both `getaddr` caching and `inv` scheduling, and can also be used for any future anti-fingerprinting measures. ACKs for top commit: sipa: utACK 0f7d4ee stratospher: reACK 0f7d4ee. naiyoma: Tested ACK 0f7d4ee danielabrozzoni: reACK 0f7d4ee Tree-SHA512: e197c3005b2522051db432948874320b74c23e01e66988ee1ee11917dac0923f58c1252fa47da24e68b08d7a355d8e5e0a3ccdfa6e4324cb901f21dfa880cd9c
2 parents 2578da6 + 0f7d4ee commit a33bd76

File tree

8 files changed

+76
-39
lines changed

8 files changed

+76
-39
lines changed

‎src/net.cpp‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ const std::string NET_MESSAGE_TYPE_OTHER = "*other*";
108108

109109
static const uint64_t RANDOMIZER_ID_NETGROUP = 0x6c0edd8036ef4036ULL; // SHA256("netgroup")[0:8]
110110
static const uint64_t RANDOMIZER_ID_LOCALHOSTNONCE = 0xd93e69e2bbfa5735ULL; // SHA256("localhostnonce")[0:8]
111-
static const uint64_t RANDOMIZER_ID_ADDRCACHE = 0x1cf2e4ddd306dda9ULL; // SHA256("addrcache")[0:8]
111+
static const uint64_t RANDOMIZER_ID_NETWORKKEY = 0x0e8a2b136c592a7dULL; // SHA256("networkkey")[0:8]
112112
//
113113
// Global state variables
114114
//
@@ -506,6 +506,13 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
506506
if (!addr_bind.IsValid()) {
507507
addr_bind = GetBindAddress(*sock);
508508
}
509+
uint64_t network_id = GetDeterministicRandomizer(RANDOMIZER_ID_NETWORKKEY)
510+
.Write(target_addr.GetNetClass())
511+
.Write(addr_bind.GetAddrBytes())
512+
// For outbound connections, the port of the bound address is randomly
513+
// assigned by the OS and would therefore not be useful for seeding.
514+
.Write(0)
515+
.Finalize();
509516
CNode* pnode = new CNode(id,
510517
std::move(sock),
511518
target_addr,
@@ -515,6 +522,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
515522
pszDest ? pszDest : "",
516523
conn_type,
517524
/*inbound_onion=*/false,
525+
network_id,
518526
CNodeOptions{
519527
.permission_flags = permission_flags,
520528
.i2p_sam_session = std::move(i2p_transient_session),
@@ -1808,6 +1816,11 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
18081816
ServiceFlags local_services = GetLocalServices();
18091817
const bool use_v2transport(local_services & NODE_P2P_V2);
18101818

1819+
uint64_t network_id = GetDeterministicRandomizer(RANDOMIZER_ID_NETWORKKEY)
1820+
.Write(inbound_onion ? NET_ONION : addr.GetNetClass())
1821+
.Write(addr_bind.GetAddrBytes())
1822+
.Write(addr_bind.GetPort()) // inbound connections use bind port
1823+
.Finalize();
18111824
CNode* pnode = new CNode(id,
18121825
std::move(sock),
18131826
CAddress{addr, NODE_NONE},
@@ -1817,6 +1830,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
18171830
/*addrNameIn=*/"",
18181831
ConnectionType::INBOUND,
18191832
inbound_onion,
1833+
network_id,
18201834
CNodeOptions{
18211835
.permission_flags = permission_flags,
18221836
.prefer_evict = discouraged,
@@ -3506,15 +3520,9 @@ std::vector<CAddress> CConnman::GetAddressesUnsafe(size_t max_addresses, size_t
35063520
std::vector<CAddress> CConnman::GetAddresses(CNode& requestor, size_t max_addresses, size_t max_pct)
35073521
{
35083522
auto local_socket_bytes = requestor.addrBind.GetAddrBytes();
3509-
uint64_t cache_id = GetDeterministicRandomizer(RANDOMIZER_ID_ADDRCACHE)
3510-
.Write(requestor.ConnectedThroughNetwork())
3511-
.Write(local_socket_bytes)
3512-
// For outbound connections, the port of the bound address is randomly
3513-
// assigned by the OS and would therefore not be useful for seeding.
3514-
.Write(requestor.IsInboundConn() ? requestor.addrBind.GetPort() : 0)
3515-
.Finalize();
3523+
uint64_t network_id = requestor.m_network_key;
35163524
const auto current_time = GetTime<std::chrono::microseconds>();
3517-
auto r = m_addr_response_caches.emplace(cache_id, CachedAddrResponse{});
3525+
auto r = m_addr_response_caches.emplace(network_id, CachedAddrResponse{});
35183526
CachedAddrResponse& cache_entry = r.first->second;
35193527
if (cache_entry.m_cache_entry_expiration < current_time) { // If emplace() added new one it has expiration 0.
35203528
cache_entry.m_addrs_response_cache = GetAddressesUnsafe(max_addresses, max_pct, /*network=*/std::nullopt);
@@ -3793,6 +3801,7 @@ CNode::CNode(NodeId idIn,
37933801
const std::string& addrNameIn,
37943802
ConnectionType conn_type_in,
37953803
bool inbound_onion,
3804+
uint64_t network_key,
37963805
CNodeOptions&& node_opts)
37973806
: m_transport{MakeTransport(idIn, node_opts.use_v2transport, conn_type_in == ConnectionType::INBOUND)},
37983807
m_permission_flags{node_opts.permission_flags},
@@ -3805,6 +3814,7 @@ CNode::CNode(NodeId idIn,
38053814
m_inbound_onion{inbound_onion},
38063815
m_prefer_evict{node_opts.prefer_evict},
38073816
nKeyedNetGroup{nKeyedNetGroupIn},
3817+
m_network_key{network_key},
38083818
m_conn_type{conn_type_in},
38093819
id{idIn},
38103820
nLocalHostNonce{nLocalHostNonceIn},

‎src/net.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,10 @@ class CNode
738738
std::atomic_bool fPauseRecv{false};
739739
std::atomic_bool fPauseSend{false};
740740

741+
/** Network key used to prevent fingerprinting our node across networks.
742+
* Influenced by the network and the bind address (+ bind port for inbounds) */
743+
const uint64_t m_network_key;
744+
741745
const ConnectionType m_conn_type;
742746

743747
/** Move all messages from the received queue to the processing queue. */
@@ -889,6 +893,7 @@ class CNode
889893
const std::string& addrNameIn,
890894
ConnectionType conn_type_in,
891895
bool inbound_onion,
896+
uint64_t network_key,
892897
CNodeOptions&& node_opts = {});
893898
CNode(const CNode&) = delete;
894899
CNode& operator=(const CNode&) = delete;

‎src/net_processing.cpp‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ class PeerManagerImpl final : public PeerManager
807807

808808
uint32_t GetFetchFlags(const Peer& peer) const;
809809

810-
std::atomic<std::chrono::microseconds> m_next_inv_to_inbounds{0us};
810+
std::map<uint64_t, std::chrono::microseconds> m_next_inv_to_inbounds_per_network_key GUARDED_BY(g_msgproc_mutex);
811811

812812
/** Number of nodes with fSyncStarted. */
813813
int nSyncStarted GUARDED_BY(cs_main) = 0;
@@ -837,12 +837,14 @@ class PeerManagerImpl final : public PeerManager
837837

838838
/**
839839
* For sending `inv`s to inbound peers, we use a single (exponentially
840-
* distributed) timer for all peers. If we used a separate timer for each
840+
* distributed) timer for all peers with the same network key. If we used a separate timer for each
841841
* peer, a spy node could make multiple inbound connections to us to
842-
* accurately determine when we received the transaction (and potentially
843-
* determine the transaction's origin). */
842+
* accurately determine when we received a transaction (and potentially
843+
* determine the transaction's origin). Each network key has its own timer
844+
* to make fingerprinting harder. */
844845
std::chrono::microseconds NextInvToInbounds(std::chrono::microseconds now,
845-
std::chrono::seconds average_interval) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
846+
std::chrono::seconds average_interval,
847+
uint64_t network_key) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
846848

847849

848850
// All of the following cache a recent block, and are protected by m_most_recent_block_mutex
@@ -1143,15 +1145,15 @@ static bool CanServeWitnesses(const Peer& peer)
11431145
}
11441146

11451147
std::chrono::microseconds PeerManagerImpl::NextInvToInbounds(std::chrono::microseconds now,
1146-
std::chrono::seconds average_interval)
1148+
std::chrono::seconds average_interval,
1149+
uint64_t network_key)
11471150
{
1148-
if (m_next_inv_to_inbounds.load() < now) {
1149-
// If this function were called from multiple threads simultaneously
1150-
// it would possible that both update the next send variable, and return a different result to their caller.
1151-
// This is not possible in practice as only the net processing thread invokes this function.
1152-
m_next_inv_to_inbounds = now + m_rng.rand_exp_duration(average_interval);
1151+
auto [it, inserted] = m_next_inv_to_inbounds_per_network_key.try_emplace(network_key, 0us);
1152+
auto& timer{it->second};
1153+
if (timer < now) {
1154+
timer = now + m_rng.rand_exp_duration(average_interval);
11531155
}
1154-
return m_next_inv_to_inbounds;
1156+
return timer;
11551157
}
11561158

11571159
bool PeerManagerImpl::IsBlockRequested(const uint256& hash)
@@ -5715,7 +5717,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
57155717
if (tx_relay->m_next_inv_send_time < current_time) {
57165718
fSendTrickle = true;
57175719
if (pto->IsInboundConn()) {
5718-
tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL);
5720+
tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL, pto->m_network_key);
57195721
} else {
57205722
tx_relay->m_next_inv_send_time = current_time + m_rng.rand_exp_duration(OUTBOUND_INVENTORY_BROADCAST_INTERVAL);
57215723
}

‎src/test/denialofservice_tests.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
6262
CAddress(),
6363
/*addrNameIn=*/"",
6464
ConnectionType::OUTBOUND_FULL_RELAY,
65-
/*inbound_onion=*/false};
65+
/*inbound_onion=*/false,
66+
/*network_key=*/0};
6667

6768
connman.Handshake(
6869
/*node=*/dummyNode1,
@@ -128,7 +129,8 @@ void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerManager&
128129
CAddress(),
129130
/*addrNameIn=*/"",
130131
connType,
131-
/*inbound_onion=*/false});
132+
/*inbound_onion=*/false,
133+
/*network_key=*/0});
132134
CNode &node = *vNodes.back();
133135
node.SetCommonVersion(PROTOCOL_VERSION);
134136

@@ -327,7 +329,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
327329
CAddress(),
328330
/*addrNameIn=*/"",
329331
ConnectionType::INBOUND,
330-
/*inbound_onion=*/false};
332+
/*inbound_onion=*/false,
333+
/*network_key=*/1};
331334
nodes[0]->SetCommonVersion(PROTOCOL_VERSION);
332335
peerLogic->InitializeNode(*nodes[0], NODE_NETWORK);
333336
nodes[0]->fSuccessfullyConnected = true;
@@ -347,7 +350,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
347350
CAddress(),
348351
/*addrNameIn=*/"",
349352
ConnectionType::INBOUND,
350-
/*inbound_onion=*/false};
353+
/*inbound_onion=*/false,
354+
/*network_key=*/1};
351355
nodes[1]->SetCommonVersion(PROTOCOL_VERSION);
352356
peerLogic->InitializeNode(*nodes[1], NODE_NETWORK);
353357
nodes[1]->fSuccessfullyConnected = true;
@@ -377,7 +381,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
377381
CAddress(),
378382
/*addrNameIn=*/"",
379383
ConnectionType::OUTBOUND_FULL_RELAY,
380-
/*inbound_onion=*/false};
384+
/*inbound_onion=*/false,
385+
/*network_key=*/2};
381386
nodes[2]->SetCommonVersion(PROTOCOL_VERSION);
382387
peerLogic->InitializeNode(*nodes[2], NODE_NETWORK);
383388
nodes[2]->fSuccessfullyConnected = true;
@@ -419,7 +424,8 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
419424
CAddress(),
420425
/*addrNameIn=*/"",
421426
ConnectionType::INBOUND,
422-
/*inbound_onion=*/false};
427+
/*inbound_onion=*/false,
428+
/*network_key=*/1};
423429
dummyNode.SetCommonVersion(PROTOCOL_VERSION);
424430
peerLogic->InitializeNode(dummyNode, NODE_NETWORK);
425431
dummyNode.fSuccessfullyConnected = true;

‎src/test/fuzz/p2p_headers_presync.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ void HeadersSyncSetup::ResetAndInitialize()
7070

7171
for (auto conn_type : conn_types) {
7272
CAddress addr{};
73-
m_connections.push_back(new CNode(id++, nullptr, addr, 0, 0, addr, "", conn_type, false));
73+
m_connections.push_back(new CNode(id++, nullptr, addr, 0, 0, addr, "", conn_type, false, 0));
7474
CNode& p2p_node = *m_connections.back();
7575

7676
connman.Handshake(

‎src/test/fuzz/util/net.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,8 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N
275275
const std::string addr_name = fuzzed_data_provider.ConsumeRandomLengthString(64);
276276
const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray(ALL_CONNECTION_TYPES);
277277
const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false};
278+
const uint64_t network_id = fuzzed_data_provider.ConsumeIntegral<uint64_t>();
279+
278280
NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
279281
if constexpr (ReturnUniquePtr) {
280282
return std::make_unique<CNode>(node_id,
@@ -286,6 +288,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N
286288
addr_name,
287289
conn_type,
288290
inbound_onion,
291+
network_id,
289292
CNodeOptions{ .permission_flags = permission_flags });
290293
} else {
291294
return CNode{node_id,
@@ -297,6 +300,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N
297300
addr_name,
298301
conn_type,
299302
inbound_onion,
303+
network_id,
300304
CNodeOptions{ .permission_flags = permission_flags }};
301305
}
302306
}

‎src/test/net_peer_connection_tests.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ void AddPeer(NodeId& id, std::vector<CNode*>& nodes, PeerManager& peerman, Connm
7272
CAddress{},
7373
/*addrNameIn=*/"",
7474
conn_type,
75-
/*inbound_onion=*/inbound_onion});
75+
/*inbound_onion=*/inbound_onion,
76+
/*network_key=*/0});
7677
CNode& node = *nodes.back();
7778
node.SetCommonVersion(PROTOCOL_VERSION);
7879

‎src/test/net_tests.cpp‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
6767
CAddress(),
6868
pszDest,
6969
ConnectionType::OUTBOUND_FULL_RELAY,
70-
/*inbound_onion=*/false);
70+
/*inbound_onion=*/false,
71+
/*network_key=*/0);
7172
BOOST_CHECK(pnode1->IsFullOutboundConn() == true);
7273
BOOST_CHECK(pnode1->IsManualConn() == false);
7374
BOOST_CHECK(pnode1->IsBlockOnlyConn() == false);
@@ -85,7 +86,8 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
8586
CAddress(),
8687
pszDest,
8788
ConnectionType::INBOUND,
88-
/*inbound_onion=*/false);
89+
/*inbound_onion=*/false,
90+
/*network_key=*/1);
8991
BOOST_CHECK(pnode2->IsFullOutboundConn() == false);
9092
BOOST_CHECK(pnode2->IsManualConn() == false);
9193
BOOST_CHECK(pnode2->IsBlockOnlyConn() == false);
@@ -103,7 +105,8 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
103105
CAddress(),
104106
pszDest,
105107
ConnectionType::OUTBOUND_FULL_RELAY,
106-
/*inbound_onion=*/false);
108+
/*inbound_onion=*/false,
109+
/*network_key=*/2);
107110
BOOST_CHECK(pnode3->IsFullOutboundConn() == true);
108111
BOOST_CHECK(pnode3->IsManualConn() == false);
109112
BOOST_CHECK(pnode3->IsBlockOnlyConn() == false);
@@ -121,7 +124,8 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
121124
CAddress(),
122125
pszDest,
123126
ConnectionType::INBOUND,
124-
/*inbound_onion=*/true);
127+
/*inbound_onion=*/true,
128+
/*network_key=*/3);
125129
BOOST_CHECK(pnode4->IsFullOutboundConn() == false);
126130
BOOST_CHECK(pnode4->IsManualConn() == false);
127131
BOOST_CHECK(pnode4->IsBlockOnlyConn() == false);
@@ -613,7 +617,8 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
613617
CAddress{},
614618
/*pszDest=*/std::string{},
615619
ConnectionType::OUTBOUND_FULL_RELAY,
616-
/*inbound_onion=*/false);
620+
/*inbound_onion=*/false,
621+
/*network_key=*/0);
617622
pnode->fSuccessfullyConnected.store(true);
618623

619624
// the peer claims to be reaching us via IPv6
@@ -667,7 +672,8 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port)
667672
/*addrBindIn=*/CService{},
668673
/*addrNameIn=*/std::string{},
669674
/*conn_type_in=*/ConnectionType::OUTBOUND_FULL_RELAY,
670-
/*inbound_onion=*/false};
675+
/*inbound_onion=*/false,
676+
/*network_key=*/0};
671677
peer_out.fSuccessfullyConnected = true;
672678
peer_out.SetAddrLocal(peer_us);
673679

@@ -688,7 +694,8 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port)
688694
/*addrBindIn=*/CService{},
689695
/*addrNameIn=*/std::string{},
690696
/*conn_type_in=*/ConnectionType::INBOUND,
691-
/*inbound_onion=*/false};
697+
/*inbound_onion=*/false,
698+
/*network_key=*/1};
692699
peer_in.fSuccessfullyConnected = true;
693700
peer_in.SetAddrLocal(peer_us);
694701

@@ -825,7 +832,8 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
825832
/*addrBindIn=*/CService{},
826833
/*addrNameIn=*/std::string{},
827834
/*conn_type_in=*/ConnectionType::OUTBOUND_FULL_RELAY,
828-
/*inbound_onion=*/false};
835+
/*inbound_onion=*/false,
836+
/*network_key=*/2};
829837

830838
const uint64_t services{NODE_NETWORK | NODE_WITNESS};
831839
const int64_t time{0};
@@ -900,7 +908,8 @@ BOOST_AUTO_TEST_CASE(advertise_local_address)
900908
CAddress{},
901909
/*pszDest=*/std::string{},
902910
ConnectionType::OUTBOUND_FULL_RELAY,
903-
/*inbound_onion=*/false);
911+
/*inbound_onion=*/false,
912+
/*network_key=*/0);
904913
};
905914
g_reachable_nets.Add(NET_CJDNS);
906915

0 commit comments

Comments
 (0)