s3api: sign every operation subresource in signature V2 (#12575)

* s3api: sign every operation subresource in signature V2 The V2 canonical resource only kept the subresources inherited from the original allowlist, so tagging, retention, legal-hold, cors, encryption, object-lock and the other keys the router dispatches on were left out of the signature. A V2 signature issued for one request therefore also verified that request with one of those keys appended, running a different operation under the signer's identity. Add every operation-selecting subresource to the signed list. Clients that already sign tagging, cors, object-lock or replication in V2 now verify as well. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3api: sign every value of a repeated signature V2 subresource canonicalizedResourceV2 kept only the last value of a repeated query key, while handlers read the first. A request signed for versionId=A could be sent as versionId=B&versionId=A and act on version B. Sign every value in order, as V2 clients do, so a second value no longer verifies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-13 07:19:18 +01:00 · 2026-11-12 17:22:59 -08:00
1 parent 5ba77af65f
commit 917cf534db
@@ -34,17 +34,33 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
 )

-// Whitelist resource list that will be used in query string for signature-V2 calculation.
-// The list should be alphabetically sorted
+// Query keys covered by a signature V2, in byte order. Every subresource that
+// selects an operation must be listed, or a signature issued for one operation
+// also authorizes the others.
 var resourceList = []string{
+	"accelerate",
 	"acl",
+	"analytics",
+	"attributes",
+	"cors",
 	"delete",
+	"encryption",
+	"intelligent-tiering",
+	"inventory",
+	"legal-hold",
 	"lifecycle",
 	"location",
 	"logging",
+	"metrics",
 	"notification",
+	"object-lock",
+	"ownershipControls",
 	"partNumber",
 	"policy",
+	"policyStatus",
+	"publicAccessBlock",
+	"renameObject",
+	"replication",
 	"requestPayment",
 	"response-cache-control",
 	"response-content-disposition",
@@ -52,6 +68,9 @@ var resourceList = []string{
 	"response-content-language",
 	"response-content-type",
 	"response-expires",
+	"retention",
+	"seaweedfs-quota",
+	"tagging",
 	"torrent",
 	"uploadId",
 	"uploads",
@@ -316,21 +335,16 @@ func getStringToSignV2(method string, encodedResource, encodedQuery string, head
 // canonicalizedResourceV2 - canonicalize the resource string for signature V2.
 func canonicalizedResourceV2(encodedResource, encodedQuery string) string {
 	queries := strings.Split(encodedQuery, "&")
-	keyval := make(map[string]string)
+	keyval := make(map[string][]string)
 	for _, query := range queries {
-		key := query
-		val := ""
-		index := strings.Index(query, "=")
-		if index != -1 {
-			key = query[:index]
-			val = query[index+1:]
-		}
-		keyval[key] = val
+		key, val, _ := strings.Cut(query, "=")
+		keyval[key] = append(keyval[key], val)
 	}

+	// Every value of a repeated key is signed: handlers read the first one.
 	var canonicalQueries []string
 	for _, resource := range resourceList {
-		if val, ok := keyval[resource]; ok {
+		for _, val := range keyval[resource] {
 			if val == "" {
 				canonicalQueries = append(canonicalQueries, resource)
 				continue
@@ -2,7 +2,9 @@ package s3api

 import (
 	"net/http"
+	"sort"
 	"strconv"
+	"strings"
 	"testing"
 	"time"

@@ -365,3 +367,154 @@ func TestDoesPresignV2MatchUnicodePath(t *testing.T) {
 		t.Fatal("Expected non-nil identity")
 	}
 }
+
+func presignV2ForTest(t *testing.T, cred *Credential, method, path, signedQuery string) *http.Request {
+	t.Helper()
+	req, err := http.NewRequest(method, "http://example.com"+path, nil)
+	if err != nil {
+		t.Fatalf("new request: %v", err)
+	}
+	req.URL.RawQuery = signedQuery
+	expires := strconv.FormatInt(time.Now().Add(time.Hour).Unix(), 10)
+	query := req.URL.Query()
+	query.Set("AWSAccessKeyId", cred.AccessKey)
+	query.Set("Expires", expires)
+	query.Set("Signature", preSignatureV2(cred, method, req.URL.EscapedPath(), req.URL.Query().Encode(), req.Header, expires))
+	req.URL.RawQuery = query.Encode()
+	return req
+}
+
+func TestSignatureV2CoversOperationSubresources(t *testing.T) {
+	if !sort.StringsAreSorted(resourceList) {
+		t.Fatal("resourceList must stay in byte order")
+	}
+	signed := make(map[string]bool, len(resourceList))
+	for _, key := range resourceList {
+		signed[key] = true
+	}
+	// list-type picks ListObjectsV2 over V1, the same action, and V2 clients never sign it.
+	for key := range operationSubresources {
+		if key != "list-type" && !signed[key] {
+			t.Errorf("operation subresource %q is not covered by signature V2", key)
+		}
+	}
+	for key := range bucketQueryActions {
+		if !signed[key] {
+			t.Errorf("bucket subresource %q is not covered by signature V2", key)
+		}
+	}
+}
+
+func TestSignatureV2RejectsAppendedSubresource(t *testing.T) {
+	iam := setupTestIAMForV2Auth()
+	cred := &Credential{
+		AccessKey: "AKIAIOSFODNN7EXAMPLE",
+		SecretKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
+	}
+
+	if _, errCode := iam.doesPresignV2SignatureMatch(presignV2ForTest(t, cred, http.MethodPut, "/bucket/key", "")); errCode != s3err.ErrNone {
+		t.Fatalf("presigned PUT as issued: %v", errCode)
+	}
+
+	for key := range operationSubresources {
+		if key == "list-type" {
+			continue
+		}
+		t.Run("presigned_"+key, func(t *testing.T) {
+			req := presignV2ForTest(t, cred, http.MethodPut, "/bucket/key", "")
+			req.URL.RawQuery += "&" + key
+			if _, errCode := iam.doesPresignV2SignatureMatch(req); errCode != s3err.ErrSignatureDoesNotMatch {
+				t.Fatalf("presigned PUT with unsigned %q: got %v, want %v", key, errCode, s3err.ErrSignatureDoesNotMatch)
+			}
+		})
+		t.Run("header_"+key, func(t *testing.T) {
+			req, err := http.NewRequest(http.MethodPut, "http://example.com/bucket/key", nil)
+			if err != nil {
+				t.Fatalf("new request: %v", err)
+			}
+			req.Header.Set("Date", time.Now().UTC().Format(http.TimeFormat))
+			req.Header.Set("Authorization", signatureV2(cred, req.Method, req.URL.EscapedPath(), req.URL.Query().Encode(), req.Header))
+			req.URL.RawQuery = key
+			if _, errCode := iam.doesSignV2Match(req); errCode != s3err.ErrSignatureDoesNotMatch {
+				t.Fatalf("signed PUT with unsigned %q: got %v, want %v", key, errCode, s3err.ErrSignatureDoesNotMatch)
+			}
+		})
+	}
+}
+
+func TestPresignV2AcceptsClientSignedSubresource(t *testing.T) {
+	iam := setupTestIAMForV2Auth()
+	cred := &Credential{
+		AccessKey: "AKIAIOSFODNN7EXAMPLE",
+		SecretKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
+	}
+	expires := strconv.FormatInt(time.Now().Add(time.Hour).Unix(), 10)
+
+	tests := []struct {
+		method, rawQuery, resource string
+	}{
+		{http.MethodPut, "tagging", "/bucket/key?tagging"},
+		{http.MethodGet, "tagging&versionId=v1", "/bucket/key?tagging&versionId=v1"},
+		{http.MethodGet, "cors", "/bucket?cors"},
+		{http.MethodPut, "retention", "/bucket/key?retention"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.method+"_"+tt.rawQuery, func(t *testing.T) {
+			// The string to sign a V2 client builds: verb, MD5, type, expiry, resource.
+			signature := calculateSignatureV2(tt.method+"\n\n\n"+expires+"\n"+tt.resource, cred.SecretKey)
+			path := tt.resource[:strings.Index(tt.resource, "?")]
+			req, err := http.NewRequest(tt.method, "http://example.com"+path+"?"+tt.rawQuery, nil)
+			if err != nil {
+				t.Fatalf("new request: %v", err)
+			}
+			query := req.URL.Query()
+			query.Set("AWSAccessKeyId", cred.AccessKey)
+			query.Set("Expires", expires)
+			query.Set("Signature", signature)
+			req.URL.RawQuery = query.Encode()
+			if _, errCode := iam.doesPresignV2SignatureMatch(req); errCode != s3err.ErrNone {
+				t.Fatalf("client-signed %s: got %v", tt.resource, errCode)
+			}
+		})
+	}
+}
+
+func TestSignatureV2RejectsRepeatedSubresource(t *testing.T) {
+	iam := setupTestIAMForV2Auth()
+	cred := &Credential{
+		AccessKey: "AKIAIOSFODNN7EXAMPLE",
+		SecretKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
+	}
+
+	if _, errCode := iam.doesPresignV2SignatureMatch(presignV2ForTest(t, cred, http.MethodPut, "/bucket/key", "retention&versionId=v1")); errCode != s3err.ErrNone {
+		t.Fatalf("presigned PUT as issued: %v", errCode)
+	}
+
+	for name, edit := range map[string]func(string) string{
+		"prepended": func(q string) string { return "versionId=v2&" + q },
+		"appended":  func(q string) string { return q + "&versionId=v2" },
+	} {
+		t.Run("presigned_"+name, func(t *testing.T) {
+			req := presignV2ForTest(t, cred, http.MethodPut, "/bucket/key", "retention&versionId=v1")
+			req.URL.RawQuery = edit(req.URL.RawQuery)
+			if _, errCode := iam.doesPresignV2SignatureMatch(req); errCode != s3err.ErrSignatureDoesNotMatch {
+				t.Fatalf("presigned PUT with a second versionId: got %v, want %v", errCode, s3err.ErrSignatureDoesNotMatch)
+			}
+		})
+		t.Run("header_"+name, func(t *testing.T) {
+			req, err := http.NewRequest(http.MethodPut, "http://example.com/bucket/key?retention&versionId=v1", nil)
+			if err != nil {
+				t.Fatalf("new request: %v", err)
+			}
+			req.Header.Set("Date", time.Now().UTC().Format(http.TimeFormat))
+			req.Header.Set("Authorization", signatureV2(cred, req.Method, req.URL.EscapedPath(), req.URL.Query().Encode(), req.Header))
+			if _, errCode := iam.doesSignV2Match(req); errCode != s3err.ErrNone {
+				t.Fatalf("signed PUT as issued: %v", errCode)
+			}
+			req.URL.RawQuery = edit(req.URL.RawQuery)
+			if _, errCode := iam.doesSignV2Match(req); errCode != s3err.ErrSignatureDoesNotMatch {
+				t.Fatalf("signed PUT with a second versionId: got %v, want %v", errCode, s3err.ErrSignatureDoesNotMatch)
+			}
+		})
+	}
+}