diff --git a/CHANGELOG.md b/CHANGELOG.md index c8017d19d..5593bac8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## v4.16.3 - 2026-10-29 + +**Security** + +* **Client IP defaults:** when `Echo#IPExtractor` is unset, `Context.RealIP()` now uses the connection peer and ignores `X-Forwarded-For` and `X-Real-IP`. This prevents clients from selecting their own default rate-limit bucket. Fixes [CVE-2026-118653](https://github.com/labstack/echo/security/advisories/GHSA-246p-cpwv-v3jq). +* **Proxy identity:** Proxy middleware always replaces upstream `X-Real-IP` with the derived client identity, including after `Connection`-nominated header removal and on WebSocket upgrades. The identity is derived once and preserved across retries. Fixes [CVE-2026-118651](https://github.com/labstack/echo/security/advisories/GHSA-99jh-6h7p-pp36). + +**Migration required for applications behind proxies** + +Configure `Echo#IPExtractor` to trust only your deployment's proxies before upgrading. Otherwise, clients behind one proxy share its address for rate limiting, access controls, logging, and proxy forwarding. See the [README migration example](README.md#client-ip-security-migration-since-v4163) for explicit XFF trust ranges; use the same trust options with `ExtractIPFromRealIPHeader` for proxies that set `X-Real-IP`. Configure the outermost proxy to replace client-supplied identity headers. Explicit extractors and custom rate-limit identifiers retain their behavior. + +**Other changes** + +* The direct-peer fallback now accepts bare IPv4 and IPv6 addresses without a port. +* Document v4 bug-fix and security-fix support dates (#3194). + +Implementation: [#3197](https://github.com/labstack/echo/pull/3197). No dependency or minimum Go-version changes; Go 1.26 or later remains required. + ## v4.16.0 - 2026-09-27 **Security** diff --git a/README.md b/README.md index 49040fd7b..ede2997bb 100644 --- a/README.md +++ b/README.md @@ -61,9 +61,32 @@ Click [here](https://github.com/sponsors/labstack) for more information on spons // go get github.com/labstack/echo/{version} go get github.com/labstack/echo/v4 ``` -Echo v4.16.1 requires Go 1.26 or later so it can use patched `golang.org/x/` dependencies. Use a supported, patched Go toolchain when building applications. +Echo v4.16.1 and later require Go 1.26 or later to use patched `golang.org/x/` dependencies. Use a supported, patched Go toolchain when building applications. -### Security defaults in v4.16.1 +Echo v4 receives bug fixes until 2026-12-31 and free security fixes until 2027-12-31, on its latest release. New projects should use [v5](https://github.com/labstack/echo). See [SECURITY.md](https://github.com/labstack/echo/blob/master/SECURITY.md) for the full policy. + +### Client IP security migration since v4.16.3 + +Echo v4.16.3 changes client IP defaults: without `Echo#IPExtractor`, `Context.RealIP()` uses the connection peer and ignores `X-Forwarded-For` and `X-Real-IP`. Proxy middleware always replaces upstream `X-Real-IP` with this result. This fixes CVE-2026-118651 and CVE-2026-118653. + +**Migration for applications behind proxies:** configure `Echo#IPExtractor` before starting the server. Otherwise, clients behind the same proxy share its IP address for rate limiting, access controls, and logging. Use `echo.ExtractIPDirect()` when there is no proxy. For XFF-based proxies, disable the broad default trust ranges and trust only your proxy addresses: + +```go +_, proxyRange, err := net.ParseCIDR("10.0.0.10/32") // replace with your proxy's address or CIDR +if err != nil { + log.Fatal(err) +} +e.IPExtractor = echo.ExtractIPFromXFFHeader( + echo.TrustLoopback(false), + echo.TrustLinkLocal(false), + echo.TrustPrivateNet(false), + echo.TrustIPRange(proxyRange), +) +``` + +If your proxy uses `X-Real-IP`, use `echo.ExtractIPFromRealIPHeader` with the same trust options. Configure the outermost proxy to replace client-supplied identity headers. Explicitly configured extractors and custom rate-limit identifiers retain their behavior. + +### Security defaults since v4.16.1 `BodyDump` captures at most 5 MiB of the request and 5 MiB of the response by default. The application and client still receive the complete bodies. Configure `BodyDumpConfig.MaxRequestBytes` and `MaxResponseBytes` to change the capture limits; `-1` disables a limit. diff --git a/client_ip_security_test.go b/client_ip_security_test.go new file mode 100644 index 000000000..e11eb0916 --- /dev/null +++ b/client_ip_security_test.go @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors + +package echo + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestContextRealIPIgnoresUntrustedHeaders(t *testing.T) { + for _, remote := range []struct { + address string + want string + }{ + {"198.51.100.10:1234", "198.51.100.10"}, + {"[2001:db8::10]:1234", "2001:db8::10"}, + {"198.51.100.10", "198.51.100.10"}, + {"2001:db8::10", "2001:db8::10"}, + {"invalid", ""}, + } { + t.Run(remote.address, func(t *testing.T) { + for _, headers := range []http.Header{ + {}, + {HeaderXForwardedFor: {"203.0.113.1"}}, + {HeaderXForwardedFor: {"203.0.113.1, 203.0.113.2", "203.0.113.3"}}, + {HeaderXRealIP: {"203.0.113.1"}}, + {HeaderXRealIP: {"[2001:db8::1]"}}, + {HeaderXForwardedFor: {"invalid"}, HeaderXRealIP: {"203.0.113.1"}}, + } { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = remote.address + req.Header = headers + assert.Equal(t, remote.want, New().NewContext(req, httptest.NewRecorder()).RealIP()) + assert.Equal(t, remote.want, (&context{request: req}).RealIP()) + } + }) + } +} + +func TestContextRealIPPreservesCustomExtractor(t *testing.T) { + e := New() + e.IPExtractor = func(r *http.Request) string { + return r.Header.Get("Custom-Client-IP") + } + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("Custom-Client-IP", "203.0.113.10") + assert.Equal(t, "203.0.113.10", e.NewContext(req, httptest.NewRecorder()).RealIP()) +} diff --git a/context.go b/context.go index dd5f11c46..07b24e0ca 100644 --- a/context.go +++ b/context.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "mime/multipart" - "net" "net/http" "net/url" "strings" @@ -40,9 +39,10 @@ type Context interface { // Scheme returns the HTTP protocol scheme, `http` or `https`. Scheme() string - // RealIP returns the client's network address based on `X-Forwarded-For` - // or `X-Real-IP` request header. - // The behavior can be configured using `Echo#IPExtractor`. + // RealIP returns the client IP from Echo#IPExtractor, or the connection peer + // when no extractor is configured. Forwarding headers are ignored by default. + // Applications behind proxies must configure an extractor that trusts only + // their own proxies to recover individual client addresses. RealIP() string // Path returns the registered path for the handler. @@ -299,24 +299,8 @@ func (c *context) RealIP() string { if c.echo != nil && c.echo.IPExtractor != nil { return c.echo.IPExtractor(c.request) } - // Fall back to legacy behavior - if ip := c.request.Header.Get(HeaderXForwardedFor); ip != "" { - i := strings.IndexAny(ip, ",") - if i > 0 { - xffip := strings.TrimSpace(ip[:i]) - xffip = strings.TrimPrefix(xffip, "[") - xffip = strings.TrimSuffix(xffip, "]") - return xffip - } - return ip - } - if ip := c.request.Header.Get(HeaderXRealIP); ip != "" { - ip = strings.TrimPrefix(ip, "[") - ip = strings.TrimSuffix(ip, "]") - return ip - } - ra, _, _ := net.SplitHostPort(c.request.RemoteAddr) - return ra + // Forwarding headers are client-controlled unless a trusted proxy extractor is configured. + return extractIP(c.request) } func (c *context) Path() string { diff --git a/context_test.go b/context_test.go index 67a2b6174..a484fff6d 100644 --- a/context_test.go +++ b/context_test.go @@ -1293,90 +1293,16 @@ func TestContext_Logger(t *testing.T) { } func TestContext_RealIP(t *testing.T) { - tests := []struct { - c Context - s string - }{ - { - &context{ - request: &http.Request{ - Header: http.Header{HeaderXForwardedFor: []string{"127.0.0.1, 127.0.1.1, "}}, - }, - }, - "127.0.0.1", - }, - { - &context{ - request: &http.Request{ - Header: http.Header{HeaderXForwardedFor: []string{"127.0.0.1,127.0.1.1"}}, - }, - }, - "127.0.0.1", - }, - { - &context{ - request: &http.Request{ - Header: http.Header{HeaderXForwardedFor: []string{"127.0.0.1"}}, - }, - }, - "127.0.0.1", - }, - { - &context{ - request: &http.Request{ - Header: http.Header{HeaderXForwardedFor: []string{"[2001:db8:85a3:8d3:1319:8a2e:370:7348], 2001:db8::1, "}}, - }, - }, - "2001:db8:85a3:8d3:1319:8a2e:370:7348", - }, - { - &context{ - request: &http.Request{ - Header: http.Header{HeaderXForwardedFor: []string{"[2001:db8:85a3:8d3:1319:8a2e:370:7348],[2001:db8::1]"}}, - }, - }, - "2001:db8:85a3:8d3:1319:8a2e:370:7348", - }, - { - &context{ - request: &http.Request{ - Header: http.Header{HeaderXForwardedFor: []string{"2001:db8:85a3:8d3:1319:8a2e:370:7348"}}, - }, - }, - "2001:db8:85a3:8d3:1319:8a2e:370:7348", - }, - { - &context{ - request: &http.Request{ - Header: http.Header{ - "X-Real-Ip": []string{"192.168.0.1"}, - }, - }, - }, - "192.168.0.1", - }, - { - &context{ - request: &http.Request{ - Header: http.Header{ - "X-Real-Ip": []string{"[2001:db8::1]"}, - }, - }, - }, - "2001:db8::1", - }, - - { - &context{ - request: &http.Request{ - RemoteAddr: "89.89.89.89:1654", - }, - }, - "89.89.89.89", - }, - } - - for _, tt := range tests { - assert.Equal(t, tt.s, tt.c.RealIP()) + for _, header := range []http.Header{ + {HeaderXForwardedFor: {"127.0.0.1, 127.0.1.1, "}}, + {HeaderXForwardedFor: {"127.0.0.1,127.0.1.1"}}, + {HeaderXForwardedFor: {"127.0.0.1"}}, + {HeaderXForwardedFor: {"[2001:db8::1], 2001:db8::2"}}, + {HeaderXRealIP: {"192.168.0.1"}}, + {HeaderXRealIP: {"[2001:db8::1]"}}, + {}, + } { + c := &context{request: &http.Request{Header: header, RemoteAddr: "89.89.89.89:1654"}} + assert.Equal(t, "89.89.89.89", c.RealIP()) } } diff --git a/echo.go b/echo.go index 99debe7a7..b50a5c129 100644 --- a/echo.go +++ b/echo.go @@ -97,7 +97,10 @@ type Echo struct { Validator Validator Renderer Renderer Logger Logger - IPExtractor IPExtractor + // IPExtractor defines how Context.RealIP determines the client IP address. + // If nil, only the connection peer is used. Applications behind a proxy must + // configure a header extractor that trusts only their own proxies. + IPExtractor IPExtractor // SchemeExtractor defines the strategy for determining the request scheme (`http` or `https`), // particularly important when behind proxies or load balancers that terminate TLS. // Used by Context.Scheme and by the HTTPS redirect, Secure and Proxy middlewares. @@ -285,7 +288,7 @@ const ( const ( // Version of Echo - Version = "4.16.2" + Version = "4.16.3" website = "https://echo.labstack.com" // http://patorjk.com/software/taag/#p=display&f=Small%20Slant&t=Echo banner = ` diff --git a/ip.go b/ip.go index dce51f55d..4de5e65c5 100644 --- a/ip.go +++ b/ip.go @@ -24,7 +24,8 @@ To retrieve IP address reliably/securely, you must let your application be aware In Echo, this can be done by configuring `Echo#IPExtractor` appropriately. This guides show you why and how. -> Note: if you don't set `Echo#IPExtractor` explicitly, Echo fallback to legacy behavior, which is not a good choice. +> Note: if you don't set `Echo#IPExtractor` explicitly, Echo uses the connection peer and ignores forwarding headers. +> Applications behind proxies must configure a trusted proxy extractor to recover individual client addresses. Let's start from two questions to know the right direction: @@ -114,10 +115,10 @@ To control this behavior, use [`TrustOption`](https://godoc.org/github.com/labst ## About default behavior -In default behavior, Echo sees all of first XFF header, X-Real-IP header and IP from network layer. - -As you might already notice, after reading this article, this is not good. -Sole reason this is default is just backward compatibility. +By default, Echo uses the connection peer from req.RemoteAddr and ignores X-Forwarded-For and X-Real-IP. +This prevents clients from choosing their own identity for rate limiting, access control, or proxy forwarding. +Behind a proxy, all clients share that proxy's address unless Echo#IPExtractor is configured. +Configure a header extractor with trust options limited to your own proxies before relying on individual client IPs. ## Private IP ranges diff --git a/middleware/client_ip_security_test.go b/middleware/client_ip_security_test.go new file mode 100644 index 000000000..35f2bcf17 --- /dev/null +++ b/middleware/client_ip_security_test.go @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors + +package middleware + +import ( + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" +) + +type clientIPTransport func(*http.Request) (*http.Response, error) + +func (f clientIPTransport) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + +func TestClientIPSecurity(t *testing.T) { + _, proxyRange, err := net.ParseCIDR("10.0.0.10/32") + assert.NoError(t, err) + trust := []echo.TrustOption{echo.TrustLoopback(false), echo.TrustLinkLocal(false), echo.TrustPrivateNet(false), echo.TrustIPRange(proxyRange)} + for _, tc := range []struct { + name string + extractor echo.IPExtractor + peer string + xff [2]string + realIP [2]string + wantIP string + }{ + {"default", nil, "198.51.100.10:1234", [2]string{"203.0.113.1", "203.0.113.2"}, [2]string{"203.0.113.3", "203.0.113.4"}, "198.51.100.10"}, + {"direct", echo.ExtractIPDirect(), "198.51.100.10:1234", [2]string{"203.0.113.1", "203.0.113.2"}, [2]string{"203.0.113.3", "203.0.113.4"}, "198.51.100.10"}, + {"XFF untrusted peer", echo.ExtractIPFromXFFHeader(trust...), "198.51.100.10:1234", [2]string{"203.0.113.1", "203.0.113.2"}, [2]string{"203.0.113.3", "203.0.113.4"}, "198.51.100.10"}, + {"XFF trusted proxy", echo.ExtractIPFromXFFHeader(trust...), "10.0.0.10:1234", [2]string{"203.0.113.1, 198.51.100.10", "203.0.113.2, 198.51.100.10"}, [2]string{"203.0.113.3", "203.0.113.4"}, "198.51.100.10"}, + {"real IP untrusted peer", echo.ExtractIPFromRealIPHeader(trust...), "198.51.100.10:1234", [2]string{"203.0.113.1", "203.0.113.2"}, [2]string{"203.0.113.3", "203.0.113.4"}, "198.51.100.10"}, + {"real IP trusted proxy", echo.ExtractIPFromRealIPHeader(trust...), "10.0.0.10:1234", [2]string{"203.0.113.1", "203.0.113.2"}, [2]string{"198.51.100.10", "198.51.100.10"}, "198.51.100.10"}, + {"custom extractor", func(*http.Request) string { return "custom-client" }, "10.0.0.10:1234", [2]string{"203.0.113.1", "203.0.113.2"}, [2]string{"203.0.113.3", "203.0.113.4"}, "custom-client"}, + } { + t.Run(tc.name, func(t *testing.T) { + request := func(i int) *http.Request { + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.RemoteAddr = tc.peer + r.Header.Set(echo.HeaderXForwardedFor, tc.xff[i]) + r.Header.Set(echo.HeaderXRealIP, tc.realIP[i]) + r.Header.Add(echo.HeaderXRealIP, "203.0.113.99") + if i == 1 { + r.Header.Add(echo.HeaderConnection, "keep-alive, x-ReAl-Ip") + r.Header.Add(echo.HeaderConnection, "X-Real-IP") + } + return r + } + t.Run("rate limit", func(t *testing.T) { + e := echo.New() + e.IPExtractor = tc.extractor + store := NewRateLimiterMemoryStoreWithConfig(RateLimiterMemoryStoreConfig{Rate: 1e-9, Burst: 1}) + e.GET("/", func(c echo.Context) error { return c.NoContent(http.StatusOK) }, RateLimiter(store)) + for i, want := range []int{http.StatusOK, http.StatusTooManyRequests} { + rec := httptest.NewRecorder() + e.ServeHTTP(rec, request(i)) + assert.Equal(t, want, rec.Code) + } + }) + t.Run("upstream header", func(t *testing.T) { + e := echo.New() + e.IPExtractor = tc.extractor + target, err := url.Parse("http://upstream.invalid") + assert.NoError(t, err) + calls := 0 + e.Use(ProxyWithConfig(ProxyConfig{ + Balancer: NewRoundRobinBalancer([]*ProxyTarget{{URL: target}}), + Transport: clientIPTransport(func(r *http.Request) (*http.Response, error) { + calls++ + assert.Equal(t, []string{tc.wantIP}, r.Header.Values(echo.HeaderXRealIP)) + return &http.Response{StatusCode: http.StatusOK, Header: make(http.Header), Body: io.NopCloser(strings.NewReader("ok")), ContentLength: 2}, nil + }), + })) + for i := range 2 { + rec := httptest.NewRecorder() + e.ServeHTTP(rec, request(i)) + assert.Equal(t, http.StatusOK, rec.Code) + } + assert.Equal(t, 2, calls) + }) + }) + } +} + +func TestClientIPTrustedProxySeparatesClients(t *testing.T) { + e := echo.New() + e.IPExtractor = echo.ExtractIPFromXFFHeader() + store := NewRateLimiterMemoryStoreWithConfig(RateLimiterMemoryStoreConfig{Rate: 1e-9, Burst: 1}) + e.GET("/", func(c echo.Context) error { return c.NoContent(http.StatusOK) }, RateLimiter(store)) + for _, client := range []string{"198.51.100.10", "198.51.100.11"} { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "10.0.0.10:1234" + req.Header.Set(echo.HeaderXForwardedFor, client) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, http.StatusOK, rec.Code) + } +} + +func TestProxyClientIPSurvivesConnectionHeader(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, []string{"198.51.100.10"}, r.Header.Values(echo.HeaderXRealIP)) + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + target, err := url.Parse(upstream.URL) + assert.NoError(t, err) + e := echo.New() + calls := 0 + e.IPExtractor = func(r *http.Request) string { + calls++ + return echo.ExtractIPDirect()(r) + } + e.Use(Proxy(NewRoundRobinBalancer([]*ProxyTarget{{URL: target}}))) + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "198.51.100.10:1234" + req.Header.Set(echo.HeaderXRealIP, "203.0.113.1") + req.Header.Set(echo.HeaderXForwardedFor, "203.0.113.2") + req.Header.Add(echo.HeaderConnection, "keep-alive, x-ReAl-Ip") + req.Header.Add(echo.HeaderConnection, "X-Real-IP") + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, 1, calls) +} diff --git a/middleware/logger_test.go b/middleware/logger_test.go index e4b783db5..a41fdfc47 100644 --- a/middleware/logger_test.go +++ b/middleware/logger_test.go @@ -63,18 +63,18 @@ func TestLoggerDefaultMW(t *testing.T) { expect: `{"time":"2020-04-28T01:26:40Z","id":"","remote_ip":"192.0.2.1","host":"example.com","method":"GET","uri":"/","user_agent":"","status":500,"error":"error\nwith\nnewlines\tand\ttabs","latency":1,"latency_human":"1µs","bytes_in":0,"bytes_out":36}` + "\n", }, { - name: "ok, remote_ip from X-Real-Ip header", + name: "ok, remote_ip ignores X-Real-Ip header", whenHeader: map[string]string{echo.HeaderXRealIP: "127.0.0.1"}, whenStatusCode: http.StatusOK, whenResponse: "test", - expect: `{"time":"2020-04-28T01:26:40Z","id":"","remote_ip":"127.0.0.1","host":"example.com","method":"GET","uri":"/","user_agent":"","status":200,"error":"","latency":1,"latency_human":"1µs","bytes_in":0,"bytes_out":4}` + "\n", + expect: `{"time":"2020-04-28T01:26:40Z","id":"","remote_ip":"192.0.2.1","host":"example.com","method":"GET","uri":"/","user_agent":"","status":200,"error":"","latency":1,"latency_human":"1µs","bytes_in":0,"bytes_out":4}` + "\n", }, { - name: "ok, remote_ip from X-Forwarded-For header", + name: "ok, remote_ip ignores X-Forwarded-For header", whenHeader: map[string]string{echo.HeaderXForwardedFor: "127.0.0.1"}, whenStatusCode: http.StatusOK, whenResponse: "test", - expect: `{"time":"2020-04-28T01:26:40Z","id":"","remote_ip":"127.0.0.1","host":"example.com","method":"GET","uri":"/","user_agent":"","status":200,"error":"","latency":1,"latency_human":"1µs","bytes_in":0,"bytes_out":4}` + "\n", + expect: `{"time":"2020-04-28T01:26:40Z","id":"","remote_ip":"192.0.2.1","host":"example.com","method":"GET","uri":"/","user_agent":"","status":200,"error":"","latency":1,"latency_human":"1µs","bytes_in":0,"bytes_out":4}` + "\n", }, } @@ -129,6 +129,7 @@ func TestLoggerWithLoggerConfig(t *testing.T) { var testCases = []struct { name string givenConfig LoggerConfig + ipExtractor echo.IPExtractor whenURI string whenMethod string whenHost string @@ -172,6 +173,7 @@ func TestLoggerWithLoggerConfig(t *testing.T) { { name: "ok, json string escape: remote_ip", givenConfig: LoggerConfig{Format: `{"remote_ip":"${remote_ip}"}`}, + ipExtractor: func(r *http.Request) string { return r.Header.Get(echo.HeaderXForwardedFor) }, whenHeader: map[string]string{echo.HeaderXForwardedFor: `\"127.0.0.1\"`}, expect: `{"remote_ip":"\\\"127.0.0.1\\\""}`, }, @@ -291,6 +293,7 @@ func TestLoggerWithLoggerConfig(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { e := echo.New() + e.IPExtractor = tc.ipExtractor req := httptest.NewRequest(http.MethodGet, cmp.Or(tc.whenURI, "/"), nil) if tc.whenFormValues != nil { @@ -396,7 +399,8 @@ func TestLoggerTemplate(t *testing.T) { "secret-form": false, "hexvalue": false, "GET": true, - "127.0.0.1": true, + "127.0.0.1": false, + "192.0.2.1": true, "\"path\":\"/users/1\"": true, "\"route\":\"/users/:id\"": true, "\"uri\":\"/\"": true, diff --git a/middleware/proxy.go b/middleware/proxy.go index a619e9e2e..5631f99de 100644 --- a/middleware/proxy.go +++ b/middleware/proxy.go @@ -232,8 +232,9 @@ func (b *roundRobinBalancer) Next(c echo.Context) *ProxyTarget { // Proxy returns a Proxy middleware. // // Proxy middleware forwards the request to upstream server using a configured load balancing technique. -// The unvalidated Forwarded header is removed. Configure Echo#IPExtractor to -// validate proxy-supplied client identities before trusting X-Real-IP upstream. +// The unvalidated Forwarded header is removed. X-Real-IP is overwritten with +// Context.RealIP(), which uses the connection peer unless Echo#IPExtractor is configured. +// Configure a trusted proxy extractor when Echo is behind a proxy. func Proxy(balancer ProxyBalancer) echo.MiddlewareFunc { c := DefaultProxyConfig c.Balancer = balancer @@ -287,11 +288,9 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc { } // Fix header - // Basically it's not good practice to unconditionally pass incoming x-real-ip header to upstream. - // However, for backward compatibility, legacy behavior is preserved unless you configure Echo#IPExtractor. - if req.Header.Get(echo.HeaderXRealIP) == "" || c.Echo().IPExtractor != nil { - req.Header.Set(echo.HeaderXRealIP, c.RealIP()) - } + // Derive the upstream identity from the configured extractor or the connection peer. + clientIP := c.RealIP() + req.Header.Set(echo.HeaderXRealIP, clientIP) // Always overwrite X-Forwarded-Proto. c.Scheme() uses an incoming X-Forwarded-Proto header only when it comes // from a trusted address (see Echo#SchemeExtractor), so clients cannot spoof the scheme seen by the upstream. // Remove the other scheme headers: X-Forwarded-Proto carries the scheme, and some upstreams (e.g. Rack) would @@ -330,7 +329,7 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc { req = c.Request() // ReverseProxy validates upgrades and closes both sides when a tunnel ends. - proxyHTTP(tgt, c, config).ServeHTTP(res, req) + proxyHTTP(tgt, c, config, clientIP).ServeHTTP(res, req) err, hasError := c.Get("_error").(error) if !hasError { @@ -355,7 +354,7 @@ func ProxyWithConfig(config ProxyConfig) echo.MiddlewareFunc { // 499 too instead of the more problematic 5xx, which does not allow to detect this situation const StatusCodeContextCanceled = 499 -func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handler { +func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig, clientIP string) http.Handler { // The HTTP transport also handles WebSocket upgrades. Normalize WebSocket // target schemes on a copy so shared balancer targets remain unchanged. targetURL := *tgt.URL @@ -387,7 +386,14 @@ func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handle c.Set("_error", httpError) } } - proxy.Transport = config.Transport + transport := config.Transport + if transport == nil { + transport = http.DefaultTransport + } + // ReverseProxy removes headers named by Connection after its Director runs. + // Restore the derived identity at the transport boundary so a client cannot + // suppress it with Connection: X-Real-IP, including on WebSocket upgrades. + proxy.Transport = proxyClientIPTransport{transport: transport, clientIP: clientIP} proxy.ModifyResponse = func(response *http.Response) error { if config.ModifyResponse != nil { if err := config.ModifyResponse(response); err != nil { @@ -411,6 +417,17 @@ func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handle }) } +type proxyClientIPTransport struct { + transport http.RoundTripper + clientIP string +} + +func (t proxyClientIPTransport) RoundTrip(req *http.Request) (*http.Response, error) { + req = req.Clone(req.Context()) + req.Header.Set(echo.HeaderXRealIP, t.clientIP) + return t.transport.RoundTrip(req) +} + type proxyUpgradeResponseWriter struct { http.ResponseWriter } diff --git a/middleware/proxy_test.go b/middleware/proxy_test.go index c470c8931..2622eb4cb 100644 --- a/middleware/proxy_test.go +++ b/middleware/proxy_test.go @@ -197,7 +197,7 @@ func TestProxyRealIPHeader(t *testing.T) { }{ {false, false, remoteAddrIP}, {false, true, extractedRealIP}, - {true, false, realIPHeaderIP}, + {true, false, remoteAddrIP}, {true, true, extractedRealIP}, } diff --git a/middleware/rate_limiter.go b/middleware/rate_limiter.go index 2746a3de1..45d29f789 100644 --- a/middleware/rate_limiter.go +++ b/middleware/rate_limiter.go @@ -42,7 +42,9 @@ var ErrRateLimitExceeded = echo.NewHTTPError(http.StatusTooManyRequests, "rate l // ErrExtractorError denotes an error raised when extractor function is unsuccessful var ErrExtractorError = echo.NewHTTPError(http.StatusForbidden, "error while extracting identifier") -// DefaultRateLimiterConfig defines default values for RateLimiterConfig +// DefaultRateLimiterConfig defines default values for RateLimiterConfig. +// The default identifier uses Context.RealIP(). Configure Echo#IPExtractor for +// trusted proxies to keep clients behind a proxy in separate rate-limit buckets. var DefaultRateLimiterConfig = RateLimiterConfig{ Skipper: DefaultSkipper, IdentifierExtractor: func(ctx echo.Context) (string, error) { diff --git a/middleware/request_logger_test.go b/middleware/request_logger_test.go index 510d34edd..48e9d2ca2 100644 --- a/middleware/request_logger_test.go +++ b/middleware/request_logger_test.go @@ -61,7 +61,7 @@ func TestRequestLoggerOK(t *testing.T) { "host": "example.com", "bytes_out": float64(2), "user_agent": "curl/7.68.0", - "remote_ip": "8.8.8.8", + "remote_ip": "192.0.2.1", "request_id": "", "time": "x", @@ -445,7 +445,7 @@ func TestRequestLogger_allFields(t *testing.T) { assert.Equal(t, time.Unix(1631045377, 0), expect.StartTime) assert.Equal(t, 10*time.Second, expect.Latency) assert.Equal(t, "HTTP/1.1", expect.Protocol) - assert.Equal(t, "8.8.8.8", expect.RemoteIP) + assert.Equal(t, "192.0.2.1", expect.RemoteIP) assert.Equal(t, "example.com", expect.Host) assert.Equal(t, http.MethodPost, expect.Method) assert.Equal(t, "/test?lang=en&checked=1&checked=2", expect.URI)