fix: address CodeRabbit review findings on security hardening
- Gate Cf-Connecting-Ip on Cloudflare CIDR ranges, not generic trusted proxies - Apply proxy CIDR allowlist to HTTP redirect listener (port 80) - Use net.JoinHostPort for registry bind address (IPv6 fix) - Add CapAdd for CHOWN/SETUID/SETGID/NET_BIND_SERVICE alongside CapDrop ALL - Add unit test for forceHSTS=true without TLS - Add migration note for network_isolation.enabled default change - Remove unnecessary FlushInterval: -1 (Go auto-detects streaming)
- commit
- 5a1e21d817b7afe75564c74ed9ccd73e1e9b6e80
- author
- brice · · 2026-03-28 05:21 UTC
- parent
- 552aaca
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +118 -21
M docs/config/network-isolation.md +11 -1
| @@ -10,11 +10,21 @@ enabled = true | |||
| 10 | 10 | network_prefix = "gordon" | |
| 11 | 11 | ``` | |
| 12 | 12 | ||
| 13 | + | ## Migration Note | |
| 14 | + | ||
| 15 | + | As of this release, `network_isolation.enabled` defaults to `true` (previously `false`). | |
| 16 | + | Existing installs that rely on a shared network must explicitly opt out: | |
| 17 | + | ||
| 18 | + | ```toml | |
| 19 | + | [network_isolation] | |
| 20 | + | enabled = false | |
| 21 | + | ``` | |
| 22 | + | ||
| 13 | 23 | ## Options | |
| 14 | 24 | ||
| 15 | 25 | | Option | Type | Default | Description | | |
| 16 | 26 | |--------|------|---------|-------------| | |
| 17 | - | | `enabled` | bool | `true` | Enable per-app network isolation | | |
| 27 | + | | `enabled` | bool | `true` | Enable per-app network isolation (changed from `false`) | | |
| 18 | 28 | | `network_prefix` | string | `"gordon"` | Prefix for created networks | | |
| 19 | 29 | ||
| 20 | 30 | ## How It Works | |
| @@ -10,11 +10,21 @@ enabled = true | |||||
| 10 | network_prefix = "gordon" | 10 | network_prefix = "gordon" | ||
| 11 | ``` | 11 | ``` | ||
| 12 | 12 | ||||
| 13 | + | ## Migration Note | |||
| 14 | + | ||||
| 15 | + | As of this release, `network_isolation.enabled` defaults to `true` (previously `false`). | |||
| 16 | + | Existing installs that rely on a shared network must explicitly opt out: | |||
| 17 | + | ||||
| 18 | + | ```toml | |||
| 19 | + | [network_isolation] | |||
| 20 | + | enabled = false | |||
| 21 | + | ``` | |||
| 22 | + | ||||
| 13 | ## Options | 23 | ## Options | ||
| 14 | 24 | ||||
| 15 | | Option | Type | Default | Description | | 25 | | Option | Type | Default | Description | | ||
| 16 | |--------|------|---------|-------------| | 26 | |--------|------|---------|-------------| | ||
| 17 | - | | `enabled` | bool | `true` | Enable per-app network isolation | | 27 | + | | `enabled` | bool | `true` | Enable per-app network isolation (changed from `false`) | |
| 18 | | `network_prefix` | string | `"gordon"` | Prefix for created networks | | 28 | | `network_prefix` | string | `"gordon"` | Prefix for created networks | | ||
| 19 | 29 | ||||
| 20 | ## How It Works | 30 | ## How It Works | ||
M internal/adapters/in/http/middleware/clientip.go +38 -5
| @@ -6,6 +6,36 @@ import ( | |||
| 6 | 6 | "strings" | |
| 7 | 7 | ) | |
| 8 | 8 | ||
| 9 | + | // cloudflareNets contains Cloudflare's published IPv4 and IPv6 ranges. | |
| 10 | + | // Cf-Connecting-Ip is only trusted when the immediate upstream matches these. | |
| 11 | + | // Source: https://www.cloudflare.com/ips/ | |
| 12 | + | var cloudflareNets = ParseTrustedProxies([]string{ | |
| 13 | + | // IPv4 | |
| 14 | + | "173.245.48.0/20", | |
| 15 | + | "103.21.244.0/22", | |
| 16 | + | "103.22.200.0/22", | |
| 17 | + | "103.31.4.0/22", | |
| 18 | + | "141.101.64.0/18", | |
| 19 | + | "108.162.192.0/18", | |
| 20 | + | "190.93.240.0/20", | |
| 21 | + | "188.114.96.0/20", | |
| 22 | + | "197.234.240.0/22", | |
| 23 | + | "198.41.128.0/17", | |
| 24 | + | "162.158.0.0/15", | |
| 25 | + | "104.16.0.0/13", | |
| 26 | + | "104.24.0.0/14", | |
| 27 | + | "172.64.0.0/13", | |
| 28 | + | "131.0.72.0/22", | |
| 29 | + | // IPv6 | |
| 30 | + | "2400:cb00::/32", | |
| 31 | + | "2606:4700::/32", | |
| 32 | + | "2803:f800::/32", | |
| 33 | + | "2405:b500::/32", | |
| 34 | + | "2405:8100::/32", | |
| 35 | + | "2a06:98c0::/29", | |
| 36 | + | "2c0f:f248::/32", | |
| 37 | + | }) | |
| 38 | + | ||
| 9 | 39 | func normalizeIP(raw string) string { | |
| 10 | 40 | ip := strings.TrimSpace(raw) | |
| 11 | 41 | if ip == "" { | |
| @@ -85,11 +115,14 @@ func GetClientIP(r *http.Request, trustedNets []*net.IPNet) string { | |||
| 85 | 115 | // Only honor proxy headers if the request comes from a trusted proxy | |
| 86 | 116 | if IsTrustedProxy(remoteIP, trustedNets) { | |
| 87 | 117 | // Cf-Connecting-IP is set by Cloudflare to the true client IP. | |
| 88 | - | // It is a single IP (not a chain) and cannot be spoofed through Cloudflare. | |
| 89 | - | // Check it first as it is the most reliable source behind Cloudflare. | |
| 90 | - | if cfIP := r.Header.Get("Cf-Connecting-Ip"); cfIP != "" { | |
| 91 | - | if ip := normalizeIP(cfIP); ip != "" { | |
| 92 | - | return ip | |
| 118 | + | // Only trust it when the immediate upstream is a known Cloudflare IP, | |
| 119 | + | // not any generic trusted proxy (e.g. local LB/nginx), because a | |
| 120 | + | // non-Cloudflare proxy would blindly forward a spoofed header. | |
| 121 | + | if IsTrustedProxy(remoteIP, cloudflareNets) { | |
| 122 | + | if cfIP := r.Header.Get("Cf-Connecting-Ip"); cfIP != "" { | |
| 123 | + | if ip := normalizeIP(cfIP); ip != "" { | |
| 124 | + | return ip | |
| 125 | + | } | |
| 93 | 126 | } | |
| 94 | 127 | } | |
| 95 | 128 | ||
| @@ -6,6 +6,36 @@ import ( | |||||
| 6 | "strings" | 6 | "strings" | ||
| 7 | ) | 7 | ) | ||
| 8 | 8 | ||||
| 9 | + | // cloudflareNets contains Cloudflare's published IPv4 and IPv6 ranges. | |||
| 10 | + | // Cf-Connecting-Ip is only trusted when the immediate upstream matches these. | |||
| 11 | + | // Source: https://www.cloudflare.com/ips/ | |||
| 12 | + | var cloudflareNets = ParseTrustedProxies([]string{ | |||
| 13 | + | // IPv4 | |||
| 14 | + | "173.245.48.0/20", | |||
| 15 | + | "103.21.244.0/22", | |||
| 16 | + | "103.22.200.0/22", | |||
| 17 | + | "103.31.4.0/22", | |||
| 18 | + | "141.101.64.0/18", | |||
| 19 | + | "108.162.192.0/18", | |||
| 20 | + | "190.93.240.0/20", | |||
| 21 | + | "188.114.96.0/20", | |||
| 22 | + | "197.234.240.0/22", | |||
| 23 | + | "198.41.128.0/17", | |||
| 24 | + | "162.158.0.0/15", | |||
| 25 | + | "104.16.0.0/13", | |||
| 26 | + | "104.24.0.0/14", | |||
| 27 | + | "172.64.0.0/13", | |||
| 28 | + | "131.0.72.0/22", | |||
| 29 | + | // IPv6 | |||
| 30 | + | "2400:cb00::/32", | |||
| 31 | + | "2606:4700::/32", | |||
| 32 | + | "2803:f800::/32", | |||
| 33 | + | "2405:b500::/32", | |||
| 34 | + | "2405:8100::/32", | |||
| 35 | + | "2a06:98c0::/29", | |||
| 36 | + | "2c0f:f248::/32", | |||
| 37 | + | }) | |||
| 38 | + | ||||
| 9 | func normalizeIP(raw string) string { | 39 | func normalizeIP(raw string) string { | ||
| 10 | ip := strings.TrimSpace(raw) | 40 | ip := strings.TrimSpace(raw) | ||
| 11 | if ip == "" { | 41 | if ip == "" { | ||
| @@ -85,11 +115,14 @@ func GetClientIP(r *http.Request, trustedNets []*net.IPNet) string { | |||||
| 85 | // Only honor proxy headers if the request comes from a trusted proxy | 115 | // Only honor proxy headers if the request comes from a trusted proxy | ||
| 86 | if IsTrustedProxy(remoteIP, trustedNets) { | 116 | if IsTrustedProxy(remoteIP, trustedNets) { | ||
| 87 | // Cf-Connecting-IP is set by Cloudflare to the true client IP. | 117 | // Cf-Connecting-IP is set by Cloudflare to the true client IP. | ||
| 88 | - | // It is a single IP (not a chain) and cannot be spoofed through Cloudflare. | 118 | + | // Only trust it when the immediate upstream is a known Cloudflare IP, |
| 89 | - | // Check it first as it is the most reliable source behind Cloudflare. | 119 | + | // not any generic trusted proxy (e.g. local LB/nginx), because a |
| 90 | - | if cfIP := r.Header.Get("Cf-Connecting-Ip"); cfIP != "" { | 120 | + | // non-Cloudflare proxy would blindly forward a spoofed header. |
| 91 | - | if ip := normalizeIP(cfIP); ip != "" { | 121 | + | if IsTrustedProxy(remoteIP, cloudflareNets) { |
| 92 | - | return ip | 122 | + | if cfIP := r.Header.Get("Cf-Connecting-Ip"); cfIP != "" { |
| 123 | + | if ip := normalizeIP(cfIP); ip != "" { | |||
| 124 | + | return ip | |||
| 125 | + | } | |||
| 93 | } | 126 | } | ||
| 94 | } | 127 | } | ||
| 95 | 128 | ||||
M internal/adapters/in/http/middleware/clientip_test.go +28 -0
| @@ -13,12 +13,15 @@ func TestGetClientIP(t *testing.T) { | |||
| 13 | 13 | // Parse trusted proxies for tests | |
| 14 | 14 | trustedNets := ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"}) | |
| 15 | 15 | noTrustedNets := []*net.IPNet{} | |
| 16 | + | // Include a Cloudflare IP in trusted proxies to test Cf-Connecting-Ip | |
| 17 | + | trustedWithCF := ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8", "173.245.48.0/20"}) | |
| 16 | 18 | ||
| 17 | 19 | tests := []struct { | |
| 18 | 20 | name string | |
| 19 | 21 | remoteAddr string | |
| 20 | 22 | xff string | |
| 21 | 23 | xRealIP string | |
| 24 | + | cfIP string | |
| 22 | 25 | trustedNets []*net.IPNet | |
| 23 | 26 | wantIP string | |
| 24 | 27 | }{ | |
| @@ -98,6 +101,28 @@ func TestGetClientIP(t *testing.T) { | |||
| 98 | 101 | trustedNets: trustedNets, | |
| 99 | 102 | wantIP: "203.0.113.50", | |
| 100 | 103 | }, | |
| 104 | + | { | |
| 105 | + | name: "Cf-Connecting-Ip honored from Cloudflare IP", | |
| 106 | + | remoteAddr: "173.245.48.1:12345", | |
| 107 | + | cfIP: "203.0.113.99", | |
| 108 | + | trustedNets: trustedWithCF, | |
| 109 | + | wantIP: "203.0.113.99", | |
| 110 | + | }, | |
| 111 | + | { | |
| 112 | + | name: "Cf-Connecting-Ip ignored from non-Cloudflare trusted proxy", | |
| 113 | + | remoteAddr: "127.0.0.1:12345", | |
| 114 | + | cfIP: "203.0.113.99", | |
| 115 | + | xff: "203.0.113.50", | |
| 116 | + | trustedNets: trustedNets, | |
| 117 | + | wantIP: "203.0.113.50", | |
| 118 | + | }, | |
| 119 | + | { | |
| 120 | + | name: "Cf-Connecting-Ip ignored from untrusted IP", | |
| 121 | + | remoteAddr: "192.168.1.100:12345", | |
| 122 | + | cfIP: "203.0.113.99", | |
| 123 | + | trustedNets: trustedNets, | |
| 124 | + | wantIP: "192.168.1.100", | |
| 125 | + | }, | |
| 101 | 126 | { | |
| 102 | 127 | name: "IPv6 RemoteAddr", | |
| 103 | 128 | remoteAddr: "[::1]:12345", | |
| @@ -116,6 +141,9 @@ func TestGetClientIP(t *testing.T) { | |||
| 116 | 141 | if tt.xRealIP != "" { | |
| 117 | 142 | req.Header.Set("X-Real-IP", tt.xRealIP) | |
| 118 | 143 | } | |
| 144 | + | if tt.cfIP != "" { | |
| 145 | + | req.Header.Set("Cf-Connecting-Ip", tt.cfIP) | |
| 146 | + | } | |
| 119 | 147 | ||
| 120 | 148 | ip := GetClientIP(req, tt.trustedNets) | |
| 121 | 149 | assert.Equal(t, tt.wantIP, ip) | |
| @@ -13,12 +13,15 @@ func TestGetClientIP(t *testing.T) { | |||||
| 13 | // Parse trusted proxies for tests | 13 | // Parse trusted proxies for tests | ||
| 14 | trustedNets := ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"}) | 14 | trustedNets := ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"}) | ||
| 15 | noTrustedNets := []*net.IPNet{} | 15 | noTrustedNets := []*net.IPNet{} | ||
| 16 | + | // Include a Cloudflare IP in trusted proxies to test Cf-Connecting-Ip | |||
| 17 | + | trustedWithCF := ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8", "173.245.48.0/20"}) | |||
| 16 | 18 | ||||
| 17 | tests := []struct { | 19 | tests := []struct { | ||
| 18 | name string | 20 | name string | ||
| 19 | remoteAddr string | 21 | remoteAddr string | ||
| 20 | xff string | 22 | xff string | ||
| 21 | xRealIP string | 23 | xRealIP string | ||
| 24 | + | cfIP string | |||
| 22 | trustedNets []*net.IPNet | 25 | trustedNets []*net.IPNet | ||
| 23 | wantIP string | 26 | wantIP string | ||
| 24 | }{ | 27 | }{ | ||
| @@ -98,6 +101,28 @@ func TestGetClientIP(t *testing.T) { | |||||
| 98 | trustedNets: trustedNets, | 101 | trustedNets: trustedNets, | ||
| 99 | wantIP: "203.0.113.50", | 102 | wantIP: "203.0.113.50", | ||
| 100 | }, | 103 | }, | ||
| 104 | + | { | |||
| 105 | + | name: "Cf-Connecting-Ip honored from Cloudflare IP", | |||
| 106 | + | remoteAddr: "173.245.48.1:12345", | |||
| 107 | + | cfIP: "203.0.113.99", | |||
| 108 | + | trustedNets: trustedWithCF, | |||
| 109 | + | wantIP: "203.0.113.99", | |||
| 110 | + | }, | |||
| 111 | + | { | |||
| 112 | + | name: "Cf-Connecting-Ip ignored from non-Cloudflare trusted proxy", | |||
| 113 | + | remoteAddr: "127.0.0.1:12345", | |||
| 114 | + | cfIP: "203.0.113.99", | |||
| 115 | + | xff: "203.0.113.50", | |||
| 116 | + | trustedNets: trustedNets, | |||
| 117 | + | wantIP: "203.0.113.50", | |||
| 118 | + | }, | |||
| 119 | + | { | |||
| 120 | + | name: "Cf-Connecting-Ip ignored from untrusted IP", | |||
| 121 | + | remoteAddr: "192.168.1.100:12345", | |||
| 122 | + | cfIP: "203.0.113.99", | |||
| 123 | + | trustedNets: trustedNets, | |||
| 124 | + | wantIP: "192.168.1.100", | |||
| 125 | + | }, | |||
| 101 | { | 126 | { | ||
| 102 | name: "IPv6 RemoteAddr", | 127 | name: "IPv6 RemoteAddr", | ||
| 103 | remoteAddr: "[::1]:12345", | 128 | remoteAddr: "[::1]:12345", | ||
| @@ -116,6 +141,9 @@ func TestGetClientIP(t *testing.T) { | |||||
| 116 | if tt.xRealIP != "" { | 141 | if tt.xRealIP != "" { | ||
| 117 | req.Header.Set("X-Real-IP", tt.xRealIP) | 142 | req.Header.Set("X-Real-IP", tt.xRealIP) | ||
| 118 | } | 143 | } | ||
| 144 | + | if tt.cfIP != "" { | |||
| 145 | + | req.Header.Set("Cf-Connecting-Ip", tt.cfIP) | |||
| 146 | + | } | |||
| 119 | 147 | ||||
| 120 | ip := GetClientIP(req, tt.trustedNets) | 148 | ip := GetClientIP(req, tt.trustedNets) | ||
| 121 | assert.Equal(t, tt.wantIP, ip) | 149 | assert.Equal(t, tt.wantIP, ip) | ||
M internal/adapters/in/http/middleware/security_test.go +16 -0
| @@ -51,6 +51,22 @@ func TestSecurityHeaders_HSTS_WithTLS(t *testing.T) { | |||
| 51 | 51 | assert.Equal(t, "max-age=31536000; includeSubDomains", rec.Header().Get("Strict-Transport-Security")) | |
| 52 | 52 | } | |
| 53 | 53 | ||
| 54 | + | func TestSecurityHeaders_HSTS_ForceWithoutTLS(t *testing.T) { | |
| 55 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 56 | + | w.WriteHeader(http.StatusOK) | |
| 57 | + | }) | |
| 58 | + | ||
| 59 | + | wrapped := SecurityHeadersWithOptions(true)(handler) | |
| 60 | + | ||
| 61 | + | req := httptest.NewRequest(http.MethodGet, "/", nil) | |
| 62 | + | // r.TLS is nil — no TLS on the connection, but forceHSTS is true | |
| 63 | + | rec := httptest.NewRecorder() | |
| 64 | + | wrapped.ServeHTTP(rec, req) | |
| 65 | + | ||
| 66 | + | assert.Equal(t, "max-age=31536000; includeSubDomains", rec.Header().Get("Strict-Transport-Security"), | |
| 67 | + | "HSTS should be set when forceHSTS is true even without TLS") | |
| 68 | + | } | |
| 69 | + | ||
| 54 | 70 | func TestSecurityHeaders_HSTS_WithForwardedProto_NotTrusted(t *testing.T) { | |
| 55 | 71 | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 56 | 72 | w.WriteHeader(http.StatusOK) | |
| @@ -51,6 +51,22 @@ func TestSecurityHeaders_HSTS_WithTLS(t *testing.T) { | |||||
| 51 | 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")) | ||
| 52 | } | 52 | } | ||
| 53 | 53 | ||||
| 54 | + | func TestSecurityHeaders_HSTS_ForceWithoutTLS(t *testing.T) { | |||
| 55 | + | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |||
| 56 | + | w.WriteHeader(http.StatusOK) | |||
| 57 | + | }) | |||
| 58 | + | ||||
| 59 | + | wrapped := SecurityHeadersWithOptions(true)(handler) | |||
| 60 | + | ||||
| 61 | + | req := httptest.NewRequest(http.MethodGet, "/", nil) | |||
| 62 | + | // r.TLS is nil — no TLS on the connection, but forceHSTS is true | |||
| 63 | + | rec := httptest.NewRecorder() | |||
| 64 | + | wrapped.ServeHTTP(rec, req) | |||
| 65 | + | ||||
| 66 | + | assert.Equal(t, "max-age=31536000; includeSubDomains", rec.Header().Get("Strict-Transport-Security"), | |||
| 67 | + | "HSTS should be set when forceHSTS is true even without TLS") | |||
| 68 | + | } | |||
| 69 | + | ||||
| 54 | func TestSecurityHeaders_HSTS_WithForwardedProto_NotTrusted(t *testing.T) { | 70 | func TestSecurityHeaders_HSTS_WithForwardedProto_NotTrusted(t *testing.T) { | ||
| 55 | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | 71 | handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| 56 | w.WriteHeader(http.StatusOK) | 72 | w.WriteHeader(http.StatusOK) | ||
M internal/adapters/in/http/proxy/forwarder.go +0 -3
| @@ -197,9 +197,6 @@ func newReverseProxy(targetURL *url.URL, originalHost string, transport http.Rou | |||
| 197 | 197 | Transport: transport, | |
| 198 | 198 | ErrorHandler: errorHandler, | |
| 199 | 199 | ModifyResponse: modifyResp, | |
| 200 | - | // Flush immediately so SSE, chunked streaming, and long-polling responses | |
| 201 | - | // are forwarded to the client without buffering delay. | |
| 202 | - | FlushInterval: -1, | |
| 203 | 200 | } | |
| 204 | 201 | } | |
| 205 | 202 | ||
| @@ -197,9 +197,6 @@ func newReverseProxy(targetURL *url.URL, originalHost string, transport http.Rou | |||||
| 197 | Transport: transport, | 197 | Transport: transport, | ||
| 198 | ErrorHandler: errorHandler, | 198 | ErrorHandler: errorHandler, | ||
| 199 | ModifyResponse: modifyResp, | 199 | ModifyResponse: modifyResp, | ||
| 200 | - | // Flush immediately so SSE, chunked streaming, and long-polling responses | |||
| 201 | - | // are forwarded to the client without buffering delay. | |||
| 202 | - | FlushInterval: -1, | |||
| 203 | } | 200 | } | ||
| 204 | } | 201 | } | ||
| 205 | 202 | ||||
M internal/adapters/out/docker/runtime.go +4 -0
| @@ -181,6 +181,10 @@ func (r *Runtime) CreateContainer(ctx context.Context, config *domain.ContainerC | |||
| 181 | 181 | Resources: resources, | |
| 182 | 182 | SecurityOpt: []string{"no-new-privileges:true"}, | |
| 183 | 183 | CapDrop: strslice.StrSlice{"ALL"}, | |
| 184 | + | // Re-add the minimal set of capabilities that standard images need. | |
| 185 | + | // Without these, containers that chown files (e.g. postgres, mysql) | |
| 186 | + | // or bind to ports <1024 (e.g. nginx) would fail to start. | |
| 187 | + | CapAdd: strslice.StrSlice{"CHOWN", "SETUID", "SETGID", "NET_BIND_SERVICE"}, | |
| 184 | 188 | } | |
| 185 | 189 | ||
| 186 | 190 | // Create network configuration for container | |
| @@ -181,6 +181,10 @@ func (r *Runtime) CreateContainer(ctx context.Context, config *domain.ContainerC | |||||
| 181 | Resources: resources, | 181 | Resources: resources, | ||
| 182 | SecurityOpt: []string{"no-new-privileges:true"}, | 182 | SecurityOpt: []string{"no-new-privileges:true"}, | ||
| 183 | CapDrop: strslice.StrSlice{"ALL"}, | 183 | CapDrop: strslice.StrSlice{"ALL"}, | ||
| 184 | + | // Re-add the minimal set of capabilities that standard images need. | |||
| 185 | + | // Without these, containers that chown files (e.g. postgres, mysql) | |||
| 186 | + | // or bind to ports <1024 (e.g. nginx) would fail to start. | |||
| 187 | + | CapAdd: strslice.StrSlice{"CHOWN", "SETUID", "SETGID", "NET_BIND_SERVICE"}, | |||
| 184 | } | 188 | } | ||
| 185 | 189 | ||||
| 186 | // Create network configuration for container | 190 | // Create network configuration for container | ||
M internal/app/run.go +19 -10
| @@ -286,10 +286,10 @@ func Run(ctx context.Context, configPath string) error { | |||
| 286 | 286 | defer svc.eventBus.Stop() | |
| 287 | 287 | ||
| 288 | 288 | // Create HTTP handlers | |
| 289 | - | registryHandler, proxyHandler := createHTTPHandlers(svc, cfg, log) | |
| 289 | + | registryHandler, proxyHandler, proxyCIDRMiddleware := createHTTPHandlers(svc, cfg, log) | |
| 290 | 290 | ||
| 291 | 291 | // Start servers, wait for listeners to bind, then sync/auto-start containers. | |
| 292 | - | return runServers(ctx, v, cfg, registryHandler, proxyHandler, svc, cleanupHandlers, log) | |
| 292 | + | return runServers(ctx, v, cfg, registryHandler, proxyHandler, proxyCIDRMiddleware, svc, cleanupHandlers, log) | |
| 293 | 293 | } | |
| 294 | 294 | ||
| 295 | 295 | func ensureTLSConfig(cfg *Config, log zerowrap.Logger) error { | |
| @@ -1669,7 +1669,9 @@ func loopbackOnly(next http.Handler, log zerowrap.Logger) http.Handler { | |||
| 1669 | 1669 | } | |
| 1670 | 1670 | ||
| 1671 | 1671 | // createHTTPHandlers creates HTTP handlers with middleware. | |
| 1672 | - | func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Handler, http.Handler) { | |
| 1672 | + | // The returned proxyCIDRMiddleware is non-nil when proxy_allowed_ips is configured; | |
| 1673 | + | // callers must also apply it to the HTTP-to-HTTPS redirect listener. | |
| 1674 | + | func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Handler, http.Handler, func(http.Handler) http.Handler) { | |
| 1673 | 1675 | // Parse trusted proxies once for all middleware chains. | |
| 1674 | 1676 | // This ensures consistent IP extraction across logging, rate limiting, and auth. | |
| 1675 | 1677 | trustedNets := middleware.ParseTrustedProxies(cfg.API.RateLimit.TrustedProxies) | |
| @@ -1700,7 +1702,8 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||
| 1700 | 1702 | // SECURITY: When proxy_allowed_ips is set, only connections from those CIDRs | |
| 1701 | 1703 | // can reach the proxy. This prevents direct-to-origin attacks that bypass | |
| 1702 | 1704 | // CDN protections (e.g. Cloudflare WAF, DDoS mitigation). | |
| 1703 | - | if proxyCIDRMiddleware := buildProxyCIDRAllowlistMiddleware(cfg, trustedNets, log); proxyCIDRMiddleware != nil { | |
| 1705 | + | proxyCIDRMiddleware := buildProxyCIDRAllowlistMiddleware(cfg, trustedNets, log) | |
| 1706 | + | if proxyCIDRMiddleware != nil { | |
| 1704 | 1707 | proxyMiddlewares = append(proxyMiddlewares, proxyCIDRMiddleware) | |
| 1705 | 1708 | } | |
| 1706 | 1709 | ||
| @@ -1714,7 +1717,7 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||
| 1714 | 1717 | ||
| 1715 | 1718 | registerAdminRoutes(registryMux, svc, cfg, trustedNets, log) | |
| 1716 | 1719 | ||
| 1717 | - | return registryMux, proxyMux | |
| 1720 | + | return registryMux, proxyMux, proxyCIDRMiddleware | |
| 1718 | 1721 | } | |
| 1719 | 1722 | ||
| 1720 | 1723 | func buildRegistryHandlerWithMiddleware( | |
| @@ -1946,7 +1949,7 @@ func registerAdminRoutes(registryMux *http.ServeMux, svc *services, cfg Config, | |||
| 1946 | 1949 | // - SIGUSR2: Triggers manual deploy for a specific route | |
| 1947 | 1950 | // The deferred signal.Stop calls ensure signal handlers are properly | |
| 1948 | 1951 | // cleaned up before program exit, preventing signal handler leaks. | |
| 1949 | - | func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler, proxyHandler http.Handler, svc *services, cleanupHandlers func(), log zerowrap.Logger) error { | |
| 1952 | + | func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler, proxyHandler http.Handler, proxyCIDRMiddleware func(http.Handler) http.Handler, svc *services, cleanupHandlers func(), log zerowrap.Logger) error { | |
| 1950 | 1953 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | |
| 1951 | 1954 | defer cancel() | |
| 1952 | 1955 | ||
| @@ -1964,10 +1967,10 @@ func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler | |||
| 1964 | 1967 | ||
| 1965 | 1968 | errChan := make(chan error, 3) | |
| 1966 | 1969 | ||
| 1967 | - | registryAddr := fmt.Sprintf("%s:%d", cfg.Server.RegistryListenAddr, cfg.Server.RegistryPort) | |
| 1970 | + | registryAddr := net.JoinHostPort(cfg.Server.RegistryListenAddr, strconv.Itoa(cfg.Server.RegistryPort)) | |
| 1968 | 1971 | registrySrv, registryReady := startServer(registryAddr, registryHandler, "registry", nil, errChan, log) | |
| 1969 | 1972 | ||
| 1970 | - | proxySrv, proxyReady, tlsSrv, tlsReady, err := startProxyServers(cfg, proxyHandler, errChan, log) | |
| 1973 | + | proxySrv, proxyReady, tlsSrv, tlsReady, err := startProxyServers(cfg, proxyHandler, proxyCIDRMiddleware, errChan, log) | |
| 1971 | 1974 | if err != nil { | |
| 1972 | 1975 | return err | |
| 1973 | 1976 | } | |
| @@ -2253,7 +2256,9 @@ func gracefulShutdown(registrySrv, proxySrv, tlsSrv *http.Server, containerSvc * | |||
| 2253 | 2256 | // startProxyServers sets up proxy server(s) depending on TLS configuration. | |
| 2254 | 2257 | // With TLS enabled, the HTTP port becomes a redirect-only server and a separate TLS server handles traffic. | |
| 2255 | 2258 | // Without TLS, a single server handles both HTTP/1.1 and cleartext HTTP/2. | |
| 2256 | - | func startProxyServers(cfg Config, proxyHandler http.Handler, errChan chan<- error, log zerowrap.Logger) (*http.Server, <-chan struct{}, *http.Server, <-chan struct{}, error) { | |
| 2259 | + | // proxyCIDRMiddleware, when non-nil, is also applied to the HTTP redirect listener | |
| 2260 | + | // so that proxy_allowed_ips cannot be bypassed via the plain-HTTP port. | |
| 2261 | + | func startProxyServers(cfg Config, proxyHandler http.Handler, proxyCIDRMiddleware func(http.Handler) http.Handler, errChan chan<- error, log zerowrap.Logger) (*http.Server, <-chan struct{}, *http.Server, <-chan struct{}, error) { | |
| 2257 | 2262 | if !cfg.Server.TLSEnabled { | |
| 2258 | 2263 | var proxyProtos http.Protocols | |
| 2259 | 2264 | proxyProtos.SetHTTP1(true) | |
| @@ -2283,7 +2288,11 @@ func startProxyServers(cfg Config, proxyHandler http.Handler, errChan chan<- err | |||
| 2283 | 2288 | http.Redirect(w, r, target, http.StatusPermanentRedirect) | |
| 2284 | 2289 | }) | |
| 2285 | 2290 | ||
| 2286 | - | proxySrv, proxyReady := startServer(fmt.Sprintf(":%d", cfg.Server.Port), redirectHandler, "proxy-redirect", nil, errChan, log) | |
| 2291 | + | var redirectWithMiddleware http.Handler = redirectHandler | |
| 2292 | + | if proxyCIDRMiddleware != nil { | |
| 2293 | + | redirectWithMiddleware = proxyCIDRMiddleware(redirectHandler) | |
| 2294 | + | } | |
| 2295 | + | proxySrv, proxyReady := startServer(fmt.Sprintf(":%d", cfg.Server.Port), redirectWithMiddleware, "proxy-redirect", nil, errChan, log) | |
| 2287 | 2296 | tlsSrv, tlsReady := startTLSServer( | |
| 2288 | 2297 | fmt.Sprintf(":%d", cfg.Server.TLSPort), | |
| 2289 | 2298 | proxyHandler, | |
| @@ -286,10 +286,10 @@ func Run(ctx context.Context, configPath string) error { | |||||
| 286 | defer svc.eventBus.Stop() | 286 | defer svc.eventBus.Stop() | ||
| 287 | 287 | ||||
| 288 | // Create HTTP handlers | 288 | // Create HTTP handlers | ||
| 289 | - | registryHandler, proxyHandler := createHTTPHandlers(svc, cfg, log) | 289 | + | registryHandler, proxyHandler, proxyCIDRMiddleware := createHTTPHandlers(svc, cfg, log) |
| 290 | 290 | ||||
| 291 | // Start servers, wait for listeners to bind, then sync/auto-start containers. | 291 | // Start servers, wait for listeners to bind, then sync/auto-start containers. | ||
| 292 | - | return runServers(ctx, v, cfg, registryHandler, proxyHandler, svc, cleanupHandlers, log) | 292 | + | return runServers(ctx, v, cfg, registryHandler, proxyHandler, proxyCIDRMiddleware, svc, cleanupHandlers, log) |
| 293 | } | 293 | } | ||
| 294 | 294 | ||||
| 295 | func ensureTLSConfig(cfg *Config, log zerowrap.Logger) error { | 295 | func ensureTLSConfig(cfg *Config, log zerowrap.Logger) error { | ||
| @@ -1669,7 +1669,9 @@ func loopbackOnly(next http.Handler, log zerowrap.Logger) http.Handler { | |||||
| 1669 | } | 1669 | } | ||
| 1670 | 1670 | ||||
| 1671 | // createHTTPHandlers creates HTTP handlers with middleware. | 1671 | // createHTTPHandlers creates HTTP handlers with middleware. | ||
| 1672 | - | func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Handler, http.Handler) { | 1672 | + | // The returned proxyCIDRMiddleware is non-nil when proxy_allowed_ips is configured; |
| 1673 | + | // callers must also apply it to the HTTP-to-HTTPS redirect listener. | |||
| 1674 | + | func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Handler, http.Handler, func(http.Handler) http.Handler) { | |||
| 1673 | // Parse trusted proxies once for all middleware chains. | 1675 | // Parse trusted proxies once for all middleware chains. | ||
| 1674 | // This ensures consistent IP extraction across logging, rate limiting, and auth. | 1676 | // This ensures consistent IP extraction across logging, rate limiting, and auth. | ||
| 1675 | trustedNets := middleware.ParseTrustedProxies(cfg.API.RateLimit.TrustedProxies) | 1677 | trustedNets := middleware.ParseTrustedProxies(cfg.API.RateLimit.TrustedProxies) | ||
| @@ -1700,7 +1702,8 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||||
| 1700 | // SECURITY: When proxy_allowed_ips is set, only connections from those CIDRs | 1702 | // SECURITY: When proxy_allowed_ips is set, only connections from those CIDRs | ||
| 1701 | // can reach the proxy. This prevents direct-to-origin attacks that bypass | 1703 | // can reach the proxy. This prevents direct-to-origin attacks that bypass | ||
| 1702 | // CDN protections (e.g. Cloudflare WAF, DDoS mitigation). | 1704 | // CDN protections (e.g. Cloudflare WAF, DDoS mitigation). | ||
| 1703 | - | if proxyCIDRMiddleware := buildProxyCIDRAllowlistMiddleware(cfg, trustedNets, log); proxyCIDRMiddleware != nil { | 1705 | + | proxyCIDRMiddleware := buildProxyCIDRAllowlistMiddleware(cfg, trustedNets, log) |
| 1706 | + | if proxyCIDRMiddleware != nil { | |||
| 1704 | proxyMiddlewares = append(proxyMiddlewares, proxyCIDRMiddleware) | 1707 | proxyMiddlewares = append(proxyMiddlewares, proxyCIDRMiddleware) | ||
| 1705 | } | 1708 | } | ||
| 1706 | 1709 | ||||
| @@ -1714,7 +1717,7 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||||
| 1714 | 1717 | ||||
| 1715 | registerAdminRoutes(registryMux, svc, cfg, trustedNets, log) | 1718 | registerAdminRoutes(registryMux, svc, cfg, trustedNets, log) | ||
| 1716 | 1719 | ||||
| 1717 | - | return registryMux, proxyMux | 1720 | + | return registryMux, proxyMux, proxyCIDRMiddleware |
| 1718 | } | 1721 | } | ||
| 1719 | 1722 | ||||
| 1720 | func buildRegistryHandlerWithMiddleware( | 1723 | func buildRegistryHandlerWithMiddleware( | ||
| @@ -1946,7 +1949,7 @@ func registerAdminRoutes(registryMux *http.ServeMux, svc *services, cfg Config, | |||||
| 1946 | // - SIGUSR2: Triggers manual deploy for a specific route | 1949 | // - SIGUSR2: Triggers manual deploy for a specific route | ||
| 1947 | // The deferred signal.Stop calls ensure signal handlers are properly | 1950 | // The deferred signal.Stop calls ensure signal handlers are properly | ||
| 1948 | // cleaned up before program exit, preventing signal handler leaks. | 1951 | // cleaned up before program exit, preventing signal handler leaks. | ||
| 1949 | - | func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler, proxyHandler http.Handler, svc *services, cleanupHandlers func(), log zerowrap.Logger) error { | 1952 | + | func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler, proxyHandler http.Handler, proxyCIDRMiddleware func(http.Handler) http.Handler, svc *services, cleanupHandlers func(), log zerowrap.Logger) error { |
| 1950 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | 1953 | ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) | ||
| 1951 | defer cancel() | 1954 | defer cancel() | ||
| 1952 | 1955 | ||||
| @@ -1964,10 +1967,10 @@ func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler | |||||
| 1964 | 1967 | ||||
| 1965 | errChan := make(chan error, 3) | 1968 | errChan := make(chan error, 3) | ||
| 1966 | 1969 | ||||
| 1967 | - | registryAddr := fmt.Sprintf("%s:%d", cfg.Server.RegistryListenAddr, cfg.Server.RegistryPort) | 1970 | + | registryAddr := net.JoinHostPort(cfg.Server.RegistryListenAddr, strconv.Itoa(cfg.Server.RegistryPort)) |
| 1968 | registrySrv, registryReady := startServer(registryAddr, registryHandler, "registry", nil, errChan, log) | 1971 | registrySrv, registryReady := startServer(registryAddr, registryHandler, "registry", nil, errChan, log) | ||
| 1969 | 1972 | ||||
| 1970 | - | proxySrv, proxyReady, tlsSrv, tlsReady, err := startProxyServers(cfg, proxyHandler, errChan, log) | 1973 | + | proxySrv, proxyReady, tlsSrv, tlsReady, err := startProxyServers(cfg, proxyHandler, proxyCIDRMiddleware, errChan, log) |
| 1971 | if err != nil { | 1974 | if err != nil { | ||
| 1972 | return err | 1975 | return err | ||
| 1973 | } | 1976 | } | ||
| @@ -2253,7 +2256,9 @@ func gracefulShutdown(registrySrv, proxySrv, tlsSrv *http.Server, containerSvc * | |||||
| 2253 | // startProxyServers sets up proxy server(s) depending on TLS configuration. | 2256 | // startProxyServers sets up proxy server(s) depending on TLS configuration. | ||
| 2254 | // With TLS enabled, the HTTP port becomes a redirect-only server and a separate TLS server handles traffic. | 2257 | // With TLS enabled, the HTTP port becomes a redirect-only server and a separate TLS server handles traffic. | ||
| 2255 | // Without TLS, a single server handles both HTTP/1.1 and cleartext HTTP/2. | 2258 | // Without TLS, a single server handles both HTTP/1.1 and cleartext HTTP/2. | ||
| 2256 | - | func startProxyServers(cfg Config, proxyHandler http.Handler, errChan chan<- error, log zerowrap.Logger) (*http.Server, <-chan struct{}, *http.Server, <-chan struct{}, error) { | 2259 | + | // proxyCIDRMiddleware, when non-nil, is also applied to the HTTP redirect listener |
| 2260 | + | // so that proxy_allowed_ips cannot be bypassed via the plain-HTTP port. | |||
| 2261 | + | func startProxyServers(cfg Config, proxyHandler http.Handler, proxyCIDRMiddleware func(http.Handler) http.Handler, errChan chan<- error, log zerowrap.Logger) (*http.Server, <-chan struct{}, *http.Server, <-chan struct{}, error) { | |||
| 2257 | if !cfg.Server.TLSEnabled { | 2262 | if !cfg.Server.TLSEnabled { | ||
| 2258 | var proxyProtos http.Protocols | 2263 | var proxyProtos http.Protocols | ||
| 2259 | proxyProtos.SetHTTP1(true) | 2264 | proxyProtos.SetHTTP1(true) | ||
| @@ -2283,7 +2288,11 @@ func startProxyServers(cfg Config, proxyHandler http.Handler, errChan chan<- err | |||||
| 2283 | http.Redirect(w, r, target, http.StatusPermanentRedirect) | 2288 | http.Redirect(w, r, target, http.StatusPermanentRedirect) | ||
| 2284 | }) | 2289 | }) | ||
| 2285 | 2290 | ||||
| 2286 | - | proxySrv, proxyReady := startServer(fmt.Sprintf(":%d", cfg.Server.Port), redirectHandler, "proxy-redirect", nil, errChan, log) | 2291 | + | var redirectWithMiddleware http.Handler = redirectHandler |
| 2292 | + | if proxyCIDRMiddleware != nil { | |||
| 2293 | + | redirectWithMiddleware = proxyCIDRMiddleware(redirectHandler) | |||
| 2294 | + | } | |||
| 2295 | + | proxySrv, proxyReady := startServer(fmt.Sprintf(":%d", cfg.Server.Port), redirectWithMiddleware, "proxy-redirect", nil, errChan, log) | |||
| 2287 | tlsSrv, tlsReady := startTLSServer( | 2296 | tlsSrv, tlsReady := startTLSServer( | ||
| 2288 | fmt.Sprintf(":%d", cfg.Server.TLSPort), | 2297 | fmt.Sprintf(":%d", cfg.Server.TLSPort), | ||
| 2289 | proxyHandler, | 2298 | proxyHandler, | ||
M internal/app/run_http_handlers_test.go +2 -2
| @@ -35,7 +35,7 @@ func TestCreateHTTPHandlers_LocalMode_DisablesAdminRoutes(t *testing.T) { | |||
| 35 | 35 | cfg.Auth.Enabled = false | |
| 36 | 36 | ||
| 37 | 37 | svc := &services{adminHandler: &adminhttp.Handler{}} | |
| 38 | - | registryHandler, _ := createHTTPHandlers(svc, cfg, zerowrap.Default()) | |
| 38 | + | registryHandler, _, _ := createHTTPHandlers(svc, cfg, zerowrap.Default()) | |
| 39 | 39 | ||
| 40 | 40 | req := httptest.NewRequest(http.MethodGet, "/admin/status", nil) | |
| 41 | 41 | req.RemoteAddr = "192.0.2.10:12345" | |
| @@ -54,7 +54,7 @@ func TestCreateHTTPHandlers_LocalMode_RestrictsRegistryToLoopback(t *testing.T) | |||
| 54 | 54 | cfg := Config{} | |
| 55 | 55 | cfg.Auth.Enabled = false | |
| 56 | 56 | ||
| 57 | - | registryHandler, _ := createHTTPHandlers(&services{}, cfg, zerowrap.Default()) | |
| 57 | + | registryHandler, _, _ := createHTTPHandlers(&services{}, cfg, zerowrap.Default()) | |
| 58 | 58 | ||
| 59 | 59 | req := httptest.NewRequest(http.MethodGet, "/v2/test/manifests/latest", nil) | |
| 60 | 60 | req.RemoteAddr = "192.0.2.20:12345" | |
| @@ -35,7 +35,7 @@ func TestCreateHTTPHandlers_LocalMode_DisablesAdminRoutes(t *testing.T) { | |||||
| 35 | cfg.Auth.Enabled = false | 35 | cfg.Auth.Enabled = false | ||
| 36 | 36 | ||||
| 37 | svc := &services{adminHandler: &adminhttp.Handler{}} | 37 | svc := &services{adminHandler: &adminhttp.Handler{}} | ||
| 38 | - | registryHandler, _ := createHTTPHandlers(svc, cfg, zerowrap.Default()) | 38 | + | registryHandler, _, _ := createHTTPHandlers(svc, cfg, zerowrap.Default()) |
| 39 | 39 | ||||
| 40 | req := httptest.NewRequest(http.MethodGet, "/admin/status", nil) | 40 | req := httptest.NewRequest(http.MethodGet, "/admin/status", nil) | ||
| 41 | req.RemoteAddr = "192.0.2.10:12345" | 41 | req.RemoteAddr = "192.0.2.10:12345" | ||
| @@ -54,7 +54,7 @@ func TestCreateHTTPHandlers_LocalMode_RestrictsRegistryToLoopback(t *testing.T) | |||||
| 54 | cfg := Config{} | 54 | cfg := Config{} | ||
| 55 | cfg.Auth.Enabled = false | 55 | cfg.Auth.Enabled = false | ||
| 56 | 56 | ||||
| 57 | - | registryHandler, _ := createHTTPHandlers(&services{}, cfg, zerowrap.Default()) | 57 | + | registryHandler, _, _ := createHTTPHandlers(&services{}, cfg, zerowrap.Default()) |
| 58 | 58 | ||||
| 59 | req := httptest.NewRequest(http.MethodGet, "/v2/test/manifests/latest", nil) | 59 | req := httptest.NewRequest(http.MethodGet, "/v2/test/manifests/latest", nil) | ||
| 60 | req.RemoteAddr = "192.0.2.20:12345" | 60 | req.RemoteAddr = "192.0.2.20:12345" | ||