Skip to content

Commit 0054e4b

Browse files
authored
fix: not forward admin API (#1629)
To merge to 2.2.3 branch. It is different from the #1628 . I think this branch still forward feature API to rippled.
1 parent 9fe9e7c commit 0054e4b

File tree

4 files changed

+73
-1
lines changed

4 files changed

+73
-1
lines changed

src/rpc/RPCEngine.hpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "data/BackendInterface.hpp"
2323
#include "rpc/Counters.hpp"
2424
#include "rpc/Errors.hpp"
25+
#include "rpc/RPCHelpers.hpp"
2526
#include "rpc/WorkQueue.hpp"
2627
#include "rpc/common/HandlerProvider.hpp"
2728
#include "rpc/common/Types.hpp"
@@ -134,8 +135,13 @@ class RPCEngine {
134135
Result
135136
buildResponse(web::Context const& ctx)
136137
{
137-
if (forwardingProxy_.shouldForward(ctx))
138+
if (forwardingProxy_.shouldForward(ctx)) {
139+
// Disallow forwarding of the admin api, only user api is allowed for security reasons.
140+
if (isAdminCmd(ctx.method, ctx.params))
141+
return Result{Status{RippledError::rpcNO_PERMISSION}};
142+
138143
return forwardingProxy_.forward(ctx);
144+
}
139145

140146
if (backend_->isTooBusy()) {
141147
LOG(log_.error()) << "Database is too busy. Rejecting request";

src/rpc/RPCHelpers.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,23 @@ specifiesCurrentOrClosedLedger(boost::json::object const& request)
12731273
return false;
12741274
}
12751275

1276+
bool
1277+
isAdminCmd(std::string const& method, boost::json::object const& request)
1278+
{
1279+
auto const isFieldSet = [&request](auto const field) {
1280+
return request.contains(field) and request.at(field).is_bool() and request.at(field).as_bool();
1281+
};
1282+
1283+
if (method == JS(ledger)) {
1284+
if (isFieldSet(JS(full)) or isFieldSet(JS(accounts)) or isFieldSet(JS(type)))
1285+
return true;
1286+
}
1287+
1288+
if (method == JS(feature) and request.contains(JS(vetoed)))
1289+
return true;
1290+
return false;
1291+
}
1292+
12761293
std::variant<ripple::uint256, Status>
12771294
getNFTID(boost::json::object const& request)
12781295
{

src/rpc/RPCHelpers.hpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,16 @@ parseIssue(boost::json::object const& issue);
557557
bool
558558
specifiesCurrentOrClosedLedger(boost::json::object const& request);
559559

560+
/**
561+
* @brief Check whether a request requires administrative privileges on rippled side.
562+
*
563+
* @param method The method name to check
564+
* @param request The request to check
565+
* @return true if the request requires ADMIN role
566+
*/
567+
bool
568+
isAdminCmd(std::string const& method, boost::json::object const& request);
569+
560570
/**
561571
* @brief Get the NFTID from the request
562572
*

tests/unit/rpc/RPCHelpersTests.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "rpc/common/Types.hpp"
2525
#include "util/Fixtures.hpp"
2626
#include "util/MockPrometheus.hpp"
27+
#include "util/NameGenerator.hpp"
2728
#include "util/TestObject.hpp"
2829

2930
#include <boost/asio/impl/spawn.hpp>
@@ -538,3 +539,41 @@ TEST_F(RPCHelpersTest, ParseIssue)
538539
std::runtime_error
539540
);
540541
}
542+
543+
struct IsAdminCmdParamTestCaseBundle {
544+
std::string testName;
545+
std::string method;
546+
std::string testJson;
547+
bool expected;
548+
};
549+
550+
struct IsAdminCmdParameterTest : public TestWithParam<IsAdminCmdParamTestCaseBundle> {};
551+
552+
static auto
553+
generateTestValuesForParametersTest()
554+
{
555+
return std::vector<IsAdminCmdParamTestCaseBundle>{
556+
{"featureVetoedTrue", "feature", R"({"vetoed": true, "feature": "foo"})", true},
557+
{"featureVetoedFalse", "feature", R"({"vetoed": false, "feature": "foo"})", true},
558+
{"ledgerFullTrue", "ledger", R"({"full": true})", true},
559+
{"ledgerAccountsTrue", "ledger", R"({"accounts": true})", true},
560+
{"ledgerTypeTrue", "ledger", R"({"type": true})", true},
561+
{"ledgerFullFalse", "ledger", R"({"full": false})", false},
562+
{"ledgerAccountsFalse", "ledger", R"({"accounts": false})", false},
563+
{"ledgerTypeFalse", "ledger", R"({"type": false})", false},
564+
{"ledgerEntry", "ledger_entry", R"({"type": false})", false}
565+
};
566+
}
567+
568+
INSTANTIATE_TEST_CASE_P(
569+
IsAdminCmdTest,
570+
IsAdminCmdParameterTest,
571+
ValuesIn(generateTestValuesForParametersTest()),
572+
tests::util::NameGenerator
573+
);
574+
575+
TEST_P(IsAdminCmdParameterTest, Test)
576+
{
577+
auto const testBundle = GetParam();
578+
EXPECT_EQ(isAdminCmd(testBundle.method, boost::json::parse(testBundle.testJson).as_object()), testBundle.expected);
579+
}

0 commit comments

Comments
 (0)