Skip to content

Commit

Permalink
net: Attempts to connect to all resolved addresses on addnode
Browse files Browse the repository at this point in the history 
Prior to this, addnode will only try to connect to a single randomized address
after IP resolution, resulting in potentially failing the connection if the picked
address is unreachable for whatever reason (e.g. being IPV6 while we do not support it).

This patches it by going over all resolved IPs until a valid one is found, or until we
exhaust them.
  • Loading branch information
@sr-gi
sr-gi committed Nov 15, 2023
1 parent b3898e9 commit 3bb2cd9
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 80 deletions.
175 changes: 95 additions & 80 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
// Resolve
const uint16_t default_port{pszDest != nullptr ? GetDefaultPort(pszDest) :
m_params.GetDefaultPort()};
std::vector<CAddress> resolvedAddresses{};
if (pszDest) {
std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)};
if (!resolved.empty()) {
Expand All @@ -436,8 +437,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());
return nullptr;
}
// Add the address to the resolved addresses vector so we can try to connect to it later on
resolvedAddresses.push_back(addrConnect);
}
}
} else {
// For resolution via proxy
resolvedAddresses.push_back(addrConnect);
}
} else {
// Connect via addrConnect directly
resolvedAddresses.push_back(addrConnect);
}

// Connect
Expand All @@ -448,102 +457,108 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
assert(!addr_bind.IsValid());
std::unique_ptr<i2p::sam::Session> i2p_transient_session;

