From d3b63b1f8756f042db9022c0a5c1065acd965261 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Fri, 13 Mar 2026 00:43:09 -0700 Subject: [PATCH] Fix SEC-06: proxy-aware gRPC rate limiting - Add grpcClientIP() helper that mirrors middleware.ClientIP for proxy-aware IP extraction from gRPC metadata - Update rateLimitInterceptor to use grpcClientIP with the TrustedProxy config setting - Only trust x-forwarded-for/x-real-ip metadata when the peer address matches the configured trusted proxy - Add 7 unit tests covering: no proxy, xff, x-real-ip preference, untrusted peer ignoring headers, no headers fallback, invalid header fallback, and no peer Security: gRPC rate limiter now extracts real client IPs behind a reverse proxy using the same trust model as the REST middleware (DEF-03). Headers from untrusted peers are ignored, preventing IP-spoofing for rate-limit bypass. Co-Authored-By: Claude Opus 4.6 --- internal/grpcserver/grpcserver.go | 63 +++++++++++-- internal/grpcserver/grpcserver_test.go | 126 +++++++++++++++++++++++++ 2 files changed, 181 insertions(+), 8 deletions(-) diff --git a/internal/grpcserver/grpcserver.go b/internal/grpcserver/grpcserver.go index 449a1fc..a46309a 100644 --- a/internal/grpcserver/grpcserver.go +++ b/internal/grpcserver/grpcserver.go @@ -289,28 +289,75 @@ func (l *grpcRateLimiter) cleanup() { // rateLimitInterceptor applies per-IP rate limiting using the same token-bucket // parameters as the REST rate limiter (10 req/s, burst 10). +// +// Security (SEC-06): uses grpcClientIP to extract the real client IP when +// behind a trusted reverse proxy, matching the REST middleware behaviour. func (s *Server) rateLimitInterceptor( ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler, ) (interface{}, error) { - ip := "" - if p, ok := peer.FromContext(ctx); ok { - host, _, err := net.SplitHostPort(p.Addr.String()) - if err == nil { - ip = host - } else { - ip = p.Addr.String() - } + var trustedProxy net.IP + if s.cfg.Server.TrustedProxy != "" { + trustedProxy = net.ParseIP(s.cfg.Server.TrustedProxy) } + ip := grpcClientIP(ctx, trustedProxy) + if ip != "" && !s.rateLimiter.allow(ip) { return nil, status.Error(codes.ResourceExhausted, "rate limit exceeded") } return handler(ctx, req) } +// grpcClientIP extracts the real client IP from gRPC context, optionally +// honouring proxy headers when the peer matches the trusted proxy. +// +// Security (SEC-06): mirrors middleware.ClientIP for the REST server. +// X-Forwarded-For and X-Real-IP metadata are only trusted when the immediate +// peer address matches trustedProxy exactly, preventing IP-spoofing attacks. +// Only the first (leftmost) value in x-forwarded-for is used (original client). +// gRPC lowercases all metadata keys, so we look up "x-forwarded-for" and +// "x-real-ip". +func grpcClientIP(ctx context.Context, trustedProxy net.IP) string { + peerIP := "" + if p, ok := peer.FromContext(ctx); ok { + host, _, err := net.SplitHostPort(p.Addr.String()) + if err == nil { + peerIP = host + } else { + peerIP = p.Addr.String() + } + } + + if trustedProxy != nil && peerIP != "" { + remoteIP := net.ParseIP(peerIP) + if remoteIP != nil && remoteIP.Equal(trustedProxy) { + // Peer is the trusted proxy — extract real client IP from metadata. + // Prefer x-real-ip (single value) over x-forwarded-for (may be a + // comma-separated list when multiple proxies are chained). + md, ok := metadata.FromIncomingContext(ctx) + if ok { + if vals := md.Get("x-real-ip"); len(vals) > 0 { + if ip := net.ParseIP(strings.TrimSpace(vals[0])); ip != nil { + return ip.String() + } + } + if vals := md.Get("x-forwarded-for"); len(vals) > 0 { + // Take the first (leftmost) address — the original client. + first, _, _ := strings.Cut(vals[0], ",") + if ip := net.ParseIP(strings.TrimSpace(first)); ip != nil { + return ip.String() + } + } + } + } + } + + return peerIP +} + // extractBearerFromMD extracts the Bearer token from gRPC metadata. // The key lookup is case-insensitive per gRPC metadata convention (all keys // are lowercased by the framework; we match on "authorization"). diff --git a/internal/grpcserver/grpcserver_test.go b/internal/grpcserver/grpcserver_test.go index 9a34e5b..6353745 100644 --- a/internal/grpcserver/grpcserver_test.go +++ b/internal/grpcserver/grpcserver_test.go @@ -19,6 +19,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" "google.golang.org/grpc/status" "google.golang.org/grpc/test/bufconn" @@ -650,3 +651,128 @@ func TestCredentialFieldsAbsentFromAccountResponse(t *testing.T) { } } } + +// ---- grpcClientIP tests (SEC-06) ---- + +// fakeAddr implements net.Addr for testing peer contexts. +type fakeAddr struct { + addr string + network string +} + +func (a fakeAddr) String() string { return a.addr } +func (a fakeAddr) Network() string { return a.network } + +// TestGRPCClientIP_NoProxy verifies that when no trusted proxy is configured +// the function returns the peer IP directly. +func TestGRPCClientIP_NoProxy(t *testing.T) { + ctx := peer.NewContext(context.Background(), &peer.Peer{ + Addr: fakeAddr{addr: "10.0.0.5:54321", network: "tcp"}, + }) + + got := grpcClientIP(ctx, nil) + if got != "10.0.0.5" { + t.Errorf("grpcClientIP(no proxy) = %q, want %q", got, "10.0.0.5") + } +} + +// TestGRPCClientIP_TrustedProxy_XForwardedFor verifies that when the peer +// matches the trusted proxy, the real client IP is extracted from +// x-forwarded-for metadata. +func TestGRPCClientIP_TrustedProxy_XForwardedFor(t *testing.T) { + proxyIP := net.ParseIP("192.168.1.1") + + ctx := peer.NewContext(context.Background(), &peer.Peer{ + Addr: fakeAddr{addr: "192.168.1.1:12345", network: "tcp"}, + }) + md := metadata.Pairs("x-forwarded-for", "203.0.113.50, 10.0.0.1") + ctx = metadata.NewIncomingContext(ctx, md) + + got := grpcClientIP(ctx, proxyIP) + if got != "203.0.113.50" { + t.Errorf("grpcClientIP(xff) = %q, want %q", got, "203.0.113.50") + } +} + +// TestGRPCClientIP_TrustedProxy_XRealIP verifies that x-real-ip is preferred +// over x-forwarded-for when both are present. +func TestGRPCClientIP_TrustedProxy_XRealIP(t *testing.T) { + proxyIP := net.ParseIP("192.168.1.1") + + ctx := peer.NewContext(context.Background(), &peer.Peer{ + Addr: fakeAddr{addr: "192.168.1.1:12345", network: "tcp"}, + }) + md := metadata.Pairs( + "x-real-ip", "198.51.100.10", + "x-forwarded-for", "203.0.113.50", + ) + ctx = metadata.NewIncomingContext(ctx, md) + + got := grpcClientIP(ctx, proxyIP) + if got != "198.51.100.10" { + t.Errorf("grpcClientIP(x-real-ip preferred) = %q, want %q", got, "198.51.100.10") + } +} + +// TestGRPCClientIP_UntrustedPeer_IgnoresHeaders verifies that forwarded +// headers are ignored when the peer does NOT match the trusted proxy. +// Security: This prevents IP-spoofing by untrusted clients. +func TestGRPCClientIP_UntrustedPeer_IgnoresHeaders(t *testing.T) { + proxyIP := net.ParseIP("192.168.1.1") + + // Peer is NOT the trusted proxy. + ctx := peer.NewContext(context.Background(), &peer.Peer{ + Addr: fakeAddr{addr: "10.0.0.99:54321", network: "tcp"}, + }) + md := metadata.Pairs( + "x-forwarded-for", "203.0.113.50", + "x-real-ip", "198.51.100.10", + ) + ctx = metadata.NewIncomingContext(ctx, md) + + got := grpcClientIP(ctx, proxyIP) + if got != "10.0.0.99" { + t.Errorf("grpcClientIP(untrusted peer) = %q, want %q", got, "10.0.0.99") + } +} + +// TestGRPCClientIP_TrustedProxy_NoHeaders verifies that when the peer matches +// the proxy but no forwarded headers are set, the peer IP is returned as fallback. +func TestGRPCClientIP_TrustedProxy_NoHeaders(t *testing.T) { + proxyIP := net.ParseIP("192.168.1.1") + + ctx := peer.NewContext(context.Background(), &peer.Peer{ + Addr: fakeAddr{addr: "192.168.1.1:12345", network: "tcp"}, + }) + + got := grpcClientIP(ctx, proxyIP) + if got != "192.168.1.1" { + t.Errorf("grpcClientIP(proxy, no headers) = %q, want %q", got, "192.168.1.1") + } +} + +// TestGRPCClientIP_TrustedProxy_InvalidHeader verifies that invalid IPs in +// headers are ignored and the peer IP is returned. +func TestGRPCClientIP_TrustedProxy_InvalidHeader(t *testing.T) { + proxyIP := net.ParseIP("192.168.1.1") + + ctx := peer.NewContext(context.Background(), &peer.Peer{ + Addr: fakeAddr{addr: "192.168.1.1:12345", network: "tcp"}, + }) + md := metadata.Pairs("x-forwarded-for", "not-an-ip") + ctx = metadata.NewIncomingContext(ctx, md) + + got := grpcClientIP(ctx, proxyIP) + if got != "192.168.1.1" { + t.Errorf("grpcClientIP(invalid header) = %q, want %q", got, "192.168.1.1") + } +} + +// TestGRPCClientIP_NoPeer verifies that an empty string is returned when +// there is no peer in the context. +func TestGRPCClientIP_NoPeer(t *testing.T) { + got := grpcClientIP(context.Background(), nil) + if got != "" { + t.Errorf("grpcClientIP(no peer) = %q, want %q", got, "") + } +}