From 7d26c8838f4e9c3abdfdbff57ee225378487e515 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 28 Oct 2025 14:16:27 -0700 Subject: [PATCH] S3: auth supports X-Forwarded-Host and X-Forwarded-Port (#7398) * add fix and tests * address comments * idiomatic * ipv6 --- weed/s3api/auth_signature_v4.go | 16 ++- weed/s3api/auth_signature_v4_test.go | 172 +++++++++++++++++++++++++++ weed/s3api/auto_signature_v4_test.go | 49 ++++++++ 3 files changed, 236 insertions(+), 1 deletion(-) diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index 3a8e59392..d0297d623 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -24,6 +24,7 @@ import ( "crypto/subtle" "encoding/hex" "io" + "net" "net/http" "regexp" "sort" @@ -592,7 +593,20 @@ func extractSignedHeaders(signedHeaders []string, r *http.Request) (http.Header, func extractHostHeader(r *http.Request) string { // Check for X-Forwarded-Host header first, which is set by reverse proxies if forwardedHost := r.Header.Get("X-Forwarded-Host"); forwardedHost != "" { - // Check if reverse proxy also forwarded the port + // Check if X-Forwarded-Host already contains a port + // This handles proxies (like Traefik, HAProxy) that include port in X-Forwarded-Host + if _, _, err := net.SplitHostPort(forwardedHost); err == nil { + // X-Forwarded-Host already contains a port (e.g., "example.com:8443" or "[::1]:8080") + // Use it as-is + return forwardedHost + } + + // An IPv6 address literal must be enclosed in square brackets. + if ip := net.ParseIP(forwardedHost); ip != nil && strings.Contains(forwardedHost, ":") { + forwardedHost = "[" + forwardedHost + "]" + } + + // X-Forwarded-Host doesn't contain a port, check if X-Forwarded-Port is provided if forwardedPort := r.Header.Get("X-Forwarded-Port"); forwardedPort != "" { // Determine the protocol to check for standard ports proto := strings.ToLower(r.Header.Get("X-Forwarded-Proto")) diff --git a/weed/s3api/auth_signature_v4_test.go b/weed/s3api/auth_signature_v4_test.go index 312e88767..16f3840c0 100644 --- a/weed/s3api/auth_signature_v4_test.go +++ b/weed/s3api/auth_signature_v4_test.go @@ -1,6 +1,7 @@ package s3api import ( + "net/http" "testing" ) @@ -89,3 +90,174 @@ func TestBuildPathWithForwardedPrefix(t *testing.T) { }) } } + +// TestExtractHostHeader tests the extractHostHeader function with various scenarios +func TestExtractHostHeader(t *testing.T) { + tests := []struct { + name string + hostHeader string + forwardedHost string + forwardedPort string + forwardedProto string + expected string + }{ + { + name: "basic host without forwarding", + hostHeader: "example.com", + forwardedHost: "", + forwardedPort: "", + forwardedProto: "", + expected: "example.com", + }, + { + name: "host with port without forwarding", + hostHeader: "example.com:8080", + forwardedHost: "", + forwardedPort: "", + forwardedProto: "", + expected: "example.com:8080", + }, + { + name: "X-Forwarded-Host without port", + hostHeader: "backend:8333", + forwardedHost: "example.com", + forwardedPort: "", + forwardedProto: "", + expected: "example.com", + }, + { + name: "X-Forwarded-Host with X-Forwarded-Port (HTTP non-standard)", + hostHeader: "backend:8333", + forwardedHost: "example.com", + forwardedPort: "8080", + forwardedProto: "http", + expected: "example.com:8080", + }, + { + name: "X-Forwarded-Host with X-Forwarded-Port (HTTPS non-standard)", + hostHeader: "backend:8333", + forwardedHost: "example.com", + forwardedPort: "8443", + forwardedProto: "https", + expected: "example.com:8443", + }, + { + name: "X-Forwarded-Host with X-Forwarded-Port (HTTP standard port 80)", + hostHeader: "backend:8333", + forwardedHost: "example.com", + forwardedPort: "80", + forwardedProto: "http", + expected: "example.com", + }, + { + name: "X-Forwarded-Host with X-Forwarded-Port (HTTPS standard port 443)", + hostHeader: "backend:8333", + forwardedHost: "example.com", + forwardedPort: "443", + forwardedProto: "https", + expected: "example.com", + }, + // Issue #6649: X-Forwarded-Host already contains port (Traefik/HAProxy style) + { + name: "X-Forwarded-Host with port already included (should not add port again)", + hostHeader: "backend:8333", + forwardedHost: "127.0.0.1:8433", + forwardedPort: "8433", + forwardedProto: "https", + expected: "127.0.0.1:8433", + }, + { + name: "X-Forwarded-Host with port, no X-Forwarded-Port header", + hostHeader: "backend:8333", + forwardedHost: "example.com:9000", + forwardedPort: "", + forwardedProto: "http", + expected: "example.com:9000", + }, + // IPv6 test cases + { + name: "IPv6 address with brackets and port in X-Forwarded-Host", + hostHeader: "backend:8333", + forwardedHost: "[::1]:8080", + forwardedPort: "8080", + forwardedProto: "http", + expected: "[::1]:8080", + }, + { + name: "IPv6 address without brackets, should add brackets with port", + hostHeader: "backend:8333", + forwardedHost: "::1", + forwardedPort: "8080", + forwardedProto: "http", + expected: "[::1]:8080", + }, + { + name: "IPv6 address without brackets and standard port, should return bracketed IPv6", + hostHeader: "backend:8333", + forwardedHost: "::1", + forwardedPort: "80", + forwardedProto: "http", + expected: "[::1]", + }, + { + name: "IPv6 address without brackets and standard HTTPS port, should return bracketed IPv6", + hostHeader: "backend:8333", + forwardedHost: "2001:db8::1", + forwardedPort: "443", + forwardedProto: "https", + expected: "[2001:db8::1]", + }, + { + name: "IPv6 address with brackets but no port, should add port", + hostHeader: "backend:8333", + forwardedHost: "[2001:db8::1]", + forwardedPort: "8080", + forwardedProto: "http", + expected: "[2001:db8::1]:8080", + }, + { + name: "IPv6 full address with brackets and port", + hostHeader: "backend:8333", + forwardedHost: "[2001:db8:85a3::8a2e:370:7334]:443", + forwardedPort: "443", + forwardedProto: "https", + expected: "[2001:db8:85a3::8a2e:370:7334]:443", + }, + { + name: "IPv4-mapped IPv6 address without brackets, should add brackets with port", + hostHeader: "backend:8333", + forwardedHost: "::ffff:127.0.0.1", + forwardedPort: "8080", + forwardedProto: "http", + expected: "[::ffff:127.0.0.1]:8080", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock request + req, err := http.NewRequest("GET", "http://"+tt.hostHeader+"/bucket/object", nil) + if err != nil { + t.Fatalf("Failed to create request: %v", err) + } + + // Set headers + req.Host = tt.hostHeader + if tt.forwardedHost != "" { + req.Header.Set("X-Forwarded-Host", tt.forwardedHost) + } + if tt.forwardedPort != "" { + req.Header.Set("X-Forwarded-Port", tt.forwardedPort) + } + if tt.forwardedProto != "" { + req.Header.Set("X-Forwarded-Proto", tt.forwardedProto) + } + + // Test the function + result := extractHostHeader(req) + if result != tt.expected { + t.Errorf("extractHostHeader() = %q, want %q", result, tt.expected) + } + }) + } +} diff --git a/weed/s3api/auto_signature_v4_test.go b/weed/s3api/auto_signature_v4_test.go index 71cae3546..d31294c99 100644 --- a/weed/s3api/auto_signature_v4_test.go +++ b/weed/s3api/auto_signature_v4_test.go @@ -450,6 +450,55 @@ func TestSignatureV4WithForwardedPort(t *testing.T) { forwardedProto: "", expectedHost: "example.com", }, + // Test cases for issue #6649: X-Forwarded-Host already contains port + { + name: "X-Forwarded-Host with port already included (Traefik/HAProxy style)", + host: "backend:8333", + forwardedHost: "127.0.0.1:8433", + forwardedPort: "8433", + forwardedProto: "https", + expectedHost: "127.0.0.1:8433", + }, + { + name: "X-Forwarded-Host with port, no X-Forwarded-Port header", + host: "backend:8333", + forwardedHost: "example.com:9000", + forwardedPort: "", + forwardedProto: "http", + expectedHost: "example.com:9000", + }, + { + name: "IPv6 with port in brackets", + host: "backend:8333", + forwardedHost: "[::1]:8080", + forwardedPort: "8080", + forwardedProto: "http", + expectedHost: "[::1]:8080", + }, + { + name: "IPv6 without port - should add port with brackets", + host: "backend:8333", + forwardedHost: "::1", + forwardedPort: "8080", + forwardedProto: "http", + expectedHost: "[::1]:8080", + }, + { + name: "IPv6 in brackets without port - should add port", + host: "backend:8333", + forwardedHost: "[2001:db8::1]", + forwardedPort: "8080", + forwardedProto: "http", + expectedHost: "[2001:db8::1]:8080", + }, + { + name: "IPv4-mapped IPv6 without port - should add port with brackets", + host: "backend:8333", + forwardedHost: "::ffff:127.0.0.1", + forwardedPort: "8080", + forwardedProto: "http", + expectedHost: "[::ffff:127.0.0.1]:8080", + }, } for _, tt := range tests {