Skip to content

server_pal rate limiting keys on the proxy, not the client #1329

Description

@aaylward

server_pal builds its GovernorLayer with the default PeerIpKeyExtractor,
which reads the socket peer address. Every Rust service in
deploy/consolidated is reached only through Caddy, so the peer is always
Caddy's container IP and all external callers share one bucket. The limit is
global, documented as per-IP.

Concretely: at the default 100 req/s, one client can spend the whole bucket and
everyone else gets 429s until it stops. #1328 fixed the units (the limit was
accidentally 1 req/100s, which hid this) but not the keying.

The C++ rail already solved this — PerClientRateLimit takes a
trusted_proxies set and keys on the ADR-0012 derived client address
(domains/platform/libs/aura/middleware.h:88), and compose.yaml already
declares the boundary via the &caddy_ip anchor and passes
TRUSTED_PROXY_CIDRS to golf_hub and portrait. The three server_pal services
don't get it.

Worth noting the naive fix is wrong: swapping in tower_governor's
SmartIpKeyExtractor makes the key fully client-controlled, because it takes
the first parseable entry in X-Forwarded-For and Caddy appends rather than
replaces. An attacker just sets the header and gets a private bucket. It needs
the aura shape — trust a configured CIDR set, take the rightmost untrusted hop,
fall back to the peer.

Also on the Go rail, same class of bug and arguably worse:
resilience4g/rate_limit/http_middleware.go:34 uses the raw X-Forwarded-For
header value as the key with no trusted-proxy check and no comma splitting — so
it's spoofable, and "a, b" keys differently from "a".

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