middleware: take the client address from the right of X-Forwarded-For (closes #181) #183

Merged
clawbot merged 1 commits from issue-181-xff-client into next 2026-10-01 22:35:18 +02:00
5 changed files with 122 additions and 14 deletions
+3 -3
View File
@@ -347,9 +347,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.
**`DNSWATCHER_DNS_INTERVAL` and `DNSWATCHER_TLS_INTERVAL`** take a positive
duration: a number followed by a unit such as `s`, `m` or `h`, for example
+2
View File
@@ -19,6 +19,8 @@ nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105
# 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: a nameserver that does not answer is saved as `error` with the
reason, and NS failure and NS recovery are notified (closes #104).
- 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is
+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
}
+84 -3
View File
@@ -342,9 +342,9 @@ func TestDashboardRendersWithSecurityHeaders(t *testing.T) {
}
}
// Addresses for the rate limit tests: a client connecting directly, a
// trusted proxy, and a client behind that proxy as its X-Real-IP
// header names it.
// Addresses for the rate limit and realIP tests: a client connecting
// directly, a trusted proxy, and a client behind that proxy as the
// proxy's X-Real-IP or X-Forwarded-For header names it.
const (
directClient = "198.51.100.1:4000"
trustedProxy = "10.0.0.1:4000"
@@ -482,3 +482,84 @@ 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",
directClient, proxiedClient, []string{"198.51.100.9"},
"198.51.100.1",
},
{
"X-Real-IP from a trusted proxy wins",
trustedProxy, proxiedClient, []string{"203.0.113.8"},
proxiedClient,
},
{
"client's own entry, then the one the proxy added",
trustedProxy, "", []string{"198.51.100.9, 203.0.113.1"},
proxiedClient,
},
{
"several trusted proxies",
trustedProxy, "",
[]string{"198.51.100.9, 203.0.113.1, 10.0.0.3, 10.0.0.2"},
proxiedClient,
},
{
"proxy adds a header line of its own",
trustedProxy, "", []string{"198.51.100.9", proxiedClient},
proxiedClient,
},
{
"every entry a trusted proxy",
trustedProxy, "", []string{"10.0.0.3, 10.0.0.2"},
"10.0.0.3",
},
{
"empty where the client address belongs",
trustedProxy, "", []string{"203.0.113.1, , 10.0.0.2"},
"10.0.0.1",
},
{
"no headers from a trusted proxy",
trustedProxy, "", 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)
}
})
}
}