Skip to content

Commit 988fe5b

Browse files
committed
Merge bitcoin#12763: Add RPC Whitelist Feature from bitcoin#12248
2081442 test: Add test for rpc_whitelist (Emil Engler) 7414d38 Add RPC Whitelist Feature from bitcoin#12248 (Jeremy Rubin) Pull request description: Summary ==== This patch adds the RPC whitelisting feature requested in bitcoin#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 2081442 Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
2 parents 995b6c8 + 2081442 commit 988fe5b

File tree

4 files changed

+162
-3
lines changed

4 files changed

+162
-3
lines changed

src/httprpc.cpp

+59-3
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,13 @@
1515
#include <util/translation.h>
1616
#include <walletinitinterface.h>
1717

18+
#include <algorithm>
19+
#include <iterator>
20+
#include <map>
1821
#include <memory>
1922
#include <stdio.h>
23+
#include <set>
24+
#include <string>
2025

2126
#include <boost/algorithm/string.hpp> // boost::trim
2227

@@ -64,6 +69,9 @@ class HTTPRPCTimerInterface : public RPCTimerInterface
6469
static std::string strRPCUserColonPass;
6570
/* Stored RPC timer interface (for unregistration) */
6671
static std::unique_ptr<HTTPRPCTimerInterface> httpRPCTimerInterface;
72+
/* RPC Auth Whitelist */
73+
static std::map<std::string, std::set<std::string>> g_rpc_whitelist;
74+
static bool g_rpc_whitelist_default = false;
6775

6876
static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const UniValue& id)
6977
{
@@ -183,18 +191,45 @@ static bool HTTPReq_JSONRPC(HTTPRequest* req, const std::string &)
183191
jreq.URI = req->GetURI();
184192

185193
std::string strReply;
194+
bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser);
195+
if (!user_has_whitelist && g_rpc_whitelist_default) {
196+
LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser);
197+
req->WriteReply(HTTP_FORBIDDEN);
198+
return false;
199+
186200
// singleton request
187-
if (valRequest.isObject()) {
201+
} else if (valRequest.isObject()) {
188202
jreq.parse(valRequest);
189-
203+
if (user_has_whitelist && !g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) {
204+
LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, jreq.strMethod);
205+
req->WriteReply(HTTP_FORBIDDEN);
206+
return false;
207+
}
190208
UniValue result = tableRPC.execute(jreq);
191209

192210
// Send reply
193211
strReply = JSONRPCReply(result, NullUniValue, jreq.id);
194212

195213
// array of requests
196-
} else if (valRequest.isArray())
214+
} else if (valRequest.isArray()) {
215+
if (user_has_whitelist) {
216+
for (unsigned int reqIdx = 0; reqIdx < valRequest.size(); reqIdx++) {
217+
if (!valRequest[reqIdx].isObject()) {
218+
throw JSONRPCError(RPC_INVALID_REQUEST, "Invalid Request object");
219+
} else {
220+
const UniValue& request = valRequest[reqIdx].get_obj();
221+
// Parse method
222+
std::string strMethod = find_value(request, "method").get_str();
223+
if (!g_rpc_whitelist[jreq.authUser].count(strMethod)) {
224+
LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, strMethod);
225+
req->WriteReply(HTTP_FORBIDDEN);
226+
return false;
227+
}
228+
}
229+
}
230+
}
197231
strReply = JSONRPCExecBatch(jreq, valRequest.get_array());
232+
}
198233
else
199234
throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error");
200235

@@ -229,6 +264,27 @@ static bool InitRPCAuthentication()
229264
{
230265
LogPrintf("Using rpcauth authentication.\n");
231266
}
267+
268+
g_rpc_whitelist_default = gArgs.GetBoolArg("-rpcwhitelistdefault", gArgs.IsArgSet("-rpcwhitelist"));
269+
for (const std::string& strRPCWhitelist : gArgs.GetArgs("-rpcwhitelist")) {
270+
auto pos = strRPCWhitelist.find(':');
271+
std::string strUser = strRPCWhitelist.substr(0, pos);
272+
bool intersect = g_rpc_whitelist.count(strUser);
273+
std::set<std::string>& whitelist = g_rpc_whitelist[strUser];
274+
if (pos != std::string::npos) {
275+
std::string strWhitelist = strRPCWhitelist.substr(pos + 1);
276+
std::set<std::string> new_whitelist;
277+
boost::split(new_whitelist, strWhitelist, boost::is_any_of(", "));
278+
if (intersect) {
279+
std::set<std::string> tmp_whitelist;
280+
std::set_intersection(new_whitelist.begin(), new_whitelist.end(),
281+
whitelist.begin(), whitelist.end(), std::inserter(tmp_whitelist, tmp_whitelist.end()));
282+
new_whitelist = std::move(tmp_whitelist);
283+
}
284+
whitelist = std::move(new_whitelist);
285+
}
286+
}
287+
232288
return true;
233289
}
234290

src/init.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,8 @@ void SetupServerArgs()
535535
gArgs.AddArg("-rpcservertimeout=<n>", strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_SERVER_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
536536
gArgs.AddArg("-rpcthreads=<n>", strprintf("Set the number of threads to service RPC calls (default: %d)", DEFAULT_HTTP_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
537537
gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
538+
gArgs.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);
539+
gArgs.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);
538540
gArgs.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);
539541
gArgs.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
540542

test/functional/rpc_whitelist.py

