Files

glm-5.1 80d1fd0fb3 Update architecture docs to address security review #003 findings

Add three ADRs (025-027) and update five spec documents to close gaps
identified in the security and bug review:

- ADR-025: Rate limiter IP source must be ConnectInfo only (C1 fix)
- ADR-026: Connector timeout ceiling of 30s for per-site timeouts (C3 fix)
- ADR-027: Admin socket resource limits — 5s timeout, 4096 byte line limit (W4 fix)

Spec changes:
- proxy.md: add rate limiter IP source section, URI error handling
  constraint, connector ceiling description, renumber sections
- operations.md: add ConnectInfo-only IP source, in-flight counter
  architectural requirement (C2), JSON format guarantee (C4), admin
  socket resource limits, 100ms drain polling interval
- config.md: fix http_port type u32→u16 (W12), tighten upstream host
  validation (W1), tighten ACME contact validation (W2), add
  X-Forwarded-Proto cross-reference, clarify alknet ADR-030 reference
- overview.md: fix ambiguous C1 reference, add ADR/OQ cross-references
- open-questions.md: update OQ-09 resolution, add OQ-13 (acme_contact
  Vec) and OQ-14 (eviction configurability)
- README.md: add ADR-025/026/027 and OQ-13/14, update doc statuses to draft

Also fix reviewer findings: alknet ADR-030 scope clarification, RFC 2616
reference updated to RFC 7230.

2026-06-12 13:17:39 +00:00

4.0 KiB

Raw Blame History

ADR-025: Rate Limiter IP Source Must Be ConnectInfo Only

Status

Accepted

Context

The rate limiter identifies clients by IP address to enforce per-IP token bucket limits. The question is: what is the authoritative source of the client IP for rate limiting?

Two potential sources exist:

ConnectInfo<SocketAddr>: The TCP peer address, extracted from TcpStream::peer_addr() before TLS handshake and propagated to axum via ConnectInfoService. This is the real client IP at the TCP level.
X-Forwarded-For header: A client-supplied HTTP header that may contain an IP address. This header is set by the proxy handler after rate limiting (the rate limiter runs as middleware before the handler), so the value present during rate limit checks is whatever the client sent — completely untrusted.

ADR-021 already establishes that the proxy is an edge proxy and client-supplied X-Forwarded-For headers are untrusted. The proxy handler replaces X-Forwarded-For with the ConnectInfo IP before forwarding upstream specifically to prevent spoofing. However, ADR-021 only addresses the proxy handler's header injection — it does not specify the rate limiter's IP source.

The current implementation checks X-Forwarded-For first and falls back to ConnectInfo, which creates two attack vectors:

Rate limit bypass: A client sends each request with a different random X-Forwarded-For value. Every request appears to come from a different IP, evading the per-IP token bucket entirely.
Denial-of-service via IP spoofing: A client sends requests with X-Forwarded-For: <victim IP>. The victim's bucket is depleted and their legitimate requests receive 429 responses.

Decision

The rate limiter must use ConnectInfo<SocketAddr> as the sole source of client IP addresses. X-Forwarded-For must not be consulted for rate limiting purposes.

The rate limiter runs as middleware before the proxy handler. At that point in the request lifecycle, no proxy headers have been injected — any X-Forwarded-For header present is from the client and is untrusted (ADR-021).

ConnectInfo<SocketAddr> is always present because each listener populates it via into_make_service_with_connect_info::<SocketAddr>(). If ConnectInfo is absent (which should not happen in normal operation), the request should be rejected rather than falling back to an untrusted header.

Rationale

The proxy is the edge proxy (ADR-021). Client-supplied headers are untrusted at the edge.
Rate limiting is a security mechanism — it must use the most trustworthy IP source available. ConnectInfo is set by the kernel's TCP stack, not by the client.
The rate limiter's position in the middleware stack (before the handler) means it sees raw client headers, not the replaced X-Forwarded-For that the proxy handler injects.
Falling back to X-Forwarded-For when ConnectInfo is absent creates a downgrade attack — an attacker could potentially strip ConnectInfo from extensions to force the fallback path. Better to reject than to accept an untrusted IP.

Consequences

Positive:

Rate limiting cannot be bypassed via header spoofing
Rate limiting cannot be weaponized to DoS legitimate clients
Consistent with ADR-021's edge proxy trust model
No ambiguity about the IP source — one source, one code path

Negative:

If the proxy is ever placed behind a trusted CDN or load balancer, ConnectInfo will reflect the CDN's IP, not the end client's. This would require a "trusted proxies" configuration (already noted as a Phase 2 consideration in ADR-021).
Requests without ConnectInfo are rejected. This should not happen in normal operation but adds a hard failure mode.

References

proxy.md — Proxy header injection, request flow
operations.md — Rate limiting design
ADR-006 — Token bucket rate limiting
ADR-021 — X-Forwarded-For edge proxy model
Security Review C1 — Rate limiter X-Forwarded-For spoofing vulnerability

4.0 KiB Raw Blame History