Skip to content

Commit

Permalink
Merge dashpay#6100: feat: make whitelist works with composite command…
Browse files Browse the repository at this point in the history
…s for platform needs

85abbb9 chore: add release notes for composite command for whitelist (Konstantin Akimov)
78ad778 feat: test composite commands in functional test for whitelist (Konstantin Akimov)
a102a59 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  dashpay/dash-issues#66
  dashpay/dash-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 dashpay#6052 dashpay#6051 dashpay#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 85abbb9
  PastaPastaPasta:
    utACK 85abbb9

Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
  • Loading branch information
PastaPastaPasta committed Jul 15, 2024
1 parent 9998ffd commit 7330982
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 5 deletions.
9 changes: 9 additions & 0 deletions doc/release-notes-6100.md
Original file line number Diff line number Diff line change
@@ -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
15 changes: 13 additions & 2 deletions src/httprpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,17 @@ class RpcHttpRequest
}
};

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
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
19 changes: 16 additions & 3 deletions test/functional/rpc_whitelist.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,27 @@
assert_equal,
str_to_b64str
)
import json
import http.client
import re
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)

# 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
Expand All @@ -39,7 +51,8 @@ def setup_chain(self):
# 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 = [
Expand All @@ -55,7 +68,7 @@ def setup_chain(self):
["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:
Expand Down

0 comments on commit 7330982

Please sign in to comment.