Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net: Split socket create/connect #11363

Merged
merged 4 commits into from
Dec 13, 2017
Merged

net: Split socket create/connect #11363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1729c29
net: split socket creation out of connection
theuni Sep 18, 2017
9e3b2f5
net: Move IsSelectableSocket check into socket creation
theuni Oct 2, 2017
df3bcf8
net: pass socket closing responsibility up to caller for outgoing con…
theuni Oct 2, 2017
3830b6e
net: use CreateSocket for binds
theuni Oct 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 28 additions & 42 deletions src/net.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,39 +417,48 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
if (addrConnect.IsValid()) {
bool proxyConnectionFailed = false;

if (GetProxy(addrConnect.GetNetwork(), proxy))
if (GetProxy(addrConnect.GetNetwork(), proxy)) {
hSocket = CreateSocket(proxy.proxy);
if (hSocket == INVALID_SOCKET) {
return nullptr;
}
connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, &proxyConnectionFailed);
else // no proxy needed (none set for target network)
} else {
// no proxy needed (none set for target network)
hSocket = CreateSocket(addrConnect);
if (hSocket == INVALID_SOCKET) {
return nullptr;
}
connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout);
}
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);
}
} else if (pszDest && GetNameProxy(proxy)) {
hSocket = CreateSocket(proxy.proxy);
if (hSocket == INVALID_SOCKET) {
return nullptr;
}
std::string host;
int port = default_port;
SplitHostPort(std::string(pszDest), port, host);
connected = ConnectThroughProxy(proxy, host, port, hSocket, nConnectTimeout, nullptr);
}
if (connected) {
if (!IsSelectableSocket(hSocket)) {
LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
CloseSocket(hSocket);
return nullptr;
}

// Add node
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);
pnode->AddRef();

return pnode;
if (!connected) {
CloseSocket(hSocket);
return nullptr;
}

return nullptr;
// Add node
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);
pnode->AddRef();

return pnode;
}

void CConnman::DumpBanlist()
Expand Down Expand Up @@ -2069,44 +2078,21 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b
return false;
}

SOCKET hListenSocket = socket(((struct sockaddr*)&sockaddr)->sa_family, SOCK_STREAM, IPPROTO_TCP);
SOCKET hListenSocket = CreateSocket(addrBind);
if (hListenSocket == INVALID_SOCKET)
{
strError = strprintf("Error: Couldn't open socket for incoming connections (socket returned error %s)", NetworkErrorString(WSAGetLastError()));
LogPrintf("%s\n", strError);
return false;
}
if (!IsSelectableSocket(hListenSocket))
{
strError = "Error: Couldn't create a listenable socket for incoming connections";
LogPrintf("%s\n", strError);
return false;
}


#ifndef WIN32
#ifdef SO_NOSIGPIPE
// Different way of disabling SIGPIPE on BSD
setsockopt(hListenSocket, SOL_SOCKET, SO_NOSIGPIPE, (void*)&nOne, sizeof(int));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOSIGPIPE handling moved to CreateSocket(addrBind)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think part of the goal is to get random socket flag-fiddling things out of net and into netbase, its kinda split between both right now. @theuni ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. If it's something we require for all sockets, imo it makes sense to do it in CreateSocket.

#endif
// Allow binding if the port is still in TIME_WAIT state after
// the program was closed and restarted.
setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (void*)&nOne, sizeof(int));
// Disable Nagle's algorithm
setsockopt(hListenSocket, IPPROTO_TCP, TCP_NODELAY, (void*)&nOne, sizeof(int));
#else
setsockopt(hListenSocket, SOL_SOCKET, SO_REUSEADDR, (const char*)&nOne, sizeof(int));
setsockopt(hListenSocket, IPPROTO_TCP, TCP_NODELAY, (const char*)&nOne, sizeof(int));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did this move? The only remaining reference to TCP_NODELAY I see is in SetSocketNoDelay, which is called on accepted sockets only.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its also called in netbase.cpp in CreateSocket, so should be set for outbound and inbound connection sockets, though not the listen socket, but that makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's set on the listen socket as well (which should pass it down to accepted sockets anyway), since BindListenPort now uses CreateSocket().

#endif

