diff --git a/docs/amp-cli-integration.md b/docs/amp-cli-integration.md index 6c5f9e8a..41cea66a 100644 --- a/docs/amp-cli-integration.md +++ b/docs/amp-cli-integration.md @@ -135,8 +135,39 @@ When enabled, management routes (`/api/auth`, `/api/user`, `/api/threads`, etc.) - Drive-by browser attacks - Remote access to management endpoints - CORS-based attacks +- Header spoofing attacks (e.g., `X-Forwarded-For: 127.0.0.1`) -**Important**: Only disable this if you understand the security implications and have other protections in place (VPN, firewall, etc.). +#### How It Works + +This restriction uses the **actual TCP connection address** (`RemoteAddr`), not HTTP headers like `X-Forwarded-For`. This prevents header spoofing attacks but has important implications: + +- ✅ **Works for direct connections**: Running CLIProxyAPI directly on your machine or server +- ⚠️ **May not work behind reverse proxies**: If deploying behind nginx, Cloudflare, or other proxies, the connection will appear to come from the proxy's IP, not localhost + +#### Reverse Proxy Deployments + +If you need to run CLIProxyAPI behind a reverse proxy (nginx, Caddy, Cloudflare Tunnel, etc.): + +1. **Disable the localhost restriction**: + ```yaml + amp-restrict-management-to-localhost: false + ``` + +2. **Use alternative security measures**: + - Firewall rules restricting access to management routes + - Proxy-level authentication (HTTP Basic Auth, OAuth) + - Network-level isolation (VPN, Tailscale, Cloudflare Access) + - Bind CLIProxyAPI to `127.0.0.1` only and access via SSH tunnel + +3. **Example nginx configuration** (blocks external access to management routes): + ```nginx + location /api/auth { deny all; } + location /api/user { deny all; } + location /api/threads { deny all; } + location /api/internal { deny all; } + ``` + +**Important**: Only disable `amp-restrict-management-to-localhost` if you understand the security implications and have other protections in place. ## Setup diff --git a/internal/api/modules/amp/routes.go b/internal/api/modules/amp/routes.go index 0f584b5d..43d140e6 100644 --- a/internal/api/modules/amp/routes.go +++ b/internal/api/modules/amp/routes.go @@ -16,14 +16,28 @@ import ( // localhostOnlyMiddleware restricts access to localhost (127.0.0.1, ::1) only. // Returns 403 Forbidden for non-localhost clients. +// +// Security: Uses RemoteAddr (actual TCP connection) instead of ClientIP() to prevent +// header spoofing attacks via X-Forwarded-For or similar headers. This means the +// middleware will not work correctly behind reverse proxies - users deploying behind +// nginx/Cloudflare should disable this feature and use firewall rules instead. func localhostOnlyMiddleware() gin.HandlerFunc { return func(c *gin.Context) { - clientIP := c.ClientIP() + // Use actual TCP connection address (RemoteAddr) to prevent header spoofing + // This cannot be forged by X-Forwarded-For or other client-controlled headers + remoteAddr := c.Request.RemoteAddr + + // RemoteAddr format is "IP:port" or "[IPv6]:port", extract just the IP + host, _, err := net.SplitHostPort(remoteAddr) + if err != nil { + // Try parsing as raw IP (shouldn't happen with standard HTTP, but be defensive) + host = remoteAddr + } // Parse the IP to handle both IPv4 and IPv6 - ip := net.ParseIP(clientIP) + ip := net.ParseIP(host) if ip == nil { - log.Warnf("Amp management: invalid client IP %s, denying access", clientIP) + log.Warnf("Amp management: invalid RemoteAddr %s, denying access", remoteAddr) c.AbortWithStatusJSON(403, gin.H{ "error": "Access denied: management routes restricted to localhost", }) @@ -32,7 +46,7 @@ func localhostOnlyMiddleware() gin.HandlerFunc { // Check if IP is loopback (127.0.0.1 or ::1) if !ip.IsLoopback() { - log.Warnf("Amp management: non-localhost IP %s attempted access, denying", clientIP) + log.Warnf("Amp management: non-localhost connection from %s attempted access, denying", remoteAddr) c.AbortWithStatusJSON(403, gin.H{ "error": "Access denied: management routes restricted to localhost", }) diff --git a/internal/api/modules/amp/routes_test.go b/internal/api/modules/amp/routes_test.go index 38da1ed6..12240981 100644 --- a/internal/api/modules/amp/routes_test.go +++ b/internal/api/modules/amp/routes_test.go @@ -220,3 +220,82 @@ func TestRegisterProviderAliases_NoAuthMiddleware(t *testing.T) { t.Fatal("routes should register even without auth middleware") } } + +func TestLocalhostOnlyMiddleware_PreventsSpoofing(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + + // Apply localhost-only middleware + r.Use(localhostOnlyMiddleware()) + r.GET("/test", func(c *gin.Context) { + c.String(http.StatusOK, "ok") + }) + + tests := []struct { + name string + remoteAddr string + forwardedFor string + expectedStatus int + description string + }{ + { + name: "spoofed_header_remote_connection", + remoteAddr: "192.168.1.100:12345", + forwardedFor: "127.0.0.1", + expectedStatus: http.StatusForbidden, + description: "Spoofed X-Forwarded-For header should be ignored", + }, + { + name: "real_localhost_ipv4", + remoteAddr: "127.0.0.1:54321", + forwardedFor: "", + expectedStatus: http.StatusOK, + description: "Real localhost IPv4 connection should work", + }, + { + name: "real_localhost_ipv6", + remoteAddr: "[::1]:54321", + forwardedFor: "", + expectedStatus: http.StatusOK, + description: "Real localhost IPv6 connection should work", + }, + { + name: "remote_ipv4", + remoteAddr: "203.0.113.42:8080", + forwardedFor: "", + expectedStatus: http.StatusForbidden, + description: "Remote IPv4 connection should be blocked", + }, + { + name: "remote_ipv6", + remoteAddr: "[2001:db8::1]:9090", + forwardedFor: "", + expectedStatus: http.StatusForbidden, + description: "Remote IPv6 connection should be blocked", + }, + { + name: "spoofed_localhost_ipv6", + remoteAddr: "203.0.113.42:8080", + forwardedFor: "::1", + expectedStatus: http.StatusForbidden, + description: "Spoofed X-Forwarded-For with IPv6 localhost should be ignored", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/test", nil) + req.RemoteAddr = tt.remoteAddr + if tt.forwardedFor != "" { + req.Header.Set("X-Forwarded-For", tt.forwardedFor) + } + + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != tt.expectedStatus { + t.Errorf("%s: expected status %d, got %d", tt.description, tt.expectedStatus, w.Code) + } + }) + } +}