Skip to content

RealIpResolver: IPv6 not supported — v6 trust_ips fails with a misleading error, v6 X-Forwarded-For is silently dropped #2596

Description

@Hespethorn

Summary

drogon::plugin::RealIpResolver only handles IPv4. This is documented in the header:

/**
 * @note This plugin currently supports only ipv4 address or cidr.
 */

Two distinct problems appear once a deployment goes dual-stack — and neither of them fails loudly.

1. IPv6 in trust_ips fails with a misleading error

CIDR::CIDR builds a trantor address with the default ipv6 = false:

// lib/src/RealIpResolver.cc
trantor::InetAddress addr(ipv4, 0);   // ipv6 defaults to false
if (addr.isIpV6())
{
    throw std::runtime_error("Ipv6 is not supported by RealIpResolver.");
}
if (addr.isUnspecified())
{
    throw std::runtime_error("Bad ipv4 address: " + ipv4);
}

But trantor's string constructor does not auto-detect the address family — isIpV6_ is taken straight from the argument:

// trantor/net/InetAddress.cc
InetAddress::InetAddress(const std::string &ip, uint16_t port, bool ipv6)
    : isIpV6_(ipv6)   // <-- comes from the parameter, not from the string
{
    ...
    if (::inet_pton(AF_INET, ip.c_str(), &addr_.sin_addr) <= 0)
    {
        return;   // leaves isUnspecified_ == true
    }
    isUnspecified_ = false;
}

So with "trust_ips": ["2001:db8::1"], isIpV6() is false, the dedicated "Ipv6 is not supported" branch is never taken, and the user gets:

Bad ipv4 address: 2001:db8::1

The message says the address is malformed, when it is in fact valid IPv6 that the plugin simply does not support. (The trantor header even documents that constructor as @param ip A IPv4 or IPv6 address., which makes the auto-detect assumption easy to make.)

2. IPv6 entries in X-Forwarded-For are silently discarded

parseAddress() splits host and port with find(':'), which collides with IPv6 syntax:

auto pos = addr.find(':');              // "2001:db8::1" -> pos == 4
...
port = std::stoi(addr.substr(pos + 1)); // stoi("db8::1") throws -> port = 0
...
return trantor::InetAddress(addr.substr(0, pos), port);   // InetAddress("2001", 0)

InetAddress("2001", 0) fails inet_pton(AF_INET, ...) and stays isUnspecified() == true. Back in the parsing loop:

while (!(ip = parser.getNext()).empty())
{
    trantor::InetAddress addr = parseAddress(ip);
    if (addr.isUnspecified() || matchCidr(addr, trustCIDRs_))
    {
        continue;   // <-- IPv6 entries are dropped here
    }
    req->attributes()->insert(attributeKey_, addr);
    return;
}
// No match, use peerAddr
req->attributes()->insert(attributeKey_, peerAddr);

Every IPv6 entry falls through to the "no match" path, and the plugin stores the TCP peer address — i.e. the reverse proxy's address.

The impact has the same shape as a misconfigured trust_ips: behind a reverse proxy all IPv6 clients collapse onto a single key. Any per-IP limiting built on GetRealAddr() (registration / login throttling, for example) then treats the entire IPv6 population as one client, while the logs look completely normal. In a dual-stack deployment that is a large share of real traffic, not a corner case.

matchCidr() is IPv4-only as well — addr.ipNetEndian() returns a 32-bit in_addr_t — so a v6 peer can never match a trusted CIDR even before parsing is considered.

Suggested direction

trantor already exposes what is needed:

bool isIpV6() const;
uint32_t ipNetEndian() const;            // v4
const uint32_t *ip6NetEndian() const;    // v6: 4 x uint32 = 16 bytes, net endian

so the change stays contained:

  • CIDR — store the address as trantor::InetAddress (or std::array<uint8_t, 16> + uint8_t prefixLen); allow prefixes up to 128 for v6 and 32 for v4.
  • matchCidr() — branch on addr.isIpV6(), compare 16 bytes through ip6NetEndian() for v6, and leave the existing 32-bit path byte-for-byte identical for v4.
  • parseAddress() — handle the shapes that actually occur in XFF: 1.2.3.4, 1.2.3.4:5678, [2001:db8::1]:5678, and bare 2001:db8::1.
  • XForwardedForParser — no change needed; it only splits on space and comma, so v6 addresses already come through intact.

One constraint worth flagging: friend class Hodor means Hodor uses CIDR(const std::string &), the CIDRs type and RealIpResolver::matchCidr. Keep those three public signatures as they are and Hodor compiles unchanged. (Its own trust_ips handling inherits the same v4-only limitation.)

If this direction looks acceptable, I'd be glad to prepare a PR — including RealIpResolverTest cases for v6 CIDR matching, v6 parsing, mixed v4/v6 trust_ips, and a v4 regression run. Or would you rather see it approached differently, e.g. dropping in_addr_t altogether in favour of InetAddress?

Notes

  • Reporting this as a feature gap rather than a regression: nothing here has changed since the plugin was merged in Resolve real ip from HttpRequest. #1321 (2022-07). v1.9.10 and master are identical in this area.
  • I checked for existing work first — no open PR or issue covers IPv6 for this plugin.
  • Everything above comes from reading lib/src/RealIpResolver.cc and trantor/net/InetAddress.{h,cc}. I have not run a reproduction, so please treat the exact error strings as derived rather than observed. Happy to attach a failing test if that would help.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions