nginx: trust X-Forwarded-For only from TRUSTED_PROXIES (closes #64)
check / check (push) Successful in 52s

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 makes each entry a CIDR, checks it with the new
"netwatch-server check-cidr", which runs the server's own
TRUSTED_PROXIES parsing, 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
This commit is contained in:
2026-09-29 06:22:56 +00:00
parent 6022cc8b02
commit a911353023
11 changed files with 129 additions and 34 deletions
+12 -6
View File
@@ -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: only inside the container, on `127.0.0.1:8081`. The image:
- Listens on port 8080 by default (override with `PORT` env var) - Listens on port 8080 by default (override with `PORT` env var)
- Trusts `X-Forwarded-For` from RFC1918 reverse proxies (10/8, 172.16/12, - Takes the client address from `X-Forwarded-For` only on requests from the
192.168/16) reverse proxies named in `TRUSTED_PROXIES`, and by default from none
- Sends access logs to stdout - Sends access logs to stdout
- Caches static assets with immutable headers - Caches static assets with immutable headers
- Stores reports in `DATA_DIR`, `/data/reports` by default, on the `/data` - 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 - `DEBUG`, default `false`: debug logging
- `DATA_DIR`, default `/data/reports`: leave unset; reports kept outside - `DATA_DIR`, default `/data/reports`: leave unset; reports kept outside
`/data` do not survive a redeploy `/data` do not survive a redeploy
- `TRUSTED_PROXIES`, default loopback and RFC1918: leave unset. The - `TRUSTED_PROXIES`, default empty: set it to the address the reverse proxy
backend's only client is nginx, on loopback, which passes on the client in front of the container connects from, as an IP address or CIDR; several
address; nginx takes it from `X-Forwarded-For` only from RFC1918 are separated by commas. nginx takes the client address from
addresses. `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 - **Health check:** the image's `HEALTHCHECK` requests
`/.well-known/healthcheck` through nginx every 30 seconds, so it fails unless `/.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 both nginx and the backend answer. upaas reads the container's health 60
+9
View File
@@ -23,6 +23,15 @@ latest run passes.
# Completed Steps # 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
an entry that is not an IP address or CIDR, as `netwatch-server check-cidr`
finds; it starts the backend with `TRUSTED_PROXIES=127.0.0.1/32`, since nginx
is its only client
- 2026-09-29: report file names can no longer collide (issue #61): each is - 2026-09-29: report file names can no longer collide (issue #61): each is
`reports-<timestamp>-<number>.jsonl.zst`, where the number goes up by one for `reports-<timestamp>-<number>.jsonl.zst`, where the number goes up by one for
each file the server starts to write, so two flushes in the same millisecond, each file the server starts to write, so two flushes in the same millisecond,
+17 -5
View File
@@ -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) | | `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`. `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 The loopback entries cover a reverse proxy on the same host. A request whose
RFC1918 ranges match `nginx.conf`. A request whose direct peer is outside this direct peer is outside this set has its forwarded headers ignored, and the
set has its forwarded headers ignored, and the direct peer is logged instead. 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`, 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 `DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from
@@ -101,8 +102,19 @@ 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 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 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 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, only nginx reaches it, and with `TRUSTED_PROXIES=127.0.0.1/32`, so it takes the
which `netwatch` owns. 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. An entry that is not an IP address or CIDR, such as a hostname or
`1.2.3`, stops the container at start with an error naming `TRUSTED_PROXIES`:
the entrypoint checks each entry with `netwatch-server check-cidr`, which parses
it as this server parses its own `TRUSTED_PROXIES`.
### Report storage ### Report storage
+16
View File
@@ -2,6 +2,9 @@
package main package main
import ( import (
"fmt"
"os"
"sneak.berlin/go/netwatch/internal/config" "sneak.berlin/go/netwatch/internal/config"
"sneak.berlin/go/netwatch/internal/globals" "sneak.berlin/go/netwatch/internal/globals"
"sneak.berlin/go/netwatch/internal/handlers" "sneak.berlin/go/netwatch/internal/handlers"
@@ -22,6 +25,19 @@ var (
) )
func main() { func main() {
// "netwatch-server check-cidr CIDR" exits 1, with the error, if
// this server would refuse CIDR in its TRUSTED_PROXIES.
// bin/entrypoint.sh runs it on each entry it gives nginx.
if len(os.Args) == 3 && os.Args[1] == "check-cidr" {
_, err := middleware.ParseTrustedProxies(os.Args[2:])
if err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
return
}
globals.Appname = Appname globals.Appname = Appname
globals.Version = Version globals.Version = Version
globals.Buildarch = Buildarch globals.Buildarch = Buildarch
+5 -4
View File
@@ -21,10 +21,11 @@ import (
) )
// defaultTrustedProxies lists the networks whose forwarded // defaultTrustedProxies lists the networks whose forwarded
// headers are honoured by default. It covers the RFC1918 // headers are honoured by default: IPv4 and IPv6 loopback,
// ranges (to match nginx.conf) plus IPv4 and IPv6 loopback, // for a reverse proxy on the same host, and the RFC1918
// because the reverse proxy shares the container and reaches // ranges. The container image does not use it:
// the backend over loopback. // 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," + const defaultTrustedProxies = "127.0.0.1/32,::1/128," +
"10.0.0.0/8,172.16.0.0/12,192.168.0.0/16" "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
@@ -29,7 +29,3 @@ func ClientIP(
) string { ) string {
return clientIP(remoteAddr, header, trusted) return clientIP(remoteAddr, header, trusted)
} }
func ParseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
return parseTrustedProxies(cidrs)
}
+6 -4
View File
@@ -64,7 +64,7 @@ func New(
_ fx.Lifecycle, _ fx.Lifecycle,
params Params, params Params,
) (*Middleware, error) { ) (*Middleware, error) {
trusted, err := parseTrustedProxies(params.Config.TrustedProxies) trusted, err := ParseTrustedProxies(params.Config.TrustedProxies)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -77,9 +77,11 @@ func New(
return s, nil return s, nil
} }
// parseTrustedProxies converts the TRUSTED_PROXIES entries into // ParseTrustedProxies converts the TRUSTED_PROXIES entries into
// prefixes, failing fast on any malformed entry. // prefixes, failing fast on any malformed entry. Each entry must be
func parseTrustedProxies(cidrs []string) ([]netip.Prefix, error) { // a CIDR; a lone address is refused. "netwatch-server check-cidr"
// runs it too.
func ParseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
prefixes := make([]netip.Prefix, 0, len(cidrs)) prefixes := make([]netip.Prefix, 0, len(cidrs))
for _, cidr := range cidrs { for _, cidr := range cidrs {
+19 -2
View File
@@ -36,13 +36,30 @@ func mustPrefixes(t *testing.T, cidrs ...string) []netip.Prefix {
return prefixes return prefixes
} }
// TestParseTrustedProxiesRejectsMalformed includes entries nginx would
// read as another address or look up as a hostname, in the CIDR form
// bin/entrypoint.sh gives "netwatch-server check-cidr".
func TestParseTrustedProxiesRejectsMalformed(t *testing.T) { func TestParseTrustedProxiesRejectsMalformed(t *testing.T) {
t.Parallel() t.Parallel()
_, err := middleware.ParseTrustedProxies([]string{"not-a-cidr"}) for _, cidr := range []string{
"not-a-cidr", "10.0.0.1", "1.2.3/32", "172.30/32", "10/32",
"cafe/32", "999.1.1.1/32", "10.0.0.0/33", "::1/129",
"fe80::1%eth0/128",
} {
_, err := middleware.ParseTrustedProxies([]string{cidr})
if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") { if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
t.Fatalf("error = %v, want one naming TRUSTED_PROXIES", err) t.Errorf("%q: error = %v, want one naming TRUSTED_PROXIES",
cidr, err)
} }
}
}
func TestParseTrustedProxiesAcceptsCIDRs(t *testing.T) {
t.Parallel()
mustPrefixes(t, "172.17.0.1/32", "10.0.0.0/8", "2001:db8::1/128",
"2001:db8::/32", "::ffff:192.0.2.1/128")
} }
type clientIPCase struct { type clientIPCase struct {
+35 -4
View File
@@ -32,16 +32,47 @@ if [ "$PORT" -eq 8081 ]; then
exit 1 exit 1
fi 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.conf includes the file written
# here, one set_real_ip_from line per entry.
#
# nginx looks up an entry it cannot read as an address as a hostname,
# and trusts what it finds (1.2.3 is found as 1.2.0.3). So each entry
# is made a CIDR, a lone address getting /128 if it is IPv6 and /32 if
# not, and netwatch-server checks it with the parsing it gives its own
# TRUSTED_PROXIES. Its error, naming the CIDR, is dropped for the one
# below, naming the entry as written. set -f keeps a * in an entry from
# becoming a list of file names.
TRUSTED_PROXIES="${TRUSTED_PROXIES:-}"
set -f
for proxy in $(printf '%s' "$TRUSTED_PROXIES" | tr ',' ' '); do
case "$proxy" in
*/*) cidr="$proxy" ;;
*:*) cidr="$proxy/128" ;;
*) cidr="$proxy/32" ;;
esac
if ! netwatch-server check-cidr "$cidr" 2> /dev/null; then
echo "entrypoint: TRUSTED_PROXIES must be IP addresses or CIDRs" \
"separated by commas; '$proxy' is neither" >&2
exit 1
fi
echo "set_real_ip_from $cidr;"
done > /etc/nginx/trusted-proxies.conf
# A stop signal is only noted here; the loop below acts on it. # A stop signal is only noted here; the loop below acts on it.
stop_requested="" stop_requested=""
trap 'stop_requested=yes' TERM INT trap 'stop_requested=yes' TERM INT
# netwatch-server runs as the netwatch user and listens on loopback # 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 # 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. # address. Its only client is nginx, so it takes the client address
# busybox su replaces itself with the command instead of staying on as # nginx passes on from 127.0.0.1 alone, whatever TRUSTED_PROXIES the
# its parent, so $! is the server's own PID. # container has. The netwatch user has no login shell, hence -s
BIND_ADDRESS=127.0.0.1 PORT=8081 \ # /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' & su -s /bin/sh netwatch -c 'exec netwatch-server' &
backend=$! backend=$!
+6 -4
View File
@@ -11,10 +11,12 @@ server {
root /usr/share/nginx/html; root /usr/share/nginx/html;
index index.html; index index.html;
# Trust RFC1918 reverse proxies for X-Forwarded-For # The client address comes from X-Forwarded-For only on a request
set_real_ip_from 10.0.0.0/8; # from the reverse proxies in TRUSTED_PROXIES: bin/entrypoint.sh
set_real_ip_from 172.16.0.0/12; # writes one set_real_ip_from line for each into this file, and
set_real_ip_from 192.168.0.0/16; # 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_header X-Forwarded-For;
real_ip_recursive on; real_ip_recursive on;
+3
View File
@@ -63,11 +63,14 @@ main() {
# nginx.conf is a template: the image renders it over its own # nginx.conf is a template: the image renders it over its own
# default.conf, with the same port and limit bin/entrypoint.sh uses. # 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" \ docker run -d --rm --name "$SERVER" \
--network "$NETWORK" --network-alias netwatch \ --network "$NETWORK" --network-alias netwatch \
-e PORT=8080 -e NGINX_ENVSUBST_FILTER='^PORT$' \ -e PORT=8080 -e NGINX_ENVSUBST_FILTER='^PORT$' \
-v "$ROOT/dist:/usr/share/nginx/html:ro" \ -v "$ROOT/dist:/usr/share/nginx/html:ro" \
-v "$ROOT/nginx.conf:/etc/nginx/templates/default.conf.template: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 "$SERVER_IMAGE" > /dev/null
# The image's own entrypoint already exposes CDP on 9222 and passes # The image's own entrypoint already exposes CDP on 9222 and passes