diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/Makefile.am | 1 | ||||
| -rw-r--r-- | src/Makefile.test.include | 1 | ||||
| -rw-r--r-- | src/init.cpp | 29 | ||||
| -rw-r--r-- | src/main.cpp | 1 | ||||
| -rw-r--r-- | src/net.cpp | 276 | ||||
| -rw-r--r-- | src/net.h | 13 | ||||
| -rw-r--r-- | src/reverselock.h | 31 | ||||
| -rw-r--r-- | src/rpcmining.cpp | 2 | ||||
| -rw-r--r-- | src/scheduler.cpp | 5 | ||||
| -rw-r--r-- | src/test/reverselock_tests.cpp | 64 |
10 files changed, 321 insertions, 102 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 4dfd56626c..390e9f1436 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -121,6 +121,7 @@ BITCOIN_CORE_H = \ protocol.h \ pubkey.h \ random.h \ + reverselock.h \ rpcclient.h \ rpcprotocol.h \ rpcserver.h \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index fc4e047c35..cc60cd92bb 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -62,6 +62,7 @@ BITCOIN_TESTS =\ test/pmt_tests.cpp \ test/policyestimator_tests.cpp \ test/pow_tests.cpp \ + test/reverselock_tests.cpp \ test/rpc_tests.cpp \ test/sanity_tests.cpp \ test/scheduler_tests.cpp \ diff --git a/src/init.cpp b/src/init.cpp index 4cdbd5edaa..5759b4b428 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -350,7 +350,6 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-whitebind=<addr>", _("Bind to given address and whitelist peers connecting to it. Use [host]:port notation for IPv6")); strUsage += HelpMessageOpt("-whitelist=<netmask>", _("Whitelist peers connecting from the given netmask or IP address. Can be specified multiple times.") + " " + _("Whitelisted peers cannot be DoS banned and their transactions are always relayed, even if they are already in the mempool, useful e.g. for a gateway")); - strUsage += HelpMessageOpt("-whiteconnections=<n>", strprintf(_("Reserve this many inbound connections for whitelisted peers (default: %d)"), 0)); #ifdef ENABLE_WALLET strUsage += HelpMessageGroup(_("Wallet options:")); @@ -778,25 +777,6 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) int nBind = std::max((int)mapArgs.count("-bind") + (int)mapArgs.count("-whitebind"), 1); int nUserMaxConnections = GetArg("-maxconnections", DEFAULT_MAX_PEER_CONNECTIONS); nMaxConnections = std::max(nUserMaxConnections, 0); - int nUserWhiteConnections = GetArg("-whiteconnections", 0); - nWhiteConnections = std::max(nUserWhiteConnections, 0); - - if ((mapArgs.count("-whitelist")) || (mapArgs.count("-whitebind"))) { - if (!(mapArgs.count("-maxconnections"))) { - // User is using whitelist feature, - // but did not specify -maxconnections parameter. - // Silently increase the default to compensate, - // so that the whitelist connection reservation feature - // does not inadvertently reduce the default - // inbound connection capacity of the network. - nMaxConnections += nWhiteConnections; - } - } else { - // User not using whitelist feature. - // Silently disable connection reservation, - // for the same reason as above. - nWhiteConnections = 0; - } // Trim requested connection counts, to fit into system limitations nMaxConnections = std::max(std::min(nMaxConnections, (int)(FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS)), 0); @@ -808,13 +788,6 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (nMaxConnections < nUserMaxConnections) InitWarning(strprintf(_("Reducing -maxconnections from %d to %d, because of system limitations."), nUserMaxConnections, nMaxConnections)); - // Connection capacity is prioritized in this order: - // outbound connections (hardcoded to 8), - // then whitelisted connections, - // then non-whitelisted connections get whatever's left (if any). - if ((nWhiteConnections > 0) && (nWhiteConnections >= (nMaxConnections - 8))) - InitWarning(strprintf(_("All non-whitelisted incoming connections will be dropped, because -whiteconnections is %d and -maxconnections is only %d."), nWhiteConnections, nMaxConnections)); - // ********************************************************* Step 3: parameter-to-internal-flags fDebug = !mapMultiArgs["-debug"].empty(); @@ -992,8 +965,6 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) LogPrintf("Using data directory %s\n", strDataDir); LogPrintf("Using config file %s\n", GetConfigFile().string()); LogPrintf("Using at most %i connections (%i file descriptors available)\n", nMaxConnections, nFD); - if (nWhiteConnections > 0) - LogPrintf("Reserving %i of these connections for whitelisted inbound peers\n", nWhiteConnections); std::ostringstream strErrors; LogPrintf("Using %u threads for script verification\n", nScriptCheckThreads); diff --git a/src/main.cpp b/src/main.cpp index 33b57a5285..35fbec6665 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4522,6 +4522,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, if (pingUsecTime > 0) { // Successful ping time measurement, replace previous pfrom->nPingUsecTime = pingUsecTime; + pfrom->nMinPingUsecTime = std::min(pfrom->nMinPingUsecTime, pingUsecTime); } else { // This should never happen sProblem = "Timing mishap"; diff --git a/src/net.cpp b/src/net.cpp index 105ccce848..4909d5fd40 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -81,7 +81,6 @@ uint64_t nLocalHostNonce = 0; static std::vector<ListenSocket> vhListenSocket; CAddrMan addrman; int nMaxConnections = DEFAULT_MAX_PEER_CONNECTIONS; -int nWhiteConnections = 0; bool fAddressesInitialized = false; std::string strSubVersion; @@ -776,6 +775,222 @@ void SocketSendData(CNode *pnode) static list<CNode*> vNodesDisconnected; +class CNodeRef { +public: + CNodeRef(CNode *pnode) : _pnode(pnode) { + LOCK(cs_vNodes); + _pnode->AddRef(); + } + + ~CNodeRef() { + LOCK(cs_vNodes); + _pnode->Release(); + } + + CNode& operator *() const {return *_pnode;}; + CNode* operator ->() const {return _pnode;}; + + CNodeRef& operator =(const CNodeRef& other) + { + if (this != &other) { + LOCK(cs_vNodes); + + _pnode->Release(); + _pnode = other._pnode; + _pnode->AddRef(); + } + return *this; + } + + CNodeRef(const CNodeRef& other): + _pnode(other._pnode) + { + LOCK(cs_vNodes); + _pnode->AddRef(); + } +private: + CNode *_pnode; +}; + +static bool ReverseCompareNodeMinPingTime(const CNodeRef &a, const CNodeRef &b) +{ + return a->nMinPingUsecTime > b->nMinPingUsecTime; +} + +static bool ReverseCompareNodeTimeConnected(const CNodeRef &a, const CNodeRef &b) +{ + return a->nTimeConnected > b->nTimeConnected; +} + +class CompareNetGroupKeyed +{ + std::vector<unsigned char> vchSecretKey; +public: + CompareNetGroupKeyed() + { + vchSecretKey.resize(32, 0); + GetRandBytes(vchSecretKey.data(), vchSecretKey.size()); + } + + bool operator()(const CNodeRef &a, const CNodeRef &b) + { + std::vector<unsigned char> vchGroupA, vchGroupB; + CSHA256 hashA, hashB; + std::vector<unsigned char> vchA(32), vchB(32); + + vchGroupA = a->addr.GetGroup(); + vchGroupB = b->addr.GetGroup(); + + hashA.Write(begin_ptr(vchGroupA), vchGroupA.size()); + hashB.Write(begin_ptr(vchGroupB), vchGroupB.size()); + + hashA.Write(begin_ptr(vchSecretKey), vchSecretKey.size()); + hashB.Write(begin_ptr(vchSecretKey), vchSecretKey.size()); + + hashA.Finalize(begin_ptr(vchA)); + hashB.Finalize(begin_ptr(vchB)); + + return vchA < vchB; + } +}; + +static bool AttemptToEvictConnection(bool fPreferNewConnection) { + std::vector<CNodeRef> vEvictionCandidates; + { + LOCK(cs_vNodes); + + BOOST_FOREACH(CNode *node, vNodes) { + if (node->fWhitelisted) + continue; + if (!node->fInbound) + continue; + if (node->fDisconnect) + continue; + if (node->addr.IsLocal()) + continue; + vEvictionCandidates.push_back(CNodeRef(node)); + } + } + + if (vEvictionCandidates.empty()) return false; + + // Protect connections with certain characteristics + + // Deterministically select 4 peers to protect by netgroup. + // An attacker cannot predict which netgroups will be protected. + static CompareNetGroupKeyed comparerNetGroupKeyed; + std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), comparerNetGroupKeyed); + vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(4, static_cast<int>(vEvictionCandidates.size())), vEvictionCandidates.end()); + + if (vEvictionCandidates.empty()) return false; + + // Protect the 8 nodes with the best ping times. + // An attacker cannot manipulate this metric without physically moving nodes closer to the target. + std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), ReverseCompareNodeMinPingTime); + vEvictionCandidates.erase(vEvictionCandidates.end() - std::min(8, static_cast<int>(vEvictionCandidates.size())), vEvictionCandidates.end()); + + if (vEvictionCandidates.empty()) return false; + + // Protect the half of the remaining nodes which have been connected the longest. + // This replicates the existing implicit behavior. + std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), ReverseCompareNodeTimeConnected); + vEvictionCandidates.erase(vEvictionCandidates.end() - static_cast<int>(vEvictionCandidates.size() / 2), vEvictionCandidates.end()); + + if (vEvictionCandidates.empty()) return false; + + // Identify the network group with the most connections + std::vector<unsigned char> naMostConnections; + unsigned int nMostConnections = 0; + std::map<std::vector<unsigned char>, std::vector<CNodeRef> > mapAddrCounts; + BOOST_FOREACH(const CNodeRef &node, vEvictionCandidates) { + mapAddrCounts[node->addr.GetGroup()].push_back(node); + + if (mapAddrCounts[node->addr.GetGroup()].size() > nMostConnections) { + nMostConnections = mapAddrCounts[node->addr.GetGroup()].size(); + naMostConnections = node->addr.GetGroup(); + } + } + + // Reduce to the network group with the most connections + vEvictionCandidates = mapAddrCounts[naMostConnections]; + + // Do not disconnect peers if there is only 1 connection from their network group + if (vEvictionCandidates.size() <= 1) + // unless we prefer the new connection (for whitelisted peers) + if (!fPreferNewConnection) + return false; + + // Disconnect the most recent connection from the network group with the most connections + std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), ReverseCompareNodeTimeConnected); + vEvictionCandidates[0]->fDisconnect = true; + + return true; +} + +static void AcceptConnection(const ListenSocket& hListenSocket) { + struct sockaddr_storage sockaddr; + socklen_t len = sizeof(sockaddr); + SOCKET hSocket = accept(hListenSocket.socket, (struct sockaddr*)&sockaddr, &len); + CAddress addr; + int nInbound = 0; + int nMaxInbound = nMaxConnections - MAX_OUTBOUND_CONNECTIONS; + + if (hSocket != INVALID_SOCKET) + if (!addr.SetSockAddr((const struct sockaddr*)&sockaddr)) + LogPrintf("Warning: Unknown socket family\n"); + + bool whitelisted = hListenSocket.whitelisted || CNode::IsWhitelistedRange(addr); + { + LOCK(cs_vNodes); + BOOST_FOREACH(CNode* pnode, vNodes) + if (pnode->fInbound) + nInbound++; + } + + if (hSocket == INVALID_SOCKET) + { + int nErr = WSAGetLastError(); + if (nErr != WSAEWOULDBLOCK) + LogPrintf("socket error accept failed: %s\n", NetworkErrorString(nErr)); + return; + } + + if (!IsSelectableSocket(hSocket)) + { + LogPrintf("connection from %s dropped: non-selectable socket\n", addr.ToString()); + CloseSocket(hSocket); + return; + } + + if (CNode::IsBanned(addr) && !whitelisted) + { + LogPrintf("connection from %s dropped (banned)\n", addr.ToString()); + CloseSocket(hSocket); + return; + } + + if (nInbound >= nMaxInbound) + { + if (!AttemptToEvictConnection(whitelisted)) { + // No connection to evict, disconnect the new connection + LogPrint("net", "failed to find an eviction candidate - connection dropped (full)\n"); + CloseSocket(hSocket); + return; + } + } + + CNode* pnode = new CNode(hSocket, addr, "", true); + pnode->AddRef(); + pnode->fWhitelisted = whitelisted; + + LogPrint("net", "connection from %s accepted\n", addr.ToString()); + + { + LOCK(cs_vNodes); + vNodes.push_back(pnode); + } +} + void ThreadSocketHandler() { unsigned int nPrevNodeCount = 0; @@ -933,64 +1148,7 @@ void ThreadSocketHandler() { if (hListenSocket.socket != INVALID_SOCKET && FD_ISSET(hListenSocket.socket, &fdsetRecv)) { - struct sockaddr_storage sockaddr; - socklen_t len = sizeof(sockaddr); - SOCKET hSocket = accept(hListenSocket.socket, (struct sockaddr*)&sockaddr, &len); - CAddress addr; - int nInbound = 0; - int nMaxInbound = nMaxConnections - MAX_OUTBOUND_CONNECTIONS; - - if (hSocket != INVALID_SOCKET) - if (!addr.SetSockAddr((const struct sockaddr*)&sockaddr)) - LogPrintf("Warning: Unknown socket family\n"); - - bool whitelisted = hListenSocket.whitelisted || CNode::IsWhitelistedRange(addr); - { - LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodes) - if (pnode->fInbound) - nInbound++; - } - - if (hSocket == INVALID_SOCKET) - { - int nErr = WSAGetLastError(); - if (nErr != WSAEWOULDBLOCK) - LogPrintf("socket error accept failed: %s\n", NetworkErrorString(nErr)); - } - else if (!IsSelectableSocket(hSocket)) - { - LogPrintf("connection from %s dropped: non-selectable socket\n", addr.ToString()); - CloseSocket(hSocket); - } - else if (nInbound >= nMaxInbound) - { - LogPrint("net", "connection from %s dropped (full)\n", addr.ToString()); - CloseSocket(hSocket); - } - else if (!whitelisted && (nInbound >= (nMaxInbound - nWhiteConnections))) - { - LogPrint("net", "connection from %s dropped (non-whitelisted)\n", addr.ToString()); - CloseSocket(hSocket); - } - else if (CNode::IsBanned(addr) && !whitelisted) - { - LogPrintf("connection from %s dropped (banned)\n", addr.ToString()); - CloseSocket(hSocket); - } - else - { - CNode* pnode = new CNode(hSocket, addr, "", true); - pnode->AddRef(); - pnode->fWhitelisted = whitelisted; - - LogPrint("net", "connection from %s accepted\n", addr.ToString()); - - { - LOCK(cs_vNodes); - vNodes.push_back(pnode); - } - } + AcceptConnection(hListenSocket); } } @@ -143,19 +143,8 @@ extern uint64_t nLocalServices; extern uint64_t nLocalHostNonce; extern CAddrMan addrman; -// The allocation of connections against the maximum allowed (nMaxConnections) -// is prioritized as follows: -// 1st: Outbound connections (MAX_OUTBOUND_CONNECTIONS) -// 2nd: Inbound connections from whitelisted peers (nWhiteConnections) -// 3rd: Inbound connections from non-whitelisted peers -// Thus, the number of connection slots for the general public to use is: -// nMaxConnections - (MAX_OUTBOUND_CONNECTIONS + nWhiteConnections) -// Any additional inbound connections beyond limits will be immediately closed - /** Maximum number of connections to simultaneously allow (aka connection slots) */ extern int nMaxConnections; -/** Number of connection slots to reserve for inbound from whitelisted peers */ -extern int nWhiteConnections; extern std::vector<CNode*> vNodes; extern CCriticalSection cs_vNodes; @@ -395,6 +384,8 @@ public: int64_t nPingUsecStart; // Last measured round-trip time. int64_t nPingUsecTime; + // Best measured round-trip time. + int64_t nMinPingUsecTime; // Whether a ping is requested. bool fPingQueued; diff --git a/src/reverselock.h b/src/reverselock.h new file mode 100644 index 0000000000..567636e16a --- /dev/null +++ b/src/reverselock.h @@ -0,0 +1,31 @@ +// Copyright (c) 2015 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_REVERSELOCK_H +#define BITCOIN_REVERSELOCK_H + +/** + * An RAII-style reverse lock. Unlocks on construction and locks on destruction. + */ +template<typename Lock> +class reverse_lock +{ +public: + + explicit reverse_lock(Lock& lock) : lock(lock) { + lock.unlock(); + } + + ~reverse_lock() { + lock.lock(); + } + +private: + reverse_lock(reverse_lock const&); + reverse_lock& operator=(reverse_lock const&); + + Lock& lock; +}; + +#endif // BITCOIN_REVERSELOCK_H diff --git a/src/rpcmining.cpp b/src/rpcmining.cpp index 620a46be15..91de37fdcc 100644 --- a/src/rpcmining.cpp +++ b/src/rpcmining.cpp @@ -119,7 +119,7 @@ UniValue generate(const UniValue& params, bool fHelp) "generate numblocks\n" "\nMine blocks immediately (before the RPC call returns)\n" "\nNote: this function can only be used on the regtest network\n" - "1. numblocks (numeric) How many blocks are generated immediately.\n" + "1. numblocks (numeric, required) How many blocks are generated immediately.\n" "\nResult\n" "[ blockhashes ] (array) hashes of blocks generated\n" "\nExamples:\n" diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 06115f5619..184ddc28ab 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -4,9 +4,10 @@ #include "scheduler.h" +#include "reverselock.h" + #include <assert.h> #include <boost/bind.hpp> -#include <boost/thread/reverse_lock.hpp> #include <utility> CScheduler::CScheduler() : nThreadsServicingQueue(0), stopRequested(false), stopWhenEmpty(false) @@ -69,7 +70,7 @@ void CScheduler::serviceQueue() { // Unlock before calling f, so it can reschedule itself or another task // without deadlocking: - boost::reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock); + reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock); f(); } } catch (...) { diff --git a/src/test/reverselock_tests.cpp b/src/test/reverselock_tests.cpp new file mode 100644 index 0000000000..e7e627ae0f --- /dev/null +++ b/src/test/reverselock_tests.cpp @@ -0,0 +1,64 @@ +// Copyright (c) 2015 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "reverselock.h" +#include "test/test_bitcoin.h" + +#include <boost/test/unit_test.hpp> + +BOOST_FIXTURE_TEST_SUITE(reverselock_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(reverselock_basics) +{ + boost::mutex mutex; + boost::unique_lock<boost::mutex> lock(mutex); + + BOOST_CHECK(lock.owns_lock()); + { + reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock); + BOOST_CHECK(!lock.owns_lock()); + } + BOOST_CHECK(lock.owns_lock()); +} + +BOOST_AUTO_TEST_CASE(reverselock_errors) +{ + boost::mutex mutex; + boost::unique_lock<boost::mutex> lock(mutex); + + // Make sure trying to reverse lock an unlocked lock fails + lock.unlock(); + + BOOST_CHECK(!lock.owns_lock()); + + bool failed = false; + try { + reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock); + } catch(...) { + failed = true; + } + + BOOST_CHECK(failed); + BOOST_CHECK(!lock.owns_lock()); + + // Make sure trying to lock a lock after it has been reverse locked fails + failed = false; + bool locked = false; + + lock.lock(); + BOOST_CHECK(lock.owns_lock()); + + try { + reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock); + lock.lock(); + locked = true; + } catch(...) { + failed = true; + } + + BOOST_CHECK(locked && failed); + BOOST_CHECK(lock.owns_lock()); +} + +BOOST_AUTO_TEST_SUITE_END() |