From 8f82b62e0d19f77cfbcbd034443e63d77e8a5460 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Wed, 26 Jan 2022 19:08:25 -0800 Subject: [PATCH] Use CIDR notation for `admin` and `secure_gateway` --- cfg/rippled-example.cfg | 25 +++- src/ripple/rpc/Role.h | 6 +- src/ripple/rpc/impl/Role.cpp | 50 +++++-- src/ripple/rpc/impl/ServerHandlerImp.cpp | 8 +- src/ripple/rpc/impl/WSInfoSub.h | 3 +- src/ripple/server/Port.h | 15 +- src/ripple/server/impl/Port.cpp | 167 +++++++++++++++-------- src/test/core/Config_test.cpp | 4 +- src/test/jtx/envconfig.h | 4 + src/test/jtx/impl/envconfig.cpp | 18 +++ src/test/rpc/Roles_test.cpp | 24 ++++ 11 files changed, 241 insertions(+), 83 deletions(-) diff --git a/cfg/rippled-example.cfg b/cfg/rippled-example.cfg index b9407d0dae..f87ad44c7a 100644 --- a/cfg/rippled-example.cfg +++ b/cfg/rippled-example.cfg @@ -200,9 +200,19 @@ # # admin = [ IP, IP, IP, ... ] # -# A comma-separated list of IP addresses. +# A comma-separated list of IP addresses or subnets. Subnets +# should be represented in "slash" notation, such as: +# 10.0.0.0/8 +# 172.16.0.0/12 +# 192.168.0.0/16 +# Those examples are ipv4, but ipv6 is also supported. +# When configuring subnets, the address must match the +# underlying network address. Otherwise, the desired IP range is +# ambiguous. For example, 10.1.2.3/24 has a network address of +# 10.1.2.0. Therefore, that subnet should be configured as +# 10.1.2.0/24. # -# When set, grants administrative command access to the specified IP +# When set, grants administrative command access to the specified # addresses. These commands may be issued over http, https, ws, or wss # if configured on the port. If not provided, the default is to not allow # administrative commands. @@ -233,9 +243,10 @@ # # secure_gateway = [ IP, IP, IP, ... ] # -# A comma-separated list of IP addresses. +# A comma-separated list of IP addresses or subnets. See the +# details for the "admin" option above. # -# When set, allows the specified IP addresses to pass HTTP headers +# When set, allows the specified addresses to pass HTTP headers # containing username and remote IP address for each session. If a # non-empty username is passed in this way, then resource controls # such as often resulting in "tooBusy" errors will be lifted. However, @@ -250,9 +261,9 @@ # proxies. Since rippled trusts these hosts, they must be # responsible for properly authenticating the remote user. # -# The same IP address cannot be used in both "admin" and "secure_gateway" -# lists for the same port. In this case, rippled will abort with an error -# message to the console shortly after startup +# If some IP addresses are included for both "admin" and +# "secure_gateway" connections, then they will be treated as +# "admin" addresses. # # ssl_key = # ssl_cert = diff --git a/src/ripple/rpc/Role.h b/src/ripple/rpc/Role.h index 1e71b7ecf5..c4f1f730c0 100644 --- a/src/ripple/rpc/Role.h +++ b/src/ripple/rpc/Role.h @@ -25,8 +25,11 @@ #include #include #include +#include +#include #include #include +#include #include namespace ripple { @@ -79,7 +82,8 @@ isUnlimited(Role const& role); bool ipAllowed( beast::IP::Address const& remoteIp, - std::vector const& adminIp); + std::vector const& nets4, + std::vector const& nets6); boost::string_view forwardedFor(http_request_type const& request); diff --git a/src/ripple/rpc/impl/Role.cpp b/src/ripple/rpc/impl/Role.cpp index 3bfc7d56a0..1807ecc20f 100644 --- a/src/ripple/rpc/impl/Role.cpp +++ b/src/ripple/rpc/impl/Role.cpp @@ -23,13 +23,14 @@ #include #include #include +#include namespace ripple { bool passwordUnrequiredOrSentCorrect(Port const& port, Json::Value const& params) { - assert(!port.admin_ip.empty()); + assert(!(port.admin_nets_v4.empty() && port.admin_nets_v6.empty())); bool const passwordRequired = (!port.admin_user.empty() || !port.admin_password.empty()); @@ -43,14 +44,40 @@ passwordUnrequiredOrSentCorrect(Port const& port, Json::Value const& params) bool ipAllowed( beast::IP::Address const& remoteIp, - std::vector const& adminIp) + std::vector const& nets4, + std::vector const& nets6) { - return std::find_if( - adminIp.begin(), - adminIp.end(), - [&remoteIp](beast::IP::Address const& ip) { - return ip.is_unspecified() || ip == remoteIp; - }) != adminIp.end(); + // To test whether the remoteIP is part of one of the configured + // subnets, first convert it to a subnet definition. For ipv4, + // this means appending /32. For ipv6, /128. Then based on protocol + // check for whether the resulting network is either a subnet of or + // equal to each configured subnet, based on boost::asio's reasoning. + // For example, 10.1.2.3 is a subnet of 10.1.2.0/24, but 10.1.2.0 is + // not. However, 10.1.2.0 is equal to the network portion of 10.1.2.0/24. + + std::string addrString = remoteIp.to_string(); + if (remoteIp.is_v4()) + { + addrString += "/32"; + auto ipNet = boost::asio::ip::make_network_v4(addrString); + for (auto const& net : nets4) + { + if (ipNet.is_subnet_of(net) || ipNet == net) + return true; + } + } + else + { + addrString += "/128"; + auto ipNet = boost::asio::ip::make_network_v6(addrString); + for (auto const& net : nets6) + { + if (ipNet.is_subnet_of(net) || ipNet == net) + return true; + } + } + + return false; } bool @@ -59,7 +86,7 @@ isAdmin( Json::Value const& params, beast::IP::Address const& remoteIp) { - return ipAllowed(remoteIp, port.admin_ip) && + return ipAllowed(remoteIp, port.admin_nets_v4, port.admin_nets_v6) && passwordUnrequiredOrSentCorrect(port, params); } @@ -77,7 +104,10 @@ requestRole( if (required == Role::ADMIN) return Role::FORBID; - if (ipAllowed(remoteIp.address(), port.secure_gateway_ip)) + if (ipAllowed( + remoteIp.address(), + port.secure_gateway_nets_v4, + port.secure_gateway_nets_v6)) { if (user.size()) return Role::IDENTIFIED; diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index 3aed453828..3ac5f04a9d 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -1060,10 +1060,6 @@ to_Port(ParsedPort const& parsed, std::ostream& log) Throw(); } p.port = *parsed.port; - if (parsed.admin_ip) - p.admin_ip = *parsed.admin_ip; - if (parsed.secure_gateway_ip) - p.secure_gateway_ip = *parsed.secure_gateway_ip; if (parsed.protocol.empty()) { @@ -1083,6 +1079,10 @@ to_Port(ParsedPort const& parsed, std::ostream& log) p.pmd_options = parsed.pmd_options; p.ws_queue_limit = parsed.ws_queue_limit; p.limit = parsed.limit; + p.admin_nets_v4 = parsed.admin_nets_v4; + p.admin_nets_v6 = parsed.admin_nets_v6; + p.secure_gateway_nets_v4 = parsed.secure_gateway_nets_v4; + p.secure_gateway_nets_v6 = parsed.secure_gateway_nets_v6; return p; } diff --git a/src/ripple/rpc/impl/WSInfoSub.h b/src/ripple/rpc/impl/WSInfoSub.h index 4bc80d5756..8f386c8ebf 100644 --- a/src/ripple/rpc/impl/WSInfoSub.h +++ b/src/ripple/rpc/impl/WSInfoSub.h @@ -45,7 +45,8 @@ public: if (ipAllowed( beast::IPAddressConversion::from_asio(ws->remote_endpoint()) .address(), - ws->port().secure_gateway_ip)) + ws->port().secure_gateway_nets_v4, + ws->port().secure_gateway_nets_v6)) { auto it = h.find("X-User"); if (it != h.end()) diff --git a/src/ripple/server/Port.h b/src/ripple/server/Port.h index ae260f6798..9dccfdf9c0 100644 --- a/src/ripple/server/Port.h +++ b/src/ripple/server/Port.h @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include #include #include @@ -30,6 +32,7 @@ #include #include #include +#include namespace boost { namespace asio { @@ -50,8 +53,10 @@ struct Port boost::asio::ip::address ip; std::uint16_t port = 0; std::set protocol; - std::vector admin_ip; - std::vector secure_gateway_ip; + std::vector admin_nets_v4; + std::vector admin_nets_v6; + std::vector secure_gateway_nets_v4; + std::vector secure_gateway_nets_v6; std::string user; std::string password; std::string admin_user; @@ -108,8 +113,10 @@ struct ParsedPort std::optional ip; std::optional port; - std::optional> admin_ip; - std::optional> secure_gateway_ip; + std::vector admin_nets_v4; + std::vector admin_nets_v6; + std::vector secure_gateway_nets_v4; + std::vector secure_gateway_nets_v6; }; void diff --git a/src/ripple/server/impl/Port.cpp b/src/ripple/server/impl/Port.cpp index 5691e7bd01..25d86025bd 100644 --- a/src/ripple/server/impl/Port.cpp +++ b/src/ripple/server/impl/Port.cpp @@ -21,8 +21,9 @@ #include #include #include - #include +#include +#include namespace ripple { @@ -47,18 +48,34 @@ operator<<(std::ostream& os, Port const& p) { os << "'" << p.name << "' (ip=" << p.ip << ":" << p.port << ", "; - if (!p.admin_ip.empty()) + if (p.admin_nets_v4.size() || p.admin_nets_v6.size()) { - os << "admin IPs:"; - for (auto const& ip : p.admin_ip) - os << ip.to_string() << ", "; + os << "admin nets:"; + for (auto const& net : p.admin_nets_v4) + { + os << net.to_string(); + os << ", "; + } + for (auto const& net : p.admin_nets_v6) + { + os << net.to_string(); + os << ", "; + } } - if (!p.secure_gateway_ip.empty()) + if (p.secure_gateway_nets_v4.size() || p.secure_gateway_nets_v6.size()) { - os << "secure_gateway IPs:"; - for (auto const& ip : p.secure_gateway_ip) - os << ip.to_string() << ", "; + os << "secure_gateway nets:"; + for (auto const& net : p.secure_gateway_nets_v4) + { + os << net.to_string(); + os << ", "; + } + for (auto const& net : p.secure_gateway_nets_v6) + { + os << net.to_string(); + os << ", "; + } } os << p.protocols() << ")"; @@ -72,65 +89,108 @@ populate( Section const& section, std::string const& field, std::ostream& log, - std::optional>& ips, - bool allowAllIps, - std::vector const& admin_ip) + std::vector& nets4, + std::vector& nets6) { auto const optResult = section.get(field); - if (optResult) + if (!optResult) + return; + + std::stringstream ss(*optResult); + std::string ip; + + while (std::getline(ss, ip, ',')) { - std::stringstream ss(*optResult); - std::string ip; - bool has_any(false); + boost::algorithm::trim(ip); + bool v4; + boost::asio::ip::network_v4 v4Net; + boost::asio::ip::network_v6 v6Net; - ips.emplace(); - while (std::getline(ss, ip, ',')) + try { + // First, check to see if 0.0.0.0 or ipv6 equivalent was configured, + // which means all IP addresses. auto const addr = beast::IP::Endpoint::from_string_checked(ip); - if (!addr) + if (addr) { - log << "Invalid value '" << ip << "' for key '" << field - << "' in [" << section.name() << "]"; - Throw(); - } - - if (is_unspecified(*addr)) - { - if (!allowAllIps) + if (is_unspecified(*addr)) { - log << addr->address() << " not allowed'" - << "' for key '" << field << "' in [" << section.name() - << "]"; - Throw(); + nets4.push_back( + boost::asio::ip::make_network_v4("0.0.0.0/0")); + nets6.push_back(boost::asio::ip::make_network_v6("::/0")); + // No reason to allow more IPs--it would be redundant. + break; + } + + // The configured address is a single IP (or else addr would + // be unset). We need this to be a subnet, so append + // the number of network bits to make a subnet of 1, + // depending on type. + v4 = addr->is_v4(); + std::string addressString = addr->to_string(); + if (v4) + { + addressString += "/32"; + v4Net = boost::asio::ip::make_network_v4(addressString); } else { - has_any = true; + addressString += "/128"; + v6Net = boost::asio::ip::make_network_v6(addressString); + } + } + else + { + // Since addr is empty, assume that the entry is + // for a subnet which includes trailing /0-32 or /0-128 + // depending on ip type. + // First, see if it's an ipv4 subnet. If not, try ipv6. + // If that throws, then there's nothing we can do with + // the entry. + try + { + v4Net = boost::asio::ip::make_network_v4(ip); + v4 = true; + } + catch (boost::system::system_error const& e) + { + v6Net = boost::asio::ip::make_network_v6(ip); + v4 = false; } } - if (has_any && !ips->empty()) + // Confirm that the address entry is the same as the subnet's + // underlying network address. + // 10.1.2.3/24 makes no sense. The underlying network address + // is 10.1.2.0/24. + if (v4) { - log << "IP specified along with " << addr->address() << " '" - << ip << "' for key '" << field << "' in [" - << section.name() << "]"; - Throw(); + if (v4Net != v4Net.canonical()) + { + log << "The configured subnet " << v4Net.to_string() + << " is not the same as the network address, which is " + << v4Net.canonical().to_string(); + Throw(); + } + nets4.push_back(v4Net); } - - auto const& address = addr->address(); - if (std::find_if( - admin_ip.begin(), - admin_ip.end(), - [&address](beast::IP::Address const& a) { - return address == a; - }) != admin_ip.end()) + else { - log << "IP specified for " << field << " is also for " - << "admin: " << ip << " in [" << section.name() << "]"; - Throw(); + if (v6Net != v6Net.canonical()) + { + log << "The configured subnet " << v6Net.to_string() + << " is not the same as the network address, which is " + << v6Net.canonical().to_string(); + Throw(); + } + nets6.push_back(v6Net); } - - ips->emplace_back(addr->address()); + } + catch (boost::system::system_error const& e) + { + log << "Invalid value '" << ip << "' for key '" << field << "' in [" + << section.name() << "]: " << e.what(); + Throw(); } } } @@ -232,14 +292,13 @@ parse_Port(ParsedPort& port, Section const& section, std::ostream& log) } } - populate(section, "admin", log, port.admin_ip, true, {}); + populate(section, "admin", log, port.admin_nets_v4, port.admin_nets_v6); populate( section, "secure_gateway", log, - port.secure_gateway_ip, - false, - port.admin_ip.value_or(std::vector{})); + port.secure_gateway_nets_v4, + port.secure_gateway_nets_v6); set(port.user, "user", section); set(port.password, "password", section); diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index 09685d32cb..45afaf6cb3 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -810,11 +810,11 @@ trustthesevalidators.gov ParsedPort rpc; if (!unexcept([&]() { parse_Port(rpc, conf["port_rpc"], log); })) return; - BEAST_EXPECT(rpc.admin_ip && (rpc.admin_ip.value().size() == 2)); + BEAST_EXPECT(rpc.admin_nets_v4.size() + rpc.admin_nets_v6.size() == 2); ParsedPort wss; if (!unexcept([&]() { parse_Port(wss, conf["port_wss_admin"], log); })) return; - BEAST_EXPECT(wss.admin_ip && (wss.admin_ip.value().size() == 1)); + BEAST_EXPECT(wss.admin_nets_v4.size() + wss.admin_nets_v6.size() == 1); } void diff --git a/src/test/jtx/envconfig.h b/src/test/jtx/envconfig.h index 755a6aa869..59b881539e 100644 --- a/src/test/jtx/envconfig.h +++ b/src/test/jtx/envconfig.h @@ -84,6 +84,10 @@ std::unique_ptr no_admin(std::unique_ptr); std::unique_ptr secure_gateway(std::unique_ptr); +std::unique_ptr admin_localnet(std::unique_ptr); + +std::unique_ptr secure_gateway_localnet(std::unique_ptr); + /// @brief adjust configuration with params needed to be a validator /// /// this is intended for use with envconfig, as in diff --git a/src/test/jtx/impl/envconfig.cpp b/src/test/jtx/impl/envconfig.cpp index b29e13ab8a..f34a444fa5 100644 --- a/src/test/jtx/impl/envconfig.cpp +++ b/src/test/jtx/impl/envconfig.cpp @@ -83,6 +83,24 @@ secure_gateway(std::unique_ptr cfg) return cfg; } +std::unique_ptr +admin_localnet(std::unique_ptr cfg) +{ + (*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 +secure_gateway_localnet(std::unique_ptr 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"); + return cfg; +} + auto constexpr defaultseed = "shUwVw52ofnCUX5m7kPTKzJdr4HEH"; std::unique_ptr diff --git a/src/test/rpc/Roles_test.cpp b/src/test/rpc/Roles_test.cpp index a56120740c..079fd2f9f2 100644 --- a/src/test/rpc/Roles_test.cpp +++ b/src/test/rpc/Roles_test.cpp @@ -269,6 +269,30 @@ class Roles_test : public beast::unit_test::suite BEAST_EXPECT(rpcRes["ip"] == "::11:22:33:44:45.55.65.75"); BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString())); } + + { + Env env{*this, envconfig(admin_localnet)}; + BEAST_EXPECT(env.rpc("ping")["result"]["role"] == "admin"); + BEAST_EXPECT(makeWSClient(env.app().config()) + ->invoke("ping")["result"]["unlimited"] + .asBool()); + } + + { + Env env{*this, envconfig(secure_gateway_localnet)}; + BEAST_EXPECT(env.rpc("ping")["result"]["role"] == "proxied"); + auto wsRes = + makeWSClient(env.app().config())->invoke("ping")["result"]; + BEAST_EXPECT( + !wsRes.isMember("unlimited") || !wsRes["unlimited"].asBool()); + + std::unordered_map headers; + headers["X-Forwarded-For"] = "12.34.56.78"; + Json::Value rpcRes = env.rpc(headers, "ping")["result"]; + BEAST_EXPECT(rpcRes["role"] == "proxied"); + BEAST_EXPECT(rpcRes["ip"] == "12.34.56.78"); + BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString())); + } } void