From 16d01d5326240cd490c0b9d0e57aca317b56e40b Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 05:16:37 +0000 Subject: [PATCH] nginx: trust X-Forwarded-For only from TRUSTED_PROXIES (closes #64) nginx trusted X-Forwarded-For from every RFC1918 address, so a client reaching it from one could write a new address on each request and get a fresh rate-limit allowance. The container's TRUSTED_PROXIES now names the reverse proxies nginx trusts, none by default. bin/entrypoint.sh refuses a value with a character no IP address or CIDR has, and writes one set_real_ip_from line per entry into /etc/nginx/trusted-proxies.conf, which nginx.conf includes. The backend is started with TRUSTED_PROXIES=127.0.0.1/32, since nginx is its only client. The viewport test mounts an empty file there. Model: opus-5-5 --- README.md | 18 ++++++++++++------ TODO.md | 8 ++++++++ backend/README.md | 20 +++++++++++++++----- backend/internal/config/config.go | 9 +++++---- bin/entrypoint.sh | 31 +++++++++++++++++++++++++++---- nginx.conf | 10 ++++++---- script/frontend-viewport-test | 3 +++ 7 files changed, 76 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 89e5c5c..2f46400 100644 --- a/README.md +++ b/README.md @@ -184,8 +184,8 @@ container: nginx serves the built frontend and passes `/api/` and only inside the container, on `127.0.0.1:8081`. The image: - Listens on port 8080 by default (override with `PORT` env var) -- Trusts `X-Forwarded-For` from RFC1918 reverse proxies (10/8, 172.16/12, - 192.168/16) +- Takes the client address from `X-Forwarded-For` only on requests from the + reverse proxies named in `TRUSTED_PROXIES`, and by default from none - Sends access logs to stdout - Caches static assets with immutable headers - Stores reports in `DATA_DIR`, `/data/reports` by default, on the `/data` @@ -224,10 +224,16 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs: - `DEBUG`, default `false`: debug logging - `DATA_DIR`, default `/data/reports`: leave unset; reports kept outside `/data` do not survive a redeploy - - `TRUSTED_PROXIES`, default loopback and RFC1918: leave unset. The - backend's only client is nginx, on loopback, which passes on the client - address; nginx takes it from `X-Forwarded-For` only from RFC1918 - addresses. + - `TRUSTED_PROXIES`, default empty: set it to the address the reverse proxy + in front of the container connects from, as an IP address or CIDR; several + are separated by commas. nginx takes the client address from + `X-Forwarded-For` only on a request from one of them, and the rate limit + counts that address. Unset, `X-Forwarded-For` is ignored and every client + behind the proxy shares the proxy's one allowance of `REPORTS_PER_MINUTE`. + Name only addresses nothing but the proxy connects from: any client that + connects from one can write its own `X-Forwarded-For`, and through a port + Docker publishes, every client may connect from the Docker network's + gateway, such as `172.17.0.1`. - **Health check:** the image's `HEALTHCHECK` requests `/.well-known/healthcheck` through nginx every 30 seconds, so it fails unless both nginx and the backend answer. upaas reads the container's health 60 diff --git a/TODO.md b/TODO.md index caa9dce..86b78a8 100644 --- a/TODO.md +++ b/TODO.md @@ -23,6 +23,14 @@ latest run passes. # Completed Steps +- 2026-09-29: nginx takes the client address from `X-Forwarded-For` only on + requests from the reverse proxies named in the container's `TRUSTED_PROXIES` + (issue #64), and by default from none, where it trusted every RFC1918 address + before, so a client could write a new address on each request and escape the + rate limit. `bin/entrypoint.sh` writes one `set_real_ip_from` line per entry + into `/etc/nginx/trusted-proxies.conf`, which `nginx.conf` includes, refusing + a value with a character no IP address or CIDR has; it starts the backend with + `TRUSTED_PROXIES=127.0.0.1/32`, since nginx is its only client - 2026-09-29: ready to run under upaas (issue #59): the image has a `HEALTHCHECK` that requests `/.well-known/healthcheck` through nginx on the port from `PORT`. The backend no longer reads a bad `PORT` as 0 or a bad diff --git a/backend/README.md b/backend/README.md index 435647f..5367aee 100644 --- a/backend/README.md +++ b/backend/README.md @@ -87,9 +87,10 @@ Internal packages in `internal/` follow standard Go project layout: | `CORS_ALLOWED_ORIGINS` | empty | Comma-separated origins whose pages may call the API; see [CORS](#cors) | `TRUSTED_PROXIES` defaults to `127.0.0.1/32,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`. -The loopback entries cover the reverse proxy that shares the container; the -RFC1918 ranges match `nginx.conf`. A request whose direct peer is outside this -set has its forwarded headers ignored, and the direct peer is logged instead. +The loopback entries cover a reverse proxy on the same host. A request whose +direct peer is outside this set has its forwarded headers ignored, and the +direct peer is logged and rate-limited instead. The container image does not use +this default; see [Container image](#container-image). A variable set to a value the server cannot use, such as `PORT=abc`, `DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from @@ -101,8 +102,17 @@ The root `Dockerfile` builds one image in which nginx listens on the public port 8080, serves the frontend, and proxies `/api/` and `/.well-known/healthcheck` to this server. The image's entrypoint, `bin/entrypoint.sh`, starts the server as user `netwatch` (uid 1000) with `BIND_ADDRESS=127.0.0.1` and `PORT=8081`, so -only nginx reaches it. `DATA_DIR` is `/data/reports`, on the `/data` volume, -which `netwatch` owns. +only nginx reaches it, and with `TRUSTED_PROXIES=127.0.0.1/32`, so it takes the +client address nginx passes on and no other. `DATA_DIR` is `/data/reports`, on +the `/data` volume, which `netwatch` owns. + +The container's own `TRUSTED_PROXIES` goes to nginx instead: IP addresses or +CIDRs, separated by commas, of the reverse proxies in front of the container. +nginx takes the client address from `X-Forwarded-For` only on a request from one +of them. Unset or empty, nginx trusts no proxy, and the client address is the +one each request comes from, so every client behind a proxy shares one rate +limit. A value with a character no IP address or CIDR has, such as a hostname, +stops the container at start with an error naming `TRUSTED_PROXIES`. ### Report storage diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index c8236d0..a258978 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -21,10 +21,11 @@ import ( ) // defaultTrustedProxies lists the networks whose forwarded -// headers are honoured by default. It covers the RFC1918 -// ranges (to match nginx.conf) plus IPv4 and IPv6 loopback, -// because the reverse proxy shares the container and reaches -// the backend over loopback. +// headers are honoured by default: IPv4 and IPv6 loopback, +// for a reverse proxy on the same host, and the RFC1918 +// ranges. The container image does not use it: +// bin/entrypoint.sh gives the server 127.0.0.1/32, since +// nginx is its only client there. const defaultTrustedProxies = "127.0.0.1/32,::1/128," + "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16" diff --git a/bin/entrypoint.sh b/bin/entrypoint.sh index e0d4fb3..938f500 100755 --- a/bin/entrypoint.sh +++ b/bin/entrypoint.sh @@ -32,16 +32,39 @@ if [ "$PORT" -eq 8081 ]; then exit 1 fi +# TRUSTED_PROXIES names the reverse proxies in front of the container, +# as IP addresses or CIDRs separated by commas. nginx takes the client +# address from X-Forwarded-For only on a request from one of them, so +# unset or empty, it trusts no one. nginx would look up a hostname at +# start and trust whatever address it found, so a value with a +# character no address has stops the container here. An entry such as +# 999.1.1.1 gets past this, and nginx refuses it at start as a +# hostname it cannot find. +TRUSTED_PROXIES="${TRUSTED_PROXIES:-}" +case "$TRUSTED_PROXIES" in + *[!0-9A-Fa-f.:/,\ ]*) + echo "entrypoint: TRUSTED_PROXIES must be IP addresses or CIDRs" \ + "separated by commas, not '$TRUSTED_PROXIES'" >&2 + exit 1 + ;; +esac +# nginx.conf includes this file; an empty one trusts no proxy. +for proxy in $(echo "$TRUSTED_PROXIES" | tr ',' ' '); do + echo "set_real_ip_from $proxy;" +done > /etc/nginx/trusted-proxies.conf + # A stop signal is only noted here; the loop below acts on it. stop_requested="" trap 'stop_requested=yes' TERM INT # netwatch-server runs as the netwatch user and listens on loopback # only, on a port other than the public one; nginx.conf proxies to this -# address. The netwatch user has no login shell, hence -s /bin/sh. -# busybox su replaces itself with the command instead of staying on as -# its parent, so $! is the server's own PID. -BIND_ADDRESS=127.0.0.1 PORT=8081 \ +# address. Its only client is nginx, so it takes the client address +# nginx passes on from 127.0.0.1 alone, whatever TRUSTED_PROXIES the +# container has. The netwatch user has no login shell, hence -s +# /bin/sh. busybox su replaces itself with the command instead of +# staying on as its parent, so $! is the server's own PID. +BIND_ADDRESS=127.0.0.1 PORT=8081 TRUSTED_PROXIES=127.0.0.1/32 \ su -s /bin/sh netwatch -c 'exec netwatch-server' & backend=$! diff --git a/nginx.conf b/nginx.conf index 697340b..0322c27 100644 --- a/nginx.conf +++ b/nginx.conf @@ -11,10 +11,12 @@ server { root /usr/share/nginx/html; index index.html; - # Trust RFC1918 reverse proxies for X-Forwarded-For - set_real_ip_from 10.0.0.0/8; - set_real_ip_from 172.16.0.0/12; - set_real_ip_from 192.168.0.0/16; + # The client address comes from X-Forwarded-For only on a request + # from the reverse proxies in TRUSTED_PROXIES: bin/entrypoint.sh + # writes one set_real_ip_from line for each into this file, and + # leaves it empty when TRUSTED_PROXIES is unset, so that by default + # the client address is the one each request comes from. + include /etc/nginx/trusted-proxies.conf; real_ip_header X-Forwarded-For; real_ip_recursive on; diff --git a/script/frontend-viewport-test b/script/frontend-viewport-test index 5a97c99..e4b53d1 100755 --- a/script/frontend-viewport-test +++ b/script/frontend-viewport-test @@ -63,11 +63,14 @@ main() { # nginx.conf is a template: the image renders it over its own # default.conf, with the same port and limit bin/entrypoint.sh uses. + # The empty file it includes trusts no proxy, as bin/entrypoint.sh + # writes it when TRUSTED_PROXIES is unset. docker run -d --rm --name "$SERVER" \ --network "$NETWORK" --network-alias netwatch \ -e PORT=8080 -e NGINX_ENVSUBST_FILTER='^PORT$' \ -v "$ROOT/dist:/usr/share/nginx/html:ro" \ -v "$ROOT/nginx.conf:/etc/nginx/templates/default.conf.template:ro" \ + -v /dev/null:/etc/nginx/trusted-proxies.conf:ro \ "$SERVER_IMAGE" > /dev/null # The image's own entrypoint already exposes CDP on 9222 and passes