fix(security): add body size limit to proxy and prevent X-Forwarded-Proto spoofing
- commit
- 0e35ff659e1fa68dbb499c3b4dd79fa5d4bb9b8d
- author
- bnema · · 2026-02-01 14:32 UTC
- parent
- 49de7a6
- signature
- unsigned
1 file · +15 -12
M internal/usecase/proxy/service.go +15 -12
| @@ -67,8 +67,15 @@ func NewService( | |||
| 67 | 67 | } | |
| 68 | 68 | } | |
| 69 | 69 | ||
| 70 | + | // maxProxyBodySize is the maximum request body size for proxied requests (512MB). | |
| 71 | + | // This prevents resource exhaustion from extremely large uploads through the proxy. | |
| 72 | + | const maxProxyBodySize = 512 << 20 | |
| 73 | + | ||
| 70 | 74 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | |
| 71 | 75 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |
| 76 | + | // SECURITY: Limit request body size to prevent resource exhaustion. | |
| 77 | + | r.Body = http.MaxBytesReader(w, r.Body, maxProxyBodySize) | |
| 78 | + | ||
| 72 | 79 | // Enrich request context with fields for downstream logging | |
| 73 | 80 | ctx := zerowrap.CtxWithFields(r.Context(), map[string]any{ | |
| 74 | 81 | zerowrap.FieldLayer: "adapter", | |
| @@ -288,13 +295,12 @@ func newReverseProxy(targetURL *url.URL, errorHandler func(http.ResponseWriter, | |||
| 288 | 295 | return &httputil.ReverseProxy{ | |
| 289 | 296 | Rewrite: func(pr *httputil.ProxyRequest) { | |
| 290 | 297 | pr.SetURL(targetURL) | |
| 291 | - | // Preserve original X-Forwarded-Proto from upstream proxy (e.g., Cloudflare) | |
| 292 | - | // before SetXForwarded overwrites it based on local connection | |
| 293 | - | origProto := pr.In.Header.Get("X-Forwarded-Proto") | |
| 298 | + | // SECURITY: Let SetXForwarded() set X-Forwarded-Proto based on the actual | |
| 299 | + | // connection state (r.TLS). We do NOT preserve the original X-Forwarded-Proto | |
| 300 | + | // from the incoming request because we cannot verify it came from a trusted | |
| 301 | + | // proxy. An attacker could spoof X-Forwarded-Proto: https on an HTTP connection | |
| 302 | + | // to trick backends into thinking the connection is secure. | |
| 294 | 303 | pr.SetXForwarded() | |
| 295 | - | if origProto != "" { | |
| 296 | - | pr.Out.Header.Set("X-Forwarded-Proto", origProto) | |
| 297 | - | } | |
| 298 | 304 | pr.Out.Host = targetURL.Host | |
| 299 | 305 | }, | |
| 300 | 306 | Transport: proxyTransport, | |
| @@ -387,12 +393,9 @@ func (s *Service) isRunningInContainer() bool { | |||
| 387 | 393 | } | |
| 388 | 394 | } | |
| 389 | 395 | ||
| 390 | - | // Check hostname pattern | |
| 391 | - | if hostname, err := os.Hostname(); err == nil { | |
| 392 | - | if len(hostname) == 12 || len(hostname) == 64 { | |
| 393 | - | return true | |
| 394 | - | } | |
| 395 | - | } | |
| 396 | + | // NOTE: Hostname length check (12 or 64 chars) was removed because it produced | |
| 397 | + | // false positives on hosts with short hostnames (e.g., "web-server-1" = 12 chars), | |
| 398 | + | // which would cause the proxy to use container network IPs instead of host port mappings. | |
| 396 | 399 | ||
| 397 | 400 | // Check environment variables | |
| 398 | 401 | if os.Getenv("KUBERNETES_SERVICE_HOST") != "" || | |
| @@ -67,8 +67,15 @@ func NewService( | |||||
| 67 | } | 67 | } | ||
| 68 | } | 68 | } | ||
| 69 | 69 | ||||
| 70 | + | // maxProxyBodySize is the maximum request body size for proxied requests (512MB). | |||
| 71 | + | // This prevents resource exhaustion from extremely large uploads through the proxy. | |||
| 72 | + | const maxProxyBodySize = 512 << 20 | |||
| 73 | + | ||||
| 70 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | 74 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | ||
| 71 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | 75 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
| 76 | + | // SECURITY: Limit request body size to prevent resource exhaustion. | |||
| 77 | + | r.Body = http.MaxBytesReader(w, r.Body, maxProxyBodySize) | |||
| 78 | + | ||||
| 72 | // Enrich request context with fields for downstream logging | 79 | // Enrich request context with fields for downstream logging | ||
| 73 | ctx := zerowrap.CtxWithFields(r.Context(), map[string]any{ | 80 | ctx := zerowrap.CtxWithFields(r.Context(), map[string]any{ | ||
| 74 | zerowrap.FieldLayer: "adapter", | 81 | zerowrap.FieldLayer: "adapter", | ||
| @@ -288,13 +295,12 @@ func newReverseProxy(targetURL *url.URL, errorHandler func(http.ResponseWriter, | |||||
| 288 | return &httputil.ReverseProxy{ | 295 | return &httputil.ReverseProxy{ | ||
| 289 | Rewrite: func(pr *httputil.ProxyRequest) { | 296 | Rewrite: func(pr *httputil.ProxyRequest) { | ||
| 290 | pr.SetURL(targetURL) | 297 | pr.SetURL(targetURL) | ||
| 291 | - | // Preserve original X-Forwarded-Proto from upstream proxy (e.g., Cloudflare) | 298 | + | // SECURITY: Let SetXForwarded() set X-Forwarded-Proto based on the actual |
| 292 | - | // before SetXForwarded overwrites it based on local connection | 299 | + | // connection state (r.TLS). We do NOT preserve the original X-Forwarded-Proto |
| 293 | - | origProto := pr.In.Header.Get("X-Forwarded-Proto") | 300 | + | // from the incoming request because we cannot verify it came from a trusted |
| 301 | + | // proxy. An attacker could spoof X-Forwarded-Proto: https on an HTTP connection | |||
| 302 | + | // to trick backends into thinking the connection is secure. | |||
| 294 | pr.SetXForwarded() | 303 | pr.SetXForwarded() | ||
| 295 | - | if origProto != "" { | |||
| 296 | - | pr.Out.Header.Set("X-Forwarded-Proto", origProto) | |||
| 297 | - | } | |||
| 298 | pr.Out.Host = targetURL.Host | 304 | pr.Out.Host = targetURL.Host | ||
| 299 | }, | 305 | }, | ||
| 300 | Transport: proxyTransport, | 306 | Transport: proxyTransport, | ||
| @@ -387,12 +393,9 @@ func (s *Service) isRunningInContainer() bool { | |||||
| 387 | } | 393 | } | ||
| 388 | } | 394 | } | ||
| 389 | 395 | ||||
| 390 | - | // Check hostname pattern | 396 | + | // NOTE: Hostname length check (12 or 64 chars) was removed because it produced |
| 391 | - | if hostname, err := os.Hostname(); err == nil { | 397 | + | // false positives on hosts with short hostnames (e.g., "web-server-1" = 12 chars), |
| 392 | - | if len(hostname) == 12 || len(hostname) == 64 { | 398 | + | // which would cause the proxy to use container network IPs instead of host port mappings. |
| 393 | - | return true | |||
| 394 | - | } | |||
| 395 | - | } | |||
| 396 | 399 | ||||
| 397 | // Check environment variables | 400 | // Check environment variables | ||
| 398 | if os.Getenv("KUBERNETES_SERVICE_HOST") != "" || | 401 | if os.Getenv("KUBERNETES_SERVICE_HOST") != "" || | ||