Skip to content

Commit 6a390cb

Browse files
authored
fix(middleware/proxy): append RealIP to X-Forwarded-For for WebSocket requests (#2994)
* <Description> The proxy middleware's WebSocket path currently sets `X-Forwarded-For` only when the header is empty, dropping the proxy's own peer IP from the chain whenever upstream proxies had already added entries. This breaks downstream services that rely on the "rightmost untrusted" rule to extract the real client IP, including echo's own `ExtractIPFromXFFHeader`. The HTTP path delegates to `net/http/httputil.ReverseProxy`, which appends `RemoteAddr` to the existing `X-Forwarded-For` chain — either inline in `ServeHTTP`'s default Director path ([reverseproxy.go#L519-L531](https://github.com/golang/go/blob/go1.26.3/src/net/http/httputil/reverseproxy.go#L519-L531)) or via the explicit [`(*ProxyRequest).SetXForwarded`](https://github.com/golang/go/blob/go1.26.3/src/net/http/httputil/reverseproxy.go#L82-L96) when a `Rewrite` callback is configured. The WebSocket path uses `proxyRaw`, which writes the request verbatim via `r.Write(out)`, so this middleware is the only place where the appending happens for WebSocket Upgrade requests. <Change> Replace the "set if empty" guard with always-append. Read values via map access to preserve multi-line `X-Forwarded-For` headers (RFC 9110 §5.3 allows combining them by joining values with commas). <Test> Added TestProxyWebSocketXForwardedFor exercising 4 cases: - no incoming X-Forwarded-For → only c.RealIP() - single-line single-entry → preserved + c.RealIP() at the tail - ingle-line comma-separated → preserved + c.RealIP() at the tail - multi-line headers (multiple X-Forwarded-For occurrences) → joined with , + c.RealIP() at the tail Each case captures the request header at WebSocket Upgrade time inside the upstream handler and asserts both the appended tail and the preserved prefix. <Backwards compatibility> The change is additive: existing entries are preserved and the proxy's own peer IP is added at the tail. Downstream readers using the standard "rightmost untrusted" rule (e.g. echo.ExtractIPFromXFFHeader) see no behavioral difference for chains where they already worked, and start returning the correct IP for chains where the proxy IP was previously dropped. <Background> We hit this in production with an Echo-based WebSocket reverse proxy fronting an internal service that uses echo.ExtractIPFromXFFHeader for IP-based authorization. Multi-hop deployments (customer proxy → our reverse proxy → backend) broke because the reverse proxy's egress IP was missing from the chain reaching the backend. * Set up upstream per test
1 parent 29727ff commit 6a390cb

2 files changed

Lines changed: 114 additions & 2 deletions

File tree

middleware/proxy.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,14 @@ func (config ProxyConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
355355
if req.Header.Get(echo.HeaderXForwardedProto) == "" {
356356
req.Header.Set(echo.HeaderXForwardedProto, c.Scheme())
357357
}
358-
if c.IsWebSocket() && req.Header.Get(echo.HeaderXForwardedFor) == "" { // For HTTP, it is automatically set by Go HTTP reverse proxy.
359-
req.Header.Set(echo.HeaderXForwardedFor, c.RealIP())
358+
if c.IsWebSocket() { // For HTTP, this is set by Go HTTP reverse proxy.
359+
// Append, not set, to preserve the incoming chain from upstream proxies.
360+
prior := req.Header[echo.HeaderXForwardedFor]
361+
if len(prior) > 0 {
362+
req.Header.Set(echo.HeaderXForwardedFor, strings.Join(prior, ", ")+", "+c.RealIP())
363+
} else {
364+
req.Header.Set(echo.HeaderXForwardedFor, c.RealIP())
365+
}
360366
}
361367

362368
retries := config.RetryCount

middleware/proxy_test.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"net/http/httptest"
1616
"net/url"
1717
"regexp"
18+
"strings"
1819
"sync"
1920
"testing"
2021
"time"
@@ -1164,3 +1165,108 @@ func TestProxyWithConfigWebSocketTLS2NonTLS(t *testing.T) {
11641165
assert.NoError(t, err)
11651166
assert.Equal(t, sendMsg, recvMsg)
11661167
}
1168+
1169+
// TestProxyWebSocketXForwardedFor verifies that for WebSocket Upgrade requests,
1170+
// the proxy middleware appends c.RealIP() to any existing X-Forwarded-For chain,
1171+
// mirroring net/http/httputil.(*ProxyRequest).SetXForwarded used by the HTTP path.
1172+
//
1173+
// Regression guard for the previous "set only if empty" behavior, which dropped
1174+
// the proxy's own peer IP from the chain whenever upstream proxies had already
1175+
// added entries.
1176+
func TestProxyWebSocketXForwardedFor(t *testing.T) {
1177+
tests := []struct {
1178+
name string
1179+
incomingXFF []string // nil = no incoming X-Forwarded-For header at all
1180+
wantPrefix string // expected join of entries preceding the appended proxy RealIP
1181+
}{
1182+
{
1183+
name: "no incoming XFF, only proxy RealIP is set",
1184+
incomingXFF: nil,
1185+
wantPrefix: "",
1186+
},
1187+
{
1188+
name: "single-line single-entry XFF is preserved with proxy RealIP appended",
1189+
incomingXFF: []string{"203.0.113.1"},
1190+
wantPrefix: "203.0.113.1",
1191+
},
1192+
{
1193+
name: "single-line comma-separated XFF is preserved with proxy RealIP appended",
1194+
incomingXFF: []string{"203.0.113.1, 10.0.0.5"},
1195+
wantPrefix: "203.0.113.1, 10.0.0.5",
1196+
},
1197+
{
1198+
name: "multi-line XFF (multiple header occurrences) is joined with proxy RealIP appended",
1199+
incomingXFF: []string{"203.0.113.1", "10.0.0.5"},
1200+
wantPrefix: "203.0.113.1, 10.0.0.5",
1201+
},
1202+
}
1203+
1204+
for _, tt := range tests {
1205+
t.Run(tt.name, func(t *testing.T) {
1206+
// Buffered so the upstream handler never blocks before the client reads.
1207+
headerCh := make(chan http.Header, 1)
1208+
1209+
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1210+
wsHandler := func(conn *websocket.Conn) {
1211+
headerCh <- conn.Request().Header.Clone()
1212+
defer conn.Close()
1213+
var msg string
1214+
if err := websocket.Message.Receive(conn, &msg); err == nil {
1215+
_ = websocket.Message.Send(conn, msg)
1216+
}
1217+
}
1218+
websocket.Server{Handler: wsHandler}.ServeHTTP(w, r)
1219+
}))
1220+
defer upstream.Close()
1221+
1222+
tgtURL, _ := url.Parse(upstream.URL)
1223+
e := echo.New()
1224+
e.Use(ProxyWithConfig(ProxyConfig{Balancer: NewRandomBalancer([]*ProxyTarget{{URL: tgtURL}})}))
1225+
proxySrv := httptest.NewServer(e)
1226+
defer proxySrv.Close()
1227+
1228+
proxyWSURL, _ := url.Parse(proxySrv.URL)
1229+
proxyWSURL.Scheme = "ws"
1230+
1231+
origin, _ := url.Parse(proxySrv.URL)
1232+
cfg := &websocket.Config{
1233+
Location: proxyWSURL,
1234+
Origin: origin,
1235+
Version: websocket.ProtocolVersionHybi13,
1236+
Header: http.Header{},
1237+
}
1238+
for _, v := range tt.incomingXFF {
1239+
cfg.Header.Add(echo.HeaderXForwardedFor, v)
1240+
}
1241+
1242+
wsConn, err := websocket.DialConfig(cfg)
1243+
assert.NoError(t, err)
1244+
defer wsConn.Close()
1245+
1246+
assert.NoError(t, websocket.Message.Send(wsConn, "ping"))
1247+
var got string
1248+
assert.NoError(t, websocket.Message.Receive(wsConn, &got))
1249+
1250+
// The handler sends to headerCh before echoing, so it arrives before Receive returns.
1251+
captured := <-headerCh
1252+
xff := captured.Get(echo.HeaderXForwardedFor)
1253+
1254+
// The middleware uses Header.Set, so the upstream sees exactly one
1255+
// X-Forwarded-For header line. Split it back into entries.
1256+
entries := strings.Split(xff, ", ")
1257+
assert.NotEmpty(t, entries, "X-Forwarded-For must be set by the proxy middleware")
1258+
1259+
// The tail entry is the proxy's c.RealIP(). When the test client dials
1260+
// via httptest.NewServer the proxy sees 127.0.0.1.
1261+
tail := entries[len(entries)-1]
1262+
assert.Equal(t, "127.0.0.1", tail,
1263+
"proxy RealIP must be appended at the tail of X-Forwarded-For")
1264+
1265+
// The remaining entries must equal the prior chain, preserving order
1266+
// and joining multi-line headers with ", ".
1267+
gotPrefix := strings.Join(entries[:len(entries)-1], ", ")
1268+
assert.Equal(t, tt.wantPrefix, gotPrefix,
1269+
"prior X-Forwarded-For entries must be preserved before the appended RealIP")
1270+
})
1271+
}
1272+
}

0 commit comments

Comments
 (0)