Skip to content

Commit

Permalink
feat: drop hard-coded restrictions for platform-user to prefer whitel…
Browse files Browse the repository at this point in the history
…ists over

Related changes for better support of whitelist for composite commands are in dashpay#6100
  • Loading branch information
knst committed Jul 9, 2024
1 parent 6e5d3f1 commit 765294c
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 231 deletions.
3 changes: 0 additions & 3 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,6 @@ void SetupServerArgs(NodeContext& node)
argsman.AddArg("-llmq-data-recovery=<n>", strprintf("Enable automated quorum data recovery (default: %u)", llmq::DEFAULT_ENABLE_QUORUM_DATA_RECOVERY), ArgsManager::ALLOW_ANY, OptionsCategory::MASTERNODE);
argsman.AddArg("-llmq-qvvec-sync=<quorum_name>:<mode>", strprintf("Defines from which LLMQ type the masternode should sync quorum verification vectors. Can be used multiple times with different LLMQ types. <mode>: %d (sync always from all quorums of the type defined by <quorum_name>), %d (sync from all quorums of the type defined by <quorum_name> if a member of any of the quorums)", (int32_t)llmq::QvvecSyncMode::Always, (int32_t)llmq::QvvecSyncMode::OnlyIfTypeMember), ArgsManager::ALLOW_ANY, OptionsCategory::MASTERNODE);
argsman.AddArg("-masternodeblsprivkey=<hex>", "Set the masternode BLS private key and enable the client to act as a masternode", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::MASTERNODE);
argsman.AddArg("-platform-user=<user>", "Set the username for the \"platform user\", a restricted user intended to be used by Dash Platform, to the specified username.", ArgsManager::ALLOW_ANY, OptionsCategory::MASTERNODE);

argsman.AddArg("-acceptnonstdtxn", strprintf("Relay and mine \"non-standard\" transactions (%sdefault: %u)", "testnet/regtest only; ", !testnetChainParams->RequireStandard()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-dustrelayfee=<amt>", strprintf("Fee rate (in %s/kB) used to define dust, the value of an output such that it will cost more than its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY);
Expand Down Expand Up @@ -1623,8 +1622,6 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc

GetMainSignals().RegisterBackgroundSignalScheduler(*node.scheduler);

tableRPC.InitPlatformRestrictions();

/* Register RPC commands regardless of -server setting so they will be
* available in the GUI RPC console even if external calls are disabled.
*/
Expand Down
91 changes: 4 additions & 87 deletions src/rpc/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <rpc/server.h>

#include <chainparams.h>
#include <node/context.h>
#include <rpc/blockchain.h>
#include <rpc/server_util.h>
#include <rpc/util.h>
Expand Down Expand Up @@ -35,10 +34,7 @@ static RPCTimerInterface* timerInterface = nullptr;
/* Map of name to timer. */
static Mutex g_deadline_timers_mutex;
static std::map<std::string, std::unique_ptr<RPCTimerBase> > deadlineTimers GUARDED_BY(g_deadline_timers_mutex);
static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler, const std::multimap<std::string, std::vector<UniValue>>& mapPlatformRestrictions);

// Any commands submitted by this user will have their commands filtered based on the mapPlatformRestrictions
static const std::string defaultPlatformUser = "platform-user";
static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler);

