middleware: take the client address from the right of X-Forwarded-For (closes #181)
check / check (push) Failing after 39s

realIP took the first X-Forwarded-For entry, which the client itself
can write, so behind a proxy that appends to the header a client chose
the address dnswatcher logs and the /metrics rate limit counts. It now
walks the entries from the right past trusted proxies, using the
existing trusted-proxy check, and takes the first that is not one; the
leftmost when all are. All X-Forwarded-For header lines are read as one
list, since a proxy may add its own line instead of appending to the
client's. An empty entry where the client address belongs falls back
to the peer address, as an empty first entry did before. X-Real-IP is
unchanged.

Model: opus-5-5
This commit is contained in:
2026-10-01 20:14:55 +00:00
parent ed0f56f144
commit 82265617da
5 changed files with 134 additions and 11 deletions
+3 -3
View File
@@ -340,9 +340,9 @@ minute, failed logins included; beyond that it answers `429 Too Many Requests`
without checking the password. A Prometheus server scraping every 15 seconds
sends 4 a minute. IPv6 addresses in one /64 count as one client. When the
request comes from a private or loopback address, such as a reverse proxy's,
the client address is taken from the `X-Real-IP` or `X-Forwarded-For` header
the proxy sets; a proxy that sets neither makes all its clients share one
allowance.
the client address is taken from the `X-Real-IP` header the proxy sets, or else
from `X-Forwarded-For`, as the last address in it that is not private or
loopback. A proxy that sets neither makes all its clients share one allowance.
### Example `.env`
+2
View File
@@ -20,6 +20,8 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
# Completed Steps
- 2026-10-01: the client address from `X-Forwarded-For` is the last entry that
is not a trusted proxy, not the first, which the client sets (closes #181).
- 2026-10-01: `/metrics` allows each client address 30 requests a minute,
counted before Basic Auth, and answers 429 beyond that (closes #101).
- 2026-10-01: the image built by `make docker` reports the `git describe`
+10 -1
View File
@@ -1,6 +1,9 @@
package middleware
import "time"
import (
"net/http"
"time"
)
// The /metrics rate limit, exported so the tests can count requests
// against it.
@@ -8,3 +11,9 @@ const (
MetricsRequestLimit = metricsRequestLimit
MetricsRequestWindow time.Duration = metricsRequestWindow
)
// RealIP is realIP, exported so the tests can check which address it
// takes as the client's.
func RealIP(r *http.Request) string {
return realIP(r)
}
+23 -7
View File
@@ -209,6 +209,12 @@ func isTrustedProxy(ip net.IP) bool {
// realIP extracts the client's real IP address from the request.
// Proxy headers are only trusted from RFC1918/loopback addresses.
//
// Each proxy adds to the end of X-Forwarded-For the address it got the
// request from, so the client can write every entry before the one the
// first trusted proxy added. The client address is therefore the
// rightmost entry that is not a trusted proxy, or the leftmost entry
// when they all are.
func realIP(r *http.Request) string {
addr := ipFromHostPort(r.RemoteAddr)
remoteIP := net.ParseIP(addr)
@@ -223,16 +229,26 @@ func realIP(r *http.Request) string {
return ip
}
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
if parts := strings.SplitN(
xff, ",", 2, //nolint:mnd
); len(parts) > 0 {
if ip := strings.TrimSpace(parts[0]); ip != "" {
return ip
}
// A proxy may add its entry as a header line of its own instead of
// appending to the line the client sent, so all lines form one list.
entries := strings.Split(
strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
)
client := strings.TrimSpace(entries[0])
for i := len(entries) - 1; i > 0; i-- {
entry := strings.TrimSpace(entries[i])
if !isTrustedProxy(net.ParseIP(entry)) {
client = entry
break
}
}
if client != "" {
return client
}
return addr
}
+96
View File
@@ -482,3 +482,99 @@ func TestMetricsRateLimitKeysOnClientAddress(t *testing.T) {
})
}
}
// TestRealIP checks which address realIP takes as the client's. Each
// element of forwardedFor is sent as an X-Forwarded-For header line of
// its own, and 198.51.100.9 is always an entry the client wrote itself.
func TestRealIP(t *testing.T) {
t.Parallel()
tests := []struct {
name string
remoteAddr string
xRealIP string
forwardedFor []string
want string
}{
{
"untrusted peer, both headers ignored",
"198.51.100.1:4000",
"203.0.113.1",
[]string{"203.0.113.2"},
"198.51.100.1",
},
{
"X-Real-IP from a trusted proxy wins",
"10.0.0.1:4000",
"203.0.113.1",
[]string{"203.0.113.2"},
"203.0.113.1",
},
{
"client's own entry, then the one the proxy added",
"10.0.0.1:4000",
"",
[]string{"198.51.100.9, 203.0.113.1"},
"203.0.113.1",
},
{
"several trusted proxies",
"10.0.0.1:4000",
"",
[]string{"198.51.100.9, 203.0.113.1, 10.0.0.3, 10.0.0.2"},
"203.0.113.1",
},
{
"proxy adds a header line of its own",
"10.0.0.1:4000",
"",
[]string{"198.51.100.9", "203.0.113.1"},
"203.0.113.1",
},
{
"every entry a trusted proxy",
"10.0.0.1:4000",
"",
[]string{"10.0.0.3, 10.0.0.2"},
"10.0.0.3",
},
{
"empty where the client address belongs",
"10.0.0.1:4000",
"",
[]string{"203.0.113.1, , 10.0.0.2"},
"10.0.0.1",
},
{
"no headers from a trusted proxy",
"10.0.0.1:4000",
"",
nil,
"10.0.0.1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/", nil,
)
req.RemoteAddr = tt.remoteAddr
if tt.xRealIP != "" {
req.Header.Set("X-Real-IP", tt.xRealIP)
}
for _, line := range tt.forwardedFor {
req.Header.Add("X-Forwarded-For", line)
}
got := middleware.RealIP(req)
if got != tt.want {
t.Errorf("realIP = %q, want %q", got, tt.want)
}
})
}
}