diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index a0417a922..a37274326 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -216,7 +216,14 @@ func (iam *IdentityAccessManagement) doesSignatureMatch(hashedPayload string, r if forwardedPrefix := r.Header.Get("X-Forwarded-Prefix"); forwardedPrefix != "" { // Try signature verification with the forwarded prefix first. // This handles cases where reverse proxies strip URL prefixes and add the X-Forwarded-Prefix header. - errCode = iam.verifySignatureWithPath(extractedSignedHeaders, hashedPayload, queryStr, path.Clean(forwardedPrefix+req.URL.Path), req.Method, foundCred.SecretKey, t, signV4Values) + // Preserve trailing slash if present in the original URL path to match S3 SDK signature + fullPath := forwardedPrefix + req.URL.Path + hasTrailingSlash := strings.HasSuffix(req.URL.Path, "/") && req.URL.Path != "/" + cleanedPath := path.Clean(fullPath) + if hasTrailingSlash && !strings.HasSuffix(cleanedPath, "/") { + cleanedPath += "/" + } + errCode = iam.verifySignatureWithPath(extractedSignedHeaders, hashedPayload, queryStr, cleanedPath, req.Method, foundCred.SecretKey, t, signV4Values) if errCode == s3err.ErrNone { return identity, errCode } @@ -369,7 +376,14 @@ func (iam *IdentityAccessManagement) doesPresignedSignatureMatch(hashedPayload s if forwardedPrefix := r.Header.Get("X-Forwarded-Prefix"); forwardedPrefix != "" { // Try signature verification with the forwarded prefix first. // This handles cases where reverse proxies strip URL prefixes and add the X-Forwarded-Prefix header. - errCode = iam.verifyPresignedSignatureWithPath(extractedSignedHeaders, hashedPayload, queryStr, path.Clean(forwardedPrefix+r.URL.Path), r.Method, foundCred.SecretKey, t, credHeader, signature) + // Preserve trailing slash if present in the original URL path to match S3 SDK signature + fullPath := forwardedPrefix + r.URL.Path + hasTrailingSlash := strings.HasSuffix(r.URL.Path, "/") && r.URL.Path != "/" + cleanedPath := path.Clean(fullPath) + if hasTrailingSlash && !strings.HasSuffix(cleanedPath, "/") { + cleanedPath += "/" + } + errCode = iam.verifyPresignedSignatureWithPath(extractedSignedHeaders, hashedPayload, queryStr, cleanedPath, r.Method, foundCred.SecretKey, t, credHeader, signature) if errCode == s3err.ErrNone { return identity, errCode } diff --git a/weed/s3api/auto_signature_v4_test.go b/weed/s3api/auto_signature_v4_test.go index 7a9599583..bf11a0906 100644 --- a/weed/s3api/auto_signature_v4_test.go +++ b/weed/s3api/auto_signature_v4_test.go @@ -322,6 +322,72 @@ func TestSignatureV4WithForwardedPrefix(t *testing.T) { } } +// Test X-Forwarded-Prefix with trailing slash preservation (GitHub issue #7223) +// This tests the specific bug where S3 SDK signs paths with trailing slashes +// but path.Clean() would remove them, causing signature verification to fail +func TestSignatureV4WithForwardedPrefixTrailingSlash(t *testing.T) { + tests := []struct { + name string + forwardedPrefix string + urlPath string + expectedPath string + }{ + { + name: "bucket listObjects with trailing slash", + forwardedPrefix: "/oss-sf-nnct", + urlPath: "/s3user-bucket1/", + expectedPath: "/oss-sf-nnct/s3user-bucket1/", + }, + { + name: "prefix path with trailing slash", + forwardedPrefix: "/s3", + urlPath: "/my-bucket/folder/", + expectedPath: "/s3/my-bucket/folder/", + }, + { + name: "root bucket with trailing slash", + forwardedPrefix: "/api/s3", + urlPath: "/test-bucket/", + expectedPath: "/api/s3/test-bucket/", + }, + { + name: "nested folder with trailing slash", + forwardedPrefix: "/storage", + urlPath: "/bucket/path/to/folder/", + expectedPath: "/storage/bucket/path/to/folder/", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + iam := newTestIAM() + + // Create a request with the URL path that has a trailing slash + r, err := newTestRequest("GET", "https://example.com"+tt.urlPath, 0, nil) + if err != nil { + t.Fatalf("Failed to create test request: %v", err) + } + + // Manually set the URL path with trailing slash to ensure it's preserved + r.URL.Path = tt.urlPath + + r.Header.Set("X-Forwarded-Prefix", tt.forwardedPrefix) + r.Header.Set("Host", "example.com") + r.Header.Set("X-Forwarded-Host", "example.com") + + // Sign the request with the full path including the trailing slash + // This simulates what S3 SDK does for listObjects operations + signV4WithPath(r, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", tt.expectedPath) + + // Test signature verification - this should succeed even with trailing slashes + _, errCode := iam.doesSignatureMatch(getContentSha256Cksum(r), r) + if errCode != s3err.ErrNone { + t.Errorf("Expected successful signature validation with trailing slash in path %q, got error: %v (code: %d)", tt.urlPath, errCode, int(errCode)) + } + }) + } +} + // Test X-Forwarded-Port support for reverse proxy scenarios func TestSignatureV4WithForwardedPort(t *testing.T) { tests := []struct { @@ -515,6 +581,73 @@ func TestPresignedSignatureV4WithForwardedPrefix(t *testing.T) { } } +// Test X-Forwarded-Prefix with trailing slash preservation for presigned URLs (GitHub issue #7223) +func TestPresignedSignatureV4WithForwardedPrefixTrailingSlash(t *testing.T) { + tests := []struct { + name string + forwardedPrefix string + originalPath string + strippedPath string + }{ + { + name: "bucket listObjects with trailing slash", + forwardedPrefix: "/oss-sf-nnct", + originalPath: "/oss-sf-nnct/s3user-bucket1/", + strippedPath: "/s3user-bucket1/", + }, + { + name: "prefix path with trailing slash", + forwardedPrefix: "/s3", + originalPath: "/s3/my-bucket/folder/", + strippedPath: "/my-bucket/folder/", + }, + { + name: "api path with trailing slash", + forwardedPrefix: "/api/s3", + originalPath: "/api/s3/test-bucket/", + strippedPath: "/test-bucket/", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + iam := newTestIAM() + + // Create a presigned request that simulates reverse proxy scenario with trailing slashes: + // 1. Client generates presigned URL with prefixed path including trailing slash + // 2. Proxy strips prefix and forwards to SeaweedFS with X-Forwarded-Prefix header + + // Start with the original request URL (what client sees) with trailing slash + r, err := newTestRequest("GET", "https://example.com"+tt.originalPath, 0, nil) + if err != nil { + t.Fatalf("Failed to create test request: %v", err) + } + + // Generate presigned URL with the original prefixed path including trailing slash + err = preSignV4WithPath(iam, r, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", 3600, tt.originalPath) + if err != nil { + t.Errorf("Failed to presign request: %v", err) + return + } + + // Now simulate what the reverse proxy does: + // 1. Strip the prefix from the URL path but preserve the trailing slash + r.URL.Path = tt.strippedPath + + // 2. Add the forwarded headers + r.Header.Set("X-Forwarded-Prefix", tt.forwardedPrefix) + r.Header.Set("Host", "example.com") + r.Header.Set("X-Forwarded-Host", "example.com") + + // Test presigned signature verification - this should succeed with trailing slashes + _, errCode := iam.doesPresignedSignatureMatch(getContentSha256Cksum(r), r) + if errCode != s3err.ErrNone { + t.Errorf("Expected successful presigned signature validation with trailing slash in path %q, got error: %v (code: %d)", tt.strippedPath, errCode, int(errCode)) + } + }) + } +} + // preSignV4WithPath adds presigned URL parameters to the request with a custom path func preSignV4WithPath(iam *IdentityAccessManagement, req *http.Request, accessKey, secretKey string, expires int64, urlPath string) error { // Create credential scope diff --git a/weed/server/master_server.go b/weed/server/master_server.go index bd83d5a96..10b54d58f 100644 --- a/weed/server/master_server.go +++ b/weed/server/master_server.go @@ -57,7 +57,7 @@ type MasterOption struct { IsFollower bool TelemetryUrl string TelemetryEnabled bool - VolumeGrowthDisabled bool + VolumeGrowthDisabled bool } type MasterServer struct { @@ -251,15 +251,18 @@ func (ms *MasterServer) proxyToLeader(f http.HandlerFunc) http.HandlerFunc { return } - targetUrl, err := url.Parse("http://" + raftServerLeader) + // determine the scheme based on HTTPS client configuration + scheme := util_http.GetGlobalHttpClient().GetHttpScheme() + + targetUrl, err := url.Parse(scheme + "://" + raftServerLeader) if err != nil { writeJsonError(w, r, http.StatusInternalServerError, - fmt.Errorf("Leader URL http://%s Parse Error: %v", raftServerLeader, err)) + fmt.Errorf("Leader URL %s://%s Parse Error: %v", scheme, raftServerLeader, err)) return } // proxy to leader - glog.V(4).Infoln("proxying to leader", raftServerLeader) + glog.V(4).Infoln("proxying to leader", raftServerLeader, "using", scheme) proxy := httputil.NewSingleHostReverseProxy(targetUrl) proxy.Transport = util_http.GetGlobalHttpClient().GetClientTransport() proxy.ServeHTTP(w, r)