1 Commits
Author SHA1 Message Date
clawbot 16d01d5326 nginx: trust X-Forwarded-For only from TRUSTED_PROXIES (closes #64)
check / check (push) Successful in 49s
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
2026-09-29 05:16:37 +00:00
7 changed files with 28 additions and 70 deletions
+2 -3
View File
@@ -29,9 +29,8 @@ latest run passes.
before, so a client could write a new address on each request and escape the 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 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 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` a value with a character no IP address or CIDR has; it starts the backend with
finds; it starts the backend with `TRUSTED_PROXIES=127.0.0.1/32`, since nginx `TRUSTED_PROXIES=127.0.0.1/32`, since nginx is its only client
is its only client
- 2026-09-29: ready to run under upaas (issue #59): the image has a - 2026-09-29: ready to run under upaas (issue #59): the image has a
`HEALTHCHECK` that requests `/.well-known/healthcheck` through nginx on the `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 port from `PORT`. The backend no longer reads a bad `PORT` as 0 or a bad
+2 -4
View File
@@ -111,10 +111,8 @@ 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 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 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 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 limit. A value with a character no IP address or CIDR has, such as a hostname,
`1.2.3`, stops the container at start with an error naming `TRUSTED_PROXIES`: 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,9 +2,6 @@
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"
@@ -25,19 +22,6 @@ 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
@@ -29,3 +29,7 @@ func ClientIP(
) string { ) string {
return clientIP(remoteAddr, header, trusted) return clientIP(remoteAddr, header, trusted)
} }
func ParseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
return parseTrustedProxies(cidrs)
}
+4 -6
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,11 +77,9 @@ 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. Each entry must be // prefixes, failing fast on any malformed entry.
// a CIDR; a lone address is refused. "netwatch-server check-cidr" func parseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
// 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 {
+3 -20
View File
@@ -36,32 +36,15 @@ 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()
for _, cidr := range []string{ _, err := middleware.ParseTrustedProxies([]string{"not-a-cidr"})
"not-a-cidr", "10.0.0.1", "1.2.3/32", "172.30/32", "10/32", if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
"cafe/32", "999.1.1.1/32", "10.0.0.0/33", "::1/129", t.Fatalf("error = %v, want one naming TRUSTED_PROXIES", err)
"fe80::1%eth0/128",
} {
_, err := middleware.ParseTrustedProxies([]string{cidr})
if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
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 {
name string name string
remoteAddr string remoteAddr string
+13 -21
View File
@@ -35,30 +35,22 @@ fi
# TRUSTED_PROXIES names the reverse proxies in front of the container, # TRUSTED_PROXIES names the reverse proxies in front of the container,
# as IP addresses or CIDRs separated by commas. nginx takes the client # 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 # 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 # unset or empty, it trusts no one. nginx would look up a hostname at
# here, one set_real_ip_from line per entry. # start and trust whatever address it found, so a value with a
# # character no address has stops the container here. An entry such as
# nginx looks up an entry it cannot read as an address as a hostname, # 999.1.1.1 gets past this, and nginx refuses it at start as a
# and trusts what it finds (1.2.3 is found as 1.2.0.3). So each entry # hostname it cannot find.
# 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:-}" TRUSTED_PROXIES="${TRUSTED_PROXIES:-}"
set -f case "$TRUSTED_PROXIES" in
for proxy in $(printf '%s' "$TRUSTED_PROXIES" | tr ',' ' '); do *[!0-9A-Fa-f.:/,\ ]*)
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" \ echo "entrypoint: TRUSTED_PROXIES must be IP addresses or CIDRs" \
"separated by commas; '$proxy' is neither" >&2 "separated by commas, not '$TRUSTED_PROXIES'" >&2
exit 1 exit 1
fi ;;
echo "set_real_ip_from $cidr;" 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 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.