+100
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2017-2019 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""
6+
A test for RPC users with restricted permissions
7+
"""
8+
from test_framework.test_framework import BitcoinTestFramework
9+
import os
10+
from test_framework.util import (
11+
get_datadir_path,
12+
assert_equal,
13+
str_to_b64str
14+
)
15+
import http.client
16+
import urllib.parse
17+
18+
def rpccall(node, user, method):
19+
url = urllib.parse.urlparse(node.url)
20+
headers = {"Authorization": "Basic " + str_to_b64str('{}:{}'.format(user[0], user[3]))}
21+
conn = http.client.HTTPConnection(url.hostname, url.port)
22+
conn.connect()
23+
conn.request('POST', '/', '{"method": "' + method + '"}', headers)
24+
resp = conn.getresponse()
25+
conn.close()
26+
return resp
27+
28+
29+
class RPCWhitelistTest(BitcoinTestFramework):
30+
def set_test_params(self):
31+
self.num_nodes = 1
32+
33+
def setup_chain(self):
34+
super().setup_chain()
35+
# 0 => Username
36+
# 1 => Password (Hashed)
37+
# 2 => Permissions
38+
# 3 => Password Plaintext
39+
self.users = [
40+
["user1", "50358aa884c841648e0700b073c32b2e$b73e95fff0748cc0b517859d2ca47d9bac1aa78231f3e48fa9222b612bd2083e", "getbestblockhash,getblockcount,", "12345"],
41+
["user2", "8650ba41296f62092377a38547f361de$4620db7ba063ef4e2f7249853e9f3c5c3592a9619a759e3e6f1c63f2e22f1d21", "getblockcount", "54321"]
42+
]
43+
# For exceptions
44+
self.strange_users = [
45+
# Test empty
46+
["strangedude", "62d67dffec03836edd698314f1b2be62$c2fb4be29bb0e3646298661123cf2d8629640979cabc268ef05ea613ab54068d", ":", "s7R4nG3R7H1nGZ"],
47+
["strangedude2", "575c012c7fe4b1e83b9d809412da3ef7$09f448d0acfc19924dd62ecb96004d3c2d4b91f471030dfe43c6ea64a8f658c1", "", "s7R4nG3R7H1nGZ"],
48+
# Test trailing comma
49+
["strangedude3", "23189c561b5975a56f4cf94030495d61$3a2f6aac26351e2257428550a553c4c1979594e36675bbd3db692442387728c0", ":getblockcount,", "s7R4nG3R7H1nGZ"],
50+
# Test overwrite
51+
["strangedude4", "990c895760a70df83949e8278665e19a$8f0906f20431ff24cb9e7f5b5041e4943bdf2a5c02a19ef4960dcf45e72cde1c", ":getblockcount, getbestblockhash", "s7R4nG3R7H1nGZ"],
52+
["strangedude4", "990c895760a70df83949e8278665e19a$8f0906f20431ff24cb9e7f5b5041e4943bdf2a5c02a19ef4960dcf45e72cde1c", ":getblockcount", "s7R4nG3R7H1nGZ"],
53+
# Testing the same permission twice
54+
["strangedude5", "d12c6e962d47a454f962eb41225e6ec8$2dd39635b155536d3c1a2e95d05feff87d5ba55f2d5ff975e6e997a836b717c9", ":getblockcount,getblockcount", "s7R4nG3R7H1nGZ"]
55+
]
56+
# These commands shouldn't be allowed for any user to test failures
57+
self.never_allowed = ["getnetworkinfo"]
58+
with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "bitcoin.conf"), 'a', encoding='utf8') as f:
59+
f.write("\nrpcwhitelistdefault=0\n")
60+
for user in self.users:
61+
f.write("rpcauth=" + user[0] + ":" + user[1] + "\n")
62+
f.write("rpcwhitelist=" + user[0] + ":" + user[2] + "\n")
63+
# Special cases
64+
for strangedude in self.strange_users:
65+
f.write("rpcauth=" + strangedude[0] + ":" + strangedude[1] + "\n")
66+
f.write("rpcwhitelist=" + strangedude[0] + strangedude[2] + "\n")
67+
68+
69+
def run_test(self):
70+
for user in self.users:
71+
permissions = user[2].replace(" ", "").split(",")
72+
# Pop all empty items
73+
i = 0
74+
while i < len(permissions):
75+
if permissions[i] == '':
76+
permissions.pop(i)
77+
78+
i += 1
79+
for permission in permissions:
80+
self.log.info("[" + user[0] + "]: Testing a permitted permission (" + permission + ")")
81+
assert_equal(200, rpccall(self.nodes[0], user, permission).status)
82+
for permission in self.never_allowed:
83+
self.log.info("[" + user[0] + "]: Testing a non permitted permission (" + permission + ")")
84+
assert_equal(403, rpccall(self.nodes[0], user, permission).status)
85+
# Now test the strange users
86+
for permission in self.never_allowed:
87+
self.log.info("Strange test 1")
88+
assert_equal(403, rpccall(self.nodes[0], self.strange_users[0], permission).status)
89+
for permission in self.never_allowed:
90+
self.log.info("Strange test 2")
91+
assert_equal(403, rpccall(self.nodes[0], self.strange_users[1], permission).status)
92+
self.log.info("Strange test 3")
93+
assert_equal(200, rpccall(self.nodes[0], self.strange_users[2], "getblockcount").status)
94+
self.log.info("Strange test 4")
95+
assert_equal(403, rpccall(self.nodes[0], self.strange_users[3], "getbestblockhash").status)
96+
self.log.info("Strange test 5")
97+
assert_equal(200, rpccall(self.nodes[0], self.strange_users[4], "getblockcount").status)
98+
99+
if __name__ == "__main__":
100+
RPCWhitelistTest().main()

test/functional/test_runner.py

+1
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@
136136
'interface_rpc.py',
137137
'rpc_psbt.py',
138138
'rpc_users.py',
139+
'rpc_whitelist.py',
139140
'feature_proxy.py',
140141
'rpc_signrawtransaction.py',
141142
'wallet_groups.py',

0 commit comments

Comments
 (0)