Merge #16248: Make whitebind/whitelist permissions more flexible

c5b404e8f1973afe071a07c63ba1038eefe13f0f Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa391844f658bd7035659b5b16695733dd56 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7ea4c3644a30092100ffc399e30e193275 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26deaaa6842f7dd7c4537ede000f965ea0189 Make whitebind/whitelist permissions more flexible (nicolas.dorier)

Pull request description:

  # Motivation

  In 0.19, bloom filter will be disabled by default. I tried to make [a PR](https://github.com/bitcoin/bitcoin/pull/16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.

  Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.

  It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.

  When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](https://github.com/bitcoin/bitcoin/pull/16176#issuecomment-500762907) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.

  Doing so will also make follow up idea very easy to implement in a backward compatible way.

  # Implementation details

  The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.

  The following permissions exists:
  * ForceRelay
  * Relay
  * NoBan
  * BloomFilter
  * Mempool

  Example:
  * `-whitelist=bloomfilter@127.0.0.1/32`.
  * `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.

  If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)

  When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist`  and add to it the permissions granted from `whitebind`.

  To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.

  `-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.

  # Follow up idea

  Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:

  * Changing `connect` at rpc and config file level to understand the permissions flags.
  * Changing the permissions of a peer at RPC level.

ACKs for top commit:
  laanwj:
    re-ACK c5b404e8f1973afe071a07c63ba1038eefe13f0f

Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577

src/Makefile.am

@@ -213,6 +213,7 @@ BITCOIN_CORE_H = \
   messagesigner.h \
   miner.h \
   net.h \
+  net_permissions.h \
   net_processing.h \
   netaddress.h \
   netbase.h \
@@ -570,6 +571,7 @@ libdash_common_a_SOURCES = \
   keystore.cpp \
   netaddress.cpp \
   netbase.cpp \
+  net_permissions.cpp \
   policy/feerate.cpp \
   protocol.cpp \
   saltedhasher.cpp \

src/init.cpp

@@ -30,6 +30,7 @@
 #include <miner.h>
 #include <netbase.h>
 #include <net.h>
+#include <net_permissions.h>
 #include <net_processing.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
@@ -2526,21 +2527,16 @@ bool AppInitMain()
         connOptions.vBinds.push_back(addrBind);
     }
     for (const std::string& strBind : gArgs.GetArgs("-whitebind")) {
-        CService addrBind;
+        NetWhitebindPermissions whitebind;
-        if (!Lookup(strBind.c_str(), addrBind, 0, false)) {
+        std::string error;
-            return InitError(ResolveErrMsg("whitebind", strBind));
+        if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(error);
-        }
+        connOptions.vWhiteBinds.push_back(whitebind);
-        if (addrBind.GetPort() == 0) {
-            return InitError(strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind));
-        }
-        connOptions.vWhiteBinds.push_back(addrBind);
     }
 
     for (const auto& net : gArgs.GetArgs("-whitelist")) {
-        CSubNet subnet;
+        NetWhitelistPermissions subnet;
-        LookupSubNet(net.c_str(), subnet);
+        std::string error;
-        if (!subnet.IsValid())
+        if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(error);
-            return InitError(strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net));
         connOptions.vWhitelistedRange.push_back(subnet);
     }
 

src/masternode/masternode-sync.cpp

@@ -181,7 +181,7 @@ void CMasternodeSync::ProcessTick(CConnman& connman)
 
         // NORMAL NETWORK MODE - TESTNET/MAINNET
         {
-            if ((pnode->fWhitelisted || pnode->m_manual_connection) && !netfulfilledman.HasFulfilledRequest(pnode->addr, strAllow)) {
+            if ((pnode->HasPermission(PF_NOBAN) || pnode->m_manual_connection) && !netfulfilledman.HasFulfilledRequest(pnode->addr, strAllow)) {
                 netfulfilledman.RemoveAllFulfilledRequests(pnode->addr);
                 netfulfilledman.AddFulfilledRequest(pnode->addr, strAllow);
                 LogPrintf("CMasternodeSync::ProcessTick -- skipping mnsync restrictions for peer=%d\n", pnode->GetId());

src/net.cpp

@@ -17,6 +17,7 @@
 #include <consensus/consensus.h>
 #include <crypto/common.h>
 #include <crypto/sha256.h>
+#include <net_permissions.h>
 #include <primitives/transaction.h>
 #include <netbase.h>
 #include <scheduler.h>
@@ -98,7 +99,6 @@ enum BindFlags {
     BF_NONE         = 0,
     BF_EXPLICIT     = (1U << 0),
     BF_REPORT_ERROR = (1U << 1),
-    BF_WHITELIST    = (1U << 2),
 };
 
 #ifndef USE_WAKEUP_PIPE
@@ -539,12 +539,10 @@ void CNode::CloseSocketDisconnect(CConnman* connman)
     statsClient.inc("peers.disconnect", 1.0f);
 }
 
-bool CConnman::IsWhitelistedRange(const CNetAddr &addr) {
+void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const {
-    for (const CSubNet& subnet : vWhitelistedRange) {
+    for (const auto& subnet : vWhitelistedRange) {
-        if (subnet.Match(addr))
+        if (subnet.m_subnet.Match(addr)) NetPermissions::AddFlag(flags, subnet.m_flags);
-            return true;
     }
-    return false;
 }
 
 std::string CNode::GetAddrName() const {
@@ -614,7 +612,8 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
         X(mapRecvBytesPerMsgCmd);
         X(nRecvBytes);
     }
-    X(fWhitelisted);
+    X(m_legacyWhitelisted);
+    X(m_permissionFlags);
 
     // It is common for nodes with good ping times to suddenly become lagged,
     // due to a new block arriving or other large transfer.
@@ -907,7 +906,7 @@ bool CConnman::AttemptToEvictConnection()
         LOCK(cs_vNodes);
 
         for (const CNode* node : vNodes) {
-            if (node->fWhitelisted)
+            if (node->HasPermission(PF_NOBAN))
                 continue;
             if (!node->fInbound)
                 continue;
@@ -1018,7 +1017,20 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
         }
     }
 
-    bool whitelisted = hListenSocket.whitelisted || IsWhitelistedRange(addr);
+    NetPermissionFlags permissionFlags = NetPermissionFlags::PF_NONE;
+    hListenSocket.AddSocketPermissionFlags(permissionFlags);
+    AddWhitelistPermissionFlags(permissionFlags, addr);
+    const bool noban = NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN);
+    bool legacyWhitelisted = false;
+    if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_ISIMPLICIT)) {
+        NetPermissions::ClearFlag(permissionFlags, PF_ISIMPLICIT);
+        if (gArgs.GetBoolArg("-whitelistforcerelay", false)) NetPermissions::AddFlag(permissionFlags, PF_FORCERELAY);
+        if (gArgs.GetBoolArg("-whitelistrelay", false)) NetPermissions::AddFlag(permissionFlags, PF_RELAY);
+        NetPermissions::AddFlag(permissionFlags, PF_MEMPOOL);
+        NetPermissions::AddFlag(permissionFlags, PF_NOBAN);
+        legacyWhitelisted = true;
+    }
+
     {
         LOCK(cs_vNodes);
         for (const CNode* pnode : vNodes) {
@@ -1068,7 +1080,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
 
     // Don't accept connections from banned peers, but if our inbound slots aren't almost full, accept
     // if the only banning reason was an automatic misbehavior ban.
-    if (!whitelisted && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0))
+    if (!noban && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0))
     {
         LogPrint(BCLog::NET, "%s (banned)\n", strDropped);
         CloseSocket(hSocket);
@@ -1102,9 +1114,15 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
     CAddress addr_bind = GetBindAddress(hSocket);
 
-    CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true);
+    ServiceFlags nodeServices = nLocalServices;
+    if (NetPermissions::HasFlag(permissionFlags, PF_BLOOMFILTER)) {
+        nodeServices = static_cast<ServiceFlags>(nodeServices | NODE_BLOOM);
+    }
+    CNode* pnode = new CNode(id, nodeServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true);
     pnode->AddRef();
-    pnode->fWhitelisted = whitelisted;
+    pnode->m_permissionFlags = permissionFlags;
+    // If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility)
+    pnode->m_legacyWhitelisted = legacyWhitelisted;
     pnode->m_prefer_evict = bannedlevel > 0;
     m_msgproc->InitializeNode(pnode);
 
@@ -2702,7 +2720,7 @@ void CConnman::ThreadMessageHandler()
 
 
 
-bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, bool fWhitelisted)
+bool CConnman::BindListenPort(const CService& addrBind, std::string& strError, NetPermissionFlags permissions)
 {
     strError = "";
     int nOne = 1;
@@ -2790,9 +2808,9 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b
     }
 #endif
 
-    vhListenSocket.push_back(ListenSocket(hListenSocket, fWhitelisted));
+    vhListenSocket.push_back(ListenSocket(hListenSocket, permissions));
 
-    if (addrBind.IsRoutable() && fDiscover && !fWhitelisted)
+    if (addrBind.IsRoutable() && fDiscover && (permissions & PF_NOBAN) == 0)
         AddLocal(addrBind, LOCAL_BIND);
 
     return true;
@@ -2889,11 +2907,11 @@ NodeId CConnman::GetNewNodeId()
 }
 
 
-bool CConnman::Bind(const CService &addr, unsigned int flags) {
+bool CConnman::Bind(const CService &addr, unsigned int flags, NetPermissionFlags permissions) {
     if (!(flags & BF_EXPLICIT) && !IsReachable(addr))
         return false;
     std::string strError;
-    if (!BindListenPort(addr, strError, (flags & BF_WHITELIST) != 0)) {
+    if (!BindListenPort(addr, strError, permissions)) {
         if ((flags & BF_REPORT_ERROR) && clientInterface) {
             clientInterface->ThreadSafeMessageBox(strError, "", CClientUIInterface::MSG_ERROR);
         }
@@ -2902,20 +2920,21 @@ bool CConnman::Bind(const CService &addr, unsigned int flags) {
     return true;
 }
 
-bool CConnman::InitBinds(const std::vector<CService>& binds, const std::vector<CService>& whiteBinds) {
+bool CConnman::InitBinds(const std::vector<CService>& binds, const std::vector<NetWhitebindPermissions>& whiteBinds)
+{
     bool fBound = false;
     for (const auto& addrBind : binds) {
-        fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR));
+        fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR), NetPermissionFlags::PF_NONE);
     }
     for (const auto& addrBind : whiteBinds) {
-        fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR | BF_WHITELIST));
+        fBound |= Bind(addrBind.m_service, (BF_EXPLICIT | BF_REPORT_ERROR), addrBind.m_flags);
     }
     if (binds.empty() && whiteBinds.empty()) {
         struct in_addr inaddr_any;
         inaddr_any.s_addr = INADDR_ANY;
         struct in6_addr inaddr6_any = IN6ADDR_ANY_INIT;
-        fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE);
+        fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE, NetPermissionFlags::PF_NONE);
-        fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE);
+        fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE, NetPermissionFlags::PF_NONE);
     }
     return fBound;
 }
@@ -3717,7 +3736,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     nVersion = 0;
     nNumWarningsSkipped = 0;
     nLastWarningTime = 0;
-    fWhitelisted = false;
     fOneShot = false;
     m_manual_connection = false;
     fClient = false; // set by version message

src/net.h

@@ -15,6 +15,7 @@
 #include <hash.h>
 #include <limitedmap.h>
 #include <netaddress.h>
+#include <net_permissions.h>
 #include <policy/feerate.h>
 #include <protocol.h>
 #include <random.h>
@@ -171,8 +172,9 @@ public:
         uint64_t nMaxOutboundLimit = 0;
         int64_t m_peer_connect_timeout = DEFAULT_PEER_CONNECT_TIMEOUT;
         std::vector<std::string> vSeedNodes;
-        std::vector<CSubNet> vWhitelistedRange;
+        std::vector<NetWhitelistPermissions> vWhitelistedRange;
-        std::vector<CService> vBinds, vWhiteBinds;
+        std::vector<NetWhitebindPermissions> vWhiteBinds;
+        std::vector<CService> vBinds;
         bool m_use_addrman_outgoing = true;
         std::vector<std::string> m_specified_outgoing;
         std::vector<std::string> m_added_nodes;
@@ -472,15 +474,17 @@ public:
 
 private:
     struct ListenSocket {
+    public:
         SOCKET socket;
-        bool whitelisted;
+        inline void AddSocketPermissionFlags(NetPermissionFlags& flags) const { NetPermissions::AddFlag(flags, m_permissions); }
-
+        ListenSocket(SOCKET socket_, NetPermissionFlags permissions_) : socket(socket_), m_permissions(permissions_) {}
-        ListenSocket(SOCKET socket_, bool whitelisted_) : socket(socket_), whitelisted(whitelisted_) {}
+    private:
+        NetPermissionFlags m_permissions;
     };
 
-    bool BindListenPort(const CService &bindAddr, std::string& strError, bool fWhitelisted = false);
+    bool BindListenPort(const CService& bindAddr, std::string& strError, NetPermissionFlags permissions);
-    bool Bind(const CService &addr, unsigned int flags);
+    bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions);
-    bool InitBinds(const std::vector<CService>& binds, const std::vector<CService>& whiteBinds);
+    bool InitBinds(const std::vector<CService>& binds, const std::vector<NetWhitebindPermissions>& whiteBinds);
     void ThreadOpenAddedConnections();
     void AddOneShot(const std::string& strDest);
     void ProcessOneShot();
@@ -517,7 +521,7 @@ private:
 
     bool AttemptToEvictConnection();
     CNode* ConnectNode(CAddress addrConnect, const char *pszDest = nullptr, bool fCountFailure = false, bool manual_connection = false);
-    bool IsWhitelistedRange(const CNetAddr &addr);
+    void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const;
 
     void DeleteNode(CNode* pnode);
 
@@ -554,7 +558,7 @@ private:
 
     // Whitelisted ranges. Any node connecting from these is automatically
     // whitelisted (as well as those connecting to whitelisted binds).
-    std::vector<CSubNet> vWhitelistedRange;
+    std::vector<NetWhitelistPermissions> vWhitelistedRange;
 
     unsigned int nSendBufferMaxSize;
     unsigned int nReceiveFloodSize;
@@ -650,7 +654,6 @@ void StartMapPort();
 void InterruptMapPort();
 void StopMapPort();
 unsigned short GetListenPort();
-bool BindListenPort(const CService &bindAddr, std::string& strError, bool fWhitelisted = false);
 
 struct CombinerAll
 {
@@ -755,7 +758,8 @@ public:
     mapMsgCmdSize mapSendBytesPerMsgCmd;
     uint64_t nRecvBytes;
     mapMsgCmdSize mapRecvBytesPerMsgCmd;
-    bool fWhitelisted;
+    NetPermissionFlags m_permissionFlags;
+    bool m_legacyWhitelisted;
     double dPingTime;
     double dPingWait;
     double dMinPing;
@@ -867,7 +871,11 @@ public:
     std::string cleanSubVer GUARDED_BY(cs_SubVer){};
     CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer
     bool m_prefer_evict{false}; // This peer is preferred for eviction.
-    bool fWhitelisted; // This peer can bypass DoS banning.
+    bool HasPermission(NetPermissionFlags permission) const {
+        return NetPermissions::HasFlag(m_permissionFlags, permission);
+    }
+    // This boolean is unusued in actual processing, only present for backward compatibility at RPC/QT level
+    bool m_legacyWhitelisted{false};
     bool fFeeler; // If true this node is being used as a short lived feeler.
     bool fOneShot;
     bool m_manual_connection;
@@ -993,6 +1001,7 @@ private:
     const ServiceFlags nLocalServices;
     const int nMyStartingHeight;
     int nSendVersion;
+    NetPermissionFlags m_permissionFlags{ PF_NONE };
     std::list<CNetMessage> vRecvMsg;  // Used only by SocketHandler thread
 
     mutable CCriticalSection cs_addrName;

src/net_permissions.cpp

@@ -0,0 +1,105 @@
+// Copyright (c) 2009-2018 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 <net_permissions.h>
+#include <util/system.h>
+#include <netbase.h>
+
+// The parse the following format "perm1,perm2@xxxxxx"
+bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, std::string& error)
+{
+    NetPermissionFlags flags = PF_NONE;
+    const auto atSeparator = str.find('@');
+
+    // if '@' is not found (ie, "xxxxx"), the caller should apply implicit permissions
+    if (atSeparator == std::string::npos) {
+        NetPermissions::AddFlag(flags, PF_ISIMPLICIT);
+        readen = 0;
+    }
+    // else (ie, "perm1,perm2@xxxxx"), let's enumerate the permissions by splitting by ',' and calculate the flags
+    else {
+        readen = 0;
+        // permissions == perm1,perm2
+        const auto permissions = str.substr(0, atSeparator);
+        while (readen < permissions.length()) {
+            const auto commaSeparator = permissions.find(',', readen);
+            const auto len = commaSeparator == std::string::npos ? permissions.length() - readen : commaSeparator - readen;
+            // permission == perm1
+            const auto permission = permissions.substr(readen, len);
+            readen += len; // We read "perm1"
+            if (commaSeparator != std::string::npos) readen++; // We read ","
+
+            if (permission == "bloomfilter" || permission == "bloom") NetPermissions::AddFlag(flags, PF_BLOOMFILTER);
+            else if (permission == "noban") NetPermissions::AddFlag(flags, PF_NOBAN);
+            else if (permission == "forcerelay") NetPermissions::AddFlag(flags, PF_FORCERELAY);
+            else if (permission == "mempool") NetPermissions::AddFlag(flags, PF_MEMPOOL);
+            else if (permission == "all") NetPermissions::AddFlag(flags, PF_ALL);
+            else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY);
+            else if (permission.length() == 0); // Allow empty entries
+            else {
+                error = strprintf(_("Invalid P2P permission: '%s'"), permission);
+                return false;
+            }
+        }
+        readen++;
+    }
+
+    output = flags;
+    error = "";
+    return true;
+}
+
+std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags)
+{
+    std::vector<std::string> strings;
+    if (NetPermissions::HasFlag(flags, PF_BLOOMFILTER)) strings.push_back("bloomfilter");
+    if (NetPermissions::HasFlag(flags, PF_NOBAN)) strings.push_back("noban");
+    if (NetPermissions::HasFlag(flags, PF_FORCERELAY)) strings.push_back("forcerelay");
+    if (NetPermissions::HasFlag(flags, PF_RELAY)) strings.push_back("relay");
+    if (NetPermissions::HasFlag(flags, PF_MEMPOOL)) strings.push_back("mempool");
+    return strings;
+}
+
+bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error)
+{
+    NetPermissionFlags flags;
+    size_t offset;
+    if (!TryParsePermissionFlags(str, flags, offset, error)) return false;
+
+    const std::string strBind = str.substr(offset);
+    CService addrBind;
+    if (!Lookup(strBind.c_str(), addrBind, 0, false)) {
+        error = strprintf(_("Cannot resolve -%s address: '%s'"), "whitebind", strBind);
+        return false;
+    }
+    if (addrBind.GetPort() == 0) {
+        error = strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind);
+        return false;
+    }
+
+    output.m_flags = flags;
+    output.m_service = addrBind;
+    error = "";
+    return true;
+}
+
+bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error)
+{
+    NetPermissionFlags flags;
+    size_t offset;
+    if (!TryParsePermissionFlags(str, flags, offset, error)) return false;
+
+    const std::string net = str.substr(offset);
+    CSubNet subnet;
+    LookupSubNet(net.c_str(), subnet);
+    if (!subnet.IsValid()) {
+        error = strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net);
+        return false;
+    }
+
+    output.m_flags = flags;
+    output.m_subnet = subnet;
+    error = "";
+    return true;
+}

src/net_permissions.h

@@ -0,0 +1,62 @@
+// Copyright (c) 2009-2018 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 <string>
+#include <vector>
+#include <netaddress.h>
+
+#ifndef BITCOIN_NET_PERMISSIONS_H
+#define BITCOIN_NET_PERMISSIONS_H
+enum NetPermissionFlags
+{
+    PF_NONE = 0,
+    // Can query bloomfilter even if -peerbloomfilters is false
+    PF_BLOOMFILTER = (1U << 1),
+    // Relay and accept transactions from this peer, even if -blocksonly is true
+    PF_RELAY = (1U << 3),
+    // Always relay transactions from this peer, even if already in mempool or rejected from policy
+    // Keep parameter interaction: forcerelay implies relay
+    PF_FORCERELAY = (1U << 2) | PF_RELAY,
+    // Can't be banned for misbehavior
+    PF_NOBAN = (1U << 4),
+    // Can query the mempool
+    PF_MEMPOOL = (1U << 5),
+
+    // True if the user did not specifically set fine grained permissions
+    PF_ISIMPLICIT = (1U << 31),
+    PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL,
+};
+class NetPermissions
+{
+public:
+    NetPermissionFlags m_flags;
+    static std::vector<std::string> ToStrings(NetPermissionFlags flags);
+    static inline bool HasFlag(const NetPermissionFlags& flags, NetPermissionFlags f)
+    {
+        return (flags & f) == f;
+    }
+    static inline void AddFlag(NetPermissionFlags& flags, NetPermissionFlags f)
+    {
+        flags = static_cast<NetPermissionFlags>(flags | f);
+    }
+    static inline void ClearFlag(NetPermissionFlags& flags, NetPermissionFlags f)
+    {
+        flags = static_cast<NetPermissionFlags>(flags & ~f);
+    }
+};
+class NetWhitebindPermissions : public NetPermissions
+{
+public:
+    static bool TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error);
+    CService m_service;
+};
+
+class NetWhitelistPermissions : public NetPermissions
+{
+public:
+    static bool TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error);
+    CSubNet m_subnet;
+};
+
+#endif // BITCOIN_NET_PERMISSIONS_H

src/net_processing.cpp

@@ -418,7 +418,7 @@ static void UpdatePreferredDownload(CNode* node, CNodeState* state) EXCLUSIVE_LO
     nPreferredDownload -= state->fPreferredDownload;
 
     // Whether this node should be marked as a preferred download node.
-    state->fPreferredDownload = (!node->fInbound || node->fWhitelisted) && !node->fOneShot && !node->fClient;
+    state->fPreferredDownload = (!node->fInbound || node->HasPermission(PF_NOBAN)) && !node->fOneShot && !node->fClient;
 
     nPreferredDownload += state->fPreferredDownload;
 }
@@ -1494,7 +1494,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c
     const CNetMsgMaker msgMaker(pfrom->GetSendVersion());
     // disconnect node in case we have reached the outbound limit for serving historical blocks
     // never disconnect whitelisted nodes
-    if (send && connman->OutboundTargetReached(true) && ( ((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->fWhitelisted)
+    if (send && connman->OutboundTargetReached(true) && ( ((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->HasPermission(PF_NOBAN))
     {
         LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom->GetId());
 
@@ -1503,7 +1503,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c
         send = false;
     }
     // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold
-    if (send && !pfrom->fWhitelisted && (
+    if (send && !pfrom->HasPermission(PF_NOBAN) && (
             (((pfrom->GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom->GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (chainActive.Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) )
        )) {
         LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold from peer=%d\n", pfrom->GetId());
@@ -2548,7 +2548,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
         bool fBlocksOnly = !fRelayTxes;
 
         // Allow whitelisted peers to send data other than blocks in blocks only mode if whitelistrelay is true
-        if (pfrom->fWhitelisted && gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY))
+        if (pfrom->HasPermission(PF_RELAY))
             fBlocksOnly = false;
 
         LOCK(cs_main);
@@ -2771,7 +2771,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
         }
 
         LOCK(cs_main);
-        if (IsInitialBlockDownload() && !pfrom->fWhitelisted) {
+        if (IsInitialBlockDownload() && !pfrom->HasPermission(PF_NOBAN)) {
             LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because node is in initial block download\n", pfrom->GetId());
             return true;
         }
@@ -2829,7 +2829,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
     if (strCommand == NetMsgType::TX || strCommand == NetMsgType::DSTX || strCommand == NetMsgType::LEGACYTXLOCKREQUEST) {
         // Stop processing the transaction early if
         // We are in blocks only mode and peer is either not whitelisted or whitelistrelay is off
-        if (!fRelayTxes && (!pfrom->fWhitelisted || !gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)))
+        if (!fRelayTxes && !pfrom->HasPermission(PF_RELAY))
         {
             LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom->GetId());
             return true;
@@ -2988,7 +2988,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
                 }
             }
 
-            if (pfrom->fWhitelisted && gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) {
+            if (pfrom->HasPermission(PF_FORCERELAY)) {
                 // Always relay transactions received from whitelisted peers, even
                 // if they were already in the mempool or rejected from it due
                 // to policy, allowing the node to function as a gateway for
@@ -3421,17 +3421,23 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
     }
 
     if (strCommand == NetMsgType::MEMPOOL) {
-        if (!(pfrom->GetLocalServices() & NODE_BLOOM) && !pfrom->fWhitelisted)
+        if (!(pfrom->GetLocalServices() & NODE_BLOOM) && !pfrom->HasPermission(PF_MEMPOOL))
+        {
+            if (!pfrom->HasPermission(PF_NOBAN))
             {
                 LogPrint(BCLog::NET, "mempool request with bloom filters disabled, disconnect peer=%d\n", pfrom->GetId());
                 pfrom->fDisconnect = true;
+            }
             return true;
         }
 
-        if (connman->OutboundTargetReached(false) && !pfrom->fWhitelisted)
+        if (connman->OutboundTargetReached(false) && !pfrom->HasPermission(PF_MEMPOOL))
+        {
+            if (!pfrom->HasPermission(PF_NOBAN))
             {
                 LogPrint(BCLog::NET, "mempool request with bandwidth limit reached, disconnect peer=%d\n", pfrom->GetId());
                 pfrom->fDisconnect = true;
+            }
             return true;
         }
 
@@ -3672,7 +3678,7 @@ bool PeerLogicValidation::SendRejectsAndCheckIfBanned(CNode* pnode, bool enable_
 
     if (state.fShouldBan) {
         state.fShouldBan = false;
-        if (pnode->fWhitelisted)
+        if (pnode->HasPermission(PF_NOBAN))
             LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode->GetLogString());
         else if (pnode->m_manual_connection)
             LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode->GetLogString());
@@ -4268,7 +4274,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
             // Check whether periodic sends should happen
             // Note: If this node is running in a Masternode mode, it makes no sense to delay outgoing txes
             // because we never produce any txes ourselves i.e. no privacy is lost in this case.
-            bool fSendTrickle = pto->fWhitelisted || fMasternodeMode;
+            bool fSendTrickle = pto->HasPermission(PF_NOBAN) || fMasternodeMode;
             if (pto->nNextInvSend < current_time) {
                 fSendTrickle = true;
                 if (pto->fInbound) {
@@ -4432,7 +4438,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
                     // Note: If all our peers are inbound, then we won't
                     // disconnect our sync peer for stalling; we have bigger
                     // problems if we can't get any outbound peers.
-                    if (!pto->fWhitelisted) {
+                    if (!pto->HasPermission(PF_NOBAN)) {
                         LogPrintf("Timeout downloading headers from peer=%d, disconnecting\n", pto->GetId());
                         pto->fDisconnect = true;
                         return true;

src/qt/rpcconsole.cpp

@@ -1246,7 +1246,7 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats)
     ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer));
     ui->peerDirection->setText(stats->nodeStats.fInbound ? tr("Inbound") : tr("Outbound"));
     ui->peerHeight->setText(QString("%1").arg(QString::number(stats->nodeStats.nStartingHeight)));
-    ui->peerWhitelisted->setText(stats->nodeStats.fWhitelisted ? tr("Yes") : tr("No"));
+    ui->peerWhitelisted->setText(stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No"));
     auto dmn = clientModel->getMasternodeList().GetMNByService(stats->nodeStats.addr);
     if (dmn == nullptr) {
         ui->peerNodeType->setText(tr("Regular"));

src/rpc/net.cpp

@@ -12,6 +12,7 @@
 #include <validation.h>
 #include <net.h>
 #include <net_processing.h>
+#include <net_permissions.h>
 #include <netbase.h>
 #include <policy/policy.h>
 #include <rpc/protocol.h>
@@ -188,7 +189,12 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
             }
             obj.pushKV("inflight", heights);
         }
-        obj.pushKV("whitelisted", stats.fWhitelisted);
+        obj.pushKV("whitelisted", stats.m_legacyWhitelisted);
+        UniValue permissions(UniValue::VARR);
+        for (const auto& permission : NetPermissions::ToStrings(stats.m_permissionFlags)) {
+            permissions.push_back(permission);
+        }
+        obj.pushKV("permissions", permissions);
 
         UniValue sendPerMsgCmd(UniValue::VOBJ);
         for (const mapMsgCmdSize::value_type &i : stats.mapSendBytesPerMsgCmd) {

src/test/netbase_tests.cpp

@@ -4,6 +4,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <netbase.h>
+#include <net_permissions.h>
 #include <protocol.h>
 #include <serialize.h>
 #include <streams.h>
@@ -449,4 +450,82 @@ BOOST_AUTO_TEST_CASE(netbase_parsenetwork)
     BOOST_CHECK_EQUAL(ParseNetwork(""), NET_UNROUTABLE);
 }
 
+BOOST_AUTO_TEST_CASE(netpermissions_test)
+{
+    std::string error;
+    NetWhitebindPermissions whitebindPermissions;
+    NetWhitelistPermissions whitelistPermissions;
+
+    // Detect invalid white bind
+    BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error));
+    BOOST_CHECK(error.find("Cannot resolve -whitebind address") != std::string::npos);
+    BOOST_CHECK(!NetWhitebindPermissions::TryParse("127.0.0.1", whitebindPermissions, error));
+    BOOST_CHECK(error.find("Need to specify a port with -whitebind") != std::string::npos);
+    BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error));
+
+    // If no permission flags, assume backward compatibility
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK(error.empty());
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_ISIMPLICIT);
+    BOOST_CHECK(NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT));
+    NetPermissions::ClearFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT);
+    BOOST_CHECK(!NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE);
+    NetPermissions::AddFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT);
+    BOOST_CHECK(NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT));
+
+    // Can set one permission
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER);
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE);
+
+    // Happy path, can parse flags
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,forcerelay@1.2.3.4:32", whitebindPermissions, error));
+    // forcerelay should also activate the relay permission
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY);
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,noban@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN);
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("all@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_ALL);
+
+    // Allow dups
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,noban,noban@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN);
+
+    // Allow empty
+    BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,,noban@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN);
+    BOOST_CHECK(NetWhitebindPermissions::TryParse(",@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE);
+    BOOST_CHECK(NetWhitebindPermissions::TryParse(",,@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE);
+
+    // Detect invalid flag
+    BOOST_CHECK(!NetWhitebindPermissions::TryParse("bloom,forcerelay,oopsie@1.2.3.4:32", whitebindPermissions, error));
+    BOOST_CHECK(error.find("Invalid P2P permission") != std::string::npos);
+
+    // Check whitelist error
+    BOOST_CHECK(!NetWhitelistPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitelistPermissions, error));
+    BOOST_CHECK(error.find("Invalid netmask specified in -whitelist") != std::string::npos);
+
+    // Happy path for whitelist parsing
+    BOOST_CHECK(NetWhitelistPermissions::TryParse("noban@1.2.3.4", whitelistPermissions, error));
+    BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, PF_NOBAN);
+    BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay@1.2.3.4/32", whitelistPermissions, error));
+    BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, PF_BLOOMFILTER | PF_FORCERELAY | PF_NOBAN | PF_RELAY);
+    BOOST_CHECK(error.empty());
+    BOOST_CHECK_EQUAL(whitelistPermissions.m_subnet.ToString(), "1.2.3.4/32");
+    BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay,mempool@1.2.3.4/32", whitelistPermissions, error));
+
+    const auto strings = NetPermissions::ToStrings(PF_ALL);
+    BOOST_CHECK_EQUAL(strings.size(), 5);
+    BOOST_CHECK(std::find(strings.begin(), strings.end(), "bloomfilter") != strings.end());
+    BOOST_CHECK(std::find(strings.begin(), strings.end(), "forcerelay") != strings.end());
+    BOOST_CHECK(std::find(strings.begin(), strings.end(), "relay") != strings.end());
+    BOOST_CHECK(std::find(strings.begin(), strings.end(), "noban") != strings.end());
+    BOOST_CHECK(std::find(strings.begin(), strings.end(), "mempool") != strings.end());
+}
+
 BOOST_AUTO_TEST_SUITE_END()

test/functional/p2p_permissions.py

@@ -0,0 +1,97 @@
+#!/usr/bin/env python3
+# Copyright (c) 2015-2018 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test p2p permission message.
+
+Test that permissions are correctly calculated and applied
+"""
+
+from test_framework.test_node import ErrorMatch
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    connect_nodes,
+    p2p_port,
+)
+
+class P2PPermissionsTests(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 2
+        self.setup_clean_chain = True
+        self.extra_args = [[],[]]
+
+    def run_test(self):
+        self.checkpermission(
+        # relay permission added
+        ["-whitelist=127.0.0.1", "-whitelistrelay"],
+        ["relay", "noban", "mempool"],
+        True)
+
+        self.checkpermission(
+        # forcerelay and relay permission added
+        # Legacy parameter interaction which set whitelistrelay to true
+        # if whitelistforcerelay is true
+        ["-whitelist=127.0.0.1", "-whitelistforcerelay"],
+        ["forcerelay", "relay", "noban", "mempool"],
+        True)
+
+        # Let's make sure permissions are merged correctly
+        # For this, we need to use whitebind instead of bind
+        # by modifying the configuration file.
+        ip_port = "127.0.0.1:{}".format(p2p_port(1))
+        self.replaceinconfig(1, "bind=127.0.0.1", "whitebind=bloomfilter,forcerelay@" + ip_port)
+        self.checkpermission(
+        ["-whitelist=noban@127.0.0.1" ],
+        # Check parameter interaction forcerelay should activate relay
+        ["noban", "bloomfilter", "forcerelay", "relay" ],
+        False)
+        self.replaceinconfig(1, "whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1")
+
+        self.checkpermission(
+        # legacy whitelistrelay should be ignored
+        ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"],
+        ["noban", "mempool"],
+        False)
+
+        self.checkpermission(
+        # legacy whitelistforcerelay should be ignored
+        ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"],
+        ["noban", "mempool"],
+        False)
+
+        self.checkpermission(
+        # missing mempool permission to be considered legacy whitelisted
+        ["-whitelist=noban@127.0.0.1"],
+        ["noban"],
+        False)
+
+        self.checkpermission(
+        # all permission added
+        ["-whitelist=all@127.0.0.1"],
+        ["forcerelay", "noban", "mempool", "bloomfilter", "relay"],
+        False)
+
+        self.stop_node(1)
+        self.nodes[1].assert_start_raises_init_error(["-whitelist=oopsie@127.0.0.1"], "Invalid P2P permission", match=ErrorMatch.PARTIAL_REGEX)
+        self.nodes[1].assert_start_raises_init_error(["-whitelist=noban@127.0.0.1:230"], "Invalid netmask specified in", match=ErrorMatch.PARTIAL_REGEX)
+        self.nodes[1].assert_start_raises_init_error(["-whitebind=noban@127.0.0.1/10"], "Cannot resolve -whitebind address", match=ErrorMatch.PARTIAL_REGEX)
+
+    def checkpermission(self, args, expectedPermissions, whitelisted):
+        self.restart_node(1, args)
+        connect_nodes(self.nodes[0], 1)
+        peerinfo = self.nodes[1].getpeerinfo()[0]
+        assert_equal(peerinfo['whitelisted'], whitelisted)
+        assert_equal(len(expectedPermissions), len(peerinfo['permissions']))
+        for p in expectedPermissions:
+            if not p in peerinfo['permissions']:
+                raise AssertionError("Expected permissions %r is not granted." % p)
+
+    def replaceinconfig(self, nodeid, old, new):
+        with open(self.nodes[nodeid].bitcoinconf, encoding="utf8") as f:
+            newText=f.read().replace(old, new)
+        with open(self.nodes[nodeid].bitcoinconf, 'w', encoding="utf8") as f:
+            f.write(newText)
+
+if __name__ == '__main__':
+    P2PPermissionsTests().main()

test/functional/test_framework/test_node.py

@@ -64,6 +64,7 @@ class TestNode():
         self.index = i
         self.datadir = datadir
         self.chain = chain
+        self.bitcoinconf = os.path.join(self.datadir, "dash.conf")
         self.stdout_dir = os.path.join(self.datadir, "stdout")
         self.stderr_dir = os.path.join(self.datadir, "stderr")
         self.rpchost = rpchost

test/functional/test_runner.py

@@ -194,6 +194,7 @@ BASE_SCRIPTS = [
     'feature_includeconf.py',
     'feature_logging.py',
     'p2p_node_network_limited.py',
+    'p2p_permissions.py',
     'feature_blocksdir.py',
     'feature_config_args.py',
     'rpc_help.py',