mirror/bitcoin
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#28834: net: attempts to connect to all resolved addresses when connecting to a node

Browse Source 
fd81a37239 net: attempts to connect to all resolved addresses when connecting to a node (Sergi Delgado Segura)

Pull request description:

  This is a follow-up of #28155 motivated by https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1362677038

  ## Rationale

  Prior to this, when establishing a network connection via `CConnman::ConnectNode`, if the connection needed address resolution, a single address would be picked at random from the resolved addresses and our node would try to connect to it. However, this would lead to the behavior of `ConnectNode` being unpredictable when the address was resolved to various ips (e.g. the address resolving to IPv4 and IPv6, but we only support one of them).

  This patches the aforementioned behavior by going over all resolved IPs until a valid one is found or until we
  exhaust them.

ACKs for top commit:
  mzumsande:
    re-ACK fd81a37239 (just looked at diff, only small logging change)
  achow101:
    ACK fd81a37239
  vasild:
    ACK fd81a37239

Tree-SHA512: fa1ebc5c84fe61dd0a7fe1113ae2d594a75ad661c43ed8984a31fc9bc50f166b2759b0d8d84ee5dc247691eff78c8156fac970af797bbcbf67492eec0353fb58
This commit is contained in:
Ava Chow committed 2024-04-25 16:08:24 -04:00
2 parents 0e2e7d1a35 fd81a37239
commit 2eff198f49
1 file changed
+29 -13
Download Patch File Download Diff File Expand all files Collapse all files
src/net.cpp
+29 -13
View File
@@ -412,6 +412,9 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
// Resolve // Resolve
const uint16_t default_port{pszDest != nullptr ? GetDefaultPort(pszDest) : const uint16_t default_port{pszDest != nullptr ? GetDefaultPort(pszDest) :
m_params.GetDefaultPort()}; m_params.GetDefaultPort()};
// Collection of addresses to try to connect to: either all dns resolved addresses if a domain name (pszDest) is provided, or addrConnect otherwise.
std::vector<CAddress> connect_to{};
if (pszDest) { if (pszDest) {
std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)}; std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)};
if (!resolved.empty()) { if (!resolved.empty()) {
@@ -432,8 +435,16 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort()); LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort());
return nullptr; return nullptr;
} }
// Add the address to the resolved addresses vector so we can try to connect to it later on
connect_to.push_back(addrConnect);
} }
} else {
// For resolution via proxy
connect_to.push_back(addrConnect);
} }
} else {
// Connect via addrConnect directly
connect_to.push_back(addrConnect);
} }
// Connect // Connect
@@ -443,16 +454,17 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
assert(!addr_bind.IsValid()); assert(!addr_bind.IsValid());
std::unique_ptr<i2p::sam::Session> i2p_transient_session; std::unique_ptr<i2p::sam::Session> i2p_transient_session;
if (addrConnect.IsValid()) { for (auto& target_addr: connect_to) {
const bool use_proxy{GetProxy(addrConnect.GetNetwork(), proxy)}; if (target_addr.IsValid()) {
const bool use_proxy{GetProxy(target_addr.GetNetwork(), proxy)};
bool proxyConnectionFailed = false; bool proxyConnectionFailed = false;
if (addrConnect.IsI2P() && use_proxy) { if (target_addr.IsI2P() && use_proxy) {
i2p::Connection conn; i2p::Connection conn;
bool connected{false}; bool connected{false};
if (m_i2p_sam_session) { if (m_i2p_sam_session) {
connected = m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed); connected = m_i2p_sam_session->Connect(target_addr, conn, proxyConnectionFailed);
} else { } else {
{ {
LOCK(m_unused_i2p_sessions_mutex); LOCK(m_unused_i2p_sessions_mutex);
@@ -464,7 +476,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
m_unused_i2p_sessions.pop(); m_unused_i2p_sessions.pop();
} }
} }
connected = i2p_transient_session->Connect(addrConnect, conn, proxyConnectionFailed); connected = i2p_transient_session->Connect(target_addr, conn, proxyConnectionFailed);
if (!connected) { if (!connected) {
LOCK(m_unused_i2p_sessions_mutex); LOCK(m_unused_i2p_sessions_mutex);
if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) { if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) {
@@ -478,16 +490,16 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
addr_bind = CAddress{conn.me, NODE_NONE}; addr_bind = CAddress{conn.me, NODE_NONE};
} }
} else if (use_proxy) { } else if (use_proxy) {
LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s:%s\n", proxy.ToString(), addrConnect.ToStringAddr(), addrConnect.GetPort()); LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s\n", proxy.ToString(), target_addr.ToStringAddrPort());
sock = ConnectThroughProxy(proxy, addrConnect.ToStringAddr(), addrConnect.GetPort(), proxyConnectionFailed); sock = ConnectThroughProxy(proxy, target_addr.ToStringAddr(), target_addr.GetPort(), proxyConnectionFailed);
} else { } else {
// no proxy needed (none set for target network) // no proxy needed (none set for target network)
sock = ConnectDirectly(addrConnect, conn_type == ConnectionType::MANUAL); sock = ConnectDirectly(target_addr, conn_type == ConnectionType::MANUAL);
} }
if (!proxyConnectionFailed) { if (!proxyConnectionFailed) {
// If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to
// the proxy, mark this as an attempt. // the proxy, mark this as an attempt.
addrman.Attempt(addrConnect, fCountFailure); addrman.Attempt(target_addr, fCountFailure);
} }
} else if (pszDest && GetNameProxy(proxy)) { } else if (pszDest && GetNameProxy(proxy)) {
std::string host; std::string host;
@@ -496,13 +508,14 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
bool proxyConnectionFailed; bool proxyConnectionFailed;
sock = ConnectThroughProxy(proxy, host, port, proxyConnectionFailed); sock = ConnectThroughProxy(proxy, host, port, proxyConnectionFailed);
} }
// Check any other resolved address (if any) if we fail to connect
if (!sock) { if (!sock) {
return nullptr; continue;
} }
NetPermissionFlags permission_flags = NetPermissionFlags::None; NetPermissionFlags permission_flags = NetPermissionFlags::None;
std::vector<NetWhitelistPermissions> whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector<NetWhitelistPermissions>{}; std::vector<NetWhitelistPermissions> whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector<NetWhitelistPermissions>{};
AddWhitelistPermissionFlags(permission_flags, addrConnect, whitelist_permissions); AddWhitelistPermissionFlags(permission_flags, target_addr, whitelist_permissions);
// Add node // Add node
NodeId id = GetNewNodeId(); NodeId id = GetNewNodeId();
@@ -512,8 +525,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
} }
CNode* pnode = new CNode(id, CNode* pnode = new CNode(id,
std::move(sock), std::move(sock),
addrConnect, target_addr,
CalculateKeyedNetGroup(addrConnect), CalculateKeyedNetGroup(target_addr),
nonce, nonce,
addr_bind, addr_bind,
pszDest ? pszDest : "", pszDest ? pszDest : "",
@@ -533,6 +546,9 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
return pnode; return pnode;
} }
return nullptr;
}
void CNode::CloseSocketDisconnect() void CNode::CloseSocketDisconnect()
{ {
fDisconnect = true; fDisconnect = true;