net_processing: don't modify addrman for private broadcast connections
What changed, and why it matters
This change prevents Bitcoin Core from storing address information learned from private broadcast connections in its internal address database (addrman). The concern is that data from these special, non-public connections could later be shared with ordinary peers, potentially leaking details about private network relationships or allowing a peer to influence which nodes your node recommends to others.
Treat as a hardening/privacy fix. Reviewers should confirm that no other code paths still allow private broadcast-derived data to enter addrman, and that the new test covers the intended invariant. No immediate emergency response is indicated by the diff alone.
Security signals we found
Information-flow control: prevents data from private broadcast channels from entering the public addrman gossip pool
Addrman integrity: reduces risk of service-bit metadata injection from non-public peers
Privacy: limits propagation of peer relationship metadata via ADDR/ADDRV2 gossip
Regression test added for the new behavior
Evidence from the diff
In net_processing.cpp, the handling of the VERSION message’s addrMe field now skips updating addrman when the connection is a PRIVATE_BROADCAST connection, in addition to the existing skip for inbound connections. Previously, only inbound connections were excluded; private broadcast outbound connections could still write service bits into addrman. A regression test verifies that a private broadcast peer advertising NODE_NETWORK does not cause addrman to record those services.
Changed components
src/net_processing.cpp VERSION message handlingAddrman (address manager) service-bit updatesPrivate broadcast connection type (ConnectionType::PRIVATE_BROADCAST)Inspect captured patch +32 / −1
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 4e102730..c7061d52 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3600,7 +3600,7 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
}
vRecv.ignore(8); // Ignore the addrMe service bits sent by the peer
vRecv >> CNetAddr::V1(addrMe);
- if (!pfrom.IsInboundConn())
+ if (!pfrom.IsInboundConn() && !pfrom.IsPrivateBroadcastConn())
{
// Overwrites potentially existing services. In contrast to this,
// unvalidated services received via gossip relay in ADDR/ADDRV2
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 1fc8d526..32801d97 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -2,6 +2,7 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#include <addrman.h>
#include <chainparams.h>
#include <clientversion.h>
#include <common/args.h>
@@ -1559,4 +1560,34 @@ BOOST_AUTO_TEST_CASE(v2transport_test)
}
}
+BOOST_AUTO_TEST_CASE(private_broadcast_version_does_not_update_addrman_services)
+{
+ LOCK(NetEventsInterface::g_msgproc_mutex);
+
+ const CNetAddr source{LookupHost("2.3.4.5", /*fAllowLookup=*/false).value()};
+ const CAddress addr{Lookup("1.2.3.4", 8333, /*fAllowLookup=*/false).value(), NODE_NONE};
+ BOOST_REQUIRE(m_node.addrman->Add({addr}, source));
+ CNode node{/*id=*/0,
+ /*sock=*/nullptr,
+ /*addrIn=*/addr,
+ /*nKeyedNetGroupIn=*/0,
+ /*nLocalHostNonceIn=*/0,
+ /*addrBindIn=*/CService{},
+ /*addrNameIn=*/"",
+ /*conn_type_in=*/ConnectionType::PRIVATE_BROADCAST,
+ /*inbound_onion=*/false,
+ /*network_key=*/0};
+
+ auto& connman = static_cast<ConnmanTestMsg&>(*m_node.connman);
+ connman.Handshake(node,
+ /*successfully_connected=*/false,
+ /*remote_services=*/NODE_NETWORK,
+ /*local_services=*/NODE_NONE,
+ /*version=*/PROTOCOL_VERSION,
+ /*relay_txs=*/true);
+
+ BOOST_CHECK_EQUAL(m_node.addrman->Select().first.nServices, NODE_NONE);
+ m_node.peerman->FinalizeNode(node);
+}
+
BOOST_AUTO_TEST_SUITE_END()
Why this scored 50/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.