fix(proxy) address Copilot and CodeRabbit review findings
- Remove CSP from global middleware (breaks proxied sites); set it only on proxy-generated error responses via proxyError() helper - HSTS now checks r.TLS only, not spoofable X-Forwarded-Proto - Fix response size limit: update Content-Length, delete Transfer-Encoding and Content-Encoding headers when substituting body; close underlying body in limitedReadCloser on limit exceeded - Preserve original Host header for DNS-pinned external routes so virtual-hosted upstreams work correctly (OriginalHost field) - Use net.JoinHostPort for IPv6-safe URL construction - Replace channel semaphore with atomic counter for connection limiting to eliminate race on config reload - MaxConcurrentConns: -1 = default (10000), 0 = no limit (matches docs) - Container shutdown now uses bounded 30s context instead of Background - Hot reload rebuilds full proxy config (not just registry domain/port) - Strip CSP/HSTS from registry proxy responses to prevent duplicates - Handle rand.Read error in request ID generation with counter fallback - Fix SSRF doc comment to match actual behavior
- commit
- 874c2db8eeaf0b6bfc2ed21890bc067a72be8925
- author
- brice · · 2026-02-07 05:44 UTC
- parent
- 97c284a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +136 -72
M internal/adapters/in/http/middleware/logging.go +9 -1
| @@ -5,8 +5,10 @@ import ( | |||
| 5 | 5 | "crypto/rand" | |
| 6 | 6 | "encoding/hex" | |
| 7 | 7 | "encoding/json" | |
| 8 | + | "fmt" | |
| 8 | 9 | "net" | |
| 9 | 10 | "net/http" | |
| 11 | + | "sync/atomic" | |
| 10 | 12 | "time" | |
| 11 | 13 | ||
| 12 | 14 | "github.com/bnema/zerowrap" | |
| @@ -122,10 +124,16 @@ func RequestLogger(log zerowrap.Logger, trustedNets ...[]*net.IPNet) func(http.H | |||
| 122 | 124 | } | |
| 123 | 125 | } | |
| 124 | 126 | ||
| 127 | + | // fallbackCounter ensures uniqueness when crypto/rand is unavailable. | |
| 128 | + | var fallbackCounter atomic.Uint64 | |
| 129 | + | ||
| 125 | 130 | // generateRequestID creates a random 16-byte hex-encoded request ID. | |
| 126 | 131 | func generateRequestID() string { | |
| 127 | 132 | b := make([]byte, 16) | |
| 128 | - | _, _ = rand.Read(b) | |
| 133 | + | if _, err := rand.Read(b); err != nil { | |
| 134 | + | // Fallback to timestamp + monotonic counter if RNG is unavailable | |
| 135 | + | return fmt.Sprintf("%x-%x", time.Now().UnixNano(), fallbackCounter.Add(1)) | |
| 136 | + | } | |
| 129 | 137 | return hex.EncodeToString(b) | |
| 130 | 138 | } | |
| 131 | 139 | ||
| @@ -5,8 +5,10 @@ import ( | |||||
| 5 | "crypto/rand" | 5 | "crypto/rand" | ||
| 6 | "encoding/hex" | 6 | "encoding/hex" | ||
| 7 | "encoding/json" | 7 | "encoding/json" | ||
| 8 | + | "fmt" | |||
| 8 | "net" | 9 | "net" | ||
| 9 | "net/http" | 10 | "net/http" | ||
| 11 | + | "sync/atomic" | |||
| 10 | "time" | 12 | "time" | ||
| 11 | 13 | ||||
| 12 | "github.com/bnema/zerowrap" | 14 | "github.com/bnema/zerowrap" | ||
| @@ -122,10 +124,16 @@ func RequestLogger(log zerowrap.Logger, trustedNets ...[]*net.IPNet) func(http.H | |||||
| 122 | } | 124 | } | ||
| 123 | } | 125 | } | ||
| 124 | 126 | ||||
| 127 | + | // fallbackCounter ensures uniqueness when crypto/rand is unavailable. | |||
| 128 | + | var fallbackCounter atomic.Uint64 | |||
| 129 | + | ||||
| 125 | // generateRequestID creates a random 16-byte hex-encoded request ID. | 130 | // generateRequestID creates a random 16-byte hex-encoded request ID. | ||
| 126 | func generateRequestID() string { | 131 | func generateRequestID() string { | ||
| 127 | b := make([]byte, 16) | 132 | b := make([]byte, 16) | ||
| 128 | - | _, _ = rand.Read(b) | 133 | + | if _, err := rand.Read(b); err != nil { |
| 134 | + | // Fallback to timestamp + monotonic counter if RNG is unavailable | |||
| 135 | + | return fmt.Sprintf("%x-%x", time.Now().UnixNano(), fallbackCounter.Add(1)) | |||
| 136 | + | } | |||
| 129 | return hex.EncodeToString(b) | 137 | return hex.EncodeToString(b) | ||
| 130 | } | 138 | } | ||
| 131 | 139 | ||||
M internal/adapters/in/http/middleware/security.go +8 -5
| @@ -24,14 +24,17 @@ func SecurityHeaders(next http.Handler) http.Handler { | |||
| 24 | 24 | w.Header().Set("Permissions-Policy", "geolocation=(), microphone=(), camera=()") | |
| 25 | 25 | ||
| 26 | 26 | // HSTS: Enforce HTTPS-only for browsers. Only set when the request | |
| 27 | - | // arrived over TLS to avoid issues with plain HTTP development setups. | |
| 28 | - | if r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https" { | |
| 27 | + | // arrived over a real TLS connection. We do NOT trust X-Forwarded-Proto | |
| 28 | + | // here because it is client-spoofable unless the request came from a | |
| 29 | + | // trusted proxy, and we have no trusted-proxy list at this layer. | |
| 30 | + | if r.TLS != nil { | |
| 29 | 31 | w.Header().Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains") | |
| 30 | 32 | } | |
| 31 | 33 | ||
| 32 | - | // CSP for proxy-generated error pages. Backends set their own CSP | |
| 33 | - | // which will override this for proxied content. | |
| 34 | - | w.Header().Set("Content-Security-Policy", "default-src 'none'; frame-ancestors 'none'") | |
| 34 | + | // NOTE: CSP is intentionally NOT set here. Gordon proxies arbitrary web | |
| 35 | + | // apps, so a blanket "default-src 'none'" would break all proxied sites. | |
| 36 | + | // Instead, CSP is set directly on proxy-generated error responses in the | |
| 37 | + | // proxy error handlers (service.go). | |
| 35 | 38 | ||
| 36 | 39 | next.ServeHTTP(w, r) | |
| 37 | 40 | }) | |
| @@ -24,14 +24,17 @@ func SecurityHeaders(next http.Handler) http.Handler { | |||||
| 24 | w.Header().Set("Permissions-Policy", "geolocation=(), microphone=(), camera=()") | 24 | w.Header().Set("Permissions-Policy", "geolocation=(), microphone=(), camera=()") | ||
| 25 | 25 | ||||
| 26 | // HSTS: Enforce HTTPS-only for browsers. Only set when the request | 26 | // HSTS: Enforce HTTPS-only for browsers. Only set when the request | ||
| 27 | - | // arrived over TLS to avoid issues with plain HTTP development setups. | 27 | + | // arrived over a real TLS connection. We do NOT trust X-Forwarded-Proto |
| 28 | - | if r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https" { | 28 | + | // here because it is client-spoofable unless the request came from a |
| 29 | + | // trusted proxy, and we have no trusted-proxy list at this layer. | |||
| 30 | + | if r.TLS != nil { | |||
| 29 | w.Header().Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains") | 31 | w.Header().Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains") | ||
| 30 | } | 32 | } | ||
| 31 | 33 | ||||
| 32 | - | // CSP for proxy-generated error pages. Backends set their own CSP | 34 | + | // NOTE: CSP is intentionally NOT set here. Gordon proxies arbitrary web |
| 33 | - | // which will override this for proxied content. | 35 | + | // apps, so a blanket "default-src 'none'" would break all proxied sites. |
| 34 | - | w.Header().Set("Content-Security-Policy", "default-src 'none'; frame-ancestors 'none'") | 36 | + | // Instead, CSP is set directly on proxy-generated error responses in the |
| 37 | + | // proxy error handlers (service.go). | |||
| 35 | 38 | ||||
| 36 | next.ServeHTTP(w, r) | 39 | next.ServeHTTP(w, r) | ||
| 37 | }) | 40 | }) | ||
M internal/adapters/in/http/middleware/security_test.go +6 -3
| @@ -28,7 +28,9 @@ func TestSecurityHeaders(t *testing.T) { | |||
| 28 | 28 | assert.Equal(t, "1; mode=block", rec.Header().Get("X-XSS-Protection")) | |
| 29 | 29 | assert.Equal(t, "strict-origin-when-cross-origin", rec.Header().Get("Referrer-Policy")) | |
| 30 | 30 | assert.Equal(t, "geolocation=(), microphone=(), camera=()", rec.Header().Get("Permissions-Policy")) | |
| 31 | - | assert.Equal(t, "default-src 'none'; frame-ancestors 'none'", rec.Header().Get("Content-Security-Policy")) | |
| 31 | + | ||
| 32 | + | // CSP should NOT be set globally (would break proxied sites) | |
| 33 | + | assert.Empty(t, rec.Header().Get("Content-Security-Policy")) | |
| 32 | 34 | ||
| 33 | 35 | // HSTS should NOT be set on plain HTTP | |
| 34 | 36 | assert.Empty(t, rec.Header().Get("Strict-Transport-Security")) | |
| @@ -49,19 +51,20 @@ func TestSecurityHeaders_HSTS_WithTLS(t *testing.T) { | |||
| 49 | 51 | assert.Equal(t, "max-age=31536000; includeSubDomains", rec.Header().Get("Strict-Transport-Security")) | |
| 50 | 52 | } | |
| 51 | 53 | ||
| 52 | - | func TestSecurityHeaders_HSTS_WithForwardedProto(t *testing.T) { | |
| 54 | + | func TestSecurityHeaders_HSTS_WithForwardedProto_NotTrusted(t *testing.T) { | |
| 53 | 55 | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 54 | 56 | w.WriteHeader(http.StatusOK) | |
| 55 | 57 | }) | |
| 56 | 58 | ||
| 57 | 59 | wrapped := SecurityHeaders(handler) | |
| 58 | 60 | ||
| 61 | + | // X-Forwarded-Proto is client-spoofable and should NOT trigger HSTS | |
| 59 | 62 | req := httptest.NewRequest(http.MethodGet, "/", nil) | |
| 60 | 63 | req.Header.Set("X-Forwarded-Proto", "https") | |
| 61 | 64 | rec := httptest.NewRecorder() | |
| 62 | 65 | wrapped.ServeHTTP(rec, req) | |
| 63 | 66 | ||
| 64 | - | assert.Equal(t, "max-age=31536000; includeSubDomains", rec.Header().Get("Strict-Transport-Security")) | |
| 67 | + | assert.Empty(t, rec.Header().Get("Strict-Transport-Security"), "HSTS should not be set based on spoofable X-Forwarded-Proto") | |
| 65 | 68 | } | |
| 66 | 69 | ||
| 67 | 70 | func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) { | |
| @@ -28,7 +28,9 @@ func TestSecurityHeaders(t *testing.T) { | |||||
| 28 | assert.Equal(t, "1; mode=block", rec.Header().Get("X-XSS-Protection")) | 28 | assert.Equal(t, "1; mode=block", rec.Header().Get("X-XSS-Protection")) | ||
| 29 | assert.Equal(t, "strict-origin-when-cross-origin", rec.Header().Get("Referrer-Policy")) | 29 | assert.Equal(t, "strict-origin-when-cross-origin", rec.Header().Get("Referrer-Policy")) | ||
| 30 | assert.Equal(t, "geolocation=(), microphone=(), camera=()", rec.Header().Get("Permissions-Policy")) | 30 | assert.Equal(t, "geolocation=(), microphone=(), camera=()", rec.Header().Get("Permissions-Policy")) | ||
| 31 | - | assert.Equal(t, "default-src 'none'; frame-ancestors 'none'", rec.Header().Get("Content-Security-Policy")) | 31 | + | |
| 32 | + | // CSP should NOT be set globally (would break proxied sites) | |||
| 33 | + | assert.Empty(t, rec.Header().Get("Content-Security-Policy")) | |||
| 32 | 34 | ||||
| 33 | // HSTS should NOT be set on plain HTTP | 35 | // HSTS should NOT be set on plain HTTP | ||
| 34 | assert.Empty(t, rec.Header().Get("Strict-Transport-Security")) | 36 | assert.Empty(t, rec.Header().Get("Strict-Transport-Security")) | ||
| @@ -49,19 +51,20 @@ func TestSecurityHeaders_HSTS_WithTLS(t *testing.T) { | |||||
| 49 | assert.Equal(t, "max-age=31536000; includeSubDomains", rec.Header().Get("Strict-Transport-Security")) | 51 | assert.Equal(t, "max-age=31536000; includeSubDomains", rec.Header().Get("Strict-Transport-Security")) | ||
| 50 | } | 52 | } | ||
| 51 | 53 | ||||
| 52 | - | func TestSecurityHeaders_HSTS_WithForwardedProto(t *testing.T) { | 54 | + | func TestSecurityHeaders_HSTS_WithForwardedProto_NotTrusted(t *testing.T) { |
| 53 | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | 55 | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| 54 | w.WriteHeader(http.StatusOK) | 56 | w.WriteHeader(http.StatusOK) | ||
| 55 | }) | 57 | }) | ||
| 56 | 58 | ||||
| 57 | wrapped := SecurityHeaders(handler) | 59 | wrapped := SecurityHeaders(handler) | ||
| 58 | 60 | ||||
| 61 | + | // X-Forwarded-Proto is client-spoofable and should NOT trigger HSTS | |||
| 59 | req := httptest.NewRequest(http.MethodGet, "/", nil) | 62 | req := httptest.NewRequest(http.MethodGet, "/", nil) | ||
| 60 | req.Header.Set("X-Forwarded-Proto", "https") | 63 | req.Header.Set("X-Forwarded-Proto", "https") | ||
| 61 | rec := httptest.NewRecorder() | 64 | rec := httptest.NewRecorder() | ||
| 62 | wrapped.ServeHTTP(rec, req) | 65 | wrapped.ServeHTTP(rec, req) | ||
| 63 | 66 | ||||
| 64 | - | assert.Equal(t, "max-age=31536000; includeSubDomains", rec.Header().Get("Strict-Transport-Security")) | 67 | + | assert.Empty(t, rec.Header().Get("Strict-Transport-Security"), "HSTS should not be set based on spoofable X-Forwarded-Proto") |
| 65 | } | 68 | } | ||
| 66 | 69 | ||||
| 67 | func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) { | 70 | func TestSecurityHeaders_PreservesExistingHeaders(t *testing.T) { | ||
M internal/app/run.go +20 -8
| @@ -869,9 +869,10 @@ func buildProxyConfig(cfg Config, log zerowrap.Logger) (*proxyConfigResult, erro | |||
| 869 | 869 | } | |
| 870 | 870 | ||
| 871 | 871 | maxConcurrentConns := cfg.Server.MaxConcurrentConns | |
| 872 | - | if maxConcurrentConns == 0 { | |
| 873 | - | maxConcurrentConns = 10000 // default | |
| 872 | + | if maxConcurrentConns < 0 { | |
| 873 | + | maxConcurrentConns = 10000 // default when explicitly set to -1 | |
| 874 | 874 | } | |
| 875 | + | // 0 means no limit (as documented in proxy.Config) | |
| 875 | 876 | ||
| 876 | 877 | return &proxyConfigResult{ | |
| 877 | 878 | proxyConfig: proxy.Config{ | |
| @@ -985,11 +986,21 @@ func setupConfigHotReload(ctx context.Context, v *viper.Viper, svc *services, lo | |||
| 985 | 986 | return | |
| 986 | 987 | } | |
| 987 | 988 | ||
| 988 | - | // Update proxy config from viper (reads directly from viper, not memory) | |
| 989 | - | svc.proxySvc.UpdateConfig(proxy.Config{ | |
| 990 | - | RegistryDomain: v.GetString("server.registry_domain"), | |
| 991 | - | RegistryPort: v.GetInt("server.registry_port"), | |
| 992 | - | }) | |
| 989 | + | // Re-unmarshal config and rebuild proxy config to pick up all changes | |
| 990 | + | var reloadCfg Config | |
| 991 | + | if err := v.Unmarshal(&reloadCfg); err != nil { | |
| 992 | + | log.Error().Err(err).Msg("failed to unmarshal config on reload") | |
| 993 | + | return | |
| 994 | + | } | |
| 995 | + | if reloadCfg.Server.GordonDomain != "" { | |
| 996 | + | reloadCfg.Server.RegistryDomain = reloadCfg.Server.GordonDomain | |
| 997 | + | } | |
| 998 | + | reloadedProxy, err := buildProxyConfig(reloadCfg, log) | |
| 999 | + | if err != nil { | |
| 1000 | + | log.Error().Err(err).Msg("failed to parse proxy config on reload") | |
| 1001 | + | return | |
| 1002 | + | } | |
| 1003 | + | svc.proxySvc.UpdateConfig(reloadedProxy.proxyConfig) | |
| 993 | 1004 | ||
| 994 | 1005 | // Clear proxy target cache to pick up external route changes | |
| 995 | 1006 | if err := svc.proxySvc.RefreshTargets(ctx); err != nil { | |
| @@ -1205,7 +1216,7 @@ func gracefulShutdown(registrySrv, proxySrv *http.Server, containerSvc *containe | |||
| 1205 | 1216 | log.Warn().Err(err).Msg("proxy server shutdown error") | |
| 1206 | 1217 | } | |
| 1207 | 1218 | ||
| 1208 | - | if err := containerSvc.Shutdown(context.Background()); err != nil { | |
| 1219 | + | if err := containerSvc.Shutdown(shutdownCtx); err != nil { | |
| 1209 | 1220 | log.Warn().Err(err).Msg("error during container shutdown") | |
| 1210 | 1221 | } | |
| 1211 | 1222 | ||
| @@ -1532,6 +1543,7 @@ func loadConfig(v *viper.Viper, configPath string) error { | |||
| 1532 | 1543 | v.SetDefault("volumes.prefix", "gordon") | |
| 1533 | 1544 | v.SetDefault("volumes.preserve", true) | |
| 1534 | 1545 | v.SetDefault("deploy.pull_policy", container.PullPolicyIfTagChanged) | |
| 1546 | + | v.SetDefault("server.max_concurrent_connections", -1) // -1 = use default (10000), 0 = no limit | |
| 1535 | 1547 | v.SetDefault("deploy.readiness_delay", "5s") | |
| 1536 | 1548 | ||
| 1537 | 1549 | ConfigureViper(v, configPath) | |
| @@ -869,9 +869,10 @@ func buildProxyConfig(cfg Config, log zerowrap.Logger) (*proxyConfigResult, erro | |||||
| 869 | } | 869 | } | ||
| 870 | 870 | ||||
| 871 | maxConcurrentConns := cfg.Server.MaxConcurrentConns | 871 | maxConcurrentConns := cfg.Server.MaxConcurrentConns | ||
| 872 | - | if maxConcurrentConns == 0 { | 872 | + | if maxConcurrentConns < 0 { |
| 873 | - | maxConcurrentConns = 10000 // default | 873 | + | maxConcurrentConns = 10000 // default when explicitly set to -1 |
| 874 | } | 874 | } | ||
| 875 | + | // 0 means no limit (as documented in proxy.Config) | |||
| 875 | 876 | ||||
| 876 | return &proxyConfigResult{ | 877 | return &proxyConfigResult{ | ||
| 877 | proxyConfig: proxy.Config{ | 878 | proxyConfig: proxy.Config{ | ||
| @@ -985,11 +986,21 @@ func setupConfigHotReload(ctx context.Context, v *viper.Viper, svc *services, lo | |||||
| 985 | return | 986 | return | ||
| 986 | } | 987 | } | ||
| 987 | 988 | ||||
| 988 | - | // Update proxy config from viper (reads directly from viper, not memory) | 989 | + | // Re-unmarshal config and rebuild proxy config to pick up all changes |
| 989 | - | svc.proxySvc.UpdateConfig(proxy.Config{ | 990 | + | var reloadCfg Config |
| 990 | - | RegistryDomain: v.GetString("server.registry_domain"), | 991 | + | if err := v.Unmarshal(&reloadCfg); err != nil { |
| 991 | - | RegistryPort: v.GetInt("server.registry_port"), | 992 | + | log.Error().Err(err).Msg("failed to unmarshal config on reload") |
| 992 | - | }) | 993 | + | return |
| 994 | + | } | |||
| 995 | + | if reloadCfg.Server.GordonDomain != "" { | |||
| 996 | + | reloadCfg.Server.RegistryDomain = reloadCfg.Server.GordonDomain | |||
| 997 | + | } | |||
| 998 | + | reloadedProxy, err := buildProxyConfig(reloadCfg, log) | |||
| 999 | + | if err != nil { | |||
| 1000 | + | log.Error().Err(err).Msg("failed to parse proxy config on reload") | |||
| 1001 | + | return | |||
| 1002 | + | } | |||
| 1003 | + | svc.proxySvc.UpdateConfig(reloadedProxy.proxyConfig) | |||
| 993 | 1004 | ||||
| 994 | // Clear proxy target cache to pick up external route changes | 1005 | // Clear proxy target cache to pick up external route changes | ||
| 995 | if err := svc.proxySvc.RefreshTargets(ctx); err != nil { | 1006 | if err := svc.proxySvc.RefreshTargets(ctx); err != nil { | ||
| @@ -1205,7 +1216,7 @@ func gracefulShutdown(registrySrv, proxySrv *http.Server, containerSvc *containe | |||||
| 1205 | log.Warn().Err(err).Msg("proxy server shutdown error") | 1216 | log.Warn().Err(err).Msg("proxy server shutdown error") | ||
| 1206 | } | 1217 | } | ||
| 1207 | 1218 | ||||
| 1208 | - | if err := containerSvc.Shutdown(context.Background()); err != nil { | 1219 | + | if err := containerSvc.Shutdown(shutdownCtx); err != nil { |
| 1209 | log.Warn().Err(err).Msg("error during container shutdown") | 1220 | log.Warn().Err(err).Msg("error during container shutdown") | ||
| 1210 | } | 1221 | } | ||
| 1211 | 1222 | ||||
| @@ -1532,6 +1543,7 @@ func loadConfig(v *viper.Viper, configPath string) error { | |||||
| 1532 | v.SetDefault("volumes.prefix", "gordon") | 1543 | v.SetDefault("volumes.prefix", "gordon") | ||
| 1533 | v.SetDefault("volumes.preserve", true) | 1544 | v.SetDefault("volumes.preserve", true) | ||
| 1534 | v.SetDefault("deploy.pull_policy", container.PullPolicyIfTagChanged) | 1545 | v.SetDefault("deploy.pull_policy", container.PullPolicyIfTagChanged) | ||
| 1546 | + | v.SetDefault("server.max_concurrent_connections", -1) // -1 = use default (10000), 0 = no limit | |||
| 1535 | v.SetDefault("deploy.readiness_delay", "5s") | 1547 | v.SetDefault("deploy.readiness_delay", "5s") | ||
| 1536 | 1548 | ||||
| 1537 | ConfigureViper(v, configPath) | 1549 | ConfigureViper(v, configPath) | ||
M internal/domain/route.go +5 -4
| @@ -9,10 +9,11 @@ type Route struct { | |||
| 9 | 9 | ||
| 10 | 10 | // ProxyTarget represents the destination for proxying requests. | |
| 11 | 11 | type ProxyTarget struct { | |
| 12 | - | Host string | |
| 13 | - | Port int | |
| 14 | - | ContainerID string | |
| 15 | - | Scheme string // "http" or "https" | |
| 12 | + | Host string | |
| 13 | + | Port int | |
| 14 | + | ContainerID string | |
| 15 | + | Scheme string // "http" or "https" | |
| 16 | + | OriginalHost string // Original hostname before DNS resolution (for Host header) | |
| 16 | 17 | } | |
| 17 | 18 | ||
| 18 | 19 | // RouteMatch represents the result of matching a request to a route. | |
| @@ -9,10 +9,11 @@ type Route struct { | |||||
| 9 | 9 | ||||
| 10 | // ProxyTarget represents the destination for proxying requests. | 10 | // ProxyTarget represents the destination for proxying requests. | ||
| 11 | type ProxyTarget struct { | 11 | type ProxyTarget struct { | ||
| 12 | - | Host string | 12 | + | Host string |
| 13 | - | Port int | 13 | + | Port int |
| 14 | - | ContainerID string | 14 | + | ContainerID string |
| 15 | - | Scheme string // "http" or "https" | 15 | + | Scheme string // "http" or "https" |
| 16 | + | OriginalHost string // Original hostname before DNS resolution (for Host header) | |||
| 16 | } | 17 | } | ||
| 17 | 18 | ||||
| 18 | // RouteMatch represents the result of matching a request to a route. | 19 | // RouteMatch represents the result of matching a request to a route. | ||
M internal/usecase/proxy/service.go +78 -40
| @@ -14,6 +14,7 @@ import ( | |||
| 14 | 14 | "strconv" | |
| 15 | 15 | "strings" | |
| 16 | 16 | "sync" | |
| 17 | + | "sync/atomic" | |
| 17 | 18 | "time" | |
| 18 | 19 | ||
| 19 | 20 | "github.com/bnema/zerowrap" | |
| @@ -54,7 +55,7 @@ type Service struct { | |||
| 54 | 55 | config Config | |
| 55 | 56 | targets map[string]*domain.ProxyTarget | |
| 56 | 57 | mu sync.RWMutex | |
| 57 | - | connSem chan struct{} // Semaphore for concurrent connection limiting | |
| 58 | + | activeConns atomic.Int64 // Atomic counter for concurrent connection limiting | |
| 58 | 59 | } | |
| 59 | 60 | ||
| 60 | 61 | // NewService creates a new proxy service. | |
| @@ -64,37 +65,35 @@ func NewService( | |||
| 64 | 65 | configSvc in.ConfigService, | |
| 65 | 66 | config Config, | |
| 66 | 67 | ) *Service { | |
| 67 | - | svc := &Service{ | |
| 68 | + | return &Service{ | |
| 68 | 69 | runtime: runtime, | |
| 69 | 70 | containerSvc: containerSvc, | |
| 70 | 71 | configSvc: configSvc, | |
| 71 | 72 | config: config, | |
| 72 | 73 | targets: make(map[string]*domain.ProxyTarget), | |
| 73 | 74 | } | |
| 74 | - | if config.MaxConcurrentConns > 0 { | |
| 75 | - | svc.connSem = make(chan struct{}, config.MaxConcurrentConns) | |
| 76 | - | } | |
| 77 | - | return svc | |
| 78 | 75 | } | |
| 79 | 76 | ||
| 80 | 77 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | |
| 81 | 78 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |
| 82 | - | // SECURITY: Limit concurrent connections to prevent resource exhaustion. | |
| 83 | - | if s.connSem != nil { | |
| 84 | - | select { | |
| 85 | - | case s.connSem <- struct{}{}: | |
| 86 | - | defer func() { <-s.connSem }() | |
| 87 | - | default: | |
| 88 | - | http.Error(w, "Service Unavailable", http.StatusServiceUnavailable) | |
| 89 | - | return | |
| 90 | - | } | |
| 91 | - | } | |
| 92 | - | ||
| 93 | 79 | // Copy config under RLock to avoid data race with UpdateConfig | |
| 94 | 80 | s.mu.RLock() | |
| 95 | 81 | cfg := s.config | |
| 96 | 82 | s.mu.RUnlock() | |
| 97 | 83 | ||
| 84 | + | // SECURITY: Limit concurrent connections to prevent resource exhaustion. | |
| 85 | + | // Uses an atomic counter so config reloads don't create a race between | |
| 86 | + | // an old and new semaphore — the counter is shared across reloads. | |
| 87 | + | if cfg.MaxConcurrentConns > 0 { | |
| 88 | + | current := s.activeConns.Add(1) | |
| 89 | + | if current > int64(cfg.MaxConcurrentConns) { | |
| 90 | + | s.activeConns.Add(-1) | |
| 91 | + | proxyError(w, "Service Unavailable", http.StatusServiceUnavailable) | |
| 92 | + | return | |
| 93 | + | } | |
| 94 | + | defer s.activeConns.Add(-1) | |
| 95 | + | } | |
| 96 | + | ||
| 98 | 97 | // SECURITY: Limit request body size to prevent resource exhaustion. | |
| 99 | 98 | if cfg.MaxBodySize > 0 { | |
| 100 | 99 | r.Body = http.MaxBytesReader(w, r.Body, cfg.MaxBodySize) | |
| @@ -124,8 +123,7 @@ func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||
| 124 | 123 | target, err := s.GetTarget(ctx, r.Host) | |
| 125 | 124 | if err != nil { | |
| 126 | 125 | log.Warn().Err(err).Msg("no route found for domain") | |
| 127 | - | w.Header().Set("Cache-Control", "no-store") | |
| 128 | - | http.NotFound(w, r) | |
| 126 | + | proxyError(w, "404 page not found", http.StatusNotFound) | |
| 129 | 127 | return | |
| 130 | 128 | } | |
| 131 | 129 | ||
| @@ -186,11 +184,19 @@ func (s *Service) GetTarget(ctx context.Context, domainName string) (*domain.Pro | |||
| 186 | 184 | return nil, err | |
| 187 | 185 | } | |
| 188 | 186 | ||
| 187 | + | // Preserve the original hostname for the Host header so virtual-hosted | |
| 188 | + | // upstreams work correctly. The resolved IP is used for dialing only. | |
| 189 | + | var originalHost string | |
| 190 | + | if resolvedIP != host { | |
| 191 | + | originalHost = host | |
| 192 | + | } | |
| 193 | + | ||
| 189 | 194 | target := &domain.ProxyTarget{ | |
| 190 | - | Host: resolvedIP, | |
| 191 | - | Port: port, | |
| 192 | - | ContainerID: "", // Not a container | |
| 193 | - | Scheme: "http", | |
| 195 | + | Host: resolvedIP, | |
| 196 | + | Port: port, | |
| 197 | + | ContainerID: "", // Not a container | |
| 198 | + | Scheme: "http", | |
| 199 | + | OriginalHost: originalHost, | |
| 194 | 200 | } | |
| 195 | 201 | ||
| 196 | 202 | // Cache external route target | |
| @@ -310,6 +316,8 @@ func (s *Service) RefreshTargets(ctx context.Context) error { | |||
| 310 | 316 | } | |
| 311 | 317 | ||
| 312 | 318 | // UpdateConfig updates the service configuration. | |
| 319 | + | // Connection limiting uses an atomic counter checked against config, so | |
| 320 | + | // changing MaxConcurrentConns takes effect immediately without any race. | |
| 313 | 321 | func (s *Service) UpdateConfig(config Config) { | |
| 314 | 322 | s.mu.Lock() | |
| 315 | 323 | s.config = config | |
| @@ -327,11 +335,17 @@ func (s *Service) modifyResponse(cfg Config) func(*http.Response) error { | |||
| 327 | 335 | if cfg.MaxResponseSize > 0 { | |
| 328 | 336 | if resp.ContentLength > cfg.MaxResponseSize { | |
| 329 | 337 | resp.Body.Close() | |
| 338 | + | const msg = "Response Too Large" | |
| 330 | 339 | resp.StatusCode = http.StatusBadGateway | |
| 331 | - | resp.Body = io.NopCloser(strings.NewReader("Response Too Large")) | |
| 332 | - | resp.ContentLength = int64(len("Response Too Large")) | |
| 340 | + | resp.Body = io.NopCloser(strings.NewReader(msg)) | |
| 341 | + | resp.ContentLength = int64(len(msg)) | |
| 333 | 342 | resp.Header.Set("Content-Type", "text/plain") | |
| 334 | 343 | resp.Header.Set("Cache-Control", "no-store") | |
| 344 | + | resp.Header.Set("Content-Security-Policy", "default-src 'none'; frame-ancestors 'none'") | |
| 345 | + | // Ensure framing/encoding headers match the substituted body | |
| 346 | + | resp.Header.Del("Transfer-Encoding") | |
| 347 | + | resp.Header.Del("Content-Encoding") | |
| 348 | + | resp.Header.Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10)) | |
| 335 | 349 | return nil | |
| 336 | 350 | } | |
| 337 | 351 | // For chunked/streaming responses where Content-Length is unknown, | |
| @@ -357,6 +371,8 @@ type limitedReadCloser struct { | |||
| 357 | 371 | ||
| 358 | 372 | func (l *limitedReadCloser) Read(p []byte) (int, error) { | |
| 359 | 373 | if l.remaining <= 0 { | |
| 374 | + | // Close the underlying body to release upstream connection/resources | |
| 375 | + | l.Close() | |
| 360 | 376 | return 0, fmt.Errorf("response body exceeded maximum size limit") | |
| 361 | 377 | } | |
| 362 | 378 | if int64(len(p)) > l.remaining { | |
| @@ -367,13 +383,26 @@ func (l *limitedReadCloser) Read(p []byte) (int, error) { | |||
| 367 | 383 | return n, err | |
| 368 | 384 | } | |
| 369 | 385 | ||
| 386 | + | // proxyError writes an error response with security headers appropriate for | |
| 387 | + | // proxy-generated error pages. CSP is set here (not in global middleware) | |
| 388 | + | // because Gordon proxies arbitrary web apps and a blanket CSP would break them. | |
| 389 | + | func proxyError(w http.ResponseWriter, msg string, code int) { | |
| 390 | + | w.Header().Set("Content-Security-Policy", "default-src 'none'; frame-ancestors 'none'") | |
| 391 | + | w.Header().Set("Cache-Control", "no-store") | |
| 392 | + | http.Error(w, msg, code) | |
| 393 | + | } | |
| 394 | + | ||
| 370 | 395 | // Helper methods | |
| 371 | 396 | ||
| 372 | 397 | // newReverseProxy creates a reverse proxy using Rewrite instead of Director to prevent | |
| 373 | 398 | // hop-by-hop header attacks. A malicious client could send "Connection: Authorization" | |
| 374 | 399 | // to strip the Authorization header when using the default Director. Rewrite processes | |
| 375 | 400 | // headers after hop-by-hop removal, ensuring headers like Authorization are preserved. | |
| 376 | - | func newReverseProxy(targetURL *url.URL, errorHandler func(http.ResponseWriter, *http.Request, error), modifyResponse func(*http.Response) error) *httputil.ReverseProxy { | |
| 401 | + | // | |
| 402 | + | // originalHost, when non-empty, overrides the Host header sent to the backend. | |
| 403 | + | // This is used for DNS-pinned targets where the proxy dials an IP but needs to | |
| 404 | + | // send the original hostname for virtual-hosted upstreams. | |
| 405 | + | func newReverseProxy(targetURL *url.URL, originalHost string, errorHandler func(http.ResponseWriter, *http.Request, error), modifyResponse func(*http.Response) error) *httputil.ReverseProxy { | |
| 377 | 406 | return &httputil.ReverseProxy{ | |
| 378 | 407 | Rewrite: func(pr *httputil.ProxyRequest) { | |
| 379 | 408 | pr.SetURL(targetURL) | |
| @@ -383,7 +412,13 @@ func newReverseProxy(targetURL *url.URL, errorHandler func(http.ResponseWriter, | |||
| 383 | 412 | // proxy. An attacker could spoof X-Forwarded-Proto: https on an HTTP connection | |
| 384 | 413 | // to trick backends into thinking the connection is secure. | |
| 385 | 414 | pr.SetXForwarded() | |
| 386 | - | pr.Out.Host = targetURL.Host | |
| 415 | + | if originalHost != "" { | |
| 416 | + | // DNS pinning: dial the resolved IP but send the original hostname | |
| 417 | + | // so virtual-hosted upstreams route the request correctly. | |
| 418 | + | pr.Out.Host = net.JoinHostPort(originalHost, targetURL.Port()) | |
| 419 | + | } else { | |
| 420 | + | pr.Out.Host = targetURL.Host | |
| 421 | + | } | |
| 387 | 422 | }, | |
| 388 | 423 | Transport: proxyTransport, | |
| 389 | 424 | ErrorHandler: errorHandler, | |
| @@ -398,27 +433,30 @@ func (s *Service) proxyToTarget(w http.ResponseWriter, r *http.Request, target * | |||
| 398 | 433 | ||
| 399 | 434 | log := zerowrap.FromCtx(r.Context()) | |
| 400 | 435 | ||
| 401 | - | targetURL, err := url.Parse(fmt.Sprintf("%s://%s:%d", target.Scheme, target.Host, target.Port)) | |
| 436 | + | // Use net.JoinHostPort for IPv6-safe URL construction (brackets are added automatically) | |
| 437 | + | targetURL, err := url.Parse(fmt.Sprintf("%s://%s", target.Scheme, net.JoinHostPort(target.Host, strconv.Itoa(target.Port)))) | |
| 402 | 438 | if err != nil { | |
| 403 | 439 | log.WrapErr(err, "failed to parse target URL") | |
| 404 | - | http.Error(w, "Internal Server Error", http.StatusInternalServerError) | |
| 440 | + | proxyError(w, "Internal Server Error", http.StatusInternalServerError) | |
| 405 | 441 | return | |
| 406 | 442 | } | |
| 407 | 443 | ||
| 408 | 444 | log.Debug().Str("target_url", targetURL.String()).Msg("proxying to target") | |
| 409 | 445 | ||
| 410 | - | proxy := newReverseProxy(targetURL, | |
| 446 | + | // If DNS pinning resolved hostname to IP, preserve the original hostname | |
| 447 | + | // for the Host header so virtual-hosted upstreams work correctly. | |
| 448 | + | originalHost := target.OriginalHost | |
| 449 | + | ||
| 450 | + | proxy := newReverseProxy(targetURL, originalHost, | |
| 411 | 451 | func(w http.ResponseWriter, _ *http.Request, err error) { | |
| 412 | 452 | var maxBytesErr *http.MaxBytesError | |
| 413 | 453 | if errors.As(err, &maxBytesErr) { | |
| 414 | 454 | log.Warn().Err(err).Str("target", targetURL.String()).Msg("proxy error: request body too large") | |
| 415 | - | w.Header().Set("Cache-Control", "no-store") | |
| 416 | - | http.Error(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | |
| 455 | + | proxyError(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | |
| 417 | 456 | return | |
| 418 | 457 | } | |
| 419 | 458 | log.Error().Err(err).Str("target", targetURL.String()).Msg("proxy error: connection failed") | |
| 420 | - | w.Header().Set("Cache-Control", "no-store") | |
| 421 | - | http.Error(w, "Service Unavailable", http.StatusServiceUnavailable) | |
| 459 | + | proxyError(w, "Service Unavailable", http.StatusServiceUnavailable) | |
| 422 | 460 | }, | |
| 423 | 461 | s.modifyResponse(cfg), | |
| 424 | 462 | ) | |
| @@ -447,22 +485,20 @@ func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | |||
| 447 | 485 | targetURL, err := url.Parse(fmt.Sprintf("http://localhost:%d", cfg.RegistryPort)) | |
| 448 | 486 | if err != nil { | |
| 449 | 487 | log.WrapErr(err, "failed to parse registry target URL") | |
| 450 | - | http.Error(w, "Internal Server Error", http.StatusInternalServerError) | |
| 488 | + | proxyError(w, "Internal Server Error", http.StatusInternalServerError) | |
| 451 | 489 | return | |
| 452 | 490 | } | |
| 453 | 491 | ||
| 454 | - | proxy := newReverseProxy(targetURL, | |
| 492 | + | proxy := newReverseProxy(targetURL, "", | |
| 455 | 493 | func(w http.ResponseWriter, _ *http.Request, err error) { | |
| 456 | 494 | var maxBytesErr *http.MaxBytesError | |
| 457 | 495 | if errors.As(err, &maxBytesErr) { | |
| 458 | 496 | log.Warn().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error: request body too large") | |
| 459 | - | w.Header().Set("Cache-Control", "no-store") | |
| 460 | - | http.Error(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | |
| 497 | + | proxyError(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | |
| 461 | 498 | return | |
| 462 | 499 | } | |
| 463 | 500 | log.Error().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error") | |
| 464 | - | w.Header().Set("Cache-Control", "no-store") | |
| 465 | - | http.Error(w, "Registry Unavailable", http.StatusServiceUnavailable) | |
| 501 | + | proxyError(w, "Registry Unavailable", http.StatusServiceUnavailable) | |
| 466 | 502 | }, | |
| 467 | 503 | func(resp *http.Response) error { | |
| 468 | 504 | resp.Header.Set("X-Proxied-By", "Gordon") | |
| @@ -476,6 +512,8 @@ func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | |||
| 476 | 512 | resp.Header.Del("X-XSS-Protection") | |
| 477 | 513 | resp.Header.Del("Referrer-Policy") | |
| 478 | 514 | resp.Header.Del("Permissions-Policy") | |
| 515 | + | resp.Header.Del("Content-Security-Policy") | |
| 516 | + | resp.Header.Del("Strict-Transport-Security") | |
| 479 | 517 | ||
| 480 | 518 | return nil | |
| 481 | 519 | }, | |
| @@ -14,6 +14,7 @@ import ( | |||||
| 14 | "strconv" | 14 | "strconv" | ||
| 15 | "strings" | 15 | "strings" | ||
| 16 | "sync" | 16 | "sync" | ||
| 17 | + | "sync/atomic" | |||
| 17 | "time" | 18 | "time" | ||
| 18 | 19 | ||||
| 19 | "github.com/bnema/zerowrap" | 20 | "github.com/bnema/zerowrap" | ||
| @@ -54,7 +55,7 @@ type Service struct { | |||||
| 54 | config Config | 55 | config Config | ||
| 55 | targets map[string]*domain.ProxyTarget | 56 | targets map[string]*domain.ProxyTarget | ||
| 56 | mu sync.RWMutex | 57 | mu sync.RWMutex | ||
| 57 | - | connSem chan struct{} // Semaphore for concurrent connection limiting | 58 | + | activeConns atomic.Int64 // Atomic counter for concurrent connection limiting |
| 58 | } | 59 | } | ||
| 59 | 60 | ||||
| 60 | // NewService creates a new proxy service. | 61 | // NewService creates a new proxy service. | ||
| @@ -64,37 +65,35 @@ func NewService( | |||||
| 64 | configSvc in.ConfigService, | 65 | configSvc in.ConfigService, | ||
| 65 | config Config, | 66 | config Config, | ||
| 66 | ) *Service { | 67 | ) *Service { | ||
| 67 | - | svc := &Service{ | 68 | + | return &Service{ |
| 68 | runtime: runtime, | 69 | runtime: runtime, | ||
| 69 | containerSvc: containerSvc, | 70 | containerSvc: containerSvc, | ||
| 70 | configSvc: configSvc, | 71 | configSvc: configSvc, | ||
| 71 | config: config, | 72 | config: config, | ||
| 72 | targets: make(map[string]*domain.ProxyTarget), | 73 | targets: make(map[string]*domain.ProxyTarget), | ||
| 73 | } | 74 | } | ||
| 74 | - | if config.MaxConcurrentConns > 0 { | |||
| 75 | - | svc.connSem = make(chan struct{}, config.MaxConcurrentConns) | |||
| 76 | - | } | |||
| 77 | - | return svc | |||
| 78 | } | 75 | } | ||
| 79 | 76 | ||||
| 80 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | 77 | // ServeHTTP handles incoming HTTP requests and proxies them to the appropriate backend. | ||
| 81 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | 78 | func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
| 82 | - | // SECURITY: Limit concurrent connections to prevent resource exhaustion. | |||
| 83 | - | if s.connSem != nil { | |||
| 84 | - | select { | |||
| 85 | - | case s.connSem <- struct{}{}: | |||
| 86 | - | defer func() { <-s.connSem }() | |||
| 87 | - | default: | |||
| 88 | - | http.Error(w, "Service Unavailable", http.StatusServiceUnavailable) | |||
| 89 | - | return | |||
| 90 | - | } | |||
| 91 | - | } | |||
| 92 | - | ||||
| 93 | // Copy config under RLock to avoid data race with UpdateConfig | 79 | // Copy config under RLock to avoid data race with UpdateConfig | ||
| 94 | s.mu.RLock() | 80 | s.mu.RLock() | ||
| 95 | cfg := s.config | 81 | cfg := s.config | ||
| 96 | s.mu.RUnlock() | 82 | s.mu.RUnlock() | ||
| 97 | 83 | ||||
| 84 | + | // SECURITY: Limit concurrent connections to prevent resource exhaustion. | |||
| 85 | + | // Uses an atomic counter so config reloads don't create a race between | |||
| 86 | + | // an old and new semaphore — the counter is shared across reloads. | |||
| 87 | + | if cfg.MaxConcurrentConns > 0 { | |||
| 88 | + | current := s.activeConns.Add(1) | |||
| 89 | + | if current > int64(cfg.MaxConcurrentConns) { | |||
| 90 | + | s.activeConns.Add(-1) | |||
| 91 | + | proxyError(w, "Service Unavailable", http.StatusServiceUnavailable) | |||
| 92 | + | return | |||
| 93 | + | } | |||
| 94 | + | defer s.activeConns.Add(-1) | |||
| 95 | + | } | |||
| 96 | + | ||||
| 98 | // SECURITY: Limit request body size to prevent resource exhaustion. | 97 | // SECURITY: Limit request body size to prevent resource exhaustion. | ||
| 99 | if cfg.MaxBodySize > 0 { | 98 | if cfg.MaxBodySize > 0 { | ||
| 100 | r.Body = http.MaxBytesReader(w, r.Body, cfg.MaxBodySize) | 99 | r.Body = http.MaxBytesReader(w, r.Body, cfg.MaxBodySize) | ||
| @@ -124,8 +123,7 @@ func (s *Service) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |||||
| 124 | target, err := s.GetTarget(ctx, r.Host) | 123 | target, err := s.GetTarget(ctx, r.Host) | ||
| 125 | if err != nil { | 124 | if err != nil { | ||
| 126 | log.Warn().Err(err).Msg("no route found for domain") | 125 | log.Warn().Err(err).Msg("no route found for domain") | ||
| 127 | - | w.Header().Set("Cache-Control", "no-store") | 126 | + | proxyError(w, "404 page not found", http.StatusNotFound) |
| 128 | - | http.NotFound(w, r) | |||
| 129 | return | 127 | return | ||
| 130 | } | 128 | } | ||
| 131 | 129 | ||||
| @@ -186,11 +184,19 @@ func (s *Service) GetTarget(ctx context.Context, domainName string) (*domain.Pro | |||||
| 186 | return nil, err | 184 | return nil, err | ||
| 187 | } | 185 | } | ||
| 188 | 186 | ||||
| 187 | + | // Preserve the original hostname for the Host header so virtual-hosted | |||
| 188 | + | // upstreams work correctly. The resolved IP is used for dialing only. | |||
| 189 | + | var originalHost string | |||
| 190 | + | if resolvedIP != host { | |||
| 191 | + | originalHost = host | |||
| 192 | + | } | |||
| 193 | + | ||||
| 189 | target := &domain.ProxyTarget{ | 194 | target := &domain.ProxyTarget{ | ||
| 190 | - | Host: resolvedIP, | 195 | + | Host: resolvedIP, |
| 191 | - | Port: port, | 196 | + | Port: port, |
| 192 | - | ContainerID: "", // Not a container | 197 | + | ContainerID: "", // Not a container |
| 193 | - | Scheme: "http", | 198 | + | Scheme: "http", |
| 199 | + | OriginalHost: originalHost, | |||
| 194 | } | 200 | } | ||
| 195 | 201 | ||||
| 196 | // Cache external route target | 202 | // Cache external route target | ||
| @@ -310,6 +316,8 @@ func (s *Service) RefreshTargets(ctx context.Context) error { | |||||
| 310 | } | 316 | } | ||
| 311 | 317 | ||||
| 312 | // UpdateConfig updates the service configuration. | 318 | // UpdateConfig updates the service configuration. | ||
| 319 | + | // Connection limiting uses an atomic counter checked against config, so | |||
| 320 | + | // changing MaxConcurrentConns takes effect immediately without any race. | |||
| 313 | func (s *Service) UpdateConfig(config Config) { | 321 | func (s *Service) UpdateConfig(config Config) { | ||
| 314 | s.mu.Lock() | 322 | s.mu.Lock() | ||
| 315 | s.config = config | 323 | s.config = config | ||
| @@ -327,11 +335,17 @@ func (s *Service) modifyResponse(cfg Config) func(*http.Response) error { | |||||
| 327 | if cfg.MaxResponseSize > 0 { | 335 | if cfg.MaxResponseSize > 0 { | ||
| 328 | if resp.ContentLength > cfg.MaxResponseSize { | 336 | if resp.ContentLength > cfg.MaxResponseSize { | ||
| 329 | resp.Body.Close() | 337 | resp.Body.Close() | ||
| 338 | + | const msg = "Response Too Large" | |||
| 330 | resp.StatusCode = http.StatusBadGateway | 339 | resp.StatusCode = http.StatusBadGateway | ||
| 331 | - | resp.Body = io.NopCloser(strings.NewReader("Response Too Large")) | 340 | + | resp.Body = io.NopCloser(strings.NewReader(msg)) |
| 332 | - | resp.ContentLength = int64(len("Response Too Large")) | 341 | + | resp.ContentLength = int64(len(msg)) |
| 333 | resp.Header.Set("Content-Type", "text/plain") | 342 | resp.Header.Set("Content-Type", "text/plain") | ||
| 334 | resp.Header.Set("Cache-Control", "no-store") | 343 | resp.Header.Set("Cache-Control", "no-store") | ||
| 344 | + | resp.Header.Set("Content-Security-Policy", "default-src 'none'; frame-ancestors 'none'") | |||
| 345 | + | // Ensure framing/encoding headers match the substituted body | |||
| 346 | + | resp.Header.Del("Transfer-Encoding") | |||
| 347 | + | resp.Header.Del("Content-Encoding") | |||
| 348 | + | resp.Header.Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10)) | |||
| 335 | return nil | 349 | return nil | ||
| 336 | } | 350 | } | ||
| 337 | // For chunked/streaming responses where Content-Length is unknown, | 351 | // For chunked/streaming responses where Content-Length is unknown, | ||
| @@ -357,6 +371,8 @@ type limitedReadCloser struct { | |||||
| 357 | 371 | ||||
| 358 | func (l *limitedReadCloser) Read(p []byte) (int, error) { | 372 | func (l *limitedReadCloser) Read(p []byte) (int, error) { | ||
| 359 | if l.remaining <= 0 { | 373 | if l.remaining <= 0 { | ||
| 374 | + | // Close the underlying body to release upstream connection/resources | |||
| 375 | + | l.Close() | |||
| 360 | return 0, fmt.Errorf("response body exceeded maximum size limit") | 376 | return 0, fmt.Errorf("response body exceeded maximum size limit") | ||
| 361 | } | 377 | } | ||
| 362 | if int64(len(p)) > l.remaining { | 378 | if int64(len(p)) > l.remaining { | ||
| @@ -367,13 +383,26 @@ func (l *limitedReadCloser) Read(p []byte) (int, error) { | |||||
| 367 | return n, err | 383 | return n, err | ||
| 368 | } | 384 | } | ||
| 369 | 385 | ||||
| 386 | + | // proxyError writes an error response with security headers appropriate for | |||
| 387 | + | // proxy-generated error pages. CSP is set here (not in global middleware) | |||
| 388 | + | // because Gordon proxies arbitrary web apps and a blanket CSP would break them. | |||
| 389 | + | func proxyError(w http.ResponseWriter, msg string, code int) { | |||
| 390 | + | w.Header().Set("Content-Security-Policy", "default-src 'none'; frame-ancestors 'none'") | |||
| 391 | + | w.Header().Set("Cache-Control", "no-store") | |||
| 392 | + | http.Error(w, msg, code) | |||
| 393 | + | } | |||
| 394 | + | ||||
| 370 | // Helper methods | 395 | // Helper methods | ||
| 371 | 396 | ||||
| 372 | // newReverseProxy creates a reverse proxy using Rewrite instead of Director to prevent | 397 | // newReverseProxy creates a reverse proxy using Rewrite instead of Director to prevent | ||
| 373 | // hop-by-hop header attacks. A malicious client could send "Connection: Authorization" | 398 | // hop-by-hop header attacks. A malicious client could send "Connection: Authorization" | ||
| 374 | // to strip the Authorization header when using the default Director. Rewrite processes | 399 | // to strip the Authorization header when using the default Director. Rewrite processes | ||
| 375 | // headers after hop-by-hop removal, ensuring headers like Authorization are preserved. | 400 | // headers after hop-by-hop removal, ensuring headers like Authorization are preserved. | ||
| 376 | - | func newReverseProxy(targetURL *url.URL, errorHandler func(http.ResponseWriter, *http.Request, error), modifyResponse func(*http.Response) error) *httputil.ReverseProxy { | 401 | + | // |
| 402 | + | // originalHost, when non-empty, overrides the Host header sent to the backend. | |||
| 403 | + | // This is used for DNS-pinned targets where the proxy dials an IP but needs to | |||
| 404 | + | // send the original hostname for virtual-hosted upstreams. | |||
| 405 | + | func newReverseProxy(targetURL *url.URL, originalHost string, errorHandler func(http.ResponseWriter, *http.Request, error), modifyResponse func(*http.Response) error) *httputil.ReverseProxy { | |||
| 377 | return &httputil.ReverseProxy{ | 406 | return &httputil.ReverseProxy{ | ||
| 378 | Rewrite: func(pr *httputil.ProxyRequest) { | 407 | Rewrite: func(pr *httputil.ProxyRequest) { | ||
| 379 | pr.SetURL(targetURL) | 408 | pr.SetURL(targetURL) | ||
| @@ -383,7 +412,13 @@ func newReverseProxy(targetURL *url.URL, errorHandler func(http.ResponseWriter, | |||||
| 383 | // proxy. An attacker could spoof X-Forwarded-Proto: https on an HTTP connection | 412 | // proxy. An attacker could spoof X-Forwarded-Proto: https on an HTTP connection | ||
| 384 | // to trick backends into thinking the connection is secure. | 413 | // to trick backends into thinking the connection is secure. | ||
| 385 | pr.SetXForwarded() | 414 | pr.SetXForwarded() | ||
| 386 | - | pr.Out.Host = targetURL.Host | 415 | + | if originalHost != "" { |
| 416 | + | // DNS pinning: dial the resolved IP but send the original hostname | |||
| 417 | + | // so virtual-hosted upstreams route the request correctly. | |||
| 418 | + | pr.Out.Host = net.JoinHostPort(originalHost, targetURL.Port()) | |||
| 419 | + | } else { | |||
| 420 | + | pr.Out.Host = targetURL.Host | |||
| 421 | + | } | |||
| 387 | }, | 422 | }, | ||
| 388 | Transport: proxyTransport, | 423 | Transport: proxyTransport, | ||
| 389 | ErrorHandler: errorHandler, | 424 | ErrorHandler: errorHandler, | ||
| @@ -398,27 +433,30 @@ func (s *Service) proxyToTarget(w http.ResponseWriter, r *http.Request, target * | |||||
| 398 | 433 | ||||
| 399 | log := zerowrap.FromCtx(r.Context()) | 434 | log := zerowrap.FromCtx(r.Context()) | ||
| 400 | 435 | ||||
| 401 | - | targetURL, err := url.Parse(fmt.Sprintf("%s://%s:%d", target.Scheme, target.Host, target.Port)) | 436 | + | // Use net.JoinHostPort for IPv6-safe URL construction (brackets are added automatically) |
| 437 | + | targetURL, err := url.Parse(fmt.Sprintf("%s://%s", target.Scheme, net.JoinHostPort(target.Host, strconv.Itoa(target.Port)))) | |||
| 402 | if err != nil { | 438 | if err != nil { | ||
| 403 | log.WrapErr(err, "failed to parse target URL") | 439 | log.WrapErr(err, "failed to parse target URL") | ||
| 404 | - | http.Error(w, "Internal Server Error", http.StatusInternalServerError) | 440 | + | proxyError(w, "Internal Server Error", http.StatusInternalServerError) |
| 405 | return | 441 | return | ||
| 406 | } | 442 | } | ||
| 407 | 443 | ||||
| 408 | log.Debug().Str("target_url", targetURL.String()).Msg("proxying to target") | 444 | log.Debug().Str("target_url", targetURL.String()).Msg("proxying to target") | ||
| 409 | 445 | ||||
| 410 | - | proxy := newReverseProxy(targetURL, | 446 | + | // If DNS pinning resolved hostname to IP, preserve the original hostname |
| 447 | + | // for the Host header so virtual-hosted upstreams work correctly. | |||
| 448 | + | originalHost := target.OriginalHost | |||
| 449 | + | ||||
| 450 | + | proxy := newReverseProxy(targetURL, originalHost, | |||
| 411 | func(w http.ResponseWriter, _ *http.Request, err error) { | 451 | func(w http.ResponseWriter, _ *http.Request, err error) { | ||
| 412 | var maxBytesErr *http.MaxBytesError | 452 | var maxBytesErr *http.MaxBytesError | ||
| 413 | if errors.As(err, &maxBytesErr) { | 453 | if errors.As(err, &maxBytesErr) { | ||
| 414 | log.Warn().Err(err).Str("target", targetURL.String()).Msg("proxy error: request body too large") | 454 | log.Warn().Err(err).Str("target", targetURL.String()).Msg("proxy error: request body too large") | ||
| 415 | - | w.Header().Set("Cache-Control", "no-store") | 455 | + | proxyError(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) |
| 416 | - | http.Error(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | |||
| 417 | return | 456 | return | ||
| 418 | } | 457 | } | ||
| 419 | log.Error().Err(err).Str("target", targetURL.String()).Msg("proxy error: connection failed") | 458 | log.Error().Err(err).Str("target", targetURL.String()).Msg("proxy error: connection failed") | ||
| 420 | - | w.Header().Set("Cache-Control", "no-store") | 459 | + | proxyError(w, "Service Unavailable", http.StatusServiceUnavailable) |
| 421 | - | http.Error(w, "Service Unavailable", http.StatusServiceUnavailable) | |||
| 422 | }, | 460 | }, | ||
| 423 | s.modifyResponse(cfg), | 461 | s.modifyResponse(cfg), | ||
| 424 | ) | 462 | ) | ||
| @@ -447,22 +485,20 @@ func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | |||||
| 447 | targetURL, err := url.Parse(fmt.Sprintf("http://localhost:%d", cfg.RegistryPort)) | 485 | targetURL, err := url.Parse(fmt.Sprintf("http://localhost:%d", cfg.RegistryPort)) | ||
| 448 | if err != nil { | 486 | if err != nil { | ||
| 449 | log.WrapErr(err, "failed to parse registry target URL") | 487 | log.WrapErr(err, "failed to parse registry target URL") | ||
| 450 | - | http.Error(w, "Internal Server Error", http.StatusInternalServerError) | 488 | + | proxyError(w, "Internal Server Error", http.StatusInternalServerError) |
| 451 | return | 489 | return | ||
| 452 | } | 490 | } | ||
| 453 | 491 | ||||
| 454 | - | proxy := newReverseProxy(targetURL, | 492 | + | proxy := newReverseProxy(targetURL, "", |
| 455 | func(w http.ResponseWriter, _ *http.Request, err error) { | 493 | func(w http.ResponseWriter, _ *http.Request, err error) { | ||
| 456 | var maxBytesErr *http.MaxBytesError | 494 | var maxBytesErr *http.MaxBytesError | ||
| 457 | if errors.As(err, &maxBytesErr) { | 495 | if errors.As(err, &maxBytesErr) { | ||
| 458 | log.Warn().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error: request body too large") | 496 | log.Warn().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error: request body too large") | ||
| 459 | - | w.Header().Set("Cache-Control", "no-store") | 497 | + | proxyError(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) |
| 460 | - | http.Error(w, "Request Entity Too Large", http.StatusRequestEntityTooLarge) | |||
| 461 | return | 498 | return | ||
| 462 | } | 499 | } | ||
| 463 | log.Error().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error") | 500 | log.Error().Err(err).Int("registry_port", cfg.RegistryPort).Msg("registry proxy error") | ||
| 464 | - | w.Header().Set("Cache-Control", "no-store") | 501 | + | proxyError(w, "Registry Unavailable", http.StatusServiceUnavailable) |
| 465 | - | http.Error(w, "Registry Unavailable", http.StatusServiceUnavailable) | |||
| 466 | }, | 502 | }, | ||
| 467 | func(resp *http.Response) error { | 503 | func(resp *http.Response) error { | ||
| 468 | resp.Header.Set("X-Proxied-By", "Gordon") | 504 | resp.Header.Set("X-Proxied-By", "Gordon") | ||
| @@ -476,6 +512,8 @@ func (s *Service) proxyToRegistry(w http.ResponseWriter, r *http.Request) { | |||||
| 476 | resp.Header.Del("X-XSS-Protection") | 512 | resp.Header.Del("X-XSS-Protection") | ||
| 477 | resp.Header.Del("Referrer-Policy") | 513 | resp.Header.Del("Referrer-Policy") | ||
| 478 | resp.Header.Del("Permissions-Policy") | 514 | resp.Header.Del("Permissions-Policy") | ||
| 515 | + | resp.Header.Del("Content-Security-Policy") | |||
| 516 | + | resp.Header.Del("Strict-Transport-Security") | |||
| 479 | 517 | ||||
| 480 | return nil | 518 | return nil | ||
| 481 | }, | 519 | }, | ||
M internal/usecase/proxy/service_test.go +6 -8
| @@ -407,8 +407,8 @@ func TestService_ConcurrentConnectionLimit(t *testing.T) { | |||
| 407 | 407 | MaxConcurrentConns: 1, | |
| 408 | 408 | }) | |
| 409 | 409 | ||
| 410 | - | assert.NotNil(t, svc.connSem, "semaphore should be initialized when MaxConcurrentConns > 0") | |
| 411 | - | assert.Equal(t, 1, cap(svc.connSem)) | |
| 410 | + | assert.Equal(t, 1, svc.config.MaxConcurrentConns) | |
| 411 | + | assert.Equal(t, int64(0), svc.activeConns.Load(), "active connections should start at 0") | |
| 412 | 412 | } | |
| 413 | 413 | ||
| 414 | 414 | func TestService_ConcurrentConnectionLimit_Disabled(t *testing.T) { | |
| @@ -420,7 +420,7 @@ func TestService_ConcurrentConnectionLimit_Disabled(t *testing.T) { | |||
| 420 | 420 | MaxConcurrentConns: 0, | |
| 421 | 421 | }) | |
| 422 | 422 | ||
| 423 | - | assert.Nil(t, svc.connSem, "semaphore should be nil when MaxConcurrentConns is 0") | |
| 423 | + | assert.Equal(t, 0, svc.config.MaxConcurrentConns, "0 means no limit") | |
| 424 | 424 | } | |
| 425 | 425 | ||
| 426 | 426 | func TestService_ConcurrentConnectionLimit_503WhenFull(t *testing.T) { | |
| @@ -432,8 +432,9 @@ func TestService_ConcurrentConnectionLimit_503WhenFull(t *testing.T) { | |||
| 432 | 432 | MaxConcurrentConns: 1, | |
| 433 | 433 | }) | |
| 434 | 434 | ||
| 435 | - | // Fill the semaphore | |
| 436 | - | svc.connSem <- struct{}{} | |
| 435 | + | // Simulate an in-flight connection by incrementing the atomic counter | |
| 436 | + | svc.activeConns.Add(1) | |
| 437 | + | defer svc.activeConns.Add(-1) | |
| 437 | 438 | ||
| 438 | 439 | // Next request should get 503 | |
| 439 | 440 | req := httptest.NewRequest("GET", "http://example.com/test", nil) | |
| @@ -441,9 +442,6 @@ func TestService_ConcurrentConnectionLimit_503WhenFull(t *testing.T) { | |||
| 441 | 442 | svc.ServeHTTP(w, req) | |
| 442 | 443 | ||
| 443 | 444 | assert.Equal(t, 503, w.Code) | |
| 444 | - | ||
| 445 | - | // Drain semaphore | |
| 446 | - | <-svc.connSem | |
| 447 | 445 | } | |
| 448 | 446 | ||
| 449 | 447 | func TestServeHTTP_MaxBodySize(t *testing.T) { | |
| @@ -407,8 +407,8 @@ func TestService_ConcurrentConnectionLimit(t *testing.T) { | |||||
| 407 | MaxConcurrentConns: 1, | 407 | MaxConcurrentConns: 1, | ||
| 408 | }) | 408 | }) | ||
| 409 | 409 | ||||
| 410 | - | assert.NotNil(t, svc.connSem, "semaphore should be initialized when MaxConcurrentConns > 0") | 410 | + | assert.Equal(t, 1, svc.config.MaxConcurrentConns) |
| 411 | - | assert.Equal(t, 1, cap(svc.connSem)) | 411 | + | assert.Equal(t, int64(0), svc.activeConns.Load(), "active connections should start at 0") |
| 412 | } | 412 | } | ||
| 413 | 413 | ||||
| 414 | func TestService_ConcurrentConnectionLimit_Disabled(t *testing.T) { | 414 | func TestService_ConcurrentConnectionLimit_Disabled(t *testing.T) { | ||
| @@ -420,7 +420,7 @@ func TestService_ConcurrentConnectionLimit_Disabled(t *testing.T) { | |||||
| 420 | MaxConcurrentConns: 0, | 420 | MaxConcurrentConns: 0, | ||
| 421 | }) | 421 | }) | ||
| 422 | 422 | ||||
| 423 | - | assert.Nil(t, svc.connSem, "semaphore should be nil when MaxConcurrentConns is 0") | 423 | + | assert.Equal(t, 0, svc.config.MaxConcurrentConns, "0 means no limit") |
| 424 | } | 424 | } | ||
| 425 | 425 | ||||
| 426 | func TestService_ConcurrentConnectionLimit_503WhenFull(t *testing.T) { | 426 | func TestService_ConcurrentConnectionLimit_503WhenFull(t *testing.T) { | ||
| @@ -432,8 +432,9 @@ func TestService_ConcurrentConnectionLimit_503WhenFull(t *testing.T) { | |||||
| 432 | MaxConcurrentConns: 1, | 432 | MaxConcurrentConns: 1, | ||
| 433 | }) | 433 | }) | ||
| 434 | 434 | ||||
| 435 | - | // Fill the semaphore | 435 | + | // Simulate an in-flight connection by incrementing the atomic counter |
| 436 | - | svc.connSem <- struct{}{} | 436 | + | svc.activeConns.Add(1) |
| 437 | + | defer svc.activeConns.Add(-1) | |||
| 437 | 438 | ||||
| 438 | // Next request should get 503 | 439 | // Next request should get 503 | ||
| 439 | req := httptest.NewRequest("GET", "http://example.com/test", nil) | 440 | req := httptest.NewRequest("GET", "http://example.com/test", nil) | ||
| @@ -441,9 +442,6 @@ func TestService_ConcurrentConnectionLimit_503WhenFull(t *testing.T) { | |||||
| 441 | svc.ServeHTTP(w, req) | 442 | svc.ServeHTTP(w, req) | ||
| 442 | 443 | ||||
| 443 | assert.Equal(t, 503, w.Code) | 444 | assert.Equal(t, 503, w.Code) | ||
| 444 | - | ||||
| 445 | - | // Drain semaphore | |||
| 446 | - | <-svc.connSem | |||
| 447 | } | 445 | } | ||
| 448 | 446 | ||||
| 449 | func TestServeHTTP_MaxBodySize(t *testing.T) { | 447 | func TestServeHTTP_MaxBodySize(t *testing.T) { | ||
M internal/usecase/proxy/ssrf.go +4 -3
| @@ -116,9 +116,10 @@ func ValidateExternalRouteTarget(host string) error { | |||
| 116 | 116 | return nil | |
| 117 | 117 | } | |
| 118 | 118 | ||
| 119 | - | // ResolveAndValidateHost resolves DNS for a hostname and validates that all | |
| 120 | - | // resolved IPs are safe (not in blocked CIDR ranges). Returns the first safe | |
| 121 | - | // resolved IP address. For raw IP addresses, validates and returns them directly. | |
| 119 | + | // ResolveAndValidateHost resolves DNS for a hostname and returns the first | |
| 120 | + | // resolved IP that is not in a blocked CIDR range. If all resolved IPs are | |
| 121 | + | // blocked, returns ErrSSRFBlocked. For raw IP addresses, validates and returns | |
| 122 | + | // them directly. | |
| 122 | 123 | // | |
| 123 | 124 | // SECURITY: This function pins DNS resolution to prevent TOCTOU/DNS-rebinding | |
| 124 | 125 | // attacks where a hostname resolves to a public IP during validation but to a | |
| @@ -116,9 +116,10 @@ func ValidateExternalRouteTarget(host string) error { | |||||
| 116 | return nil | 116 | return nil | ||
| 117 | } | 117 | } | ||
| 118 | 118 | ||||
| 119 | - | // ResolveAndValidateHost resolves DNS for a hostname and validates that all | 119 | + | // ResolveAndValidateHost resolves DNS for a hostname and returns the first |
| 120 | - | // resolved IPs are safe (not in blocked CIDR ranges). Returns the first safe | 120 | + | // resolved IP that is not in a blocked CIDR range. If all resolved IPs are |
| 121 | - | // resolved IP address. For raw IP addresses, validates and returns them directly. | 121 | + | // blocked, returns ErrSSRFBlocked. For raw IP addresses, validates and returns |
| 122 | + | // them directly. | |||
| 122 | // | 123 | // | ||
| 123 | // SECURITY: This function pins DNS resolution to prevent TOCTOU/DNS-rebinding | 124 | // SECURITY: This function pins DNS resolution to prevent TOCTOU/DNS-rebinding | ||
| 124 | // attacks where a hostname resolves to a public IP during validation but to a | 125 | // attacks where a hostname resolves to a public IP during validation but to a | ||