From 7e624d53550d3f6a87c4c2ac6f887e89c40e0465 Mon Sep 17 00:00:00 2001 From: zuzuviewer <750938164@qq.com> Date: Thu, 30 Oct 2025 09:01:18 +0800 Subject: [PATCH] * Fix s3 auth with proxy request (#7403) * * Fix s3 auth with proxy request * * 6649 Add unit test for signature v4 * address comments * fix for tests * ipv6 * address comments * setting scheme Works for both cases (direct HTTPS and behind proxy) * trim for ipv6 * Corrected Scheme Precedence Order * trim * accurate --------- Co-authored-by: chrislu Co-authored-by: Chris Lu --- weed/s3api/auth_signature_v4.go | 101 ++++++++++++------ weed/s3api/auth_signature_v4_test.go | 4 +- weed/s3api/auto_signature_v4_test.go | 151 +++++++++++++++++++++++++++ 3 files changed, 223 insertions(+), 33 deletions(-) diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index d0297d623..4d273de6a 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -591,44 +591,83 @@ func extractSignedHeaders(signedHeaders []string, r *http.Request) (http.Header, // extractHostHeader returns the value of host header if available. 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 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 + "]" + forwardedHost := r.Header.Get("X-Forwarded-Host") + forwardedPort := r.Header.Get("X-Forwarded-Port") + forwardedProto := r.Header.Get("X-Forwarded-Proto") + + // Determine the effective scheme with correct order of precedence: + // 1. X-Forwarded-Proto (most authoritative, reflects client's original protocol) + // 2. r.TLS (authoritative for direct connection to server) + // 3. r.URL.Scheme (fallback, may not always be set correctly) + // 4. Default to "http" + scheme := "http" + if r.URL.Scheme != "" { + scheme = r.URL.Scheme + } + if r.TLS != nil { + scheme = "https" + } + if forwardedProto != "" { + scheme = forwardedProto + } + + var host, port string + if forwardedHost != "" { + // X-Forwarded-Host can be a comma-separated list of hosts when there are multiple proxies. + // Use only the first host in the list and trim spaces for robustness. + if comma := strings.Index(forwardedHost, ","); comma != -1 { + host = strings.TrimSpace(forwardedHost[:comma]) + } else { + host = strings.TrimSpace(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")) - // Only add port if it's not the standard port for the protocol - if (proto == "https" && forwardedPort != "443") || (proto != "https" && forwardedPort != "80") { - return forwardedHost + ":" + forwardedPort + port = forwardedPort + if h, p, err := net.SplitHostPort(host); err == nil { + host = h + if port == "" { + port = p } } - // Using reverse proxy with X-Forwarded-Host (standard port or no port forwarded). - return forwardedHost + } else { + host = r.Host + if host == "" { + host = r.URL.Host + } + if h, p, err := net.SplitHostPort(host); err == nil { + host = h + port = p + } } - hostHeaderValue := r.Host - // For standard requests, this should be fine. - if r.Host != "" { - return hostHeaderValue + // If we have a non-default port, join it with the host. + // net.JoinHostPort will handle bracketing for IPv6. + if port != "" && !isDefaultPort(scheme, port) { + // Strip existing brackets before calling JoinHostPort, which automatically adds + // brackets for IPv6 addresses. This prevents double-bracketing like [[::1]]:8080. + // Using Trim handles both well-formed and malformed bracketed hosts. + host = strings.Trim(host, "[]") + return net.JoinHostPort(host, port) } - // If no host header is found, then check for host URL value. - if r.URL.Host != "" { - hostHeaderValue = r.URL.Host + + // No port or default port, just ensure host is correctly formatted (IPv6 brackets). + if strings.Contains(host, ":") && !strings.HasPrefix(host, "[") { + return "[" + host + "]" + } + return host +} + +func isDefaultPort(scheme, port string) bool { + if port == "" { + return true + } + + switch port { + case "80": + return strings.EqualFold(scheme, "http") + case "443": + return strings.EqualFold(scheme, "https") + default: + return false } - return hostHeaderValue } // getScope generate a string of a specific date, an AWS region, and a service. diff --git a/weed/s3api/auth_signature_v4_test.go b/weed/s3api/auth_signature_v4_test.go index 16f3840c0..6850e9d2b 100644 --- a/weed/s3api/auth_signature_v4_test.go +++ b/weed/s3api/auth_signature_v4_test.go @@ -216,12 +216,12 @@ func TestExtractHostHeader(t *testing.T) { expected: "[2001:db8::1]:8080", }, { - name: "IPv6 full address with brackets and port", + name: "IPv6 full address with brackets and default port (should strip port)", hostHeader: "backend:8333", forwardedHost: "[2001:db8:85a3::8a2e:370:7334]:443", forwardedPort: "443", forwardedProto: "https", - expected: "[2001:db8:85a3::8a2e:370:7334]:443", + expected: "[2001:db8:85a3::8a2e:370:7334]", }, { name: "IPv4-mapped IPv6 address without brackets, should add brackets with port", diff --git a/weed/s3api/auto_signature_v4_test.go b/weed/s3api/auto_signature_v4_test.go index d31294c99..6039081a1 100644 --- a/weed/s3api/auto_signature_v4_test.go +++ b/weed/s3api/auto_signature_v4_test.go @@ -392,6 +392,125 @@ func TestSignatureV4WithForwardedPrefixTrailingSlash(t *testing.T) { } } +func TestSignatureV4WithoutProxy(t *testing.T) { + tests := []struct { + name string + host string + proto string + expectedHost string + }{ + { + name: "HTTP with non-standard port", + host: "backend:8333", + proto: "http", + expectedHost: "backend:8333", + }, + { + name: "HTTPS with non-standard port", + host: "backend:8333", + proto: "https", + expectedHost: "backend:8333", + }, + { + name: "HTTP with standard port", + host: "backend:80", + proto: "http", + expectedHost: "backend", + }, + { + name: "HTTPS with standard port", + host: "backend:443", + proto: "https", + expectedHost: "backend", + }, + { + name: "HTTP without port", + host: "backend", + proto: "http", + expectedHost: "backend", + }, + { + name: "HTTPS without port", + host: "backend", + proto: "https", + expectedHost: "backend", + }, + { + name: "IPv6 HTTP with non-standard port", + host: "[::1]:8333", + proto: "http", + expectedHost: "[::1]:8333", + }, + { + name: "IPv6 HTTPS with non-standard port", + host: "[::1]:8333", + proto: "https", + expectedHost: "[::1]:8333", + }, + { + name: "IPv6 HTTP with standard port", + host: "[::1]:80", + proto: "http", + expectedHost: "[::1]", + }, + { + name: "IPv6 HTTPS with standard port", + host: "[::1]:443", + proto: "https", + expectedHost: "[::1]", + }, + { + name: "IPv6 HTTP without port", + host: "::1", + proto: "http", + expectedHost: "[::1]", + }, + { + name: "IPv6 HTTPS without port", + host: "::1", + proto: "https", + expectedHost: "[::1]", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + iam := newTestIAM() + + // Create a request + r, err := newTestRequest("GET", tt.proto+"://"+tt.host+"/test-bucket/test-object", 0, nil) + if err != nil { + t.Fatalf("Failed to create test request: %v", err) + } + + // Set the mux variables manually since we're not going through the actual router + r = mux.SetURLVars(r, map[string]string{ + "bucket": "test-bucket", + "object": "test-object", + }) + + // Set forwarded headers + r.Header.Set("Host", tt.host) + + // First, verify that extractHostHeader returns the expected value + extractedHost := extractHostHeader(r) + if extractedHost != tt.expectedHost { + t.Errorf("extractHostHeader() = %q, want %q", extractedHost, tt.expectedHost) + } + + // Sign the request with the expected host header + // We need to temporarily modify the Host header for signing + signV4WithPath(r, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", r.URL.Path) + + // Test signature verification + _, _, errCode := iam.doesSignatureMatch(r) + if errCode != s3err.ErrNone { + t.Errorf("Expected successful signature validation, got error: %v (code: %d)", errCode, int(errCode)) + } + }) + } +} + // Test X-Forwarded-Port support for reverse proxy scenarios func TestSignatureV4WithForwardedPort(t *testing.T) { tests := []struct { @@ -467,6 +586,38 @@ func TestSignatureV4WithForwardedPort(t *testing.T) { forwardedProto: "http", expectedHost: "example.com:9000", }, + { + name: "X-Forwarded-Host with standard https port already included (Traefik/HAProxy style)", + host: "backend:443", + forwardedHost: "127.0.0.1:443", + forwardedPort: "443", + forwardedProto: "https", + expectedHost: "127.0.0.1", + }, + { + name: "X-Forwarded-Host with standard http port already included (Traefik/HAProxy style)", + host: "backend:80", + forwardedHost: "127.0.0.1:80", + forwardedPort: "80", + forwardedProto: "http", + expectedHost: "127.0.0.1", + }, + { + name: "IPv6 X-Forwarded-Host with standard https port already included (Traefik/HAProxy style)", + host: "backend:443", + forwardedHost: "[::1]:443", + forwardedPort: "443", + forwardedProto: "https", + expectedHost: "[::1]", + }, + { + name: "IPv6 X-Forwarded-Host with standard http port already included (Traefik/HAProxy style)", + host: "backend:80", + forwardedHost: "[::1]:80", + forwardedPort: "80", + forwardedProto: "http", + expectedHost: "[::1]", + }, { name: "IPv6 with port in brackets", host: "backend:8333",