From af0fca530e4d8311bcb24a14c416e5ad7c30ff78 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 13 Jul 2023 14:17:30 +0200 Subject: [PATCH] netbase: use reliable send() during SOCKS5 handshake `send(2)` can be interrupted or for another reason it may not fully complete sending all the bytes. We should be ready to retry the send with the remaining bytes. This is what `Sock::SendComplete()` does, thus use it in `Socks5()`. Since `Sock::SendComplete()` takes a `CThreadInterrupt` argument, change also the recv part of `Socks5()` to use `CThreadInterrupt` instead of a boolean. Easier reviewed with `git show -b` (ignore white-space changes). --- src/net.cpp | 3 +- src/netbase.cpp | 201 ++++++++++++++++++--------------------- src/netbase.h | 6 +- src/test/fuzz/socks5.cpp | 4 +- 4 files changed, 104 insertions(+), 110 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 994abd986d0..6dce17c6d03 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3260,7 +3260,6 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) // Start threads // assert(m_msgproc); - InterruptSocks5(false); interruptNet.reset(); flagInterruptMsgProc = false; @@ -3332,7 +3331,7 @@ void CConnman::Interrupt() condMsgProc.notify_all(); interruptNet(); - InterruptSocks5(true); + g_socks5_interrupt(); if (semOutbound) { for (int i=0; i interruptSocks5Recv(false); +CThreadInterrupt g_socks5_interrupt; std::vector WrappedGetAddrInfo(const std::string& name, bool allow_lookup) { @@ -269,7 +269,7 @@ enum class IntrRecvError { * IntrRecvError::OK only if all of the specified number of bytes were * read. * - * @see This function can be interrupted by calling InterruptSocks5(bool). + * @see This function can be interrupted by calling g_socks5_interrupt(). * Sockets can be made non-blocking with Sock::SetNonBlocking(). */ static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, std::chrono::milliseconds timeout, const Sock& sock) @@ -297,8 +297,9 @@ static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, std::chrono::m return IntrRecvError::NetworkError; } } - if (interruptSocks5Recv) + if (g_socks5_interrupt) { return IntrRecvError::Interrupted; + } curTime = Now(); } return len == 0 ? IntrRecvError::OK : IntrRecvError::Timeout; @@ -331,103 +332,93 @@ static std::string Socks5ErrorString(uint8_t err) bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* auth, const Sock& sock) { - IntrRecvError recvr; - LogPrint(BCLog::NET, "SOCKS5 connecting %s\n", strDest); - if (strDest.size() > 255) { - return error("Hostname too long"); - } - // Construct the version identifier/method selection message - std::vector vSocks5Init; - vSocks5Init.push_back(SOCKSVersion::SOCKS5); // We want the SOCK5 protocol - if (auth) { - vSocks5Init.push_back(0x02); // 2 method identifiers follow... - vSocks5Init.push_back(SOCKS5Method::NOAUTH); - vSocks5Init.push_back(SOCKS5Method::USER_PASS); - } else { - vSocks5Init.push_back(0x01); // 1 method identifier follows... - vSocks5Init.push_back(SOCKS5Method::NOAUTH); - } - ssize_t ret = sock.Send(vSocks5Init.data(), vSocks5Init.size(), MSG_NOSIGNAL); - if (ret != (ssize_t)vSocks5Init.size()) { - return error("Error sending to proxy"); - } - uint8_t pchRet1[2]; - if (InterruptibleRecv(pchRet1, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) { - LogPrintf("Socks5() connect to %s:%d failed: InterruptibleRecv() timeout or other failure\n", strDest, port); - return false; - } - if (pchRet1[0] != SOCKSVersion::SOCKS5) { - return error("Proxy failed to initialize"); - } - if (pchRet1[1] == SOCKS5Method::USER_PASS && auth) { - // Perform username/password authentication (as described in RFC1929) - std::vector vAuth; - vAuth.push_back(0x01); // Current (and only) version of user/pass subnegotiation - if (auth->username.size() > 255 || auth->password.size() > 255) - return error("Proxy username or password too long"); - vAuth.push_back(auth->username.size()); - vAuth.insert(vAuth.end(), auth->username.begin(), auth->username.end()); - vAuth.push_back(auth->password.size()); - vAuth.insert(vAuth.end(), auth->password.begin(), auth->password.end()); - ret = sock.Send(vAuth.data(), vAuth.size(), MSG_NOSIGNAL); - if (ret != (ssize_t)vAuth.size()) { - return error("Error sending authentication to proxy"); - } - LogPrint(BCLog::PROXY, "SOCKS5 sending proxy authentication %s:%s\n", auth->username, auth->password); - uint8_t pchRetA[2]; - if (InterruptibleRecv(pchRetA, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) { - return error("Error reading proxy authentication response"); + try { + IntrRecvError recvr; + LogPrint(BCLog::NET, "SOCKS5 connecting %s\n", strDest); + if (strDest.size() > 255) { + return error("Hostname too long"); } - if (pchRetA[0] != 0x01 || pchRetA[1] != 0x00) { - return error("Proxy authentication unsuccessful"); + // Construct the version identifier/method selection message + std::vector vSocks5Init; + vSocks5Init.push_back(SOCKSVersion::SOCKS5); // We want the SOCK5 protocol + if (auth) { + vSocks5Init.push_back(0x02); // 2 method identifiers follow... + vSocks5Init.push_back(SOCKS5Method::NOAUTH); + vSocks5Init.push_back(SOCKS5Method::USER_PASS); + } else { + vSocks5Init.push_back(0x01); // 1 method identifier follows... + vSocks5Init.push_back(SOCKS5Method::NOAUTH); } - } else if (pchRet1[1] == SOCKS5Method::NOAUTH) { - // Perform no authentication - } else { - return error("Proxy requested wrong authentication method %02x", pchRet1[1]); - } - std::vector vSocks5; - vSocks5.push_back(SOCKSVersion::SOCKS5); // VER protocol version - vSocks5.push_back(SOCKS5Command::CONNECT); // CMD CONNECT - vSocks5.push_back(0x00); // RSV Reserved must be 0 - vSocks5.push_back(SOCKS5Atyp::DOMAINNAME); // ATYP DOMAINNAME - vSocks5.push_back(strDest.size()); // Length<=255 is checked at beginning of function - vSocks5.insert(vSocks5.end(), strDest.begin(), strDest.end()); - vSocks5.push_back((port >> 8) & 0xFF); - vSocks5.push_back((port >> 0) & 0xFF); - ret = sock.Send(vSocks5.data(), vSocks5.size(), MSG_NOSIGNAL); - if (ret != (ssize_t)vSocks5.size()) { - return error("Error sending to proxy"); - } - uint8_t pchRet2[4]; - if ((recvr = InterruptibleRecv(pchRet2, 4, g_socks5_recv_timeout, sock)) != IntrRecvError::OK) { - if (recvr == IntrRecvError::Timeout) { - /* If a timeout happens here, this effectively means we timed out while connecting - * to the remote node. This is very common for Tor, so do not print an - * error message. */ + sock.SendComplete(vSocks5Init, g_socks5_recv_timeout, g_socks5_interrupt); + uint8_t pchRet1[2]; + if (InterruptibleRecv(pchRet1, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) { + LogPrintf("Socks5() connect to %s:%d failed: InterruptibleRecv() timeout or other failure\n", strDest, port); return false; + } + if (pchRet1[0] != SOCKSVersion::SOCKS5) { + return error("Proxy failed to initialize"); + } + if (pchRet1[1] == SOCKS5Method::USER_PASS && auth) { + // Perform username/password authentication (as described in RFC1929) + std::vector vAuth; + vAuth.push_back(0x01); // Current (and only) version of user/pass subnegotiation + if (auth->username.size() > 255 || auth->password.size() > 255) + return error("Proxy username or password too long"); + vAuth.push_back(auth->username.size()); + vAuth.insert(vAuth.end(), auth->username.begin(), auth->username.end()); + vAuth.push_back(auth->password.size()); + vAuth.insert(vAuth.end(), auth->password.begin(), auth->password.end()); + sock.SendComplete(vAuth, g_socks5_recv_timeout, g_socks5_interrupt); + LogPrint(BCLog::PROXY, "SOCKS5 sending proxy authentication %s:%s\n", auth->username, auth->password); + uint8_t pchRetA[2]; + if (InterruptibleRecv(pchRetA, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) { + return error("Error reading proxy authentication response"); + } + if (pchRetA[0] != 0x01 || pchRetA[1] != 0x00) { + return error("Proxy authentication unsuccessful"); + } + } else if (pchRet1[1] == SOCKS5Method::NOAUTH) { + // Perform no authentication } else { - return error("Error while reading proxy response"); + return error("Proxy requested wrong authentication method %02x", pchRet1[1]); } - } - if (pchRet2[0] != SOCKSVersion::SOCKS5) { - return error("Proxy failed to accept request"); - } - if (pchRet2[1] != SOCKS5Reply::SUCCEEDED) { - // Failures to connect to a peer that are not proxy errors - LogPrintf("Socks5() connect to %s:%d failed: %s\n", strDest, port, Socks5ErrorString(pchRet2[1])); - return false; - } - if (pchRet2[2] != 0x00) { // Reserved field must be 0 - return error("Error: malformed proxy response"); - } - uint8_t pchRet3[256]; - switch (pchRet2[3]) - { + std::vector vSocks5; + vSocks5.push_back(SOCKSVersion::SOCKS5); // VER protocol version + vSocks5.push_back(SOCKS5Command::CONNECT); // CMD CONNECT + vSocks5.push_back(0x00); // RSV Reserved must be 0 + vSocks5.push_back(SOCKS5Atyp::DOMAINNAME); // ATYP DOMAINNAME + vSocks5.push_back(strDest.size()); // Length<=255 is checked at beginning of function + vSocks5.insert(vSocks5.end(), strDest.begin(), strDest.end()); + vSocks5.push_back((port >> 8) & 0xFF); + vSocks5.push_back((port >> 0) & 0xFF); + sock.SendComplete(vSocks5, g_socks5_recv_timeout, g_socks5_interrupt); + uint8_t pchRet2[4]; + if ((recvr = InterruptibleRecv(pchRet2, 4, g_socks5_recv_timeout, sock)) != IntrRecvError::OK) { + if (recvr == IntrRecvError::Timeout) { + /* If a timeout happens here, this effectively means we timed out while connecting + * to the remote node. This is very common for Tor, so do not print an + * error message. */ + return false; + } else { + return error("Error while reading proxy response"); + } + } + if (pchRet2[0] != SOCKSVersion::SOCKS5) { + return error("Proxy failed to accept request"); + } + if (pchRet2[1] != SOCKS5Reply::SUCCEEDED) { + // Failures to connect to a peer that are not proxy errors + LogPrintf("Socks5() connect to %s:%d failed: %s\n", strDest, port, Socks5ErrorString(pchRet2[1])); + return false; + } + if (pchRet2[2] != 0x00) { // Reserved field must be 0 + return error("Error: malformed proxy response"); + } + uint8_t pchRet3[256]; + switch (pchRet2[3]) { case SOCKS5Atyp::IPV4: recvr = InterruptibleRecv(pchRet3, 4, g_socks5_recv_timeout, sock); break; case SOCKS5Atyp::IPV6: recvr = InterruptibleRecv(pchRet3, 16, g_socks5_recv_timeout, sock); break; - case SOCKS5Atyp::DOMAINNAME: - { + case SOCKS5Atyp::DOMAINNAME: { recvr = InterruptibleRecv(pchRet3, 1, g_socks5_recv_timeout, sock); if (recvr != IntrRecvError::OK) { return error("Error reading from proxy"); @@ -437,15 +428,18 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a break; } default: return error("Error: malformed proxy response"); + } + if (recvr != IntrRecvError::OK) { + return error("Error reading from proxy"); + } + if (InterruptibleRecv(pchRet3, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) { + return error("Error reading from proxy"); + } + LogPrint(BCLog::NET, "SOCKS5 connected %s\n", strDest); + return true; + } catch (const std::runtime_error& e) { + return error("Error during SOCKS5 proxy handshake: %s", e.what()); } - if (recvr != IntrRecvError::OK) { - return error("Error reading from proxy"); - } - if (InterruptibleRecv(pchRet3, 2, g_socks5_recv_timeout, sock) != IntrRecvError::OK) { - return error("Error reading from proxy"); - } - LogPrint(BCLog::NET, "SOCKS5 connected %s\n", strDest); - return true; } std::unique_ptr CreateSockTCP(const CService& address_family) @@ -678,11 +672,6 @@ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out) return false; } -void InterruptSocks5(bool interrupt) -{ - interruptSocks5Recv = interrupt; -} - bool IsBadPort(uint16_t port) { /* Don't forget to update doc/p2p-bad-ports.md if you change this list. */ diff --git a/src/netbase.h b/src/netbase.h index 1da4f5c51de..985d10f3842 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -220,7 +221,10 @@ bool ConnectSocketDirectly(const CService &addrConnect, const Sock& sock, int nT */ bool ConnectThroughProxy(const Proxy& proxy, const std::string& strDest, uint16_t port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed); -void InterruptSocks5(bool interrupt); +/** + * Interrupt SOCKS5 reads or writes. + */ +extern CThreadInterrupt g_socks5_interrupt; /** * Connect to a specified destination service through an already connected diff --git a/src/test/fuzz/socks5.cpp b/src/test/fuzz/socks5.cpp index 05b8312ab2a..af81fcb593c 100644 --- a/src/test/fuzz/socks5.cpp +++ b/src/test/fuzz/socks5.cpp @@ -32,7 +32,9 @@ FUZZ_TARGET(socks5, .init = initialize_socks5) ProxyCredentials proxy_credentials; proxy_credentials.username = fuzzed_data_provider.ConsumeRandomLengthString(512); proxy_credentials.password = fuzzed_data_provider.ConsumeRandomLengthString(512); - InterruptSocks5(fuzzed_data_provider.ConsumeBool()); + if (fuzzed_data_provider.ConsumeBool()) { + g_socks5_interrupt(); + } // Set FUZZED_SOCKET_FAKE_LATENCY=1 to exercise recv timeout code paths. This // will slow down fuzzing. g_socks5_recv_timeout = (fuzzed_data_provider.ConsumeBool() && std::getenv("FUZZED_SOCKET_FAKE_LATENCY") != nullptr) ? 1ms : default_socks5_recv_timeout;