refactor(registry): use shared IP extraction from middleware
- commit
- 61fda114514fade5a0ff62e57154ee1f6c5c34e5
- author
- bnema · · 2026-01-18 03:16 UTC
- parent
- 8d262c1
- signature
- unsigned
2 files · +9 -82
M internal/adapters/in/http/registry/ratelimit.go +4 -78
| @@ -3,19 +3,18 @@ package registry | |||
| 3 | 3 | ||
| 4 | 4 | import ( | |
| 5 | 5 | "encoding/json" | |
| 6 | - | "net" | |
| 7 | 6 | "net/http" | |
| 8 | - | "strings" | |
| 9 | 7 | ||
| 10 | 8 | "github.com/bnema/zerowrap" | |
| 11 | 9 | ||
| 12 | 10 | "gordon/internal/adapters/dto" | |
| 11 | + | "gordon/internal/adapters/in/http/middleware" | |
| 13 | 12 | "gordon/internal/boundaries/out" | |
| 14 | 13 | ) | |
| 15 | 14 | ||
| 16 | 15 | // RateLimitMiddleware creates rate limiting middleware for the registry API. | |
| 17 | 16 | // It uses the provided RateLimiter interfaces for global and per-IP limits. | |
| 18 | - | // IP extraction (trusted proxy handling) remains in this HTTP adapter. | |
| 17 | + | // IP extraction uses shared middleware.GetClientIP for trusted proxy handling. | |
| 19 | 18 | func RateLimitMiddleware( | |
| 20 | 19 | globalLimiter out.RateLimiter, | |
| 21 | 20 | ipLimiter out.RateLimiter, | |
| @@ -30,7 +29,7 @@ func RateLimitMiddleware( | |||
| 30 | 29 | } | |
| 31 | 30 | ||
| 32 | 31 | // Parse trusted proxy CIDRs once at middleware creation | |
| 33 | - | trustedNets := parseTrustedProxies(trustedProxies) | |
| 32 | + | trustedNets := middleware.ParseTrustedProxies(trustedProxies) | |
| 34 | 33 | ||
| 35 | 34 | return func(next http.Handler) http.Handler { | |
| 36 | 35 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| @@ -43,7 +42,7 @@ func RateLimitMiddleware( | |||
| 43 | 42 | } | |
| 44 | 43 | ||
| 45 | 44 | // Check per-IP rate limit | |
| 46 | - | ip := getClientIP(r, trustedNets) | |
| 45 | + | ip := middleware.GetClientIP(r, trustedNets) | |
| 47 | 46 | if !ipLimiter.Allow(ctx, "ip:"+ip) { | |
| 48 | 47 | sendRateLimitError(w) | |
| 49 | 48 | return | |
| @@ -54,47 +53,6 @@ func RateLimitMiddleware( | |||
| 54 | 53 | } | |
| 55 | 54 | } | |
| 56 | 55 | ||
| 57 | - | // parseTrustedProxies converts a list of IP addresses and CIDR ranges to net.IPNet. | |
| 58 | - | func parseTrustedProxies(proxies []string) []*net.IPNet { | |
| 59 | - | var nets []*net.IPNet | |
| 60 | - | for _, proxy := range proxies { | |
| 61 | - | // Try parsing as CIDR | |
| 62 | - | _, ipNet, err := net.ParseCIDR(proxy) | |
| 63 | - | if err == nil { | |
| 64 | - | nets = append(nets, ipNet) | |
| 65 | - | continue | |
| 66 | - | } | |
| 67 | - | // Try parsing as single IP | |
| 68 | - | ip := net.ParseIP(proxy) | |
| 69 | - | if ip != nil { | |
| 70 | - | // Convert single IP to /32 or /128 CIDR | |
| 71 | - | bits := 32 | |
| 72 | - | if ip.To4() == nil { | |
| 73 | - | bits = 128 | |
| 74 | - | } | |
| 75 | - | nets = append(nets, &net.IPNet{IP: ip, Mask: net.CIDRMask(bits, bits)}) | |
| 76 | - | } | |
| 77 | - | } | |
| 78 | - | return nets | |
| 79 | - | } | |
| 80 | - | ||
| 81 | - | // isTrustedProxy checks if the given IP is from a trusted proxy. | |
| 82 | - | func isTrustedProxy(ip string, trustedNets []*net.IPNet) bool { | |
| 83 | - | if len(trustedNets) == 0 { | |
| 84 | - | return false | |
| 85 | - | } | |
| 86 | - | parsedIP := net.ParseIP(ip) | |
| 87 | - | if parsedIP == nil { | |
| 88 | - | return false | |
| 89 | - | } | |
| 90 | - | for _, ipNet := range trustedNets { | |
| 91 | - | if ipNet.Contains(parsedIP) { | |
| 92 | - | return true | |
| 93 | - | } | |
| 94 | - | } | |
| 95 | - | return false | |
| 96 | - | } | |
| 97 | - | ||
| 98 | 56 | // sendRateLimitError sends an HTTP 429 response in Docker Registry API format. | |
| 99 | 57 | func sendRateLimitError(w http.ResponseWriter) { | |
| 100 | 58 | w.Header().Set("Content-Type", "application/json") | |
| @@ -108,35 +66,3 @@ func sendRateLimitError(w http.ResponseWriter) { | |||
| 108 | 66 | }}, | |
| 109 | 67 | }) | |
| 110 | 68 | } | |
| 111 | - | ||
| 112 | - | // getClientIP extracts the client IP address from the request. | |
| 113 | - | // It only honors X-Forwarded-For and X-Real-IP headers when the request | |
| 114 | - | // originates from a trusted proxy. This prevents attackers from spoofing | |
| 115 | - | // their IP address when Gordon is exposed directly without a reverse proxy. | |
| 116 | - | func getClientIP(r *http.Request, trustedNets []*net.IPNet) string { | |
| 117 | - | // Extract the direct connection IP from RemoteAddr | |
| 118 | - | remoteIP, _, err := net.SplitHostPort(r.RemoteAddr) | |
| 119 | - | if err != nil { | |
| 120 | - | remoteIP = r.RemoteAddr | |
| 121 | - | } | |
| 122 | - | ||
| 123 | - | // Only honor proxy headers if the request comes from a trusted proxy | |
| 124 | - | if isTrustedProxy(remoteIP, trustedNets) { | |
| 125 | - | // Check X-Forwarded-For for proxied requests | |
| 126 | - | if xff := r.Header.Get("X-Forwarded-For"); xff != "" { | |
| 127 | - | // Take first IP in chain (original client) | |
| 128 | - | if first, _, found := strings.Cut(xff, ","); found { | |
| 129 | - | return strings.TrimSpace(first) | |
| 130 | - | } | |
| 131 | - | return strings.TrimSpace(xff) | |
| 132 | - | } | |
| 133 | - | ||
| 134 | - | // Check X-Real-IP | |
| 135 | - | if xri := r.Header.Get("X-Real-IP"); xri != "" { | |
| 136 | - | return xri | |
| 137 | - | } | |
| 138 | - | } | |
| 139 | - | ||
| 140 | - | // Fall back to RemoteAddr (direct connection IP) | |
| 141 | - | return remoteIP | |
| 142 | - | } | |
| @@ -3,19 +3,18 @@ package registry | |||||
| 3 | 3 | ||||
| 4 | import ( | 4 | import ( | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | - | "net" | |||
| 7 | "net/http" | 6 | "net/http" | ||
| 8 | - | "strings" | |||
| 9 | 7 | ||||
| 10 | "github.com/bnema/zerowrap" | 8 | "github.com/bnema/zerowrap" | ||
| 11 | 9 | ||||
| 12 | "gordon/internal/adapters/dto" | 10 | "gordon/internal/adapters/dto" | ||
| 11 | + | "gordon/internal/adapters/in/http/middleware" | |||
| 13 | "gordon/internal/boundaries/out" | 12 | "gordon/internal/boundaries/out" | ||
| 14 | ) | 13 | ) | ||
| 15 | 14 | ||||
| 16 | // RateLimitMiddleware creates rate limiting middleware for the registry API. | 15 | // RateLimitMiddleware creates rate limiting middleware for the registry API. | ||
| 17 | // It uses the provided RateLimiter interfaces for global and per-IP limits. | 16 | // It uses the provided RateLimiter interfaces for global and per-IP limits. | ||
| 18 | - | // IP extraction (trusted proxy handling) remains in this HTTP adapter. | 17 | + | // IP extraction uses shared middleware.GetClientIP for trusted proxy handling. |
| 19 | func RateLimitMiddleware( | 18 | func RateLimitMiddleware( | ||
| 20 | globalLimiter out.RateLimiter, | 19 | globalLimiter out.RateLimiter, | ||
| 21 | ipLimiter out.RateLimiter, | 20 | ipLimiter out.RateLimiter, | ||
| @@ -30,7 +29,7 @@ func RateLimitMiddleware( | |||||
| 30 | } | 29 | } | ||
| 31 | 30 | ||||
| 32 | // Parse trusted proxy CIDRs once at middleware creation | 31 | // Parse trusted proxy CIDRs once at middleware creation | ||
| 33 | - | trustedNets := parseTrustedProxies(trustedProxies) | 32 | + | trustedNets := middleware.ParseTrustedProxies(trustedProxies) |
| 34 | 33 | ||||
| 35 | return func(next http.Handler) http.Handler { | 34 | return func(next http.Handler) http.Handler { | ||
| 36 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 35 | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| @@ -43,7 +42,7 @@ func RateLimitMiddleware( | |||||
| 43 | } | 42 | } | ||
| 44 | 43 | ||||
| 45 | // Check per-IP rate limit | 44 | // Check per-IP rate limit | ||
| 46 | - | ip := getClientIP(r, trustedNets) | 45 | + | ip := middleware.GetClientIP(r, trustedNets) |
| 47 | if !ipLimiter.Allow(ctx, "ip:"+ip) { | 46 | if !ipLimiter.Allow(ctx, "ip:"+ip) { | ||
| 48 | sendRateLimitError(w) | 47 | sendRateLimitError(w) | ||
| 49 | return | 48 | return | ||
| @@ -54,47 +53,6 @@ func RateLimitMiddleware( | |||||
| 54 | } | 53 | } | ||
| 55 | } | 54 | } | ||
| 56 | 55 | ||||
| 57 | - | // parseTrustedProxies converts a list of IP addresses and CIDR ranges to net.IPNet. | |||
| 58 | - | func parseTrustedProxies(proxies []string) []*net.IPNet { | |||
| 59 | - | var nets []*net.IPNet | |||
| 60 | - | for _, proxy := range proxies { | |||
| 61 | - | // Try parsing as CIDR | |||
| 62 | - | _, ipNet, err := net.ParseCIDR(proxy) | |||
| 63 | - | if err == nil { | |||
| 64 | - | nets = append(nets, ipNet) | |||
| 65 | - | continue | |||
| 66 | - | } | |||
| 67 | - | // Try parsing as single IP | |||
| 68 | - | ip := net.ParseIP(proxy) | |||
| 69 | - | if ip != nil { | |||
| 70 | - | // Convert single IP to /32 or /128 CIDR | |||
| 71 | - | bits := 32 | |||
| 72 | - | if ip.To4() == nil { | |||
| 73 | - | bits = 128 | |||
| 74 | - | } | |||
| 75 | - | nets = append(nets, &net.IPNet{IP: ip, Mask: net.CIDRMask(bits, bits)}) | |||
| 76 | - | } | |||
| 77 | - | } | |||
| 78 | - | return nets | |||
| 79 | - | } | |||
| 80 | - | ||||
| 81 | - | // isTrustedProxy checks if the given IP is from a trusted proxy. | |||
| 82 | - | func isTrustedProxy(ip string, trustedNets []*net.IPNet) bool { | |||
| 83 | - | if len(trustedNets) == 0 { | |||
| 84 | - | return false | |||
| 85 | - | } | |||
| 86 | - | parsedIP := net.ParseIP(ip) | |||
| 87 | - | if parsedIP == nil { | |||
| 88 | - | return false | |||
| 89 | - | } | |||
| 90 | - | for _, ipNet := range trustedNets { | |||
| 91 | - | if ipNet.Contains(parsedIP) { | |||
| 92 | - | return true | |||
| 93 | - | } | |||
| 94 | - | } | |||
| 95 | - | return false | |||
| 96 | - | } | |||
| 97 | - | ||||
| 98 | // sendRateLimitError sends an HTTP 429 response in Docker Registry API format. | 56 | // sendRateLimitError sends an HTTP 429 response in Docker Registry API format. | ||
| 99 | func sendRateLimitError(w http.ResponseWriter) { | 57 | func sendRateLimitError(w http.ResponseWriter) { | ||
| 100 | w.Header().Set("Content-Type", "application/json") | 58 | w.Header().Set("Content-Type", "application/json") | ||
| @@ -108,35 +66,3 @@ func sendRateLimitError(w http.ResponseWriter) { | |||||
| 108 | }}, | 66 | }}, | ||
| 109 | }) | 67 | }) | ||
| 110 | } | 68 | } | ||
| 111 | - | ||||
| 112 | - | // getClientIP extracts the client IP address from the request. | |||
| 113 | - | // It only honors X-Forwarded-For and X-Real-IP headers when the request | |||
| 114 | - | // originates from a trusted proxy. This prevents attackers from spoofing | |||
| 115 | - | // their IP address when Gordon is exposed directly without a reverse proxy. | |||
| 116 | - | func getClientIP(r *http.Request, trustedNets []*net.IPNet) string { | |||
| 117 | - | // Extract the direct connection IP from RemoteAddr | |||
| 118 | - | remoteIP, _, err := net.SplitHostPort(r.RemoteAddr) | |||
| 119 | - | if err != nil { | |||
| 120 | - | remoteIP = r.RemoteAddr | |||
| 121 | - | } | |||
| 122 | - | ||||
| 123 | - | // Only honor proxy headers if the request comes from a trusted proxy | |||
| 124 | - | if isTrustedProxy(remoteIP, trustedNets) { | |||
| 125 | - | // Check X-Forwarded-For for proxied requests | |||
| 126 | - | if xff := r.Header.Get("X-Forwarded-For"); xff != "" { | |||
| 127 | - | // Take first IP in chain (original client) | |||
| 128 | - | if first, _, found := strings.Cut(xff, ","); found { | |||
| 129 | - | return strings.TrimSpace(first) | |||
| 130 | - | } | |||
| 131 | - | return strings.TrimSpace(xff) | |||
| 132 | - | } | |||
| 133 | - | ||||
| 134 | - | // Check X-Real-IP | |||
| 135 | - | if xri := r.Header.Get("X-Real-IP"); xri != "" { | |||
| 136 | - | return xri | |||
| 137 | - | } | |||
| 138 | - | } | |||
| 139 | - | ||||
| 140 | - | // Fall back to RemoteAddr (direct connection IP) | |||
| 141 | - | return remoteIP | |||
| 142 | - | } | |||
M internal/adapters/in/http/registry/ratelimit_test.go +5 -4
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | ||
| 10 | 10 | "github.com/stretchr/testify/assert" | |
| 11 | 11 | ||
| 12 | + | "gordon/internal/adapters/in/http/middleware" | |
| 12 | 13 | "gordon/internal/adapters/out/ratelimit" | |
| 13 | 14 | outmocks "gordon/internal/boundaries/out/mocks" | |
| 14 | 15 | ) | |
| @@ -220,7 +221,7 @@ func TestRateLimitMiddleware_Integration(t *testing.T) { | |||
| 220 | 221 | ||
| 221 | 222 | func TestGetClientIP(t *testing.T) { | |
| 222 | 223 | // Parse trusted proxies for tests | |
| 223 | - | trustedNets := parseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"}) | |
| 224 | + | trustedNets := middleware.ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"}) | |
| 224 | 225 | noTrustedNets := []*net.IPNet{} | |
| 225 | 226 | ||
| 226 | 227 | tests := []struct { | |
| @@ -313,7 +314,7 @@ func TestGetClientIP(t *testing.T) { | |||
| 313 | 314 | req.Header.Set("X-Real-IP", tt.xRealIP) | |
| 314 | 315 | } | |
| 315 | 316 | ||
| 316 | - | ip := getClientIP(req, tt.trustedNets) | |
| 317 | + | ip := middleware.GetClientIP(req, tt.trustedNets) | |
| 317 | 318 | assert.Equal(t, tt.wantIP, ip) | |
| 318 | 319 | }) | |
| 319 | 320 | } | |
| @@ -372,8 +373,8 @@ func TestParseTrustedProxies(t *testing.T) { | |||
| 372 | 373 | ||
| 373 | 374 | for _, tt := range tests { | |
| 374 | 375 | t.Run(tt.name, func(t *testing.T) { | |
| 375 | - | nets := parseTrustedProxies(tt.proxies) | |
| 376 | - | got := isTrustedProxy(tt.testIP, nets) | |
| 376 | + | nets := middleware.ParseTrustedProxies(tt.proxies) | |
| 377 | + | got := middleware.IsTrustedProxy(tt.testIP, nets) | |
| 377 | 378 | assert.Equal(t, tt.want, got) | |
| 378 | 379 | }) | |
| 379 | 380 | } | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | 9 | ||||
| 10 | "github.com/stretchr/testify/assert" | 10 | "github.com/stretchr/testify/assert" | ||
| 11 | 11 | ||||
| 12 | + | "gordon/internal/adapters/in/http/middleware" | |||
| 12 | "gordon/internal/adapters/out/ratelimit" | 13 | "gordon/internal/adapters/out/ratelimit" | ||
| 13 | outmocks "gordon/internal/boundaries/out/mocks" | 14 | outmocks "gordon/internal/boundaries/out/mocks" | ||
| 14 | ) | 15 | ) | ||
| @@ -220,7 +221,7 @@ func TestRateLimitMiddleware_Integration(t *testing.T) { | |||||
| 220 | 221 | ||||
| 221 | func TestGetClientIP(t *testing.T) { | 222 | func TestGetClientIP(t *testing.T) { | ||
| 222 | // Parse trusted proxies for tests | 223 | // Parse trusted proxies for tests | ||
| 223 | - | trustedNets := parseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"}) | 224 | + | trustedNets := middleware.ParseTrustedProxies([]string{"127.0.0.1", "10.0.0.0/8"}) |
| 224 | noTrustedNets := []*net.IPNet{} | 225 | noTrustedNets := []*net.IPNet{} | ||
| 225 | 226 | ||||
| 226 | tests := []struct { | 227 | tests := []struct { | ||
| @@ -313,7 +314,7 @@ func TestGetClientIP(t *testing.T) { | |||||
| 313 | req.Header.Set("X-Real-IP", tt.xRealIP) | 314 | req.Header.Set("X-Real-IP", tt.xRealIP) | ||
| 314 | } | 315 | } | ||
| 315 | 316 | ||||
| 316 | - | ip := getClientIP(req, tt.trustedNets) | 317 | + | ip := middleware.GetClientIP(req, tt.trustedNets) |
| 317 | assert.Equal(t, tt.wantIP, ip) | 318 | assert.Equal(t, tt.wantIP, ip) | ||
| 318 | }) | 319 | }) | ||
| 319 | } | 320 | } | ||
| @@ -372,8 +373,8 @@ func TestParseTrustedProxies(t *testing.T) { | |||||
| 372 | 373 | ||||
| 373 | for _, tt := range tests { | 374 | for _, tt := range tests { | ||
| 374 | t.Run(tt.name, func(t *testing.T) { | 375 | t.Run(tt.name, func(t *testing.T) { | ||
| 375 | - | nets := parseTrustedProxies(tt.proxies) | 376 | + | nets := middleware.ParseTrustedProxies(tt.proxies) |
| 376 | - | got := isTrustedProxy(tt.testIP, nets) | 377 | + | got := middleware.IsTrustedProxy(tt.testIP, nets) |
| 377 | assert.Equal(t, tt.want, got) | 378 | assert.Equal(t, tt.want, got) | ||
| 378 | }) | 379 | }) | ||
| 379 | } | 380 | } | ||