Merge #12763: Add RPC Whitelist Feature from #12248

2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)

Pull request description:

  Summary
  ====

  This patch adds the RPC whitelisting feature requested in #12248. RPC Whitelists help enforce application policies for services being built on top of Bitcoin Core (e.g., your Lightning Node maybe shouldn't be adding new peers). The aim of this PR is not to make it advisable to connect your Bitcoin node to arbitrary services, but to reduce risk and prevent unintended access.

  Using RPC Whitelists
  ====
  The way it works is you specify (in your bitcoin.conf) configurations such as

  ```
  rpcauth=user1:4cc74397d6e9972e5ee7671fd241$11849357f26a5be7809c68a032bc2b16ab5dcf6348ef3ed1cf30dae47b8bcc71
  rpcauth=user2:181b4a25317bff60f3749adee7d6bca0$d9c331474f1322975fa170a2ffbcb176ba11644211746b27c1d317f265dd4ada
  rpcauth=user3:a6c8a511b53b1edcf69c36984985e$13cfba0e626db19061c9d61fa58e712d0319c11db97ad845fa84517f454f6675
  rpcwhitelist=user1:getnetworkinfo
  rpcwhitelist=user2:getnetworkinfo,getwalletinfo, getbestblockhash
  rpcwhitelistdefault=0
  ```

  Now user1 can only call getnetworkinfo, user2 can only call getnetworkinfo or getwalletinfo, while user3 can still call all RPCs.

  If any rpcwhitelist is set, act as if all users are subject to whitelists unless rpcwhitelistdefault is set to 0. If rpcwhitelistdefault is set to 1 and no rpcwhitelist is set, act as if all users are subject to whitelists.

  Review Request
  =====
  In addition to normal review, would love specific review from someone working on LN (e.g., @ roasbeef) and someone working on an infrastructure team at an exchange (e.g., @ jimpo) to check that this works well with their system.

  Notes
  =====

  The rpc list is spelling sensitive -- whitespace is stripped though. Spelling errors fail towards the RPC call being blocked, which is safer.

  It was unclear to me if HTTPReq_JSONRPC is the best function to patch this functionality into, or if it would be better to place it in exec or somewhere else.

  It was also unclear to me if it would be preferred to cache the whitelists on startup or parse them on every RPC as is done with multiUserAuthorized. I opted for the cached approach as I thought it was a bit cleaner.

  Future Work
  =====

  In a future PR, I would like to add an inheritance scheme. This seemed more controversial so I didn't want to include that here. Inheritance semantics are tricky, but it would also make these whitelists easier to read.

  It also might be good to add a `getrpcwhitelist` command to facilitate permission discovery.

  Tests
  =====
  Thanks to @ emilengler for adding tests for this feature. The tests cover all cases except for where `rpcwhitelistdefault=1` is used, given difficulties around testing with the current test framework.

ACKs for top commit:
  laanwj:
    ACK 2081442c421cc4376e5d7839f68fbe7630e89103

Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf

src/httprpc.cpp

@@ -15,7 +15,12 @@
 #include <util/translation.h>
 #include <walletinitinterface.h>
 
+#include <algorithm>
+#include <iterator>
+#include <map>
 #include <memory>
+#include <set>
+#include <string>
 
 #include <boost/algorithm/string.hpp> // boost::trim
 
@@ -63,6 +68,9 @@ private:
 static std::string strRPCUserColonPass;
 /* Stored RPC timer interface (for unregistration) */
 static std::unique_ptr<HTTPRPCTimerInterface> httpRPCTimerInterface;
+/* RPC Auth Whitelist */
+static std::map<std::string, std::set<std::string>> g_rpc_whitelist;
+static bool g_rpc_whitelist_default = false;
 
 static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const UniValue& id)
 {
@@ -185,18 +193,45 @@ static bool HTTPReq_JSONRPC(const util::Ref& context, HTTPRequest* req)
         jreq.URI = req->GetURI();
 
         std::string strReply;
-        // singleton request
+        bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser);
-        if (valRequest.isObject()) {
+        if (!user_has_whitelist && g_rpc_whitelist_default) {
-            jreq.parse(valRequest);
+            LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser);
+            req->WriteReply(HTTP_FORBIDDEN);
+            return false;
 
+        // singleton request
+        } else if (valRequest.isObject()) {
+            jreq.parse(valRequest);
+            if (user_has_whitelist && !g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) {
+                LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, jreq.strMethod);
+                req->WriteReply(HTTP_FORBIDDEN);
+                return false;
+            }
             UniValue result = tableRPC.execute(jreq);
 
             // Send reply
             strReply = JSONRPCReply(result, NullUniValue, jreq.id);
 
         // array of requests
