Improve handling of the X-Forwarded-For and Forwarded HTTP headers

src/ripple/rpc/impl/Role.cpp

@@ -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;
+}
+
 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(),
@@ -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 {};

src/test/rpc/Roles_test.cpp

@@ -20,9 +20,12 @@
 #include <ripple/beast/unit_test.h>
 #include <ripple/protocol/ErrorCodes.h>
 #include <ripple/protocol/jss.h>
-#include <string>
 #include <test/jtx.h>
 #include <test/jtx/WSClient.h>
+
+#include <boost/asio/ip/address_v4.hpp>
+
+#include <string>
 #include <unordered_map>
 
 namespace ripple {
@@ -31,6 +34,14 @@ namespace test {
 
 class Roles_test : public beast::unit_test::suite
 {
+    bool
+    isValidIpAddress(std::string const& addr)
+    {
+        boost::system::error_code ec;
+        boost::asio::ip::make_address(addr, ec);
+        return !ec.failed();
+    }
+
     void
     testRoles()
     {
@@ -63,31 +74,65 @@ class Roles_test : public beast::unit_test::suite
                 !wsRes.isMember("unlimited") || !wsRes["unlimited"].asBool());
 
             std::unordered_map<std::string, std::string> headers;
+            Json::Value rpcRes;
+
+            // IPv4 tests.
             headers["X-Forwarded-For"] = "12.34.56.78";
-            auto rpcRes = env.rpc(headers, "ping")["result"];
+            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()));
 
             headers["X-Forwarded-For"] = "87.65.43.21, 44.33.22.11";
             rpcRes = env.rpc(headers, "ping")["result"];
             BEAST_EXPECT(rpcRes["ip"] == "87.65.43.21");
-            headers.erase("X-Forwarded-For");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
 
+            headers["X-Forwarded-For"] = "87.65.43.21:47011, 44.33.22.11";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["ip"] == "87.65.43.21");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers = {};
             headers["Forwarded"] = "for=88.77.66.55";
             rpcRes = env.rpc(headers, "ping")["result"];
             BEAST_EXPECT(rpcRes["ip"] == "88.77.66.55");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
 
             headers["Forwarded"] =
                 "what=where;for=55.66.77.88;for=nobody;"
                 "who=3";
             rpcRes = env.rpc(headers, "ping")["result"];
             BEAST_EXPECT(rpcRes["ip"] == "55.66.77.88");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
 
             headers["Forwarded"] =
-                "what=where;for=55.66.77.88, 99.00.11.22;"
+                "what=where; for=55.66.77.88, for=99.00.11.22;"
                 "who=3";
             rpcRes = env.rpc(headers, "ping")["result"];
             BEAST_EXPECT(rpcRes["ip"] == "55.66.77.88");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["Forwarded"] =
+                "what=where; For=99.88.77.66, for=55.66.77.88;"
+                "who=3";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["ip"] == "99.88.77.66");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["Forwarded"] =
+                "what=where; for=\"55.66.77.88:47011\";"
+                "who=3";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["ip"] == "55.66.77.88");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["Forwarded"] =
+                "what=where; For= \" 99.88.77.66 \" ,for=11.22.33.44;"
+                "who=3";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["ip"] == "99.88.77.66");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
 
             wsRes = makeWSClient(env.app().config(), true, 2, headers)
                         ->invoke("ping")["result"];
@@ -99,10 +144,218 @@ class Roles_test : public beast::unit_test::suite
             rpcRes = env.rpc(headers, "ping")["result"];
             BEAST_EXPECT(rpcRes["role"] == "identified");
             BEAST_EXPECT(rpcRes["username"] == name);
-            BEAST_EXPECT(rpcRes["ip"] == "55.66.77.88");
+            BEAST_EXPECT(rpcRes["ip"] == "99.88.77.66");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
             wsRes = makeWSClient(env.app().config(), true, 2, headers)
                         ->invoke("ping")["result"];
             BEAST_EXPECT(wsRes["unlimited"].asBool());
