Skip to content

Commit 24bf2df

Browse files
committed
<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.
1 parent 29727ff commit 24bf2df

2 files changed

Lines changed: 131 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: 123 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,125 @@ 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+
var (
1178+
mu sync.Mutex
1179+
capturedHeader http.Header
1180+
)
1181+
1182+
// Upstream that captures the request header at WS Upgrade time, then echoes.
1183+
upstreamHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1184+
wsHandler := func(conn *websocket.Conn) {
1185+
mu.Lock()
1186+
capturedHeader = conn.Request().Header.Clone()
1187+
mu.Unlock()
1188+
1189+
defer conn.Close()
1190+
1191+
var msg string
1192+
if err := websocket.Message.Receive(conn, &msg); err == nil {
1193+
_ = websocket.Message.Send(conn, msg)
1194+
}
1195+
}
1196+
websocket.Server{Handler: wsHandler}.ServeHTTP(w, r)
1197+
})
1198+
upstream := httptest.NewServer(upstreamHandler)
1199+
defer upstream.Close()
1200+
1201+
tgtURL, _ := url.Parse(upstream.URL)
1202+
e := echo.New()
1203+
balancer := NewRandomBalancer([]*ProxyTarget{{URL: tgtURL}})
1204+
e.Use(ProxyWithConfig(ProxyConfig{Balancer: balancer}))
1205+
proxySrv := httptest.NewServer(e)
1206+
defer proxySrv.Close()
1207+
1208+
proxyWSURL, _ := url.Parse(proxySrv.URL)
1209+
proxyWSURL.Scheme = "ws"
1210+
1211+
tests := []struct {
1212+
name string
1213+
incomingXFF []string // nil = no incoming X-Forwarded-For header at all
1214+
wantPrefix string // expected join of entries preceding the appended proxy RealIP
1215+
}{
1216+
{
1217+
name: "no incoming XFF, only proxy RealIP is set",
1218+
incomingXFF: nil,
1219+
wantPrefix: "",
1220+
},
1221+
{
1222+
name: "single-line single-entry XFF is preserved with proxy RealIP appended",
1223+
incomingXFF: []string{"203.0.113.1"},
1224+
wantPrefix: "203.0.113.1",
1225+
},
1226+
{
1227+
name: "single-line comma-separated XFF is preserved with proxy RealIP appended",
1228+
incomingXFF: []string{"203.0.113.1, 10.0.0.5"},
1229+
wantPrefix: "203.0.113.1, 10.0.0.5",
1230+
},
1231+
{
1232+
name: "multi-line XFF (multiple header occurrences) is joined with proxy RealIP appended",
1233+
incomingXFF: []string{"203.0.113.1", "10.0.0.5"},
1234+
wantPrefix: "203.0.113.1, 10.0.0.5",
1235+
},
1236+
}
1237+
1238+
for _, tt := range tests {
1239+
t.Run(tt.name, func(t *testing.T) {
1240+
mu.Lock()
1241+
capturedHeader = nil
1242+
mu.Unlock()
1243+
1244+
origin, _ := url.Parse(proxySrv.URL)
1245+
cfg := &websocket.Config{
1246+
Location: proxyWSURL,
1247+
Origin: origin,
1248+
Version: websocket.ProtocolVersionHybi13,
1249+
Header: http.Header{},
1250+
}
1251+
1252+
for _, v := range tt.incomingXFF {
1253+
cfg.Header.Add(echo.HeaderXForwardedFor, v)
1254+
}
1255+
1256+
wsConn, err := websocket.DialConfig(cfg)
1257+
assert.NoError(t, err)
1258+
1259+
defer wsConn.Close()
1260+
1261+
// Exchange one message to ensure the upstream handler has captured the header.
1262+
assert.NoError(t, websocket.Message.Send(wsConn, "ping"))
1263+
1264+
var got string
1265+
assert.NoError(t, websocket.Message.Receive(wsConn, &got))
1266+
1267+
mu.Lock()
1268+
xff := capturedHeader.Get(echo.HeaderXForwardedFor)
1269+
mu.Unlock()
1270+
1271+
// The middleware uses Header.Set, so the upstream sees exactly one
1272+
// X-Forwarded-For header line. Split it back into entries.
1273+
entries := strings.Split(xff, ", ")
1274+
assert.NotEmpty(t, entries, "X-Forwarded-For must be set by the proxy middleware")
1275+
1276+
// The tail entry is the proxy's c.RealIP(). When the test client dials
1277+
// via httptest.NewServer the proxy sees 127.0.0.1.
1278+
tail := entries[len(entries)-1]
1279+
assert.Equal(t, "127.0.0.1", tail,
1280+
"proxy RealIP must be appended at the tail of X-Forwarded-For")
1281+
1282+
// The remaining entries must equal the prior chain, preserving order
1283+
// and joining multi-line headers with ", ".
1284+
gotPrefix := strings.Join(entries[:len(entries)-1], ", ")
1285+
assert.Equal(t, tt.wantPrefix, gotPrefix,
1286+
"prior X-Forwarded-For entries must be preserved before the appended RealIP")
1287+
})
1288+
}
1289+
}

0 commit comments

Comments
 (0)