-        } else if (valRequest.isArray())
+        } else if (valRequest.isArray()) {
+            if (user_has_whitelist) {
+                for (unsigned int reqIdx = 0; reqIdx < valRequest.size(); reqIdx++) {
+                    if (!valRequest[reqIdx].isObject()) {
+                        throw JSONRPCError(RPC_INVALID_REQUEST, "Invalid Request object");
+                    } else {
+                        const UniValue& request = valRequest[reqIdx].get_obj();
+                        // Parse method
+                        std::string strMethod = find_value(request, "method").get_str();
+                        if (!g_rpc_whitelist[jreq.authUser].count(strMethod)) {
+                            LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, strMethod);
+                            req->WriteReply(HTTP_FORBIDDEN);
+                            return false;
+                        }
+                    }
+                }
+            }
             strReply = JSONRPCExecBatch(jreq, valRequest.get_array());
+        }
         else
             throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error");
 
@@ -231,6 +266,27 @@ static bool InitRPCAuthentication()
     {
         LogPrintf("Using rpcauth authentication.\n");
     }
+
+    g_rpc_whitelist_default = gArgs.GetBoolArg("-rpcwhitelistdefault", gArgs.IsArgSet("-rpcwhitelist"));
+    for (const std::string& strRPCWhitelist : gArgs.GetArgs("-rpcwhitelist")) {
+        auto pos = strRPCWhitelist.find(':');
+        std::string strUser = strRPCWhitelist.substr(0, pos);
+        bool intersect = g_rpc_whitelist.count(strUser);
+        std::set<std::string>& whitelist = g_rpc_whitelist[strUser];
+        if (pos != std::string::npos) {
+            std::string strWhitelist = strRPCWhitelist.substr(pos + 1);
+            std::set<std::string> new_whitelist;
+            boost::split(new_whitelist, strWhitelist, boost::is_any_of(", "));
+            if (intersect) {
+                std::set<std::string> tmp_whitelist;
+                std::set_intersection(new_whitelist.begin(), new_whitelist.end(),
+                       whitelist.begin(), whitelist.end(), std::inserter(tmp_whitelist, tmp_whitelist.end()));
+                new_whitelist = std::move(tmp_whitelist);
+            }
+            whitelist = std::move(new_whitelist);
+        }
+    }
+
     return true;
 }
 

src/init.cpp