+
+            // IPv6 tests.
+            headers = {};
+            headers["X-Forwarded-For"] =
+                "2001:db8:3333:4444:5555:6666:7777:8888";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:7777:8888");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["X-Forwarded-For"] =
+                "2001:db8:3333:4444:5555:6666:7777:9999, a:b:c:d:e:f, "
+                "g:h:i:j:k:l";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:7777:9999");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["X-Forwarded-For"] =
+                "[2001:db8:3333:4444:5555:6666:7777:8888]";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:7777:8888");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["X-Forwarded-For"] =
+                "[2001:db8:3333:4444:5555:6666:7777:9999], [a:b:c:d:e:f], "
+                "[g:h:i:j:k:l]";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:7777:9999");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers = {};
+            headers["Forwarded"] =
+                "for=\"[2001:db8:3333:4444:5555:6666:7777:aaaa]\"";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:7777:aaaa");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["Forwarded"] =
+                "For=\"[2001:db8:bb:cc:dd:ee:ff::]:2345\", for=99.00.11.22";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(rpcRes["ip"] == "2001:db8:bb:cc:dd:ee:ff::");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["Forwarded"] =
+                "proto=http;FOR=\"[2001:db8:11:22:33:44:55:66]\""
+                ";by=203.0.113.43";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(rpcRes["ip"] == "2001:db8:11:22:33:44:55:66");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            // IPv6 (dual) tests.
+            headers = {};
+            headers["X-Forwarded-For"] = "2001:db8:3333:4444:5555:6666:1.2.3.4";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:1.2.3.4");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["X-Forwarded-For"] =
+                "2001:db8:3333:4444:5555:6666:5.6.7.8, a:b:c:d:e:f, "
+                "g:h:i:j:k:l";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:5.6.7.8");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["X-Forwarded-For"] =
+                "[2001:db8:3333:4444:5555:6666:9.10.11.12]";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:9.10.11.12");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["X-Forwarded-For"] =
+                "[2001:db8:3333:4444:5555:6666:13.14.15.16], [a:b:c:d:e:f], "
+                "[g:h:i:j:k:l]";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:13.14.15.16");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers = {};
+            headers["Forwarded"] =
+                "for=\"[2001:db8:3333:4444:5555:6666:20.19.18.17]\"";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(
+                rpcRes["ip"] == "2001:db8:3333:4444:5555:6666:20.19.18.17");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["Forwarded"] =
+                "For=\"[2001:db8:bb:cc::24.23.22.21]\", for=99.00.11.22";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(rpcRes["ip"] == "2001:db8:bb:cc::24.23.22.21");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+
+            headers["Forwarded"] =
+                "proto=http;FOR=\"[::11:22:33:44:45.55.65.75]:234\""
+                ";by=203.0.113.43";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "proxied");
+            BEAST_EXPECT(rpcRes["ip"] == "::11:22:33:44:45.55.65.75");
+            BEAST_EXPECT(isValidIpAddress(rpcRes["ip"].asString()));
+        }
+    }
+
+    void
+    testInvalidIpAddresses()
+    {
+        using namespace test::jtx;
+
+        {
+            Env env(*this);
+
+            std::unordered_map<std::string, std::string> headers;
+            Json::Value rpcRes;
+
+            // No "for=" in Forwarded.
+            headers["Forwarded"] = "for 88.77.66.55";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            headers["Forwarded"] = "by=88.77.66.55";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            // Empty field.
+            headers = {};
+            headers["Forwarded"] = "for=";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            headers = {};
+            headers["X-Forwarded-For"] = "     ";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            // Empty quotes.
+            headers = {};
+            headers["Forwarded"] = "for= \"    \" ";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            headers = {};
+            headers["X-Forwarded-For"] = "\"\"";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            // Unbalanced outer quotes.
+            headers = {};
+            headers["X-Forwarded-For"] = "\"12.34.56.78   ";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            headers["X-Forwarded-For"] = "12.34.56.78\"";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            // Unbalanced square brackets for IPv6.
+            headers = {};
+            headers["Forwarded"] = "FOR=[2001:db8:bb:cc::";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            headers = {};
+            headers["X-Forwarded-For"] = "2001:db8:bb:cc::24.23.22.21]";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            // Empty square brackets.
+            headers = {};
+            headers["Forwarded"] = "FOR=[]";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
+
+            headers = {};
+            headers["X-Forwarded-For"] = "\"  [      ]  \"";
+            rpcRes = env.rpc(headers, "ping")["result"];
+            BEAST_EXPECT(rpcRes["role"] == "admin");
+            BEAST_EXPECT(!rpcRes.isMember("ip"));
         }
     }
 
@@ -111,6 +364,7 @@ public:
     run() override
     {
         testRoles();
+        testInvalidIpAddresses();
     }
 };