net: rate limit the processing of incoming addr messages (#1818)

* Add ADDR rate limiter

* Change limit on regtest only

* Add flags for MAX_ADDR_RATE_PER_SECOND and MAX_ADDR_PROCESSING_TOKEN_BUCKET in defid

* Increase MAX_ADDR_RATE_PER_SECOND_REGTEST

* Add GetDoubleArg() to fix the issue

* lint: add locale dependancy

---------

Co-authored-by: Mihailo Milenkovic <mihailo.milenkovic84@gmail.com>

src/init.cpp

@@ -635,6 +635,8 @@ void SetupServerArgs()
     gArgs.AddArg("-negativeinterest", "(experimental) Track negative interest values", ArgsManager::ALLOW_ANY, OptionsCategory::HIDDEN);
     gArgs.AddArg("-rpc-governance-accept-neutral", "Allow voting with neutral votes for JellyFish purpose", ArgsManager::ALLOW_ANY, OptionsCategory::HIDDEN);
     gArgs.AddArg("-dftxworkers=<n>", strprintf("No. of parallel workers associated with the DfTx related work pool. Stock splits, parallel processing of the chain where appropriate, etc use this worker pool (default: %d)", DEFAULT_DFTX_WORKERS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    gArgs.AddArg("-maxaddrratepersecond=<n>", strprintf("Sets MAX_ADDR_RATE_PER_SECOND limit for ADDR messages(default: %f)", MAX_ADDR_RATE_PER_SECOND), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    gArgs.AddArg("-maxaddrprocessingtokenbucket=<n>", strprintf("Sets MAX_ADDR_PROCESSING_TOKEN_BUCKET limit for ADDR messages(default: %d)", MAX_ADDR_PROCESSING_TOKEN_BUCKET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
 
 #if HAVE_DECL_DAEMON
     gArgs.AddArg("-daemon", "Run in the background as a daemon and accept commands", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -1248,6 +1250,17 @@ bool AppInitParameterInteraction()
         return InitError("peertimeout cannot be configured with a negative value.");
     }
 
+    maxAddrRatePerSecond = gArgs.GetDoubleArg("-maxaddrratepersecond", Params().NetworkIDString() == CBaseChainParams::REGTEST ? MAX_ADDR_RATE_PER_SECOND_REGTEST : MAX_ADDR_RATE_PER_SECOND);
+    std::cout << maxAddrRatePerSecond << std::endl;
+    if (maxAddrRatePerSecond <= static_cast<double>(0)) {
+        return InitError("maxaddrratepersecond cannot be configured with a negative value.");
+    }
+
+    maxAddrProcessingTokenBucket = gArgs.GetArg("-maxaddrprocessingtokenbucket", MAX_ADDR_PROCESSING_TOKEN_BUCKET);
+    if (maxAddrProcessingTokenBucket <= 0) {
+        return InitError("maxaddrprocessingtokenbucket cannot be configured with a negative value.");
+    }
+
     if (gArgs.IsArgSet("-minrelaytxfee")) {
         CAmount n = 0;
         if (!ParseMoney(gArgs.GetArg("-minrelaytxfee", ""), n)) {

src/net.cpp

@@ -533,6 +533,8 @@ void CNode::copyStats(CNodeStats &stats)
         LOCK(cs_feeFilter);
         X(minFeeFilter);
     }
+    X(nProcessedAddrs);
+    X(nRatelimitedAddrs);
 
     // It is common for nodes with good ping times to suddenly become lagged,
     // due to a new block arriving or other large transfer.
@@ -2648,6 +2650,8 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     } else {
         LogPrint(BCLog::NET, "Added connection peer=%d\n", id);
     }
+
+    nAddrTokenTimestamp = GetTimeMicros();
 }
 
 CNode::~CNode()

src/net.h

@@ -576,6 +576,8 @@ class CNodeStats
     CAddress addr;
     // Bind address of our side of the connection
     CAddress addrBind;
+    uint64_t nProcessedAddrs;
+    uint64_t nRatelimitedAddrs;
 };
 
 
@@ -709,6 +711,16 @@ class CNode
     std::vector<CAddress> vAddrToSend;
     CRollingBloomFilter addrKnown;
     bool fGetAddr{false};
+
+    /** Number of addresses that can be processed from this peer. */
+    double nAddrTokenBucket{1}; // initialize to 1 to allow self-announcement
+
+    /** When nAddrTokenBucket was last updated, in microseconds */
+    int64_t nAddrTokenTimestamp;
+
+    std::atomic<uint64_t> nProcessedAddrs{};
+    std::atomic<uint64_t> nRatelimitedAddrs{};
+
     std::set<uint256> setKnown;
     int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0};
     int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0};

src/net_processing.cpp

@@ -83,6 +83,9 @@ static_assert(INBOUND_PEER_TX_DELAY >= MAX_GETDATA_RANDOM_DELAY,
 /** Limit to avoid sending big packets. Not used in processing incoming GETDATA for compatibility */
 static const unsigned int MAX_GETDATA_SZ = 1000;
 
+double maxAddrRatePerSecond;
+size_t maxAddrProcessingTokenBucket;
+
 
 struct COrphanTx {
     // When modifying, adapt the copy of this definition in tests/DoS_tests.
@@ -2138,6 +2141,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
             {
                 connman->PushMessage(pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR));
                 pfrom->fGetAddr = true;
+
+                // When requesting a getaddr, accept an additional MAX_ADDR_TO_SEND addresses in response
+                // (bypassing the MAX_ADDR_PROCESSING_TOKEN_BUCKET limit).
+                pfrom->nAddrTokenBucket += MAX_ADDR_TO_SEND;
             }
             connman->MarkAddressGood(pfrom->addr);
         }
@@ -2252,11 +2259,40 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
         std::vector<CAddress> vAddrOk;
         int64_t nNow = GetAdjustedTime();
         int64_t nSince = nNow - 10 * 60;
+
+        // track rate limiting within this message
+        uint64_t nProcessedAddrs = 0;
+        uint64_t nRatelimitedAddrs = 0;
+
+        // Update/increment addr rate limiting bucket.
+        const uint64_t nCurrentTime = GetTimeMicros();
+        if (pfrom->nAddrTokenBucket < maxAddrProcessingTokenBucket) {
+            const uint64_t nTimeElapsed = std::max(nCurrentTime - pfrom->nAddrTokenTimestamp, uint64_t(0));
+            const double nIncrement = nTimeElapsed * maxAddrRatePerSecond / 1e6;
+            pfrom->nAddrTokenBucket = std::min<double>(pfrom->nAddrTokenBucket + nIncrement, maxAddrProcessingTokenBucket);
+        }
+        pfrom->nAddrTokenTimestamp = nCurrentTime;
+
+        // Randomize entries before processing, to prevent an attacker to
+        // determine which entries will make it through the rate limit
+        random_shuffle(vAddr.begin(), vAddr.end(), GetRandInt);
+
         for (CAddress& addr : vAddr)
         {
             if (interruptMsgProc)
                 return true;
 
+            // Apply rate limiting
+            if (!pfrom->HasPermission(PF_NOBAN)) {
+                if (pfrom->nAddrTokenBucket < 1.0) {
+                    nRatelimitedAddrs++;
+                    continue;
+                }
+                pfrom->nAddrTokenBucket -= 1.0;
+            }
+
+            nProcessedAddrs++;
+
             // We only bother storing full nodes, though this may include
             // things which we would not make an outbound connection to, in
             // part because we may make feeler connections to them.
@@ -2277,6 +2313,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
             if (fReachable)
                 vAddrOk.push_back(addr);
         }
+        pfrom->nProcessedAddrs += nProcessedAddrs;
+        pfrom->nRatelimitedAddrs += nRatelimitedAddrs;
+
+        LogPrint(BCLog::NET, "Received addr: %u addresses (%u processed, %u rate-limited) peer=%d\n",
+                 vAddr.size(), nProcessedAddrs, nRatelimitedAddrs, pfrom->GetId());
+
         connman->AddNewAddresses(vAddrOk, pfrom->addr, 2 * 60 * 60);
         if (vAddr.size() < 1000)
             pfrom->fGetAddr = false;

src/net_processing.h

@@ -21,6 +21,22 @@ static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100;
 static constexpr bool DEFAULT_ENABLE_BIP61{false};
 static const bool DEFAULT_PEERBLOOMFILTERS = false;
 
+/** The maximum rate of address records we're willing to process on average. Can be bypassed using
+ *  the NetPermissionFlags::Addr permission. */
+static constexpr double MAX_ADDR_RATE_PER_SECOND{0.1};
+
+/** The maximum rate of address records we're willing to process on average. Can be bypassed using
+ *  the NetPermissionFlags::Addr permission. */
+static constexpr double MAX_ADDR_RATE_PER_SECOND_REGTEST{100000};
+
+/** The soft limit of the address processing token bucket (the regular MAX_ADDR_RATE_PER_SECOND
+ *  based increments won't go above this, but the MAX_ADDR_TO_SEND increment following GETADDR
+ *  is exempt from this limit. */
+static constexpr size_t MAX_ADDR_PROCESSING_TOKEN_BUCKET{MAX_ADDR_TO_SEND};
+
+extern double maxAddrRatePerSecond;
+extern size_t maxAddrProcessingTokenBucket;
+
 class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface {
 private:
     CConnman* const connman;

src/rpc/net.cpp

@@ -105,6 +105,8 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
             "       n,                        (numeric) The heights of blocks we're currently asking from this peer\n"
             "       ...\n"
             "    ],\n"
+            "    \"addr_processed\": n,       (numeric) The total number of addresses processed, excluding those dropped due to rate limiting\n"
+            "    \"addr_rate_limited\": n,    (numeric) The total number of addresses dropped due to rate limiting\n"
             "    \"whitelisted\": true|false, (boolean) Whether the peer is whitelisted\n"
             "    \"minfeefilter\": n,         (numeric) The minimum fee rate for transactions this peer accepts\n"
             "    \"bytessent_per_msg\": {\n"
@@ -179,6 +181,8 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
             }
             obj.pushKV("inflight", heights);
         }
+        obj.pushKV("addr_processed", stats.nProcessedAddrs);
+        obj.pushKV("addr_rate_limited", stats.nRatelimitedAddrs);
         obj.pushKV("whitelisted", stats.m_legacyWhitelisted);
         UniValue permissions(UniValue::VARR);
         for (const auto& permission : NetPermissions::ToStrings(stats.m_permissionFlags)) {

src/util/system.cpp

@@ -510,6 +510,14 @@ int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const
     return nDefault;
 }
 
+double ArgsManager::GetDoubleArg(const std::string& strArg, double fDefault) const
+{
+    if (IsArgNegated(strArg)) return 0;
+    std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
+    if (found_res.first) return atof(found_res.second.c_str());
+    return fDefault;
+}
+
 bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const
 {
     if (IsArgNegated(strArg)) return false;

src/util/system.h

@@ -47,7 +47,7 @@ bool error(const char* fmt, const Args&... args)
     return false;
 }
 
-// Adding some generic function pointers to keep things contained without taking 
+// Adding some generic function pointers to keep things contained without taking
 // dependencies on HTTPServer on libs that don't need it
 typedef std::function<std::pair<bool, std::string>(const std::string&)> HTTPHeaderQueryFunc;
 typedef std::function<void(const std::string&, const std::string&)> HTTPHeaderWriterFunc;
@@ -235,6 +235,15 @@ class ArgsManager
      */
     int64_t GetArg(const std::string& strArg, int64_t nDefault) const;
 
+    /**
+     * Return integer argument or default value
+     *
+     * @param strArg Argument to get (e.g. "-foo")
+     * @param fDefault (e.g. 1.0)
+     * @return command-line argument (0.0 if invalid number) or default value
+     */
+    double GetDoubleArg(const std::string& strArg, double fDefault) const;
+
     /**
      * Return boolean argument or default value
      *

test/lint/lint-locale-dependence.sh

@@ -22,6 +22,7 @@ KNOWN_VIOLATIONS=(
     "src/util/strencodings.cpp:.*strtoul"
     "src/util/strencodings.h:.*atoi"
     "src/util/system.cpp:.*atoi"
+    "src/util/system.cpp:.*atof"
     "src/validation.cpp:.*stoi"
     "src/masternodes/govvariables/attributes.cpp:.*isspace"
     "src/masternodes/validation.cpp:.*stoi"