fix(indexer): bucket rate-limit key by IPv6 /64 (#282) #776

Merged

Brouie merged 1 commit from qa/282-ratelimit-ipv6-bucket into main

2026-06-05 07:16:24 +00:00

Brouie commented

2026-06-05 06:59:31 +00:00

(Migrated from gitlab.com)

Implemented per your call ("we do not have a CG-Connecting-IP or X-Forwarded-For ... proceed with the implementation for rate limiting IP addresses").

Since there's no trusted forwarded header, I kept the key on the socket peer IP (a forwarded-header extractor would only add a spoofable bucket key on a wrong trust boundary). The remaining gap was the default PeerIpKeyExtractor keying on the FULL IPv6 address — a client can rotate through its own /64 (the smallest block a host gets) to multiply its limit. So I added a custom KeyExtractor that collapses IPv6 to its /64 prefix; IPv4 keeps the full address. It goes through the single apply_rate_limit_layer chokepoint, so both the general limiter and the lcd-heavy one (#278) get it.

Tests: rate_limit_key_tests — two addresses in the same /64 share one key, different /64s and IPv4 stay independent, and the extractor reads ConnectInfo and buckets by /64. The existing burst->429 integration tests (lcd_heavy_route_rate_limit_returns_429, cg_cmc_orderbook_lcd_heavy_rate_limit_returns_429, rate_limit_returns_429_when_exceeded) stay green.

If you later put it behind a proxy that DOES inject a trusted client-IP header, that's a one-line extractor swap — flag me and I'll wire SmartIp with the trusted hop.

Implemented per your call ("we do not have a CG-Connecting-IP or X-Forwarded-For ... proceed with the implementation for rate limiting IP addresses"). Since there's no trusted forwarded header, I kept the key on the socket peer IP (a forwarded-header extractor would only add a spoofable bucket key on a wrong trust boundary). The remaining gap was the default PeerIpKeyExtractor keying on the FULL IPv6 address — a client can rotate through its own /64 (the smallest block a host gets) to multiply its limit. So I added a custom KeyExtractor that collapses IPv6 to its /64 prefix; IPv4 keeps the full address. It goes through the single apply_rate_limit_layer chokepoint, so both the general limiter and the lcd-heavy one (#278) get it. Tests: rate_limit_key_tests — two addresses in the same /64 share one key, different /64s and IPv4 stay independent, and the extractor reads ConnectInfo and buckets by /64. The existing burst->429 integration tests (lcd_heavy_route_rate_limit_returns_429, cg_cmc_orderbook_lcd_heavy_rate_limit_returns_429, rate_limit_returns_429_when_exceeded) stay green. If you later put it behind a proxy that DOES inject a trusted client-IP header, that's a one-line extractor swap — flag me and I'll wire SmartIp with the trusted hop.

Brouie commented

2026-06-05 06:59:47 +00:00

(Migrated from gitlab.com)

mentioned in issue #282

PlasticDigits commented

2026-06-05 07:14:31 +00:00

(Migrated from gitlab.com)

Security review

Commit reviewed: 5b236b2a4d0273ba34749d3b2ae3bf88832cb6cc
Scope: indexer/src/api/mod.rs — custom PeerIpV6Slash64KeyExtractor + rate_limit_ip_key() wired into apply_rate_limit_layer (covers global and LCD-heavy governors).

Outcome: FINDINGS: 0 medium+

Summary

This MR closes the IPv6 /64 rotation bypass described in #282 for a directly exposed deployment (peer-IP keying, no spoofable forwarded-header extractor). The extractor reads ConnectInfo<SocketAddr> from the TCP accept path (not client-controlled HTTP headers), and GovernorError::UnableToExtractKey fails closed with HTTP 500 per tower_governor defaults — not a bypass.

Checked attack paths (no medium+ issues):

Rate-limit bypass via header spoofing: Not applicable; key is socket peer IP only.
IPv6 host-bit rotation within /64: Collapsed to network prefix — fix is correct for stated topology.
Missing ConnectInfo: Returns 500, does not pass requests through; production serve() always uses into_make_service_with_connect_info.
IPv4-mapped IPv6 (::ffff:…) on dual-stack bind: Would collapse all mapped v4 peers to :: (shared bucket). Default API_BIND=127.0.0.1 is IPv4-only in this repo; not treated as a confirmed prod exposure without evidence of :: bind.

Existing integration tests (security.rs 429 burst cases) and new unit tests (rate_limit_key_tests) provide adequate coverage for the changed keying logic.

Inline threads: none (no medium+ findings).

## Security review **Commit reviewed:** `5b236b2a4d0273ba34749d3b2ae3bf88832cb6cc` **Scope:** `indexer/src/api/mod.rs` — custom `PeerIpV6Slash64KeyExtractor` + `rate_limit_ip_key()` wired into `apply_rate_limit_layer` (covers global and LCD-heavy governors). **Outcome:** `FINDINGS: 0` medium+ ### Summary This MR closes the IPv6 /64 rotation bypass described in #282 for a directly exposed deployment (peer-IP keying, no spoofable forwarded-header extractor). The extractor reads `ConnectInfo<SocketAddr>` from the TCP accept path (not client-controlled HTTP headers), and `GovernorError::UnableToExtractKey` fails closed with HTTP 500 per tower_governor defaults — not a bypass. **Checked attack paths (no medium+ issues):** - **Rate-limit bypass via header spoofing:** Not applicable; key is socket peer IP only. - **IPv6 host-bit rotation within /64:** Collapsed to network prefix — fix is correct for stated topology. - **Missing ConnectInfo:** Returns 500, does not pass requests through; production `serve()` always uses `into_make_service_with_connect_info`. - **IPv4-mapped IPv6 (`::ffff:…`) on dual-stack bind:** Would collapse all mapped v4 peers to `::` (shared bucket). Default `API_BIND=127.0.0.1` is IPv4-only in this repo; not treated as a confirmed prod exposure without evidence of `::` bind. Existing integration tests (`security.rs` 429 burst cases) and new unit tests (`rate_limit_key_tests`) provide adequate coverage for the changed keying logic. **Inline threads:** none (no medium+ findings).

PlasticDigits commented

2026-06-05 07:16:25 +00:00

(Migrated from gitlab.com)

mentioned in commit e70f2f9e75

mentioned in commit e70f2f9e7515267b73116a46f74151ffb99e43ef