struct RPCCommandExecutionInfo
{
Expand Down Expand Up @@ -146,21 +142,6 @@ std::string CRPCTable::help(const std::string& strCommand, const std::string& st
return strRet;
}

void CRPCTable::InitPlatformRestrictions()
{
mapPlatformRestrictions = {
{"getassetunlockstatuses", {}},
{"getbestblockhash", {}},
{"getblockhash", {}},
{"getblockcount", {}},
{"getbestchainlock", {}},
{"quorum sign", {static_cast<uint8_t>(Params().GetConsensus().llmqTypePlatform)}},
{"quorum verify", {}},
{"submitchainlock", {}},
{"verifyislock", {}},
};
}

static RPCHelpMan help()
{
return RPCHelpMan{"help",
Expand Down Expand Up @@ -525,80 +506,16 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const
UniValue result;
for (const auto& command : it->second) {
const JSONRPCRequest new_request{subcommand.empty() ? request : request.squashed() };
if (ExecuteCommand(*command, new_request, result, &command == &it->second.back(), mapPlatformRestrictions)) {
if (ExecuteCommand(*command, new_request, result, &command == &it->second.back())) {
return result;
}
}
}
throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found");
}

static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler, const std::multimap<std::string, std::vector<UniValue>>& mapPlatformRestrictions)
{
const NodeContext& node = EnsureAnyNodeContext(request.context);
// Before executing the RPC Command, filter commands from platform rpc user
if (node.mn_activeman && request.authUser == gArgs.GetArg("-platform-user", defaultPlatformUser)) {
// replace this with structured binding in c++20
std::string command_name = command.name;
if (!command.subname.empty()) command_name += " " + command.subname;
const auto& it = mapPlatformRestrictions.equal_range(command_name);
const auto& allowed_begin = it.first;
const auto& allowed_end = it.second;
/**
* allowed_begin and allowed_end are iterators that represent a range of [method_name, vec_params]
* For example, assume allowed = `quorum sign platformLlmqType`, `quorum verify` and `verifyislock`
* this range will look like:
*
* if request.strMethod == "quorum":
* [
* "quorum sign", [platformLlmqType],
* "quorum verify", []
* ]
* if request.strMethod == "verifyislock"
* [
* "verifyislock", []
* ]
*/

// If the requested method is not available in mapPlatformRestrictions
if (allowed_begin == allowed_end) {
throw JSONRPCError(RPC_PLATFORM_RESTRICTION, strprintf("Method \"%s\" prohibited", request.strMethod));
}

auto isValidRequest = [&request, &allowed_begin, &allowed_end]() {
for (auto itRequest = allowed_begin; itRequest != allowed_end; ++itRequest) {
// This is an individual group of parameters that is valid
// This will look something like `["sign", platformLlmqType]` from above.
const auto& vecAllowedParam = itRequest->second;
// An empty vector of allowed parameters represents that any parameter is allowed.
if (vecAllowedParam.empty()) {
return true;
}
if (request.params.empty()) {
throw JSONRPCError(RPC_PLATFORM_RESTRICTION, strprintf("Method \"%s\" has parameter restrictions.", request.strMethod));
}

if (request.params.size() < vecAllowedParam.size()) {
continue;
}

if (std::equal(vecAllowedParam.begin(), vecAllowedParam.end(),
request.params.getValues().begin(),
[](const UniValue& left, const UniValue& right) {
return left.type() == right.type() && left.getValStr() == right.getValStr();
})) {
return true;
}
}
return false;
};

// Try if any of the mapPlatformRestrictions entries matches the current request
if (!isValidRequest()) {
throw JSONRPCError(RPC_PLATFORM_RESTRICTION, "Request doesn't comply with the parameter restrictions.");
}
}

static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler)
{
try
{
RPCCommandExecution execution(request.strMethod);
Expand Down
3 changes: 0 additions & 3 deletions src/rpc/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,10 @@ class CRPCTable
{
private:
std::map<std::pair<std::string, std::string>, std::vector<const CRPCCommand*>> mapCommands;
std::multimap<std::string, std::vector<UniValue>> mapPlatformRestrictions;
public:
CRPCTable();
std::string help(const std::string& name, const std::string& strSubCommand, const JSONRPCRequest& helpreq) const;

void InitPlatformRestrictions();

/**
* Execute a method.
* @param request The JSONRPCRequest to execute
Expand Down
92 changes: 92 additions & 0 deletions test/functional/rpc_external_queue.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
#!/usr/bin/env python3
# Copyright (c) 2024 The Dash Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test that commands submitted by the platform user are filtered."""

from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import str_to_b64str, assert_equal

import http.client
import json
import os
import urllib.parse


class HTTPBasicsTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
self.supports_cli = False

def setup_nodes(self):
self.add_nodes(self.num_nodes)
self.start_nodes()

def setup_chain(self):
super().setup_chain()
# Append rpcauth to dash.conf before initialization
rpcauthplatform = "rpcauth=platform-user:dd88fd676186f48553775d6fb5a2d344$bc1f7898698ead19c6ec7ff47055622dd7101478f1ff6444103d3dc03cd77c13"
# rpcuser : platform-user
# rpcpassword : password123
rpcauthoperator = "rpcauth=operator:e9b45dd0b61a7be72155535435365a3a$8fb7470bc6f74d8ceaf9a23f49b06127723bd563b3ed5d9cea776ef01803d191"
# rpcuser : operator
# rpcpassword : otherpassword

masternodeblskey="masternodeblsprivkey=58af6e39bb4d86b22bda1a02b134c2f5b71caffa1377540b02f7f1ad122f59e0"

with open(os.path.join(self.options.tmpdir+"/node0", "dash.conf"), 'a', encoding='utf8') as f:
f.write(masternodeblskey+"\n")
f.write(rpcauthplatform+"\n")
f.write(rpcauthoperator+"\n")

def run_test(self):
url = urllib.parse.urlparse(self.nodes[0].url)

def send_command(method, params, auth, expected_status, should_not_match=False):
conn = http.client.HTTPConnection(url.hostname, url.port)
conn.connect()
body = {"method": method}
if len(params):
body["params"] = params
conn.request('POST', '/', json.dumps(body), {"Authorization": "Basic " + str_to_b64str(auth)})
resp = conn.getresponse()
if should_not_match:
assert resp.status != expected_status
else:
assert_equal(resp.status, expected_status)
conn.close()

rpcuser_authpair_platform = "platform-user:password123"
rpcuser_authpair_operator = "operator:otherpassword"
rpcuser_authpair_wrong = "platform-user:rpcpasswordwrong"

external_log_str = "HTTP: Calling handler for external user"
always_expected_log_str = "ThreadRPCServer method="

self.log.info('Try using a incorrect password for platform-user...')
send_command("getbestblockhash", [], rpcuser_authpair_wrong, 401)

self.log.info("Check that there's no external queue by default")
with self.nodes[0].assert_debug_log(expected_msgs=[always_expected_log_str], unexpected_msgs = [external_log_str]):
send_command("getbestblockhash", [], rpcuser_authpair_platform, 200)
with self.nodes[0].assert_debug_log(expected_msgs=[always_expected_log_str], unexpected_msgs = [external_log_str]):
send_command("getbestblockhash", [], rpcuser_authpair_operator, 200)

self.log.info("Restart node with -rpcexternaluser")
self.restart_node(0, extra_args=["-rpcexternaluser=platform-user"])

with self.nodes[0].assert_debug_log(expected_msgs=[always_expected_log_str, external_log_str]):
send_command("getbestblockhash", [], rpcuser_authpair_platform, 200)
with self.nodes[0].assert_debug_log(expected_msgs=[always_expected_log_str], unexpected_msgs = [external_log_str]):
send_command("getbestblockhash", [], rpcuser_authpair_operator, 200)

self.log.info("Restart node with multiple external users")
self.restart_node(0, extra_args=["-rpcexternaluser=platform-user,operator"])
with self.nodes[0].assert_debug_log(expected_msgs=[always_expected_log_str, external_log_str]):
send_command("getbestblockhash", [], rpcuser_authpair_platform, 200)
with self.nodes[0].assert_debug_log(expected_msgs=[always_expected_log_str, external_log_str]):
send_command("getbestblockhash", [], rpcuser_authpair_operator, 200)


if __name__ == '__main__':
HTTPBasicsTest().main()
137 changes: 0 additions & 137 deletions test/functional/rpc_platform_filter.py

This file was deleted.

2 changes: 1 addition & 1 deletion test/functional/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@
'wallet_send.py --descriptors',
'wallet_create_tx.py --descriptors',
'p2p_fingerprint.py',
'rpc_platform_filter.py',
'rpc_external_queue.py',
'rpc_wipewallettxes.py',
'feature_dip0020_activation.py',
'feature_uacomment.py',
Expand Down

0 comments on commit 765294c

Please sign in to comment.