Skip to content

fix(plugin): support ipv6 in RealIpResolver - #2598

Open
Hespethorn wants to merge 1 commit into
drogonframework:masterfrom
Hespethorn:fix/realipresolver-ipv6
Open

Hespethorn wants to merge 1 commit into
drogonframework:masterfrom
Hespethorn:fix/realipresolver-ipv6

Conversation

@Hespethorn

Copy link
Copy Markdown

Closes #2596.

RealIpResolver only handled ipv4, which the header documented as
@note This plugin currently supports only ipv4 address or cidr. In a
dual-stack deployment this failed silently in two ways: ipv6 entries in
trust_ips were rejected with a misleading error, and ipv6 entries in
X-Forwarded-For were dropped, making the plugin fall back to the reverse
proxy's address.

What changed

CIDR now stores the network address as a network-order byte string plus a
prefix length, instead of a 32-bit in_addr_t + mask.

struct CIDR
{
    explicit CIDR(const std::string &ipOrCidr);
    std::string network_;     // 4 bytes for ipv4, 16 for ipv6
    uint16_t prefixLen_{32};  // 0-32 for ipv4, 0-128 for ipv6
};

matchCidr() compares the leading prefixLen_ bits of the address bytes
(trantor::InetAddress::toIpNetEndian() returns 4 bytes for ipv4 and 16 for
ipv6, in network byte order). Different byte-string lengths mean the two
address families can never match, so no family branch is needed.

CIDR::CIDR() now accepts ipv6 addresses and prefix lengths up to 128. The
InetAddress string constructor does not detect the address family - isIpV6_
comes straight from its third argument - so an ipv6 literal is first tried as
ipv4, fails inet_pton(AF_INET, ...) and comes back unspecified, rather than
raising the ipv6-specific error. The constructor therefore retries as ipv6
before rejecting an entry, which also makes the
"Ipv6 is not supported by RealIpResolver." throw (unreachable in practice)
go away.

parseAddress() handles the shapes that occur in X-Forwarded-For:

entry result
1.2.3.4 ipv4, no port
1.2.3.4:5678 ipv4 + port (exactly one colon)
[2001:db8::1]:5678 bracketed ipv6 + port
[2001:db8::1] bracketed ipv6, no port
2001:db8::1 bare ipv6, no port (several colons)

Unparseable entries still yield an unspecified address, which the existing
loop already skips, so the previous behaviour for malformed input is preserved.

XForwardedForParser needed no change - it only splits on space and comma, so
ipv6 entries already come through intact.

Backward compatibility

The ipv4 path is bit-identical to the previous implementation for every prefix
from 1 to 32, so no existing trust_ips configuration changes behaviour.
Verified by translating both the old masked comparison and the new byte-prefix
comparison into a script and comparing them per prefix over large random
samples: zero disagreements for prefixes 1-32.

A zero prefix length is handled explicitly, so the implementation no longer
relies on 0xffffffffu << 32, which is undefined behaviour.

Tests

Extended RealIpResolverTest from 5 to 14 cases (42 CHECK + 14 REQUIRE
assertions, 56 in total), adding ipv6 coverage to a trust_ips list that mixes
ipv4 and ipv6 entries:

  • bare ipv6, [ipv6]:port and [ipv6] parsing
  • an ipv6 entry inside a trusted cidr is skipped
  • a fully trusted chain falls back to the peer address
  • mixed ipv4/ipv6 header
  • ipv4-mapped ipv6 (::ffff:4.4.4.4)
  • a prefix length that is not a multiple of 8, checked in both directions
    (/33 - one address inside, one outside)

All 14 cases pass, and the five pre-existing ipv4 cases are unchanged.

Notes

  • Reported 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).
  • friend class Hodor uses CIDR(const std::string &), the CIDRs type and
    RealIpResolver::matchCidr. All three public signatures are unchanged, so
    Hodor compiles without modification.

RealIpResolver only handled ipv4. In a dual-stack deployment, ipv6 entries
in trust_ips were rejected as malformed ipv4, and ipv6 entries in
X-Forwarded-For were silently dropped, causing the plugin to fall back to
the reverse proxy's address.

Store the CIDR network address as a network-order byte string plus a prefix
length (0-32 for ipv4, 0-128 for ipv6) instead of in_addr_t + mask, and
compare the leading prefix bits. parseAddress now handles bare ipv6,
bracketed ipv6 with and without a port, and ipv4 with a port.

The ipv4 path is bit-identical for prefixes 1-32, so no existing
configuration changes behaviour. A zero prefix length is now handled
explicitly, so the implementation no longer relies on 0xffffffffu << 32,
which is undefined behaviour.

Closes drogonframework#2596

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant