API rate limiter keys on peer IP with no forwarded-header extractor

Brouie commented

2026-06-03 07:12:10 +00:00

(Migrated from gitlab.com)

Severity: Medium (depends on the deployment topology)
Reachability: Affects every client of the public API.
Affected: apply_rate_limit_layer (indexer/src/api/mod.rs).
Root cause: the rate limiter keys on the direct socket peer IP and never reads a forwarded-client header, so behind a reverse proxy every client shares one bucket.

Summary

apply_rate_limit_layer builds GovernorConfigBuilder::default(), whose default key extractor is the peer IP. .use_headers() is set, but that only adds X-RateLimit-* to responses — it does not make the extractor read X-Forwarded-For / a real-client header.

Two failure modes depending on how it's deployed:

Behind a reverse proxy (the normal prod setup): every request arrives from the proxy's IP, so the limiter sees one IP and throttles all clients into a single shared bucket — a self-inflicted global DoS, and one abuser can starve everyone.
Directly exposed: it works per-client, but an attacker rotating IPs (a single IPv6 /64 is plenty) sidesteps it.

Either way the limit isn't doing what it's meant to. This also undercuts the lcd-heavy throttle that other reports lean on.

Current codebase

mod.rs apply_rate_limit_layer: GovernorConfigBuilder::default().per_second(rps).burst_size(...).use_headers().finish() — default (peer-IP) key extractor, no forwarded-header extractor.

Recommended direction

Use a key extractor that reads the real client IP from a trusted proxy header (e.g. a SmartIp/forwarded extractor with the trusted proxy configured), matched to the actual deployment.
If the service is meant to be directly exposed, document that and consider per-/64 bucketing for IPv6.

Acceptance criteria

Behind the intended proxy, distinct clients get distinct buckets (verified with spoofed X-Forwarded-For from the trusted proxy only).
The trusted-proxy header is not honored from arbitrary upstreams.

Test plan (abuse)

case	expect
many clients via proxy	each limited independently, no shared-bucket collapse
forged forwarded header from untrusted source	ignored

**Severity:** Medium (depends on the deployment topology) **Reachability:** Affects every client of the public API. **Affected:** `apply_rate_limit_layer` (`indexer/src/api/mod.rs`). **Root cause:** the rate limiter keys on the direct socket peer IP and never reads a forwarded-client header, so behind a reverse proxy every client shares one bucket. ## Summary `apply_rate_limit_layer` builds `GovernorConfigBuilder::default()`, whose default key extractor is the peer IP. `.use_headers()` is set, but that only adds `X-RateLimit-*` to responses — it does not make the extractor read `X-Forwarded-For` / a real-client header. Two failure modes depending on how it's deployed: - **Behind a reverse proxy** (the normal prod setup): every request arrives from the proxy's IP, so the limiter sees one IP and throttles all clients into a single shared bucket — a self-inflicted global DoS, and one abuser can starve everyone. - **Directly exposed**: it works per-client, but an attacker rotating IPs (a single IPv6 /64 is plenty) sidesteps it. Either way the limit isn't doing what it's meant to. This also undercuts the lcd-heavy throttle that other reports lean on. ## Current codebase - `mod.rs` `apply_rate_limit_layer`: `GovernorConfigBuilder::default().per_second(rps).burst_size(...).use_headers().finish()` — default (peer-IP) key extractor, no forwarded-header extractor. ## Recommended direction 1. Use a key extractor that reads the real client IP from a trusted proxy header (e.g. a SmartIp/forwarded extractor with the trusted proxy configured), matched to the actual deployment. 2. If the service is meant to be directly exposed, document that and consider per-/64 bucketing for IPv6. ## Acceptance criteria - [ ] Behind the intended proxy, distinct clients get distinct buckets (verified with spoofed `X-Forwarded-For` from the trusted proxy only). - [ ] The trusted-proxy header is not honored from arbitrary upstreams. ## Test plan (abuse) | case | expect | |---|---| | many clients via proxy | each limited independently, no shared-bucket collapse | | forged forwarded header from untrusted source | ignored |

PlasticDigits commented

2026-06-03 10:49:37 +00:00

(Migrated from gitlab.com)

Not sure but we are planning to deploy on render which uses cloudflare - in the past ip detection has not been an issue - but need to investigate specifically how this is handled. Research requried

Brouie commented

2026-06-04 06:30:07 +00:00

(Migrated from gitlab.com)

Holding this on your infra call, per your note — quick confirmation of why it can't be implemented blind.

The limiter keys on the socket peer IP (tower_governor PeerIpKeyExtractor default; into_make_service_with_connect_info::<SocketAddr>), no forwarded-header extractor. The fix is small in code (swap in a SmartIp/XFF key extractor) but its SECURITY correctness depends entirely on the trusted-proxy boundary: on Render-behind-Cloudflare every request arrives from a small set of CF/Render egress IPs, which is exactly the shared-bucket-collapse this finding warns about — so that topology makes the risk MORE likely, not less. And trusting a forwarded header from the wrong hop lets anyone spoof their bucket key, which is worse than today.

So: I need from you the client-IP propagation Render/Cloudflare actually uses (CF-Connecting-IP vs X-Forwarded-For, and which hop is trusted) before wiring the extractor. Once you confirm the trusted hop I'll do the (small) code change + an IPv6 /64 bucket. This pairs with #278 (both about whether the public API throttle is actually effective per-client). No code from me until the topology is pinned. @PlasticDigits

Holding this on your infra call, per your note — quick confirmation of why it can't be implemented blind. The limiter keys on the socket peer IP (tower_governor `PeerIpKeyExtractor` default; `into_make_service_with_connect_info::<SocketAddr>`), no forwarded-header extractor. The fix is small in code (swap in a SmartIp/XFF key extractor) but its SECURITY correctness depends entirely on the trusted-proxy boundary: on Render-behind-Cloudflare every request arrives from a small set of CF/Render egress IPs, which is exactly the shared-bucket-collapse this finding warns about — so that topology makes the risk MORE likely, not less. And trusting a forwarded header from the wrong hop lets anyone spoof their bucket key, which is worse than today. So: I need from you the client-IP propagation Render/Cloudflare actually uses (`CF-Connecting-IP` vs `X-Forwarded-For`, and which hop is trusted) before wiring the extractor. Once you confirm the trusted hop I'll do the (small) code change + an IPv6 /64 bucket. This pairs with #278 (both about whether the public API throttle is actually effective per-client). No code from me until the topology is pinned. @PlasticDigits

PlasticDigits commented

2026-06-05 03:54:21 +00:00

(Migrated from gitlab.com)

We do not have a CG-Connecting-IP or X-Forwarded-For as far as Im aware.
Proceed with the implementation for rate limiting IP addresses

We do not have a CG-Connecting-IP or X-Forwarded-For as far as Im aware. Proceed with the implementation for rate limiting IP addresses

Brouie commented

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

(Migrated from gitlab.com)

mentioned in merge request !776

Brouie commented

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

(Migrated from gitlab.com)

Done per your call (no CG-Connecting-IP / X-Forwarded-For -> rate limit on IP). MR !776.

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 the wrong trust boundary. The gap left was the default extractor keying on the FULL IPv6 address, so a client can rotate through its own /64 (smallest block a host gets) to multiply its limit. Added a custom KeyExtractor that collapses IPv6 to its /64 prefix; IPv4 keeps the full address. It runs 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 (same /64 -> one key; different /64 and IPv4 -> distinct; the extractor reads ConnectInfo and buckets by /64). Existing burst->429 integration tests stay green.

If the deployment later sits behind a proxy that injects a trusted client-IP header, it's a one-line swap to a SmartIp extractor with the trusted hop — flag me. @PlasticDigits

Done per your call (no CG-Connecting-IP / X-Forwarded-For -> rate limit on IP). MR !776. 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 the wrong trust boundary. The gap left was the default extractor keying on the FULL IPv6 address, so a client can rotate through its own /64 (smallest block a host gets) to multiply its limit. Added a custom KeyExtractor that collapses IPv6 to its /64 prefix; IPv4 keeps the full address. It runs 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 (same /64 -> one key; different /64 and IPv4 -> distinct; the extractor reads ConnectInfo and buckets by /64). Existing burst->429 integration tests stay green. If the deployment later sits behind a proxy that injects a trusted client-IP header, it's a one-line swap to a SmartIp extractor with the trusted hop — flag me. @PlasticDigits

PlasticDigits commented

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

(Migrated from gitlab.com)

mentioned in commit e70f2f9e75

mentioned in commit e70f2f9e7515267b73116a46f74151ffb99e43ef

PlasticDigits commented

2026-06-05 09:37:54 +00:00

(Migrated from gitlab.com)

Approved, but we should just disable ipv6 by default as it seems to be introducing uncessary risks, and ipv4 is quite cheap anyway

ghost1 commented

2026-06-05 09:42:39 +00:00

(Migrated from gitlab.com)

mentioned in commit 17e7cfb74f

mentioned in commit 17e7cfb74f5ed7f550c131e412793e517effeaba

PlasticDigits commented

2026-06-05 09:43:00 +00:00

(Migrated from gitlab.com)

mentioned in merge request !779

PlasticDigits commented

2026-06-05 09:43:10 +00:00

(Migrated from gitlab.com)

Implementation complete — MR !779

Per your direction (no CF-Connecting-IP / X-Forwarded-For; disable IPv6 by default instead of /64 bucketing):

What changed

Rate limiter keys on socket peer IP via PeerIpKeyExtractor — no forwarded-header extractor (would be spoofable without a trusted hop).
API listener is IPv4-only by default (API_IPV6_ENABLED off); IPv6 API_BIND rejected at config parse.
Docs updated: docs/indexer-invariants.md, skills/AGENTS_INDEXER_API_LCD_SECURITY.md.

Acceptance criteria

Criterion	Result	How verified
Distinct clients via proxy (XFF from trusted proxy)	SKIP	No trusted forwarded header in this deployment; peer IP is the client on direct Render exposure. Distinct IPv4 peers get distinct buckets: `cargo test --lib peer_ip_extractor_keys_distinct_ipv4_peers`
Forged forwarded header from untrusted source ignored	PASS	`cargo test --lib peer_ip_extractor_ignores_forwarded_headers`
IPv6 /64 rotation abuse mitigated	PASS	IPv6 disabled by default; `cargo test --lib api_listener` + `cargo test --lib ipv6_bind_rejected`

Full lib suite: cd indexer && cargo test --lib — 105 passed.

MR: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/61
Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/282

## Implementation complete — MR !779 Per your direction (no `CF-Connecting-IP` / `X-Forwarded-For`; disable IPv6 by default instead of /64 bucketing): ### What changed - Rate limiter keys on **socket peer IP** via `PeerIpKeyExtractor` — no forwarded-header extractor (would be spoofable without a trusted hop). - API listener is **IPv4-only by default** (`API_IPV6_ENABLED` off); IPv6 `API_BIND` rejected at config parse. - Docs updated: `docs/indexer-invariants.md`, `skills/AGENTS_INDEXER_API_LCD_SECURITY.md`. ### Acceptance criteria | Criterion | Result | How verified | |-----------|--------|--------------| | Distinct clients via proxy (XFF from trusted proxy) | **SKIP** | No trusted forwarded header in this deployment; peer IP is the client on direct Render exposure. Distinct IPv4 peers get distinct buckets: `cargo test --lib peer_ip_extractor_keys_distinct_ipv4_peers` | | Forged forwarded header from untrusted source ignored | **PASS** | `cargo test --lib peer_ip_extractor_ignores_forwarded_headers` | | IPv6 /64 rotation abuse mitigated | **PASS** | IPv6 disabled by default; `cargo test --lib api_listener` + `cargo test --lib ipv6_bind_rejected` | Full lib suite: `cd indexer && cargo test --lib` — **105 passed**. MR: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/61 Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/282

PlasticDigits commented

2026-06-05 09:55:26 +00:00

(Migrated from gitlab.com)

mentioned in merge request !783

PlasticDigits commented

2026-06-05 09:56:37 +00:00

(Migrated from gitlab.com)

mentioned in commit 2e2de53890

mentioned in commit 2e2de53890588abff85e491f7871bd75a219d0b5

PlasticDigits commented

2026-06-05 10:01:03 +00:00

(Migrated from gitlab.com)

mentioned in merge request !785

PlasticDigits commented

2026-06-05 11:02:31 +00:00

(Migrated from gitlab.com)

Verification — issue #282 (agent:verify)

Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/282
Implementation: MR !779 (merged) — peer-IP rate limiting, no forwarded-header extractor, IPv4-only API by default.

Deployment decision (from issue thread)

Render exposure has no trusted CF-Connecting-IP / X-Forwarded-For. Rate limits therefore key on socket peer IP (PeerIpKeyExtractor); a SmartIp/XFF extractor would be spoofable. IPv6 is disabled by default (API_IPV6_ENABLED off) to mitigate /64 address-rotation abuse.

Acceptance criteria

Criterion	Result	Verification
Behind intended proxy, distinct clients get distinct buckets (spoofed XFF from trusted proxy only)	SKIP	No trusted forwarded header in this deployment; peer IP is the client on direct exposure. Distinct IPv4 peers → distinct keys: `cargo test --lib peer_ip_extractor_keys_distinct_ipv4_peers` — ok
Trusted-proxy header not honored from arbitrary upstreams	PASS	`cargo test --lib peer_ip_extractor_ignores_forwarded_headers` — ok
IPv6 `/64` rotation abuse mitigated	PASS	`API_IPV6_ENABLED` defaults off; `cargo test --lib ipv6_bind_rejected` + `ipv6_bind_allowed_when_ipv6_enabled` + `api_listener` — ok
Rate limiter still returns 429 under burst (regression)	PASS	`cargo test --test security rate_limit` — 3 tests ok (`rate_limit_returns_429_when_exceeded`, LCD-heavy + CG/CMC paths)
Docs / invariants aligned	PASS	`docs/indexer-invariants.md`, `skills/AGENTS_INDEXER_API_LCD_SECURITY.md`, `indexer/.env.example` document peer-IP keying + IPv4-only default

Commands (2026-06-05)

cd indexer && cargo test --lib rate_limit_key          # 2 passed
cd indexer && cargo test --lib api_listener ipv6       # 4 passed
cd indexer && cargo test --lib                       # 113 passed
cd indexer && cargo test --test security rate_limit -j 1 -- --test-threads=1  # 3 passed

Overall: PASS (with topology SKIP on proxy/XFF criterion per product direction).

## Verification — issue #282 (agent:verify) **Issue:** https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/282 **Implementation:** MR !779 (merged) — peer-IP rate limiting, no forwarded-header extractor, IPv4-only API by default. ### Deployment decision (from issue thread) Render exposure has **no trusted** `CF-Connecting-IP` / `X-Forwarded-For`. Rate limits therefore key on **socket peer IP** (`PeerIpKeyExtractor`); a SmartIp/XFF extractor would be spoofable. IPv6 is **disabled by default** (`API_IPV6_ENABLED` off) to mitigate `/64` address-rotation abuse. ### Acceptance criteria | Criterion | Result | Verification | |-----------|--------|----------------| | Behind intended proxy, distinct clients get distinct buckets (spoofed XFF from trusted proxy only) | **SKIP** | No trusted forwarded header in this deployment; peer IP is the client on direct exposure. Distinct IPv4 peers → distinct keys: `cargo test --lib peer_ip_extractor_keys_distinct_ipv4_peers` — **ok** | | Trusted-proxy header not honored from arbitrary upstreams | **PASS** | `cargo test --lib peer_ip_extractor_ignores_forwarded_headers` — **ok** | | IPv6 `/64` rotation abuse mitigated | **PASS** | `API_IPV6_ENABLED` defaults off; `cargo test --lib ipv6_bind_rejected` + `ipv6_bind_allowed_when_ipv6_enabled` + `api_listener` — **ok** | | Rate limiter still returns 429 under burst (regression) | **PASS** | `cargo test --test security rate_limit` — 3 tests **ok** (`rate_limit_returns_429_when_exceeded`, LCD-heavy + CG/CMC paths) | | Docs / invariants aligned | **PASS** | `docs/indexer-invariants.md`, `skills/AGENTS_INDEXER_API_LCD_SECURITY.md`, `indexer/.env.example` document peer-IP keying + IPv4-only default | ### Commands (2026-06-05) ```bash cd indexer && cargo test --lib rate_limit_key # 2 passed cd indexer && cargo test --lib api_listener ipv6 # 4 passed cd indexer && cargo test --lib # 113 passed cd indexer && cargo test --test security rate_limit -j 1 -- --test-threads=1 # 3 passed ``` **Overall: PASS** (with topology SKIP on proxy/XFF criterion per product direction).

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-05 11:02:34 +00:00

PlasticDigits commented

2026-06-05 14:10:16 +00:00

(Migrated from gitlab.com)

mentioned in merge request !818

Rows
Columns

API rate limiter keys on peer IP with no forwarded-header extractor #282

Summary

Current codebase

Recommended direction

Acceptance criteria

Test plan (abuse)

Implementation complete — MR !779

What changed

Acceptance criteria

Verification — issue #282 (agent:verify)

Deployment decision (from issue thread)

Acceptance criteria

Commands (2026-06-05)