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.

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

Improved handling for HTTP X-Forwarded-For and Forwarded #4009

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
146 changes: 130 additions & 16 deletions src/ripple/rpc/impl/Role.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,18 +122,125 @@ requestInboundEndpoint(
remoteAddress, role == Role::PROXY, forwardedFor);
}

static boost::string_view
extractIpAddrFromField(boost::string_view field)
{
// Lambda to trim leading and trailing spaces on the field.
auto trim = [](boost::string_view str) -> boost::string_view {
boost::string_view ret = str;

// Only do the work if there's at least one leading space.
if (!ret.empty() && ret.front() == ' ')
{
std::size_t const firstNonSpace = ret.find_first_not_of(' ');
if (firstNonSpace == boost::string_view::npos)
// We know there's at least one leading space. So if we got
// npos, then it must be all spaces. Return empty string_view.
return {};

ret = ret.substr(firstNonSpace);
}
// Trim trailing spaces.
if (!ret.empty())
{
// Only do the work if there's at least one trailing space.
if (unsigned char const c = ret.back();
c == ' ' || c == '\r' || c == '\n')
{
std::size_t const lastNonSpace = ret.find_last_not_of(" \r\n");
if (lastNonSpace == boost::string_view::npos)
// We know there's at least one leading space. So if we
// got npos, then it must be all spaces.
return {};

ret = ret.substr(0, lastNonSpace + 1);
}
}
return ret;
};

boost::string_view ret = trim(field);
if (ret.empty())
return {};

// If there are surrounding quotes, strip them.
if (ret.front() == '"')
{
ret.remove_prefix(1);
if (ret.empty() || ret.back() != '"')
return {}; // Unbalanced double quotes.

ret.remove_suffix(1);

// Strip leading and trailing spaces that were inside the quotes.
ret = trim(ret);
}
if (ret.empty())
return {};

// If we have an IPv6 or IPv6 (dual) address wrapped in square brackets,
// then we need to remove the square brackets.
if (ret.front() == '[')
{
// Remove leading '['.
ret.remove_prefix(1);

// We may have an IPv6 address in square brackets. Scan up to the
// closing square bracket.
auto const closeBracket =
std::find_if_not(ret.begin(), ret.end(), [](unsigned char c) {
return std::isxdigit(c) || c == ':' || c == '.' || c == ' ';
});

// If the string does not close with a ']', then it's not valid IPv6
// or IPv6 (dual).
if (closeBracket == ret.end() || (*closeBracket) != ']')
return {};

// Remove trailing ']'
ret = ret.substr(0, closeBracket - ret.begin());
ret = trim(ret);
}
if (ret.empty())
return {};

// If this is an IPv6 address (after unwrapping from square brackets),
// then there cannot be an appended port. In that case we're done.
{
// Skip any leading hex digits.
auto const colon =
std::find_if_not(ret.begin(), ret.end(), [](unsigned char c) {
return std::isxdigit(c) || c == ' ';
});

// If the string starts with optional hex digits followed by a colon
// it's an IVv6 address. We're done.
if (colon == ret.end() || (*colon) == ':')
return ret;
}

// If there's a port appended to the IP address, strip that by
// terminating at the colon.
if (std::size_t colon = ret.find(':'); colon != boost::string_view::npos)
ret = ret.substr(0, colon);

return ret;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would boost::regex be better? It's certainly a bit less code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was a good question to ask. At this prompt I explored using boost::regex to do the work. I don't have a lot of experience with boost::regex, so maybe I'm missing something. But from what I can see boost::regex does not support boost::string_view very well if at all.

The problem is that ripple::forwardedFor() is designed to return a boost::string_view extracted from the passed in http_request_type const&. I believe this design is intended to avoid the memory allocations that are involved with std::string creation. After poking at boost::regex for a few hours I've been unable to locate the operations I would need to perform in order to get boost::regex to operate on and produce the needed boost::string_view. A few web searches helped to reinforce this suspicion.

Maybe you have more experience with boost::regex and can steer me in the right direction? Failing that, I'm afraid the current character manipulations are the best I can do.

I did not look into using std::regex since, disappointingly, std::regex is not portable across platforms.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. I didn't consider boost::string_view.

boost::string_view
forwardedFor(http_request_type const& request)
{
auto it = request.find(boost::beast::http::field::forwarded);
if (it != request.end())
// Look for the Forwarded field in the request.
if (auto it = request.find(boost::beast::http::field::forwarded);
it != request.end())
{
auto ascii_tolower = [](char c) -> char {
return ((static_cast<unsigned>(c) - 65U) < 26) ? c + 'a' - 'A' : c;
};

// Look for the first (case insensitive) "for="
static std::string const forStr{"for="};
auto found = std::search(
char const* found = std::search(
it->value().begin(),
it->value().end(),
forStr.begin(),
Expand All @@ -146,22 +253,29 @@ forwardedFor(http_request_type const& request)
return {};

found += forStr.size();
std::size_t const pos([&]() {
std::size_t const pos{
boost::string_view(found, it->value().end() - found).find(';')};
if (pos == boost::string_view::npos)
return it->value().size() - forStr.size();
return pos;
}());

return *boost::beast::http::token_list(boost::string_view(found, pos))
.begin();

// We found a "for=". Scan for the end of the IP address.
std::size_t const pos = [&found, &it]() {
std::size_t pos =
boost::string_view(found, it->value().end() - found)
.find_first_of(",;");
if (pos != boost::string_view::npos)
return pos;

return it->value().size() - forStr.size();
}();

return extractIpAddrFromField({found, pos});
}

it = request.find("X-Forwarded-For");
if (it != request.end())
// Look for the X-Forwarded-For field in the request.
if (auto it = request.find("X-Forwarded-For"); it != request.end())
{
return *boost::beast::http::token_list(it->value()).begin();
// The first X-Forwarded-For entry may be terminated by a comma.
std::size_t found = it->value().find(',');
if (found == boost::string_view::npos)
found = it->value().length();
return extractIpAddrFromField(it->value().substr(0, found));
}

return {};
Expand Down
Loading