From 61bfe730292b374f2f53c6adab06d9a08ac49263 Mon Sep 17 00:00:00 2001 From: chrislu Date: Sun, 14 Sep 2025 12:50:19 -0700 Subject: [PATCH] Revert "Merge branch 'master' into feature/mq-kafka-gateway-m1" This reverts commit 78382b380ebcedf54221b79fa4703408c65095ef, reversing changes made to 56aa5278affc2f17ebfa8050b5936894147d7720. --- weed/s3api/auth_signature_v4.go | 18 +--- weed/s3api/auto_signature_v4_test.go | 133 --------------------------- 2 files changed, 2 insertions(+), 149 deletions(-) diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index a37274326..a0417a922 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -216,14 +216,7 @@ 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. - // 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) + errCode = iam.verifySignatureWithPath(extractedSignedHeaders, hashedPayload, queryStr, path.Clean(forwardedPrefix+req.URL.Path), req.Method, foundCred.SecretKey, t, signV4Values) if errCode == s3err.ErrNone { return identity, errCode } @@ -376,14 +369,7 @@ 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. - // 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) + errCode = iam.verifyPresignedSignatureWithPath(extractedSignedHeaders, hashedPayload, queryStr, path.Clean(forwardedPrefix+r.URL.Path), 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 bf11a0906..7a9599583 100644 --- a/weed/s3api/auto_signature_v4_test.go +++ b/weed/s3api/auto_signature_v4_test.go @@ -322,72 +322,6 @@ 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 { @@ -581,73 +515,6 @@ 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