@@ -754,6 +754,8 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-rpcservertimeout=<n>", strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_SERVER_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
     argsman.AddArg("-rpcthreads=<n>", strprintf("Set the number of threads to service RPC calls (default: %d)", DEFAULT_HTTP_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
     argsman.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+    argsman.AddArg("-rpcwhitelist=<whitelist>", "Set a whitelist to filter incoming RPC calls for a specific user. The field <whitelist> comes in the format: <USERNAME>:<rpc 1>,<rpc 2>,...,<rpc n>. If multiple whitelists are set for a given user, they are set-intersected. See -rpcwhitelistdefault documentation for information on default whitelist behavior.", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+    argsman.AddArg("-rpcwhitelistdefault", "Sets default behavior for rpc whitelisting. Unless rpcwhitelistdefault is set to 0, if any -rpcwhitelist is set, the rpc server acts as if all rpc users are subject to empty-unless-otherwise-specified whitelists. If rpcwhitelistdefault is set to 1 and no -rpcwhitelist is set, rpc server acts as if all rpc users are subject to empty whitelists.", ArgsManager::ALLOW_BOOL, OptionsCategory::RPC);
     argsman.AddArg("-rpcworkqueue=<n>", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
     argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
 

test/functional/rpc_whitelist.py

@@ -0,0 +1,100 @@
+#!/usr/bin/env python3
+# Copyright (c) 2017-2019 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""
+A test for RPC users with restricted permissions
+"""
+from test_framework.test_framework import BitcoinTestFramework
+import os
+from test_framework.util import (
+    get_datadir_path,
+    assert_equal,
+    str_to_b64str
+)
+import http.client
+import urllib.parse
+
+def rpccall(node, user, method):
+    url = urllib.parse.urlparse(node.url)
+    headers = {"Authorization": "Basic " + str_to_b64str('{}:{}'.format(user[0], user[3]))}
+    conn = http.client.HTTPConnection(url.hostname, url.port)
+    conn.connect()
+    conn.request('POST', '/', '{"method": "' + method + '"}', headers)
+    resp = conn.getresponse()
+    conn.close()
+    return resp
+
+
+class RPCWhitelistTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+
+    def setup_chain(self):
+        super().setup_chain()
+        # 0 => Username
+        # 1 => Password (Hashed)
+        # 2 => Permissions
+        # 3 => Password Plaintext
+        self.users = [
+            ["user1", "50358aa884c841648e0700b073c32b2e$b73e95fff0748cc0b517859d2ca47d9bac1aa78231f3e48fa9222b612bd2083e", "getbestblockhash,getblockcount,", "12345"],
+            ["user2", "8650ba41296f62092377a38547f361de$4620db7ba063ef4e2f7249853e9f3c5c3592a9619a759e3e6f1c63f2e22f1d21", "getblockcount", "54321"]
+        ]
+        # For exceptions
+        self.strange_users = [
+            # Test empty
+            ["strangedude", "62d67dffec03836edd698314f1b2be62$c2fb4be29bb0e3646298661123cf2d8629640979cabc268ef05ea613ab54068d", ":", "s7R4nG3R7H1nGZ"],
+            ["strangedude2", "575c012c7fe4b1e83b9d809412da3ef7$09f448d0acfc19924dd62ecb96004d3c2d4b91f471030dfe43c6ea64a8f658c1", "", "s7R4nG3R7H1nGZ"],
+            # Test trailing comma
+            ["strangedude3", "23189c561b5975a56f4cf94030495d61$3a2f6aac26351e2257428550a553c4c1979594e36675bbd3db692442387728c0", ":getblockcount,", "s7R4nG3R7H1nGZ"],
+            # Test overwrite
+            ["strangedude4", "990c895760a70df83949e8278665e19a$8f0906f20431ff24cb9e7f5b5041e4943bdf2a5c02a19ef4960dcf45e72cde1c", ":getblockcount, getbestblockhash", "s7R4nG3R7H1nGZ"],
+            ["strangedude4", "990c895760a70df83949e8278665e19a$8f0906f20431ff24cb9e7f5b5041e4943bdf2a5c02a19ef4960dcf45e72cde1c", ":getblockcount", "s7R4nG3R7H1nGZ"],
+            # Testing the same permission twice
+            ["strangedude5", "d12c6e962d47a454f962eb41225e6ec8$2dd39635b155536d3c1a2e95d05feff87d5ba55f2d5ff975e6e997a836b717c9", ":getblockcount,getblockcount", "s7R4nG3R7H1nGZ"]
+        ]
+        # These commands shouldn't be allowed for any user to test failures
+        self.never_allowed = ["getnetworkinfo"]
+        with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "dash.conf"), 'a', encoding='utf8') as f:
+            f.write("\nrpcwhitelistdefault=0\n")
+            for user in self.users:
+                f.write("rpcauth=" + user[0] + ":" + user[1] + "\n")
+                f.write("rpcwhitelist=" + user[0] + ":" + user[2] + "\n")
+            # Special cases
+            for strangedude in self.strange_users:
+                f.write("rpcauth=" + strangedude[0] + ":" + strangedude[1] + "\n")
+                f.write("rpcwhitelist=" + strangedude[0] + strangedude[2] + "\n")
+
+
+    def run_test(self):
+        for user in self.users:
+            permissions = user[2].replace(" ", "").split(",")
+            # Pop all empty items
+            i = 0
+            while i < len(permissions):
+                if permissions[i] == '':
+                    permissions.pop(i)
+
+                i += 1
+            for permission in permissions:
+                self.log.info("[" + user[0] + "]: Testing a permitted permission (" + permission + ")")
+                assert_equal(200, rpccall(self.nodes[0], user, permission).status)
+            for permission in self.never_allowed:
+                self.log.info("[" + user[0] + "]: Testing a non permitted permission (" + permission + ")")
+                assert_equal(403, rpccall(self.nodes[0], user, permission).status)
+        # Now test the strange users
+        for permission in self.never_allowed:
+            self.log.info("Strange test 1")
+            assert_equal(403, rpccall(self.nodes[0], self.strange_users[0], permission).status)
+        for permission in self.never_allowed:
+            self.log.info("Strange test 2")
+            assert_equal(403, rpccall(self.nodes[0], self.strange_users[1], permission).status)
+        self.log.info("Strange test 3")
+        assert_equal(200, rpccall(self.nodes[0], self.strange_users[2], "getblockcount").status)
+        self.log.info("Strange test 4")
+        assert_equal(403, rpccall(self.nodes[0], self.strange_users[3], "getbestblockhash").status)
+        self.log.info("Strange test 5")
+        assert_equal(200, rpccall(self.nodes[0], self.strange_users[4], "getblockcount").status)
+
+if __name__ == "__main__":
+    RPCWhitelistTest().main()

test/functional/test_runner.py

@@ -151,6 +151,7 @@ BASE_SCRIPTS = [
     'interface_rpc.py',
     'rpc_psbt.py',
     'rpc_users.py',
+    'rpc_whitelist.py',
     'feature_proxy.py',
     'rpc_signrawtransaction.py',
     'p2p_addrv2_relay.py',