From b6988de098a26c96095a7dbbd952a622f1a06481 Mon Sep 17 00:00:00 2001 From: Harris Date: Mon, 4 May 2020 20:33:52 +0200 Subject: [PATCH 1/5] RPC: Add getrpcwhitelist method --- src/httprpc.cpp | 5 ++ src/httprpc.h | 6 +++ src/rpc/server.cpp | 35 +++++++++++++ test/functional/rpc_getrpcwhitelist.py | 70 +++++++++++++++++++++++++ test/functional/test_runner.py | 1 + test/lint/lint-circular-dependencies.py | 1 + 6 files changed, 118 insertions(+) create mode 100755 test/functional/rpc_getrpcwhitelist.py diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 661406e122..56bbe4a680 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -327,3 +327,8 @@ void StopHTTPRPC() httpRPCTimerInterface.reset(); } } + +const std::set& GetWhitelistedRpcs(const std::string& user_name) +{ + return g_rpc_whitelist.at(user_name); +} diff --git a/src/httprpc.h b/src/httprpc.h index 404d13083f..ab25c1a100 100644 --- a/src/httprpc.h +++ b/src/httprpc.h @@ -6,6 +6,8 @@ #define BITCOIN_HTTPRPC_H #include +#include +#include /** Start HTTP RPC subsystem. * Precondition; HTTP and RPC has been started. @@ -31,4 +33,8 @@ void InterruptREST(); */ void StopREST(); +/** Returns a collection of whitelisted RPCs for the given user + */ +const std::set& GetWhitelistedRpcs(const std::string& user_name); + #endif // BITCOIN_HTTPRPC_H diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index d3c5a19326..9b098d8dd5 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -252,9 +253,43 @@ static RPCHelpMan getrpcinfo() }; } +static RPCHelpMan getrpcwhitelist() +{ + return RPCHelpMan{"getrpcwhitelist", + "\nReturns whitelisted RPCs for the current user.\n", + {}, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::ARR, "methods", "List of RPCs that the user is allowed to call", + { + {RPCResult::Type::STR, "rpc", "rpc command"}, + }}, + } + }, + RPCExamples{ + HelpExampleCli("getrpcwhitelist", "") + + HelpExampleRpc("getrpcwhitelist", "")}, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ + UniValue whitelisted_rpcs(UniValue::VARR); + const std::set& whitelist = GetWhitelistedRpcs(request.authUser); + for (const auto& rpc : whitelist) { + whitelisted_rpcs.push_back(rpc); + } + + UniValue result(UniValue::VOBJ); + result.pushKV("methods", whitelisted_rpcs); + + return result; +} + }; +} + static const CRPCCommand vRPCCommands[]{ /* Overall control/query calls */ {"control", &getrpcinfo}, + {"control", &getrpcwhitelist}, {"control", &help}, {"control", &stop}, {"control", &uptime}, diff --git a/test/functional/rpc_getrpcwhitelist.py b/test/functional/rpc_getrpcwhitelist.py new file mode 100755 index 0000000000..d83d316bbe --- /dev/null +++ b/test/functional/rpc_getrpcwhitelist.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2020 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 getrpcwhitelist RPC call. +""" +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + get_datadir_path, + str_to_b64str +) +import http.client +import json +import os +import urllib.parse + +def call_rpc(node, settings, rpc): + url = urllib.parse.urlparse(node.url) + headers = {"Authorization": "Basic " + str_to_b64str('{}:{}'.format(settings[0], settings[2]))} + conn = http.client.HTTPConnection(url.hostname, url.port) + conn.connect() + conn.request('POST', '/', '{"method": "' + rpc + '"}', headers) + resp = conn.getresponse() + code = resp.status + if code == 200: + json_ret = json.loads(resp.read().decode()) + else: + json_ret = {"result": None} + conn.close() + return {"status": code, "json": json_ret['result']} + +class RPCWhitelistTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def setup_chain(self): + super().setup_chain() + self.settings = ["dummy", + "4e799db4b65924f4468b1c9ff3a68109$5fcd282dcaf4ae74599934a543626c0a11e7e83ead30f07b182058ead8e85da9", + "dummypwd", + "getbalance,getrpcwhitelist,getwalletinfo"] + self.settings_forbidden = ["dummy2", + "f3d319f64b076012f75626c9d895fced$7f55381a24fda02c5de7c18fc377f56fc573149b4d6f83daa9fd584210b51f99", + "dummy2pwd", + "getbalance,getwalletinfo"] + + # declare rpc-whitelisting entries + with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "bitcoin.conf"), 'a', encoding='utf8') as f: + f.write("\nrpcwhitelistdefault=0\n") + f.write("rpcauth={}:{}\n".format(self.settings[0], self.settings[1])) + f.write("rpcwhitelist={}:{}\n".format(self.settings[0], self.settings[3])) + f.write("rpcauth={}:{}\n".format(self.settings_forbidden[0], self.settings_forbidden[1])) + f.write("rpcwhitelist={}:{}\n".format(self.settings_forbidden[0], self.settings_forbidden[3])) + + def run_test(self): + self.log.info("Test getrpcwhitelist") + whitelisted = self.settings[3].split(',') + + # should return allowed rpcs + result = call_rpc(self.nodes[0], self.settings, 'getrpcwhitelist') + assert_equal(200, result['status']) + assert_equal(result['json']['methods'], whitelisted) + # should fail because user has no rpcwhitelist-rpc entry in bitcoin.conf + result = call_rpc(self.nodes[0], self.settings_forbidden, 'getrpcwhitelist') + assert_equal(result['status'], 403) + +if __name__ == "__main__": + RPCWhitelistTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index fbf48a0e4d..58de9733cc 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -215,6 +215,7 @@ BASE_SCRIPTS = [ 'interface_usdt_validation.py', 'rpc_users.py', 'rpc_whitelist.py', + 'rpc_getrpcwhitelist.py', 'feature_proxy.py', 'wallet_signrawtransactionwithwallet.py --legacy-wallet', 'wallet_signrawtransactionwithwallet.py --descriptors', diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index e366a08bd2..c072b20e6b 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -13,6 +13,7 @@ import sys EXPECTED_CIRCULAR_DEPENDENCIES = ( "chainparamsbase -> common/args -> chainparamsbase", + "httprpc -> rpc/server -> httprpc", "node/blockstorage -> validation -> node/blockstorage", "node/utxo_snapshot -> validation -> node/utxo_snapshot", "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel", From cdc008f2d821dea37ad6c64454d328960a90526d Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 31 May 2020 00:23:48 +0000 Subject: [PATCH 2/5] RPC: getrpcwhitelist: Return methods as a JSON Object for future expansion to sub-call permissions --- src/rpc/server.cpp | 4 ++-- test/functional/rpc_getrpcwhitelist.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 9b098d8dd5..b45ed386b6 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -272,10 +272,10 @@ static RPCHelpMan getrpcwhitelist() + HelpExampleRpc("getrpcwhitelist", "")}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - UniValue whitelisted_rpcs(UniValue::VARR); + UniValue whitelisted_rpcs(UniValue::VOBJ); const std::set& whitelist = GetWhitelistedRpcs(request.authUser); for (const auto& rpc : whitelist) { - whitelisted_rpcs.push_back(rpc); + whitelisted_rpcs.pushKV(rpc, NullUniValue); } UniValue result(UniValue::VOBJ); diff --git a/test/functional/rpc_getrpcwhitelist.py b/test/functional/rpc_getrpcwhitelist.py index d83d316bbe..91fa9ce741 100755 --- a/test/functional/rpc_getrpcwhitelist.py +++ b/test/functional/rpc_getrpcwhitelist.py @@ -56,7 +56,7 @@ class RPCWhitelistTest(BitcoinTestFramework): def run_test(self): self.log.info("Test getrpcwhitelist") - whitelisted = self.settings[3].split(',') + whitelisted = {method: None for method in self.settings[3].split(',')} # should return allowed rpcs result = call_rpc(self.nodes[0], self.settings, 'getrpcwhitelist') From e52d7d20330278328f8885ad805346cb76c0eef9 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 1 Oct 2023 20:32:23 +0000 Subject: [PATCH 3/5] Bugfix: RPC: Correct types in getrpcwhitelist RPC docs --- src/rpc/server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index b45ed386b6..66f4d2cf20 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -261,9 +261,9 @@ static RPCHelpMan getrpcwhitelist() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::ARR, "methods", "List of RPCs that the user is allowed to call", + {RPCResult::Type::OBJ_DYN, "methods", "List of RPCs that the user is allowed to call", { - {RPCResult::Type::STR, "rpc", "rpc command"}, + {RPCResult::Type::NONE, "rpc", "Key is name of RPC method, value is null"}, }}, } }, From 651c171539f97d7f9aced649e33a66da81485ef6 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Fri, 29 Sep 2023 13:01:25 +0000 Subject: [PATCH 4/5] RPC: getrpcwhitelist: Return all methods (or none) if no explicit whitelist defined --- src/httprpc.cpp | 16 ++++++++++++++-- src/httprpc.h | 2 +- test/functional/rpc_getrpcwhitelist.py | 4 ++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 56bbe4a680..ae9a821348 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -328,7 +328,19 @@ void StopHTTPRPC() } } -const std::set& GetWhitelistedRpcs(const std::string& user_name) +std::set GetWhitelistedRpcs(const std::string& user_name) { - return g_rpc_whitelist.at(user_name); + if (auto it = g_rpc_whitelist.find(user_name); it != g_rpc_whitelist.end()) { + return it->second; + } + if (g_rpc_whitelist_default) { + return std::set(); + } + + // Build a list of every method + std::set allowed_methods; + for (const auto& method_name : tableRPC.listCommands()) { + allowed_methods.insert(method_name); + } + return allowed_methods; } diff --git a/src/httprpc.h b/src/httprpc.h index ab25c1a100..55a1e271dd 100644 --- a/src/httprpc.h +++ b/src/httprpc.h @@ -35,6 +35,6 @@ void StopREST(); /** Returns a collection of whitelisted RPCs for the given user */ -const std::set& GetWhitelistedRpcs(const std::string& user_name); +std::set GetWhitelistedRpcs(const std::string& user_name); #endif // BITCOIN_HTTPRPC_H diff --git a/test/functional/rpc_getrpcwhitelist.py b/test/functional/rpc_getrpcwhitelist.py index 91fa9ce741..41e6b45bca 100755 --- a/test/functional/rpc_getrpcwhitelist.py +++ b/test/functional/rpc_getrpcwhitelist.py @@ -66,5 +66,9 @@ class RPCWhitelistTest(BitcoinTestFramework): result = call_rpc(self.nodes[0], self.settings_forbidden, 'getrpcwhitelist') assert_equal(result['status'], 403) + # should return a long list of allowed RPC methods (ie, all of them) + result = self.nodes[0].getrpcwhitelist() + assert len(result['methods']) > 10 + if __name__ == "__main__": RPCWhitelistTest().main() From d59e47c9df310cee7675d8690f195b859c5729ac Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Tue, 10 Oct 2023 00:31:00 +0000 Subject: [PATCH 5/5] Bugfix: QA/fuzz: Add getrpcwhitelist to RPC_COMMANDS_SAFE_FOR_FUZZING --- src/test/fuzz/rpc.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 270cab58e2..f77f9afd55 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -146,6 +146,7 @@ const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ "getrawmempool", "getrawtransaction", "getrpcinfo", + "getrpcwhitelist", "gettxout", "gettxoutsetinfo", "gettxspendingprevout",