diff --git a/ip.go b/ip.go index dfab5ab90..e6b3e6939 100644 --- a/ip.go +++ b/ip.go @@ -93,6 +93,20 @@ e.IPExtractor = echo.ExtractIPFromXFFHeader( - Ref: https://godoc.org/github.com/labstack/echo#TrustOption +If your outermost proxy strips incoming `X-Forwarded-For` headers and is the +sole writer of the chain (as in some Azure Front Door setups), the client +address is the leftmost entry instead of the first untrusted entry from the +right: use `XFFLeftmost()` to scan from the left. + +```go +e.IPExtractor = echo.ExtractIPFromXFFHeader( + echo.XFFLeftmost(), +) +``` + +Do not use `XFFLeftmost()` otherwise — the leftmost entry is +client-controlled and can be spoofed. + ## Case 3. With proxies using `X-Real-IP` header `X-Real-IP` is another HTTP header to relay clients' IP addresses, but it carries only one address unlike XFF. @@ -138,6 +152,7 @@ type ipChecker struct { trustLoopback bool trustLinkLocal bool trustPrivateNet bool + xffLeftmost bool } // TrustOption is config for which IP address to trust @@ -171,6 +186,25 @@ func TrustIPRange(ipRange *net.IPNet) TrustOption { } } +// XFFLeftmost configures ExtractIPFromXFFHeader to return the first untrusted IP +// reading from the left of the X-Forwarded-For chain instead of the default +// first untrusted IP reading from the right. +// +// Use this only when the outermost proxy in front of your application strips +// incoming X-Forwarded-For headers and is the sole writer of the chain, as in +// some Azure Front Door setups: the leftmost entry is then the client address +// recorded by your own edge proxy. Otherwise the leftmost entry is +// client-controlled and can be spoofed — a client can prepend arbitrary +// addresses to the header before it reaches your infrastructure. +// +// This option has no effect on ExtractIPFromRealIPHeader or +// ExtractSchemeFromHeaders. +func XFFLeftmost() TrustOption { + return func(c *ipChecker) { + c.xffLeftmost = true + } +} + func newIPChecker(configs []TrustOption) *ipChecker { checker := &ipChecker{trustLoopback: true, trustLinkLocal: true, trustPrivateNet: true} for _, configure := range configs { @@ -248,7 +282,13 @@ func ExtractIPFromXFFHeader(options ...TrustOption) IPExtractor { return directIP } ips := append(strings.Split(strings.Join(xffs, ","), ","), directIP) - for i := len(ips) - 1; i >= 0; i-- { + // Scan order: from the right (nearest the server, the default) or + // from the left (furthest from the server) with XFFLeftmost. + start, end, step := len(ips)-1, -1, -1 + if checker.xffLeftmost { + start, end, step = 0, len(ips), 1 + } + for i := start; i != end; i += step { ips[i] = strings.TrimSpace(ips[i]) ips[i] = strings.TrimPrefix(ips[i], "[") ips[i] = strings.TrimSuffix(ips[i], "]") diff --git a/ip_test.go b/ip_test.go index b20368616..9df2627a2 100644 --- a/ip_test.go +++ b/ip_test.go @@ -715,6 +715,104 @@ func TestExtractIPFromXFFHeader(t *testing.T) { } } +func TestExtractIPFromXFFHeaderLeftmost(t *testing.T) { + var testCases = []struct { + whenRequest http.Request + name string + expectIP string + givenTrustOptions []TrustOption + }{ + { + name: "leftmost picks client over Front Door egress proxy (issue #2803)", + whenRequest: http.Request{ + Header: http.Header{ + HeaderXForwardedFor: []string{"203.0.113.195, 203.0.113.199"}, + }, + RemoteAddr: "10.0.0.5:8080", + }, + expectIP: "203.0.113.195", + }, + { + name: "leftmost skips trusted entries on the left", + whenRequest: http.Request{ + Header: http.Header{ + HeaderXForwardedFor: []string{"10.0.0.9, 203.0.113.195, 203.0.113.199"}, + }, + RemoteAddr: "10.0.0.5:8080", + }, + expectIP: "203.0.113.195", + }, + { + name: "all entries trusted, falls back to leftmost entry like the default scan", + whenRequest: http.Request{ + Header: http.Header{ + HeaderXForwardedFor: []string{"10.0.0.9, 10.0.0.10"}, + }, + RemoteAddr: "10.0.0.5:8080", + }, + expectIP: "10.0.0.9", + }, + { + name: "invalid entry, extract IP from remote addr", + whenRequest: http.Request{ + Header: http.Header{ + HeaderXForwardedFor: []string{"not-an-ip, 203.0.113.195"}, + }, + RemoteAddr: "10.0.0.5:8080", + }, + expectIP: "10.0.0.5", + }, + { + name: "request has no XFF header, extracts IP from request remote addr", + whenRequest: http.Request{ + RemoteAddr: "203.0.113.1:8080", + }, + expectIP: "203.0.113.1", + }, + { + name: "multiple XFF header values are joined, leftmost wins", + whenRequest: http.Request{ + Header: http.Header{ + HeaderXForwardedFor: []string{"203.0.113.195", "203.0.113.199"}, + }, + RemoteAddr: "10.0.0.5:8080", + }, + expectIP: "203.0.113.195", + }, + { + name: "IPv6 entries with brackets, leftmost wins", + whenRequest: http.Request{ + Header: http.Header{ + HeaderXForwardedFor: []string{"[2001:db8::1], [2001:db8::2]"}, + }, + RemoteAddr: "[fd00::1]:8080", + }, + expectIP: "2001:db8::1", + }, + { + name: "trust options still apply: trusted leftmost entry is skipped", + givenTrustOptions: []TrustOption{ + TrustIPRange(mustParseCIDR("203.0.113.195/32")), + }, + whenRequest: http.Request{ + Header: http.Header{ + HeaderXForwardedFor: []string{"203.0.113.195, 203.0.113.199"}, + }, + RemoteAddr: "10.0.0.5:8080", + }, + expectIP: "203.0.113.199", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + options := append([]TrustOption{XFFLeftmost()}, tc.givenTrustOptions...) + extractedIP := ExtractIPFromXFFHeader(options...)(&tc.whenRequest) + assert.Equal(t, tc.expectIP, extractedIP) + }) + } +} + func TestLegacyIPExtractor(t *testing.T) { var testCases = []struct { name string