Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate grpc port info in GRPCServer ctor #4728

Merged
merged 11 commits into from
Feb 7, 2024
Merged

Validate grpc port info in GRPCServer ctor #4728

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
47f55ae
Fix #4015, #4557: Do not validate GRPC IP/Port/Protocol information i…
ckeshava Jun 7, 2023
54d443a
fix the compiler error by including ConfigSections in ServerInfo unit…
ckeshava Sep 25, 2023
dc4a1be
use const& whereever possible, addressing Ed's suggestions
ckeshava Sep 27, 2023
429593d
alphabetically order the #defines
ckeshava Oct 10, 2023
f61e718
alphabetically order all the #defines (not relevant to the GRPCServer…
ckeshava Oct 10, 2023
2f882af
Merge branch 'develop' into grpc3
ckeshava Oct 10, 2023
52beb3f
Merge branch 'develop' into grpc3
intelliot Oct 31, 2023
33f51ca
Merge branch 'develop' into grpc3
intelliot Dec 2, 2023
9212481
Merge branch 'develop' into grpc3
intelliot Feb 1, 2024
724492d
Merge branch 'develop' into grpc3
intelliot Feb 5, 2024
f7dd7b8
Merge branch 'develop' into grpc3
intelliot Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/ripple/app/main/GRPCServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <ripple/resource/Fees.h>

#include <ripple/beast/net/IPAddressConversion.h>
#include <ripple/core/ConfigSections.h>

namespace ripple {

Expand Down Expand Up @@ -427,9 +428,9 @@ GRPCServerImpl::GRPCServerImpl(Application& app)
: app_(app), journal_(app_.journal("gRPC Server"))
{
// if present, get endpoint from config
if (app_.config().exists("port_grpc"))
if (app_.config().exists(SECTION_PORT_GRPC))
{
const auto& section = app_.config().section("port_grpc");
Section section = app_.config().section(SECTION_PORT_GRPC);
ximinez marked this conversation as resolved.
Show resolved Hide resolved

auto const optIp = section.get("ip");
if (!optIp)
Expand Down Expand Up @@ -659,7 +660,7 @@ GRPCServerImpl::setupListeners()
secureGatewayIPs_));
}
return requests;
};
}

bool
GRPCServerImpl::start()
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/app/misc/NetworkOPs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2707,9 +2707,9 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters)
}
}

if (app_.config().exists("port_grpc"))
if (app_.config().exists(SECTION_PORT_GRPC))
{
auto const& grpcSection = app_.config().section("port_grpc");
auto const& grpcSection = app_.config().section(SECTION_PORT_GRPC);
auto const optPort = grpcSection.get("port");
if (optPort && grpcSection.get("ip"))
{
Expand Down
2 changes: 2 additions & 0 deletions src/ripple/core/ConfigSections.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ struct ConfigSection
#define SECTION_SWEEP_INTERVAL "sweep_interval"
#define SECTION_NETWORK_ID "network_id"

#define SECTION_PORT_GRPC "port_grpc"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the definitions in this list are alphabetical. Could you move this one to slot in alphabetically with the bulk of them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last 6 #defines are not in alphabetical order, I've fixed them too


} // namespace ripple

#endif
9 changes: 7 additions & 2 deletions src/ripple/rpc/impl/ServerHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <ripple/basics/make_SSLContext.h>
#include <ripple/beast/net/IPAddressConversion.h>
#include <ripple/beast/rfc2616.h>
#include <ripple/core/ConfigSections.h>
#include <ripple/core/JobQueue.h>
#include <ripple/json/json_reader.h>
#include <ripple/json/to_string.h>
Expand All @@ -46,9 +47,7 @@
#include <boost/algorithm/string.hpp>
#include <boost/beast/http/fields.hpp>
#include <boost/beast/http/string_body.hpp>
#include <boost/type_traits.hpp>
#include <algorithm>
#include <mutex>
#include <stdexcept>

namespace ripple {
Expand Down Expand Up @@ -1149,6 +1148,12 @@ parse_Ports(Config const& config, std::ostream& log)
log << "Missing section: [" << name << "]";
Throw<std::exception>();
}

// grpc ports are parsed by GRPCServer class. Do not validate
// grpc port information in this file.
if (name == SECTION_PORT_GRPC)
continue;

ParsedPort parsed = common;
parsed.name = name;
parse_Port(parsed, config[name], log);
Expand Down
5 changes: 5 additions & 0 deletions src/test/jtx/envconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
namespace ripple {
namespace test {

// frequently used macros defined here for convinience.
#define PORT_WS "port_ws"
#define PORT_RPC "port_rpc"
#define PORT_PEER "port_peer"

extern std::atomic<bool> envUseIPv4;

inline const char*
Expand Down
67 changes: 33 additions & 34 deletions src/test/jtx/impl/envconfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <test/jtx/envconfig.h>

#include <ripple/core/ConfigSections.h>
#include <test/jtx/Env.h>
#include <test/jtx/amount.h>

namespace ripple {
Expand Down Expand Up @@ -57,22 +56,22 @@ setupConfigForUnitTests(Config& cfg)
cfg.deprecatedClearSection(ConfigSection::importNodeDatabase());
cfg.legacy("database_path", "");
cfg.setupControl(true, true, true);
cfg["server"].append("port_peer");
cfg["port_peer"].set("ip", getEnvLocalhostAddr());
cfg["port_peer"].set("port", port_peer);
cfg["port_peer"].set("protocol", "peer");

cfg["server"].append("port_rpc");
cfg["port_rpc"].set("ip", getEnvLocalhostAddr());
cfg["port_rpc"].set("admin", getEnvLocalhostAddr());
cfg["port_rpc"].set("port", port_rpc);
cfg["port_rpc"].set("protocol", "http,ws2");

cfg["server"].append("port_ws");
cfg["port_ws"].set("ip", getEnvLocalhostAddr());
cfg["port_ws"].set("admin", getEnvLocalhostAddr());
cfg["port_ws"].set("port", port_ws);
cfg["port_ws"].set("protocol", "ws");
cfg["server"].append(PORT_PEER);
cfg[PORT_PEER].set("ip", getEnvLocalhostAddr());
cfg[PORT_PEER].set("port", port_peer);
cfg[PORT_PEER].set("protocol", "peer");

cfg["server"].append(PORT_RPC);
cfg[PORT_RPC].set("ip", getEnvLocalhostAddr());
cfg[PORT_RPC].set("admin", getEnvLocalhostAddr());
cfg[PORT_RPC].set("port", port_rpc);
cfg[PORT_RPC].set("protocol", "http,ws2");

cfg["server"].append(PORT_WS);
cfg[PORT_WS].set("ip", getEnvLocalhostAddr());
cfg[PORT_WS].set("admin", getEnvLocalhostAddr());
cfg[PORT_WS].set("port", port_ws);
cfg[PORT_WS].set("protocol", "ws");
cfg.SSL_VERIFY = false;
}

Expand All @@ -81,35 +80,35 @@ namespace jtx {
std::unique_ptr<Config>
no_admin(std::unique_ptr<Config> cfg)
{
(*cfg)["port_rpc"].set("admin", "");
(*cfg)["port_ws"].set("admin", "");
(*cfg)[PORT_RPC].set("admin", "");
(*cfg)[PORT_WS].set("admin", "");
return cfg;
}

std::unique_ptr<Config>
secure_gateway(std::unique_ptr<Config> cfg)
{
(*cfg)["port_rpc"].set("admin", "");
(*cfg)["port_ws"].set("admin", "");
(*cfg)["port_rpc"].set("secure_gateway", getEnvLocalhostAddr());
(*cfg)[PORT_RPC].set("admin", "");
(*cfg)[PORT_WS].set("admin", "");
(*cfg)[PORT_RPC].set("secure_gateway", getEnvLocalhostAddr());
return cfg;
}

std::unique_ptr<Config>
admin_localnet(std::unique_ptr<Config> cfg)
{
(*cfg)["port_rpc"].set("admin", "127.0.0.0/8");
(*cfg)["port_ws"].set("admin", "127.0.0.0/8");
(*cfg)[PORT_RPC].set("admin", "127.0.0.0/8");
(*cfg)[PORT_WS].set("admin", "127.0.0.0/8");
return cfg;
}

std::unique_ptr<Config>
secure_gateway_localnet(std::unique_ptr<Config> cfg)
{
(*cfg)["port_rpc"].set("admin", "");
(*cfg)["port_ws"].set("admin", "");
(*cfg)["port_rpc"].set("secure_gateway", "127.0.0.0/8");
(*cfg)["port_ws"].set("secure_gateway", "127.0.0.0/8");
(*cfg)[PORT_RPC].set("admin", "");
(*cfg)[PORT_WS].set("admin", "");
(*cfg)[PORT_RPC].set("secure_gateway", "127.0.0.0/8");
(*cfg)[PORT_WS].set("secure_gateway", "127.0.0.0/8");
return cfg;
}

Expand All @@ -127,7 +126,7 @@ validator(std::unique_ptr<Config> cfg, std::string const& seed)
std::unique_ptr<Config>
port_increment(std::unique_ptr<Config> cfg, int increment)
{
for (auto const sectionName : {"port_peer", "port_rpc", "port_ws"})
for (auto const sectionName : {PORT_PEER, PORT_RPC, PORT_WS})
{
Section& s = (*cfg)[sectionName];
auto const port = s.get<std::int32_t>("port");
Expand All @@ -143,8 +142,8 @@ std::unique_ptr<Config>
addGrpcConfig(std::unique_ptr<Config> cfg)
{
std::string port_grpc = std::to_string(port_base + 3);
(*cfg)["port_grpc"].set("ip", getEnvLocalhostAddr());
(*cfg)["port_grpc"].set("port", port_grpc);
(*cfg)[SECTION_PORT_GRPC].set("ip", getEnvLocalhostAddr());
(*cfg)[SECTION_PORT_GRPC].set("port", port_grpc);
return cfg;
}

Expand All @@ -154,9 +153,9 @@ addGrpcConfigWithSecureGateway(
std::string const& secureGateway)
{
std::string port_grpc = std::to_string(port_base + 3);
(*cfg)["port_grpc"].set("ip", getEnvLocalhostAddr());
(*cfg)["port_grpc"].set("port", port_grpc);
(*cfg)["port_grpc"].set("secure_gateway", secureGateway);
(*cfg)[SECTION_PORT_GRPC].set("ip", getEnvLocalhostAddr());
(*cfg)[SECTION_PORT_GRPC].set("port", port_grpc);
(*cfg)[SECTION_PORT_GRPC].set("secure_gateway", secureGateway);
return cfg;
}

Expand Down
21 changes: 13 additions & 8 deletions src/test/rpc/ReportingETL_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <ripple/beast/unit_test.h>
#include <ripple/rpc/impl/Tuning.h>

#include <ripple/core/ConfigSections.h>
#include <test/jtx.h>
#include <test/jtx/Env.h>
#include <test/jtx/envconfig.h>
Expand Down Expand Up @@ -56,7 +57,8 @@ class ReportingETL_test : public beast::unit_test::suite
testcase("GetLedger");
using namespace test::jtx;
std::unique_ptr<Config> config = envconfig(addGrpcConfig);
std::string grpcPort = *(*config)["port_grpc"].get<std::string>("port");
std::string grpcPort =
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down Expand Up @@ -498,7 +500,8 @@ class ReportingETL_test : public beast::unit_test::suite
testcase("GetLedgerData");
using namespace test::jtx;
std::unique_ptr<Config> config = envconfig(addGrpcConfig);
std::string grpcPort = *(*config)["port_grpc"].get<std::string>("port");
std::string grpcPort =
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));
auto grpcLedgerData = [&grpcPort](
auto sequence, std::string marker = "") {
Expand Down Expand Up @@ -620,7 +623,8 @@ class ReportingETL_test : public beast::unit_test::suite
testcase("GetLedgerDiff");
using namespace test::jtx;
std::unique_ptr<Config> config = envconfig(addGrpcConfig);
std::string grpcPort = *(*config)["port_grpc"].get<std::string>("port");
std::string grpcPort =
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

auto grpcLedgerDiff = [&grpcPort](
Expand Down Expand Up @@ -735,7 +739,8 @@ class ReportingETL_test : public beast::unit_test::suite
testcase("GetLedgerDiff");
using namespace test::jtx;
std::unique_ptr<Config> config = envconfig(addGrpcConfig);
std::string grpcPort = *(*config)["port_grpc"].get<std::string>("port");
std::string grpcPort =
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

auto grpcLedgerEntry = [&grpcPort](auto sequence, auto key) {
Expand Down Expand Up @@ -895,7 +900,7 @@ class ReportingETL_test : public beast::unit_test::suite
std::unique_ptr<Config> config = envconfig(
addGrpcConfigWithSecureGateway, getEnvLocalhostAddr());
std::string grpcPort =
*(*config)["port_grpc"].get<std::string>("port");
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down Expand Up @@ -955,7 +960,7 @@ class ReportingETL_test : public beast::unit_test::suite
std::unique_ptr<Config> config =
envconfig(addGrpcConfigWithSecureGateway, secureGatewayIp);
std::string grpcPort =
*(*config)["port_grpc"].get<std::string>("port");
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down Expand Up @@ -1008,7 +1013,7 @@ class ReportingETL_test : public beast::unit_test::suite
std::unique_ptr<Config> config = envconfig(
addGrpcConfigWithSecureGateway, getEnvLocalhostAddr());
std::string grpcPort =
*(*config)["port_grpc"].get<std::string>("port");
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down Expand Up @@ -1065,7 +1070,7 @@ class ReportingETL_test : public beast::unit_test::suite
std::unique_ptr<Config> config =
envconfig(addGrpcConfigWithSecureGateway, secureGatewayIp);
std::string grpcPort =
*(*config)["port_grpc"].get<std::string>("port");
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down
3 changes: 2 additions & 1 deletion src/test/rpc/ServerInfo_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <ripple/app/misc/NetworkOPs.h>
#include <ripple/beast/unit_test.h>
#include <ripple/core/ConfigSections.h>
#include <ripple/protocol/jss.h>
#include <test/jtx.h>

Expand Down Expand Up @@ -105,7 +106,7 @@ admin = 127.0.0.1
auto const rpc_port =
(*config)["port_rpc"].get<unsigned int>("port");
auto const grpc_port =
(*config)["port_grpc"].get<unsigned int>("port");
(*config)[SECTION_PORT_GRPC].get<unsigned int>("port");
auto const ws_port = (*config)["port_ws"].get<unsigned int>("port");
BEAST_EXPECT(grpc_port);
BEAST_EXPECT(rpc_port);
Expand Down