Summary
#173 added a source-keyed RateLimitLayer on the gateway using ConnectInfo<SocketAddr> — the real TCP peer — and deliberately did not use X-Forwarded-For. That was the right call, and the reason is specific to this deployment rather than the usual generic warning.
Behind the Caddy edge, the TCP peer is always an internal container address, so the source limiter collapses to a small number of internal buckets. Bounded and not attacker-controlled, which is what makes it safe — but it gives no per-external-client granularity.
Why XFF was rejected rather than deferred lazily
deploy/docker-compose.yml has two internal callers of sms-gateway, not one:
caddy, the TLS-terminating edge, which would set a trustworthy X-Forwarded-For.admin, which sets SMS_API_URL: http://sms-gateway:8080 and calls the gateway directly, bypassing Caddy entirely for its server-side requests.
A blanket "trust X-Forwarded-For" configuration cannot distinguish them, and Docker Compose assigns container addresses dynamically, so there is no static address to pin an allowlist against without adding that infrastructure.
Getting this half-right is worse than not doing it. A limiter that trusts an attacker-supplied X-Forwarded-For is both trivially bypassable (a fresh header value per request is a fresh bucket) and actively harmful (an attacker can attribute their traffic to a victim's address and deny them service). The current implementation has neither property.
Scope
Real per-external-client source limiting, which needs all of:
- A pinned trusted-proxy identity for Caddy — a static address on the compose network, or a shared secret header, or mTLS between edge and gateway. Decide which; each has different operational cost.
- A way to distinguish
admin's direct calls from proxied traffic, or a decision to route admin through Caddy too so there is exactly one trusted hop. The latter is simpler and worth weighing first.
- Explicit configuration of how many hops to trust and from whom, never blanket trust.
- The same question answered for the Helm chart (
deploy/charts/vsms), where the topology is a Service and the peer address is a pod IP.
Acceptance
- With the trusted hop configured, two distinct external clients behind the edge land in distinct buckets — demonstrated, not asserted.
- A request carrying a forged
X-Forwarded-For from an untrusted peer does not get a fresh bucket, and cannot be attributed to another client's bucket. Both halves must be shown; the second is the one that turns a rate limiter into a denial-of-service tool if it is wrong.
Deferred from #173, where it is recorded under Reviewer Focus.
Summary
#173 added a source-keyed
RateLimitLayeron the gateway usingConnectInfo<SocketAddr>— the real TCP peer — and deliberately did not useX-Forwarded-For. That was the right call, and the reason is specific to this deployment rather than the usual generic warning.Behind the Caddy edge, the TCP peer is always an internal container address, so the source limiter collapses to a small number of internal buckets. Bounded and not attacker-controlled, which is what makes it safe — but it gives no per-external-client granularity.
Why XFF was rejected rather than deferred lazily
deploy/docker-compose.ymlhas two internal callers ofsms-gateway, not one:caddy, the TLS-terminating edge, which would set a trustworthyX-Forwarded-For.admin, which setsSMS_API_URL: http://sms-gateway:8080and calls the gateway directly, bypassing Caddy entirely for its server-side requests.A blanket "trust
X-Forwarded-For" configuration cannot distinguish them, and Docker Compose assigns container addresses dynamically, so there is no static address to pin an allowlist against without adding that infrastructure.Getting this half-right is worse than not doing it. A limiter that trusts an attacker-supplied
X-Forwarded-Foris both trivially bypassable (a fresh header value per request is a fresh bucket) and actively harmful (an attacker can attribute their traffic to a victim's address and deny them service). The current implementation has neither property.Scope
Real per-external-client source limiting, which needs all of:
admin's direct calls from proxied traffic, or a decision to routeadminthrough Caddy too so there is exactly one trusted hop. The latter is simpler and worth weighing first.deploy/charts/vsms), where the topology is a Service and the peer address is a pod IP.Acceptance
X-Forwarded-Forfrom an untrusted peer does not get a fresh bucket, and cannot be attributed to another client's bucket. Both halves must be shown; the second is the one that turns a rate limiter into a denial-of-service tool if it is wrong.Deferred from #173, where it is recorded under Reviewer Focus.