if (addrConnect.IsValid()) {
const bool use_proxy{GetProxy(addrConnect.GetNetwork(), proxy)};
bool proxyConnectionFailed = false;
for (auto& addrConnect: resolvedAddresses) {
if (addrConnect.IsValid()) {
const bool use_proxy{GetProxy(addrConnect.GetNetwork(), proxy)};
bool proxyConnectionFailed = false;

if (addrConnect.IsI2P() && use_proxy) {
i2p::Connection conn;
if (addrConnect.IsI2P() && use_proxy) {
i2p::Connection conn;

if (m_i2p_sam_session) {
connected = m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed);
} else {
{
LOCK(m_unused_i2p_sessions_mutex);
if (m_unused_i2p_sessions.empty()) {
i2p_transient_session =
std::make_unique<i2p::sam::Session>(proxy.proxy, &interruptNet);
} else {
i2p_transient_session.swap(m_unused_i2p_sessions.front());
m_unused_i2p_sessions.pop();
if (m_i2p_sam_session) {
connected = m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed);
} else {
{
LOCK(m_unused_i2p_sessions_mutex);
if (m_unused_i2p_sessions.empty()) {
i2p_transient_session =
std::make_unique<i2p::sam::Session>(proxy.proxy, &interruptNet);
} else {
i2p_transient_session.swap(m_unused_i2p_sessions.front());
m_unused_i2p_sessions.pop();
}
}
}
connected = i2p_transient_session->Connect(addrConnect, conn, proxyConnectionFailed);
if (!connected) {
LOCK(m_unused_i2p_sessions_mutex);
if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) {
m_unused_i2p_sessions.emplace(i2p_transient_session.release());
connected = i2p_transient_session->Connect(addrConnect, conn, proxyConnectionFailed);
if (!connected) {
LOCK(m_unused_i2p_sessions_mutex);
if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) {
m_unused_i2p_sessions.emplace(i2p_transient_session.release());
}
}
}
}

if (connected) {
sock = std::move(conn.sock);
addr_bind = CAddress{conn.me, NODE_NONE};
if (connected) {
sock = std::move(conn.sock);
addr_bind = CAddress{conn.me, NODE_NONE};
}
} else if (use_proxy) {
sock = CreateSock(proxy.proxy);
if (!sock) {
return nullptr;
}
connected = ConnectThroughProxy(proxy, addrConnect.ToStringAddr(), addrConnect.GetPort(),
*sock, nConnectTimeout, proxyConnectionFailed);
} else {
// no proxy needed (none set for target network)
sock = CreateSock(addrConnect);
if (!sock) {
continue;
}
connected = ConnectSocketDirectly(addrConnect, *sock, nConnectTimeout,
conn_type == ConnectionType::MANUAL);
}
} else if (use_proxy) {
sock = CreateSock(proxy.proxy);
if (!sock) {
return nullptr;
if (!proxyConnectionFailed) {
// 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.
addrman.Attempt(addrConnect, fCountFailure);
}
connected = ConnectThroughProxy(proxy, addrConnect.ToStringAddr(), addrConnect.GetPort(),
*sock, nConnectTimeout, proxyConnectionFailed);
} else {
// no proxy needed (none set for target network)
sock = CreateSock(addrConnect);
} else if (pszDest && GetNameProxy(proxy)) {
sock = CreateSock(proxy.proxy);
if (!sock) {
return nullptr;
}
connected = ConnectSocketDirectly(addrConnect, *sock, nConnectTimeout,
conn_type == ConnectionType::MANUAL);
std::string host;
uint16_t port{default_port};
SplitHostPort(std::string(pszDest), port, host);
bool proxyConnectionFailed;
connected = ConnectThroughProxy(proxy, host, port, *sock, nConnectTimeout,
proxyConnectionFailed);
}
if (!proxyConnectionFailed) {
// 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.
addrman.Attempt(addrConnect, fCountFailure);

// Check any other resolved address (if any) if we fail to connect
if (!connected) {
continue;
}
} else if (pszDest && GetNameProxy(proxy)) {
sock = CreateSock(proxy.proxy);
if (!sock) {
return nullptr;

// Add node
NodeId id = GetNewNodeId();
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
if (!addr_bind.IsValid()) {
addr_bind = GetBindAddress(*sock);
}
std::string host;
uint16_t port{default_port};
SplitHostPort(std::string(pszDest), port, host);
bool proxyConnectionFailed;
connected = ConnectThroughProxy(proxy, host, port, *sock, nConnectTimeout,
proxyConnectionFailed);
}
if (!connected) {
return nullptr;
}
CNode* pnode = new CNode(id,
std::move(sock),
addrConnect,
CalculateKeyedNetGroup(addrConnect),
nonce,
addr_bind,
pszDest ? pszDest : "",
conn_type,
/*inbound_onion=*/false,
CNodeOptions{
.i2p_sam_session = std::move(i2p_transient_session),
.recv_flood_size = nReceiveFloodSize,
.use_v2transport = use_v2transport,
});
pnode->AddRef();

// Add node
NodeId id = GetNewNodeId();
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
if (!addr_bind.IsValid()) {
addr_bind = GetBindAddress(*sock);
}
CNode* pnode = new CNode(id,
std::move(sock),
addrConnect,
CalculateKeyedNetGroup(addrConnect),
nonce,
addr_bind,
pszDest ? pszDest : "",
conn_type,
/*inbound_onion=*/false,
CNodeOptions{
.i2p_sam_session = std::move(i2p_transient_session),
.recv_flood_size = nReceiveFloodSize,
.use_v2transport = use_v2transport,
});
pnode->AddRef();
// We're making a new connection, harvest entropy from the time (and our peer count)
RandAddEvent((uint32_t)id);

// We're making a new connection, harvest entropy from the time (and our peer count)
RandAddEvent((uint32_t)id);
return pnode;
}

return pnode;
return nullptr;
}

void CNode::CloseSocketDisconnect()
Expand Down
4 changes: 4 additions & 0 deletions test/functional/feature_proxy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def setup_nodes(self):
if self.have_ipv6:
args[3] = ['-listen', f'-proxy=[{self.conf3.addr[0]}]:{self.conf3.addr[1]}','-proxyrandomize=0', '-noonion']
self.add_nodes(self.num_nodes, extra_args=args)

self.start_nodes()

def network_test(self, node, addr, network):
Expand Down Expand Up @@ -187,7 +188,10 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
addr = "node.noumenon:8333"
self.log.debug(f"Test: outgoing DNS name connection through node for address {addr}")
node.addnode(addr, "onetry")
# FIXME: DEADLOCK IS HERE
self.log.debug(f"Test: DEADLOCK")
cmd = proxies[3].queue.get()

assert isinstance(cmd, Socks5Command)
assert_equal(cmd.atyp, AddressType.DOMAINNAME)
assert_equal(cmd.addr, b"node.noumenon")
Expand Down

0 comments on commit 3bb2cd9

Please sign in to comment.