glm-5.1
ceb59ad9b9
Critical findings resolved: - C1: Site routing is global (per-listener TOML, global runtime lookup) - C2: X-Forwarded-For replaces (not appends) — edge proxy model (ADR-021) - C3: Hop-by-hop header handling rules specified (proxy.md) - C4: ACME failure behavior defined (tls.md) - C5: Startup sequence with fail-fast semantics (operations.md) - C6: Per-listener Router instances with shared global state (overview.md) - C7: Rate limiter adopts new params on next request, no state clear (operations.md) Warnings resolved: - W1: Admin socket wire protocol specified - W2: Host header port stripped, hostnames only in config - W3: HTTP redirect URL construction with port handling - W4: /health on HTTPS matches regardless of Host header - W5: Static config changes logged as warning during reload - W6: Reload operations serialized via Mutex - W7: http_port validation rules added (9 new rules total) - W8: upstream format validation (host:port required, no scheme) - W9: TLS error handling table (SNI, version, cipher failures) - W10: IPv6 rate limited per /64 prefix - W11: Graceful shutdown sequence specified (6 steps) - W12: Error response bodies: minimal plain text, no version disclosure - W13: upstream_scheme HTTPS uses system CA store - W14: allow_wildcard_bind is OR between config and CLI - W15: ADR-010 Phase 2 list updated (timeouts moved to Phase 1) - W17: LoggingConfig static/restart note added Suggestions applied: - S2: ConnectInfo propagation note - S3: Case-insensitive host matching (RFC 7230) - S5: Response streaming behavior (chunk-by-chunk) - S6: Token bucket nodelay semantics - S7: File watching explicitly out of scope - S8: All paths forwarded without filtering - S9: shutdown_timeout_secs referenced in shutdown description - S11: Consolidated defaults table in config.md
74 lines
3.1 KiB
Markdown
74 lines
3.1 KiB
Markdown
# ADR-021: X-Forwarded-For Edge Proxy Model
|
|
|
|
## Status
|
|
|
|
Accepted
|
|
|
|
## Context
|
|
|
|
The reverse proxy terminates TLS and injects standard proxy headers (Host,
|
|
X-Real-IP, X-Forwarded-For, X-Forwarded-Proto) before forwarding requests to
|
|
upstream services. The question is how to handle the `X-Forwarded-For` header
|
|
when the request already contains one.
|
|
|
|
Two approaches exist:
|
|
|
|
1. **Append**: Add the client IP to any existing `X-Forwarded-For` value.
|
|
This preserves the proxy chain history but trusts that existing values are
|
|
legitimate. In a chained proxy scenario (e.g., CDN → reverse proxy →
|
|
backend), appending is correct because the CDN's `X-Forwarded-For` value
|
|
is trustworthy.
|
|
|
|
2. **Replace**: Set `X-Forwarded-For` to the client's IP address from
|
|
`ConnectInfo<SocketAddr>`, discarding any existing value. This is correct
|
|
when the proxy is the edge proxy directly facing the internet, because
|
|
client-provided `X-Forwarded-For` headers are untrusted — any client can
|
|
inject arbitrary values.
|
|
|
|
This proxy is deployed as the **edge proxy** — it sits directly in front of the
|
|
internet with no trusted proxies upstream. All client connections come directly
|
|
to this proxy.
|
|
|
|
## Decision
|
|
|
|
Set `X-Forwarded-For` to the client's IP address from `ConnectInfo<SocketAddr>`,
|
|
**replacing** any existing value rather than appending.
|
|
|
|
The proxy is an edge proxy. There are no trusted proxies upstream, so existing
|
|
`X-Forwarded-For` headers from clients cannot be trusted. Replacing prevents
|
|
header spoofing attacks where a malicious client injects fake IP addresses to
|
|
confuse upstream services or bypass IP-based access controls.
|
|
|
|
## Rationale
|
|
|
|
- The proxy is the edge proxy — it directly faces the internet. No CDN or
|
|
other trusted proxy sits in front of it.
|
|
- Client-provided `X-Forwarded-For` headers are untrusted. Any client can send
|
|
`X-Forwarded-For: 1.2.3.4` to spoof their IP.
|
|
- Appending to an untrusted header creates a security vulnerability: upstream
|
|
services (like Gitea) may use `X-Forwarded-For` for IP-based rate limiting or
|
|
access control. Spoofed values would bypass these protections.
|
|
- `X-Real-IP` is also set to the client's IP from `ConnectInfo`, providing a
|
|
trustworthy header for upstream services that need the real client IP.
|
|
- If this proxy is ever placed behind a CDN or other trusted proxy, the header
|
|
handling model should be revisited. A "trusted proxies" configuration can be
|
|
added in Phase 2 to support chained proxy scenarios.
|
|
|
|
## Consequences
|
|
|
|
**Positive:**
|
|
- Prevents `X-Forwarded-For` spoofing attacks
|
|
- Upstream services receive the real client IP, not a spoofed one
|
|
- Simple, predictable header handling — no trust model to configure
|
|
- Consistent with the proxy's role as the edge proxy
|
|
|
|
**Negative:**
|
|
- If the proxy is placed behind a CDN or other proxy in the future, the header
|
|
handling must be updated to support a "trusted proxies" model
|
|
- Does not preserve legitimate proxy chain history (not applicable in our
|
|
deployment — there are no proxies upstream)
|
|
|
|
## References
|
|
|
|
- [proxy.md](../proxy.md)
|
|
- Architecture Review C2 (X-Forwarded-For security model) |