// Set to non-blocking, incoming connections will also inherit this
if (!SetSocketNonBlocking(hListenSocket, true)) {
CloseSocket(hListenSocket);
strError = strprintf("BindListenPort: Setting listening socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError()));
LogPrintf("%s\n", strError);
return false;
}

// some systems don't have IPV6_V6ONLY but are always v6only; others do have the option
// and enable it by default or not. Try to enable it, if possible.
if (addrBind.IsIPv6()) {
Expand Down
70 changes: 32 additions & 38 deletions src/netbase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,11 @@ std::string Socks5ErrorString(uint8_t err)
}

/** Connect using SOCKS5 (as described in RFC1928) */
static bool Socks5(const std::string& strDest, int port, const ProxyCredentials *auth, SOCKET& hSocket)
static bool Socks5(const std::string& strDest, int port, const ProxyCredentials *auth, const SOCKET& hSocket)
{
IntrRecvError recvr;
LogPrint(BCLog::NET, "SOCKS5 connecting %s\n", strDest);
if (strDest.size() > 255) {
CloseSocket(hSocket);
return error("Hostname too long");
}
// Accepted authentication methods
Expand All @@ -338,17 +337,14 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
}
ssize_t ret = send(hSocket, (const char*)vSocks5Init.data(), vSocks5Init.size(), MSG_NOSIGNAL);
if (ret != (ssize_t)vSocks5Init.size()) {
CloseSocket(hSocket);
return error("Error sending to proxy");
}
uint8_t pchRet1[2];
if ((recvr = InterruptibleRecv(pchRet1, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) {
CloseSocket(hSocket);
LogPrintf("Socks5() connect to %s:%d failed: InterruptibleRecv() timeout or other failure\n", strDest, port);
return false;
}
if (pchRet1[0] != SOCKSVersion::SOCKS5) {
CloseSocket(hSocket);
return error("Proxy failed to initialize");
}
if (pchRet1[1] == SOCKS5Method::USER_PASS && auth) {
Expand All @@ -363,23 +359,19 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
vAuth.insert(vAuth.end(), auth->password.begin(), auth->password.end());
ret = send(hSocket, (const char*)vAuth.data(), vAuth.size(), MSG_NOSIGNAL);
if (ret != (ssize_t)vAuth.size()) {
CloseSocket(hSocket);
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 ((recvr = InterruptibleRecv(pchRetA, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) {
CloseSocket(hSocket);
return error("Error reading proxy authentication response");
}
if (pchRetA[0] != 0x01 || pchRetA[1] != 0x00) {
CloseSocket(hSocket);
return error("Proxy authentication unsuccessful");
}
} else if (pchRet1[1] == SOCKS5Method::NOAUTH) {
// Perform no authentication
} else {
CloseSocket(hSocket);
return error("Proxy requested wrong authentication method %02x", pchRet1[1]);
}
std::vector<uint8_t> vSocks5;
Expand All @@ -393,12 +385,10 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
vSocks5.push_back((port >> 0) & 0xFF);
ret = send(hSocket, (const char*)vSocks5.data(), vSocks5.size(), MSG_NOSIGNAL);
if (ret != (ssize_t)vSocks5.size()) {
CloseSocket(hSocket);
return error("Error sending to proxy");
}
uint8_t pchRet2[4];
if ((recvr = InterruptibleRecv(pchRet2, 4, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) {
CloseSocket(hSocket);
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
Expand All @@ -409,17 +399,14 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
}
}
if (pchRet2[0] != SOCKSVersion::SOCKS5) {
CloseSocket(hSocket);
return error("Proxy failed to accept request");
}
if (pchRet2[1] != SOCKS5Reply::SUCCEEDED) {
// Failures to connect to a peer that are not proxy errors
CloseSocket(hSocket);
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
CloseSocket(hSocket);
return error("Error: malformed proxy response");
}
uint8_t pchRet3[256];
Expand All @@ -431,41 +418,42 @@ static bool Socks5(const std::string& strDest, int port, const ProxyCredentials
{
recvr = InterruptibleRecv(pchRet3, 1, SOCKS5_RECV_TIMEOUT, hSocket);
if (recvr != IntrRecvError::OK) {
CloseSocket(hSocket);
return error("Error reading from proxy");
}
int nRecv = pchRet3[0];
recvr = InterruptibleRecv(pchRet3, nRecv, SOCKS5_RECV_TIMEOUT, hSocket);
break;
}
default: CloseSocket(hSocket); return error("Error: malformed proxy response");
default: return error("Error: malformed proxy response");
}
if (recvr != IntrRecvError::OK) {
CloseSocket(hSocket);
return error("Error reading from proxy");
}
if ((recvr = InterruptibleRecv(pchRet3, 2, SOCKS5_RECV_TIMEOUT, hSocket)) != IntrRecvError::OK) {
CloseSocket(hSocket);
return error("Error reading from proxy");
}
LogPrint(BCLog::NET, "SOCKS5 connected %s\n", strDest);
return true;
}

bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout)
SOCKET CreateSocket(const CService &addrConnect)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to change the return values values of all the "return false"s in here - INVALID_SOCKET is (currently) ~0, so we will pretend the socket creation succeeded.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, don't know how I missed those.

{
hSocketRet = INVALID_SOCKET;

struct sockaddr_storage sockaddr;
socklen_t len = sizeof(sockaddr);
if (!addrConnect.GetSockAddr((struct sockaddr*)&sockaddr, &len)) {
LogPrintf("Cannot connect to %s: unsupported network\n", addrConnect.ToString());
return false;
LogPrintf("Cannot create socket for %s: unsupported network\n", addrConnect.ToString());
return INVALID_SOCKET;
}

SOCKET hSocket = socket(((struct sockaddr*)&sockaddr)->sa_family, SOCK_STREAM, IPPROTO_TCP);
if (hSocket == INVALID_SOCKET)
return false;
return INVALID_SOCKET;

if (!IsSelectableSocket(hSocket)) {
CloseSocket(hSocket);
LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
return INVALID_SOCKET;
}

#ifdef SO_NOSIGPIPE
int set = 1;
Expand All @@ -479,9 +467,23 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int
// Set to non-blocking
if (!SetSocketNonBlocking(hSocket, true)) {
CloseSocket(hSocket);
return error("ConnectSocketDirectly: Setting socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError()));
LogPrintf("ConnectSocketDirectly: Setting socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError()));
}
return hSocket;
}

bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout)
{
struct sockaddr_storage sockaddr;
socklen_t len = sizeof(sockaddr);
if (hSocket == INVALID_SOCKET) {
LogPrintf("Cannot connect to %s: invalid socket\n", addrConnect.ToString());
return false;
}
if (!addrConnect.GetSockAddr((struct sockaddr*)&sockaddr, &len)) {
LogPrintf("Cannot connect to %s: unsupported network\n", addrConnect.ToString());
return false;
}
if (connect(hSocket, (struct sockaddr*)&sockaddr, len) == SOCKET_ERROR)
{
int nErr = WSAGetLastError();
Expand All @@ -496,13 +498,11 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int
if (nRet == 0)
{
LogPrint(BCLog::NET, "connection to %s timeout\n", addrConnect.ToString());
CloseSocket(hSocket);
return false;
}
if (nRet == SOCKET_ERROR)
{
LogPrintf("select() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError()));
CloseSocket(hSocket);
return false;
}
socklen_t nRetSize = sizeof(nRet);
Expand All @@ -513,13 +513,11 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int
#endif
{
LogPrintf("getsockopt() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError()));
CloseSocket(hSocket);
return false;
}
if (nRet != 0)
{
LogPrintf("connect() to %s failed after select(): %s\n", addrConnect.ToString(), NetworkErrorString(nRet));
CloseSocket(hSocket);
return false;
}
}
Expand All @@ -530,12 +528,9 @@ bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int
#endif
{
LogPrintf("connect() to %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError()));
CloseSocket(hSocket);
return false;
}
}

hSocketRet = hSocket;
return true;
}

Expand Down Expand Up @@ -587,9 +582,8 @@ bool IsProxy(const CNetAddr &addr) {
return false;
}

bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed)
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocket, int nTimeout, bool *outProxyConnectionFailed)
{
SOCKET hSocket = INVALID_SOCKET;
// first connect to proxy server
if (!ConnectSocketDirectly(proxy.proxy, hSocket, nTimeout)) {
if (outProxyConnectionFailed)
Expand All @@ -601,14 +595,14 @@ bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int
ProxyCredentials random_auth;
static std::atomic_int counter(0);
random_auth.username = random_auth.password = strprintf("%i", counter++);
if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket))
if (!Socks5(strDest, (unsigned short)port, &random_auth, hSocket)) {
return false;
}
} else {
if (!Socks5(strDest, (unsigned short)port, 0, hSocket))
if (!Socks5(strDest, (unsigned short)port, 0, hSocket)) {
return false;
}
}

hSocketRet = hSocket;
return true;
}
bool LookupSubNet(const char* pszName, CSubNet& ret)
Expand Down
5 changes: 3 additions & 2 deletions src/netbase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLoo
bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);
CService LookupNumeric(const char *pszName, int portDefault = 0);
bool LookupSubNet(const char *pszName, CSubNet& subnet);
bool ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRet, int nTimeout);
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed);
SOCKET CreateSocket(const CService &addrConnect);
bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocketRet, int nTimeout);
bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed);
/** Return readable error string for a network error code */
std::string NetworkErrorString(int err);
/** Close socket and set hSocket to INVALID_SOCKET */
Expand Down