From 845e3a34c49abcc634b5a10ccdd6b10fb4fcf449 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 28 Nov 2022 16:34:50 +0000 Subject: [PATCH 1/3] [net processing] Ensure transaction announcements are only queued for fully connected peers --- src/net_processing.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0d5be42e0e3..70e7eb85d8a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -295,7 +295,7 @@ struct Peer { std::atomic m_last_mempool_req{0s}; /** The next time after which we will send an `inv` message containing * transaction announcements to this peer. */ - std::chrono::microseconds m_next_inv_send_time GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; + std::chrono::microseconds m_next_inv_send_time GUARDED_BY(m_tx_inventory_mutex){0}; /** Minimum fee rate with which to filter transaction announcements to this node. See BIP133. */ std::atomic m_fee_filter_received{0}; @@ -2011,8 +2011,15 @@ void PeerManagerImpl::RelayTransaction(const uint256& txid, const uint256& wtxid auto tx_relay = peer.GetTxRelay(); if (!tx_relay) continue; - const uint256& hash{peer.m_wtxid_relay ? wtxid : txid}; LOCK(tx_relay->m_tx_inventory_mutex); + // Only queue transactions for announcement once the version handshake + // is completed. The time of arrival for these transactions is + // otherwise at risk of leaking to a spy, if the spy is able to + // distinguish transactions received during the handshake from the rest + // in the announcement. + if (tx_relay->m_next_inv_send_time == 0s) continue; + + const uint256& hash{peer.m_wtxid_relay ? wtxid : txid}; if (!tx_relay->m_tx_inventory_known_filter.contains(hash)) { tx_relay->m_tx_inventory_to_send.insert(hash); } From ce63fca13e9b500e9f687d80a457175ac967a371 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 28 Nov 2022 16:37:24 +0000 Subject: [PATCH 2/3] [net processing] Assume that TxRelay::m_tx_inventory_to_send is empty pre-verack This commit documents our assumption about TxRelay::m_tx_inventory_to_send being empty prior to version handshake completion. The added Assume acts as testing oracle for our fuzzing tests to potentially detect if the assumption is violated. --- src/net_processing.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 70e7eb85d8a..6d5eb3a449d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3435,6 +3435,20 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } } + if (auto tx_relay = peer->GetTxRelay()) { + // `TxRelay::m_tx_inventory_to_send` must be empty before the + // version handshake is completed as + // `TxRelay::m_next_inv_send_time` is first initialised in + // `SendMessages` after the verack is received. Any transactions + // received during the version handshake would otherwise + // immediately be advertised without random delay, potentially + // leaking the time of arrival to a spy. + Assume(WITH_LOCK( + tx_relay->m_tx_inventory_mutex, + return tx_relay->m_tx_inventory_to_send.empty() && + tx_relay->m_next_inv_send_time == 0s)); + } + pfrom.fSuccessfullyConnected = true; return; } From 8f2dac54096c20afd8fd12c21a2ee5465fea085e Mon Sep 17 00:00:00 2001 From: dergoegge Date: Wed, 23 Nov 2022 18:12:42 +0000 Subject: [PATCH 3/3] [test] Add p2p_tx_privacy.py --- test/functional/p2p_tx_privacy.py | 78 +++++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 79 insertions(+) create mode 100755 test/functional/p2p_tx_privacy.py diff --git a/test/functional/p2p_tx_privacy.py b/test/functional/p2p_tx_privacy.py new file mode 100755 index 00000000000..b885ccdf5df --- /dev/null +++ b/test/functional/p2p_tx_privacy.py @@ -0,0 +1,78 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Test that transaction announcements are only queued for peers that have +successfully completed the version handshake. + +Topology: + + tx_originator ----> node[0] <---- spy + +We test that a transaction sent by tx_originator is only relayed to spy +if it was received after spy's version handshake completed. + +1. Fully connect tx_originator +2. Connect spy (no version handshake) +3. tx_originator sends tx1 +4. spy completes the version handshake +5. tx_originator sends tx2 +6. We check that only tx2 is announced on the spy interface +""" +from test_framework.messages import ( + msg_wtxidrelay, + msg_verack, + msg_tx, + CInv, + MSG_WTX, +) +from test_framework.p2p import ( + P2PInterface, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.wallet import MiniWallet + +class P2PTxSpy(P2PInterface): + def __init__(self): + super().__init__() + self.all_invs = [] + + def on_version(self, message): + self.send_message(msg_wtxidrelay()) + + def on_inv(self, message): + self.all_invs += message.inv + + def wait_for_inv_match(self, expected_inv): + self.wait_until(lambda: len(self.all_invs) == 1 and self.all_invs[0] == expected_inv) + +class TxPrivacyTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def run_test(self): + self.wallet = MiniWallet(self.nodes[0]) + self.wallet.rescan_utxos() + + tx_originator = self.nodes[0].add_p2p_connection(P2PInterface()) + spy = self.nodes[0].add_p2p_connection(P2PTxSpy(), wait_for_verack=False) + spy.wait_for_verack() + + # tx_originator sends tx1 + tx1 = self.wallet.create_self_transfer()["tx"] + tx_originator.send_and_ping(msg_tx(tx1)) + + # Spy sends the verack + spy.send_and_ping(msg_verack()) + + # tx_originator sends tx2 + tx2 = self.wallet.create_self_transfer()["tx"] + tx_originator.send_and_ping(msg_tx(tx2)) + + # Spy should only get an inv for the second transaction as the first + # one was received pre-verack with the spy + spy.wait_for_inv_match(CInv(MSG_WTX, tx2.calc_sha256(True))) + +if __name__ == '__main__': + TxPrivacyTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index e2c13a67056..785a97a48b1 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -318,6 +318,7 @@ BASE_SCRIPTS = [ 'rpc_deriveaddresses.py', 'rpc_deriveaddresses.py --usecli', 'p2p_ping.py', + 'p2p_tx_privacy.py', 'rpc_scanblocks.py', 'p2p_sendtxrcncl.py', 'rpc_scantxoutset.py',