Skip to content

Commit

Permalink
Merge bitcoin#15759: p2p: Add 2 outbound block-relay-only connections
Browse files Browse the repository at this point in the history 
0ba0802 Disconnect peers violating blocks-only mode (Suhas Daftuar)
937eba9 doc: improve comments relating to block-relay-only peers (Suhas Daftuar)
430f489 Don't relay addr messages to block-relay-only peers (Suhas Daftuar)
3a5e885 Add 2 outbound block-relay-only connections (Suhas Daftuar)
b83f51a Add comment explaining intended use of m_tx_relay (Suhas Daftuar)
e75c39c Check that tx_relay is initialized before access (Suhas Daftuar)
c4aa2ba [refactor] Change tx_relay structure to be unique_ptr (Suhas Daftuar)
4de0dba [refactor] Move tx relay state to separate structure (Suhas Daftuar)
26a93bc Remove unused variable (Suhas Daftuar)

Pull request description:

  Transaction relay is optimized for a combination of redundancy/robustness as well as bandwidth minimization -- as a result transaction relay leaks information that adversaries can use to infer the network topology.

  Network topology is better kept private for (at least) two reasons:

  (a) Knowledge of the network graph can make it easier to find the source IP of a given transaction.

  (b) Knowledge of the network graph could be used to split a target node or nodes from the honest network (eg by knowing which peers to attack in order to achieve a network split).

  We can eliminate the risks of (b) by separating block relay from transaction relay; inferring network connectivity from the relay of blocks/block headers is much more expensive for an adversary.

  After this commit, bitcoind will make 2 additional outbound connections that are only used for block relay. (In the future, we might consider rotating our transaction-relay peers to help limit the effects of (a).)

ACKs for top commit:
  sipa:
    ACK 0ba0802
  ajtowns:
    ACK 0ba0802 -- code review, ran tests. ran it on mainnet for a couple of days with MAX_BLOCKS_ONLY_CONNECTIONS upped from 2 to 16 and didn't observe any unexpected behaviour: it disconnected a couple of peers that tried sending inv's, and it successfully did compact block relay with some block relay peers.
  TheBlueMatt:
    re-utACK 0ba0802. Pointed out that stats.fRelayTxes was sometimes uninitialized for blocksonly peers (though its not a big deal and only effects RPC), which has since been fixed here. Otherwise changes are pretty trivial so looks good.
  jnewbery:
    utACK 0ba0802
  jamesob:
    ACK bitcoin@0ba0802

Tree-SHA512: 4c3629434472c7dd4125253417b1be41967a508c3cfec8af5a34cad685464fbebbb6558f0f8f5c0d4463e3ffa4fa3aabd58247692cb9ab8395f4993078b9bcdf
  • Loading branch information
@fanquake @vijaydasmp
fanquake authored and vijaydasmp committed May 19, 2022
1 parent 53776b4 commit f0ce263
Show file tree
Hide file tree
Showing 6 changed files with 302 additions and 166 deletions.
3 changes: 2 additions & 1 deletion src/init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2447,7 +2447,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
CConnman::Options connOptions;
connOptions.nLocalServices = nLocalServices;
connOptions.nMaxConnections = nMaxConnections;
connOptions.nMaxOutbound = std::min(MAX_OUTBOUND_CONNECTIONS, connOptions.nMaxConnections);
connOptions.m_max_outbound_full_relay = std::min(MAX_OUTBOUND_FULL_RELAY_CONNECTIONS, connOptions.nMaxConnections);
connOptions.m_max_outbound_block_relay = std::min(MAX_BLOCKS_ONLY_CONNECTIONS, connOptions.nMaxConnections-connOptions.m_max_outbound_full_relay);
connOptions.nMaxAddnode = MAX_ADDNODE_CONNECTIONS;
connOptions.nMaxFeeler = 1;
connOptions.nBestHeight = chain_active_height;
Expand Down
68 changes: 46 additions & 22 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ static CAddress GetBindAddress(SOCKET sock)
return addr_bind;
}

CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection)
CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection, bool block_relay_only)
{
if (pszDest == nullptr) {
bool fAllowLocal = Params().AllowMultiplePorts() && addrConnect.GetPort() != GetListenPort();
Expand Down Expand Up @@ -478,7 +478,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
NodeId id = GetNewNodeId();
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
CAddress addr_bind = GetBindAddress(hSocket);
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false);
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", false, block_relay_only);
pnode->AddRef();
statsClient.inc("peers.connect", 1.0f);

Expand Down Expand Up @@ -561,10 +561,12 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
X(nServices);
X(addr);
X(addrBind);
stats.m_mapped_as = addr.GetMappedAS(m_asmap);
{
LOCK(cs_filter);
X(fRelayTxes);
stats.m_mapped_as = addr.GetMappedAS(m_asmap);
if (m_tx_relay != nullptr) {
LOCK(m_tx_relay->cs_filter);
stats.fRelayTxes = m_tx_relay->fRelayTxes;
} else {
stats.fRelayTxes = false;
}
X(nLastSend);
X(nLastRecv);
Expand Down Expand Up @@ -936,11 +938,17 @@ bool CConnman::AttemptToEvictConnection()
}
}

LOCK(node->cs_filter);
bool peer_relay_txes = false;
bool peer_filter_not_null = false;
if (node->m_tx_relay != nullptr) {
LOCK(node->m_tx_relay->cs_filter);
peer_relay_txes = node->m_tx_relay->fRelayTxes;
peer_filter_not_null = node->m_tx_relay->pfilter != nullptr;
}
NodeEvictionCandidate candidate = {node->GetId(), node->nTimeConnected, node->nMinPingUsecTime,
node->nLastBlockTime, node->nLastTXTime,
HasAllDesirableServiceFlags(node->nServices),
node->fRelayTxes, node->pfilter != nullptr, node->nKeyedNetGroup,
peer_relay_txes, peer_filter_not_null, node->nKeyedNetGroup,
node->m_prefer_evict};
vEvictionCandidates.push_back(candidate);
}
Expand Down Expand Up @@ -1014,7 +1022,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
CAddress addr;
int nInbound = 0;
int nVerifiedInboundMasternodes = 0;
int nMaxInbound = nMaxConnections - (nMaxOutbound + nMaxFeeler);
int nMaxInbound = nMaxConnections - m_max_outbound;

if (hSocket != INVALID_SOCKET) {
if (!addr.SetSockAddr((const struct sockaddr*)&sockaddr)) {
Expand Down Expand Up @@ -2060,7 +2068,7 @@ int CConnman::GetExtraOutboundCount()
}
}
}
return std::max(nOutbound - nMaxOutbound, 0);
return std::max(nOutbound - m_max_outbound_full_relay - m_max_outbound_block_relay, 0);
}

void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
Expand Down Expand Up @@ -2120,8 +2128,8 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
CAddress addrConnect;

// Only connect out to one peer per network group (/16 for IPv4).
// This is only done for mainnet and testnet
int nOutbound = 0;
int nOutboundFullRelay = 0;
int nOutboundBlockRelay = 0;
std::set<std::vector<unsigned char> > setConnected;
if (!Params().AllowMultipleAddressesFromGroup()) {
LOCK(cs_vNodes);
Expand All @@ -2133,7 +2141,11 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
// also have the added issue that they're attacker controlled and could be used
// to prevent us from connecting to particular hosts if we used them here.
setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap));
nOutbound++;
if (pnode->m_tx_relay == nullptr) {
nOutboundBlockRelay++;
} else if (!pnode->fFeeler) {
nOutboundFullRelay++;
}
}
}
}
Expand Down Expand Up @@ -2163,7 +2175,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
//
bool fFeeler = false;

