From 7330982631412162161dfdd17e9a090baf4e0f92 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 15 Jul 2024 11:43:38 -0500 Subject: [PATCH] Merge #6100: feat: make whitelist works with composite commands for platform needs 85abbb97b4cbc292d81a29e94f75ee20028e8186 chore: add release notes for composite command for whitelist (Konstantin Akimov) 78ad778bb0ef88c6d24505699b0dfdea9891aa83 feat: test composite commands in functional test for whitelist (Konstantin Akimov) a102a5978718f399aa71c22afad720f2f686eac6 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented https://github.com/dashpay/dash-issues/issues/66 https://github.com/dashpay/dash-issues/issues/65 ## What was done? Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see https://github.com/dashpay/dash/pull/6052 https://github.com/dashpay/dash/pull/6051 https://github.com/dashpay/dash/pull/6055 and other related PRs This PR makes whitelist feature to be compatible with composite commands. Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things. Platform at their side can use config such as this one (created based on shumkov's example): ``` rpc: { host: '127.0.0.1', port: 9998, users: [ { user: 'dashmate', password: 'rpcpassword', whitelist: null, lowPriority: false, }, { username: 'platform-dapi', password: 'rpcpassword', whitelist: [], lowPriority: true, }, { username: 'platform-drive-consensus', password: 'rpcpassword', whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status] lowPriority: false, }, { username: 'platform-drive-other', password: 'rpcpassword', whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status] ], lowPriority: true, }, ], allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'], }, ``` ## How Has This Been Tested? Updated functional tests, see commits ## Breaking Changes n/a ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: LGTM, utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186 PastaPastaPasta: utACK 85abbb97b4cbc292d81a29e94f75ee20028e8186 Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b --- doc/release-notes-6100.md | 9 +++++++++ src/httprpc.cpp | 15 +++++++++++++-- test/functional/rpc_whitelist.py | 19 ++++++++++++++++--- 3 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 doc/release-notes-6100.md diff --git a/doc/release-notes-6100.md b/doc/release-notes-6100.md new file mode 100644 index 0000000000..656e0b0aac --- /dev/null +++ b/doc/release-notes-6100.md @@ -0,0 +1,9 @@ +## Remote Procedure Call (RPC) Changes + +### Improved support of composite commands + +Dash Core's composite commands such as `quorum list` or `bls generate` now are compatible with a whitelist feature. + +For example, whitelist `getblockcount,quorumlist` will let to call commands `getblockcount`, `quorum list`, but not `quorum sign` + +Note, that adding simple `quorum` in whitelist will allow to use all kind of `quorum...` commands, such as `quorum`, `quorum list`, `quorum sign`, etc diff --git a/src/httprpc.cpp b/src/httprpc.cpp index af02b53c55..5a5f0f601c 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -101,6 +101,17 @@ public: } }; +static bool whitelisted(JSONRPCRequest jreq) +{ + if (g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) return true; + + // check for composite command after + if (!jreq.params.isArray() || jreq.params.empty()) return false; + if (!jreq.params[0].isStr()) return false; + + return g_rpc_whitelist[jreq.authUser].count(jreq.strMethod + jreq.params[0].get_str()); +} + static bool JSONErrorReply(RpcHttpRequest& rpcRequest, const UniValue& objError, const UniValue& id) { // Send error reply from json-rpc error object @@ -226,7 +237,7 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req) jreq.parse(valRequest); rpcRequest.command = jreq.strMethod; - if (user_has_whitelist && !g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) { + if (user_has_whitelist && !whitelisted(jreq)) { LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, jreq.strMethod); return rpcRequest.send_reply(HTTP_FORBIDDEN); } @@ -245,7 +256,7 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req) 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)) { + if (!whitelisted(jreq)) { LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, strMethod); return rpcRequest.send_reply(HTTP_FORBIDDEN); } diff --git a/test/functional/rpc_whitelist.py b/test/functional/rpc_whitelist.py index 77aa79ec37..534d37ffbd 100755 --- a/test/functional/rpc_whitelist.py +++ b/test/functional/rpc_whitelist.py @@ -12,7 +12,9 @@ from test_framework.util import ( assert_equal, str_to_b64str ) +import json import http.client +import re import urllib.parse def rpccall(node, user, method): @@ -20,7 +22,17 @@ def rpccall(node, user, method): 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) + + # composite commands are presented without space in whitelist + # but space can't be ommitted when using CLI/http rpc + # for sack of test, substitute missing space for quorum composite command + params = [] + if re.match(r"^quorum[^ ]", method): + params = [method[6:]] + method = "quorum" + query = {"method" : method, "params" : params} + + conn.request('POST', '/', json.dumps(query), headers) resp = conn.getresponse() conn.close() return resp @@ -39,7 +51,8 @@ class RPCWhitelistTest(BitcoinTestFramework): # 3 => Password Plaintext self.users = [ ["user1", "50358aa884c841648e0700b073c32b2e$b73e95fff0748cc0b517859d2ca47d9bac1aa78231f3e48fa9222b612bd2083e", "getbestblockhash,getblockcount,", "12345"], - ["user2", "8650ba41296f62092377a38547f361de$4620db7ba063ef4e2f7249853e9f3c5c3592a9619a759e3e6f1c63f2e22f1d21", "getblockcount", "54321"] + ["user2", "8650ba41296f62092377a38547f361de$4620db7ba063ef4e2f7249853e9f3c5c3592a9619a759e3e6f1c63f2e22f1d21", "getblockcount", "54321"], + ["platform-user", "8650ba41296f62092377a38547f361de$4620db7ba063ef4e2f7249853e9f3c5c3592a9619a759e3e6f1c63f2e22f1d21", "getblockcount,quorumlist", "54321"], ] # For exceptions self.strange_users = [ @@ -55,7 +68,7 @@ class RPCWhitelistTest(BitcoinTestFramework): ["strangedude5", "d12c6e962d47a454f962eb41225e6ec8$2dd39635b155536d3c1a2e95d05feff87d5ba55f2d5ff975e6e997a836b717c9", ":getblockcount,getblockcount", "s7R4nG3R7H1nGZ"] ] # These commands shouldn't be allowed for any user to test failures - self.never_allowed = ["getnetworkinfo"] + self.never_allowed = ["getnetworkinfo", "quorum sign"] 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: