fix(gateway): forward the real client IP to the backend on every call (last-login IP was the docker addr) #264
@@ -56,6 +56,9 @@ func Middleware(logger *zap.Logger) gin.HandlerFunc {
|
|||||||
zap.String("path", route),
|
zap.String("path", route),
|
||||||
zap.Int("status", status),
|
zap.Int("status", status),
|
||||||
zap.Duration("latency", elapsed),
|
zap.Duration("latency", elapsed),
|
||||||
|
// The gateway forwards the real caller as X-Forwarded-For (and Caddy does for /_gm), which
|
||||||
|
// gin resolves here — so the access log carries the client IP, not the gateway's connection.
|
||||||
|
zap.String("client_ip", c.ClientIP()),
|
||||||
}
|
}
|
||||||
fields = append(fields, TraceFieldsFromContext(ctx)...)
|
fields = append(fields, TraceFieldsFromContext(ctx)...)
|
||||||
|
|
||||||
|
|||||||
@@ -151,7 +151,10 @@ dropped). Horizontal scaling is explicit future work.
|
|||||||
and GCG are unaffected** (they stay decoded concrete characters, §9.1).
|
and GCG are unaffected** (they stay decoded concrete characters, §9.1).
|
||||||
- **gateway ↔ backend (sync)**: plain HTTP REST/JSON. The gateway injects
|
- **gateway ↔ backend (sync)**: plain HTTP REST/JSON. The gateway injects
|
||||||
`X-User-ID` (and the session's trusted `X-Platform`, §3) for authenticated
|
`X-User-ID` (and the session's trusted `X-Platform`, §3) for authenticated
|
||||||
requests; `backend` never re-derives identity or platform from the body. Because every sync call targets the one backend host, the
|
requests, plus the caller's real IP as **`X-Forwarded-For`** on **every** call
|
||||||
|
(carried on the request context), so the backend records the real caller — the
|
||||||
|
account's last-login IP, chat/feedback moderation, the access log — rather than
|
||||||
|
the gateway's own connection address; `backend` never re-derives identity or platform from the body. Because every sync call targets the one backend host, the
|
||||||
gateway's REST client widens its keep-alive pool well past the stdlib default
|
gateway's REST client widens its keep-alive pool well past the stdlib default
|
||||||
of 2 idle connections per host; otherwise the per-request connection churn
|
of 2 idle connections per host; otherwise the per-request connection churn
|
||||||
exhausts ephemeral ports and burns gateway CPU under load (see
|
exhausts ephemeral ports and burns gateway CPU under load (see
|
||||||
@@ -1496,7 +1499,8 @@ Two contours, two secret/variable prefixes (`TEST_` / `PROD_`):
|
|||||||
forwards the domain to `scrabble:80`, so the in-compose caddy serves plain HTTP
|
forwards the domain to `scrabble:80`, so the in-compose caddy serves plain HTTP
|
||||||
(`CADDY_SITE_ADDRESS=:80`). The in-compose caddy **trusts X-Forwarded-For from
|
(`CADDY_SITE_ADDRESS=:80`). The in-compose caddy **trusts X-Forwarded-For from
|
||||||
private-range upstreams** (`trusted_proxies private_ranges`), so the real client IP —
|
private-range upstreams** (`trusted_proxies private_ranges`), so the real client IP —
|
||||||
used for chat-moderation logging and the gateway's per-IP rate limiting — survives the
|
used for the gateway's per-IP rate limiting and, forwarded on to the backend, the account's
|
||||||
|
last-login IP, chat/feedback moderation and the access log — survives the
|
||||||
host-caddy hop; in prod (no host caddy) public clients are untrusted and Caddy uses the
|
host-caddy hop; in prod (no host caddy) public clients are untrusted and Caddy uses the
|
||||||
real peer, so the single config is correct and spoof-safe in both contours. The
|
real peer, so the single config is correct and spoof-safe in both contours. The
|
||||||
**bot-link mTLS material** (a private CA + gateway/bot leaves, CN=`gateway`) is
|
**bot-link mTLS material** (a private CA + gateway/bot leaves, CN=`gateway`) is
|
||||||
|
|||||||
@@ -139,6 +139,26 @@ func platformFromContext(ctx context.Context) string {
|
|||||||
return p
|
return p
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// clientIPCtxKey types the request-context slot the originating client IP rides in.
|
||||||
|
type clientIPCtxKey struct{}
|
||||||
|
|
||||||
|
// WithClientIP returns a copy of ctx carrying the originating client IP that do injects as
|
||||||
|
// X-Forwarded-For on every downstream backend request — so the backend records the real caller
|
||||||
|
// (the account's last-login IP, chat/feedback moderation, the access log) rather than the gateway's
|
||||||
|
// own connection. An empty IP leaves ctx unchanged, so no header is sent.
|
||||||
|
func WithClientIP(ctx context.Context, ip string) context.Context {
|
||||||
|
if ip == "" {
|
||||||
|
return ctx
|
||||||
|
}
|
||||||
|
return context.WithValue(ctx, clientIPCtxKey{}, ip)
|
||||||
|
}
|
||||||
|
|
||||||
|
// clientIPFromContext returns the client IP stored by WithClientIP, or an empty string when none.
|
||||||
|
func clientIPFromContext(ctx context.Context) string {
|
||||||
|
ip, _ := ctx.Value(clientIPCtxKey{}).(string)
|
||||||
|
return ip
|
||||||
|
}
|
||||||
|
|
||||||
// do performs one REST call. userID, when non-empty, is forwarded as X-User-ID;
|
// do performs one REST call. userID, when non-empty, is forwarded as X-User-ID;
|
||||||
// clientIP, when non-empty, as X-Forwarded-For (for chat moderation); the trusted
|
// clientIP, when non-empty, as X-Forwarded-For (for chat moderation); the trusted
|
||||||
// platform carried on ctx (see WithPlatform), when present, as X-Platform. A non-2xx
|
// platform carried on ctx (see WithPlatform), when present, as X-Platform. A non-2xx
|
||||||
@@ -160,6 +180,13 @@ func (c *Client) do(ctx context.Context, method, path, userID, clientIP string,
|
|||||||
if userID != "" {
|
if userID != "" {
|
||||||
req.Header.Set("X-User-ID", userID)
|
req.Header.Set("X-User-ID", userID)
|
||||||
}
|
}
|
||||||
|
// The client IP rides X-Forwarded-For so the backend records the real caller. It comes from the
|
||||||
|
// explicit param (chat/feedback) or, for every other call, the request context (WithClientIP, set
|
||||||
|
// once per request in the Connect edge) — so the backend never falls back to the gateway's own
|
||||||
|
// connection address.
|
||||||
|
if clientIP == "" {
|
||||||
|
clientIP = clientIPFromContext(ctx)
|
||||||
|
}
|
||||||
if clientIP != "" {
|
if clientIP != "" {
|
||||||
req.Header.Set("X-Forwarded-For", clientIP)
|
req.Header.Set("X-Forwarded-For", clientIP)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -122,3 +122,44 @@ func TestXPlatformInjection(t *testing.T) {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestXForwardedForInjection verifies the client IP carried on the context (WithClientIP) rides every
|
||||||
|
// backend call as X-Forwarded-For — not just chat/feedback, which pass it explicitly — so the backend
|
||||||
|
// records the real caller (e.g. the account's last-login IP on the profile fetch) rather than the
|
||||||
|
// gateway's own connection. Absent when no client IP is set.
|
||||||
|
func TestXForwardedForInjection(t *testing.T) {
|
||||||
|
var gotXFF string
|
||||||
|
var hadHeader bool
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
gotXFF = r.Header.Get("X-Forwarded-For")
|
||||||
|
_, hadHeader = r.Header["X-Forwarded-For"]
|
||||||
|
_, _ = w.Write([]byte(`{}`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
c, err := backendclient.New(srv.URL, "localhost:9090", 2*time.Second)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("backendclient: %v", err)
|
||||||
|
}
|
||||||
|
defer func() { _ = c.Close() }()
|
||||||
|
|
||||||
|
t.Run("client IP on ctx rides a non-chat call", func(t *testing.T) {
|
||||||
|
ctx := backendclient.WithClientIP(context.Background(), "203.0.113.7")
|
||||||
|
if _, err := c.Profile(ctx, "user-1"); err != nil {
|
||||||
|
t.Fatalf("Profile: %v", err)
|
||||||
|
}
|
||||||
|
if gotXFF != "203.0.113.7" {
|
||||||
|
t.Fatalf("X-Forwarded-For = %q, want 203.0.113.7", gotXFF)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("no client IP omits the header", func(t *testing.T) {
|
||||||
|
hadHeader = true
|
||||||
|
if _, err := c.Profile(context.Background(), "user-1"); err != nil {
|
||||||
|
t.Fatalf("Profile: %v", err)
|
||||||
|
}
|
||||||
|
if hadHeader {
|
||||||
|
t.Fatal("X-Forwarded-For must be absent when no client IP is set")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|||||||
@@ -418,6 +418,10 @@ func (s *Server) Execute(ctx context.Context, req *connect.Request[edgev1.Execut
|
|||||||
return nil, connect.NewError(connect.CodeNotFound, errUnknownMessageType(msgType))
|
return nil, connect.NewError(connect.CodeNotFound, errUnknownMessageType(msgType))
|
||||||
}
|
}
|
||||||
clientIP := peerIP(req.Peer().Addr, req.Header())
|
clientIP := peerIP(req.Peer().Addr, req.Header())
|
||||||
|
// Carry the client IP on the context so the backend client injects it as X-Forwarded-For on every
|
||||||
|
// downstream REST call for this request — the account's last-login IP, chat/feedback moderation and
|
||||||
|
// the backend access log, not only the chat/feedback calls that pass it explicitly.
|
||||||
|
ctx = backendclient.WithClientIP(ctx, clientIP)
|
||||||
|
|
||||||
tr := transcode.Request{Payload: req.Msg.GetPayload(), ClientIP: clientIP}
|
tr := transcode.Request{Payload: req.Msg.GetPayload(), ClientIP: clientIP}
|
||||||
if op.Auth {
|
if op.Auth {
|
||||||
|
|||||||
Reference in New Issue
Block a user