if (nOutbound >= nMaxOutbound && !GetTryNewOutboundPeer()) {
if (nOutboundFullRelay >= m_max_outbound_full_relay && nOutboundBlockRelay >= m_max_outbound_block_relay && !GetTryNewOutboundPeer()) {
int64_t nTime = GetTimeMicros(); // The current time right now (in microseconds).
if (nTime > nNextFeeler) {
nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL);
Expand Down Expand Up @@ -2255,7 +2267,14 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
}
}

OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, false, fFeeler);
// Open this connection as block-relay-only if we're already at our
// full-relay capacity, but not yet at our block-relay peer limit.
// (It should not be possible for fFeeler to be set if we're not
// also at our block-relay peer limit, but check against that as
// well for sanity.)
bool block_relay_only = nOutboundBlockRelay < m_max_outbound_block_relay && !fFeeler && nOutboundFullRelay >= m_max_outbound_full_relay;

OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, false, fFeeler, false, block_relay_only);
}
}
}
Expand Down Expand Up @@ -2484,7 +2503,7 @@ void CConnman::ThreadOpenMasternodeConnections()
}

// if successful, this moves the passed grant to the constructed node
void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler, bool manual_connection, bool masternode_connection, bool masternode_probe_connection)
void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound, const char *pszDest, bool fOneShot, bool fFeeler, bool manual_connection, bool masternode_connection, bool masternode_probe_connection, bool block_relay_only)
{
//
// Initiate outbound network connection
Expand Down Expand Up @@ -2519,7 +2538,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
};

LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- connecting to %s\n", __func__, getIpStr());
CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, manual_connection);
CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, manual_connection, block_relay_only);

if (!pnode) {
LogPrint(BCLog::NET_NETCONN, "CConnman::%s -- ConnectNode failed for %s\n", __func__, getIpStr());
Expand Down Expand Up @@ -2894,7 +2913,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)

if (semOutbound == nullptr) {
// initialize semaphore
semOutbound = MakeUnique<CSemaphore>(std::min((nMaxOutbound + nMaxFeeler), nMaxConnections));
semOutbound = MakeUnique<CSemaphore>(std::min(m_max_outbound, nMaxConnections));
}
if (semAddnode == nullptr) {
// initialize semaphore
Expand Down Expand Up @@ -3024,7 +3043,7 @@ void CConnman::Interrupt()
InterruptSocks5(true);

if (semOutbound) {
for (int i=0; i<(nMaxOutbound + nMaxFeeler); i++) {
for (int i=0; i<m_max_outbound; i++) {
semOutbound->post();
}
}
Expand Down Expand Up @@ -3596,14 +3615,17 @@ int CConnman::GetBestHeight() const

unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }

CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, bool fInboundIn)
CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, bool fInboundIn, bool block_relay_only)
: nTimeConnected(GetSystemTimeInSeconds()),
addr(addrIn),
addrBind(addrBindIn),
fInbound(fInboundIn),
nKeyedNetGroup(nKeyedNetGroupIn),
addrKnown(5000, 0.001),
filterInventoryKnown(50000, 0.000001),
// Don't relay addr messages to peers that we connect to as block-relay-only
// peers (to prevent adversaries from inferring these links from addr
// traffic).
m_addr_relay_peer(!block_relay_only),
id(idIn),
nLocalHostNonce(nLocalHostNonceIn),
nLocalServices(nLocalServicesIn),
Expand All @@ -3612,7 +3634,9 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
hSocket = hSocketIn;
addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn;
hashContinue = uint256();
filterInventoryKnown.reset();
if (!block_relay_only) {
m_tx_relay = MakeUnique<TxRelay>();
}

for (const std::string &msg : getAllNetMessageTypes())
mapRecvBytesPerMsgCmd[msg] = 0;
Expand Down
Loading

0 comments on commit f0ce263

Please sign in to comment.