s3: resolve versionId tagging requests to the version-tagging actions (#11874)

* s3: resolve versionId tagging requests to the version-tagging actions

A request for an object's tags with versionId acted on a historical
version but authorized against the current-object tagging action, so an
explicit Deny on s3:*ObjectVersionTagging never applied.

* iam: map PutObjectVersionTagging and DeleteObjectVersionTagging to the tagging action

* s3: only resolve tagging when the query value is empty

The tagging routes match ?tagging with an empty value, so a non-empty one
is not a tagging request and would resolve a tagging grant onto the plain
read or version read that actually serves it.

* s3: keep resolving tagging for any query value

The routes match ?tagging whenever the key is present, so tagging=x is a
tagging request served by the tagging handlers and must resolve to the
tagging actions.

weed/iam/helpers.go

@@ -104,13 +104,15 @@ var baseS3ActionMap = map[string]string{
 	"DeleteObjectVersion": s3_constants.ACTION_WRITE,
 	"GetObjectVersion":    s3_constants.ACTION_READ,
 	// Tagging operations
-	"GetObjectTagging":        s3_constants.ACTION_TAGGING,
-	"GetObjectVersionTagging": s3_constants.ACTION_TAGGING,
-	"PutObjectTagging":        s3_constants.ACTION_TAGGING,
-	"DeleteObjectTagging":     s3_constants.ACTION_TAGGING,
-	"GetBucketTagging":        s3_constants.ACTION_TAGGING,
-	"PutBucketTagging":        s3_constants.ACTION_TAGGING,
-	"DeleteBucketTagging":     s3_constants.ACTION_TAGGING,
+	"GetObjectTagging":           s3_constants.ACTION_TAGGING,
+	"GetObjectVersionTagging":    s3_constants.ACTION_TAGGING,
+	"PutObjectTagging":           s3_constants.ACTION_TAGGING,
+	"PutObjectVersionTagging":    s3_constants.ACTION_TAGGING,
+	"DeleteObjectTagging":        s3_constants.ACTION_TAGGING,
+	"DeleteObjectVersionTagging": s3_constants.ACTION_TAGGING,
+	"GetBucketTagging":           s3_constants.ACTION_TAGGING,
+	"PutBucketTagging":           s3_constants.ACTION_TAGGING,
+	"DeleteBucketTagging":        s3_constants.ACTION_TAGGING,
 	// ACL operations
 	"PutObjectAcl":        s3_constants.ACTION_WRITE_ACP,
 	"GetObjectAcl":        s3_constants.ACTION_READ_ACP,

weed/s3api/s3_action_resolver.go

@@ -182,20 +182,33 @@ func resolveFromQueryParameters(query url.Values, method string, hasObject bool)
 		}
 	}
 
-	// Tagging operations
+	// Tagging operations. The routes match ?tagging whenever the key is
+	// present (gorilla/mux does not compare the value), so any value counts.
+	// A versionId retargets the request at a specific version, so the
+	// version-tagging action applies, not the current-object one.
 	if query.Has("tagging") {
+		versioned := hasObject && query.Get("versionId") != ""
 		switch method {
 		case http.MethodGet:
+			if versioned {
+				return s3_constants.S3_ACTION_GET_OBJECT_VERSION_TAGGING
+			}
 			if hasObject {
 				return s3_constants.S3_ACTION_GET_OBJECT_TAGGING
 			}
 			return s3_constants.S3_ACTION_GET_BUCKET_TAGGING
 		case http.MethodPut:
+			if versioned {
+				return s3_constants.S3_ACTION_PUT_OBJECT_VERSION_TAGGING
+			}
 			if hasObject {
 				return s3_constants.S3_ACTION_PUT_OBJECT_TAGGING
 			}
 			return s3_constants.S3_ACTION_PUT_BUCKET_TAGGING
 		case http.MethodDelete:
+			if versioned {
+				return s3_constants.S3_ACTION_DELETE_OBJECT_VERSION_TAGGING
+			}
 			if hasObject {
 				return s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING
 			}

weed/s3api/s3_action_resolver_test.go

@@ -114,6 +114,38 @@ func TestResolveS3Action_AttributesBeforeVersionId(t *testing.T) {
 	}
 }
 
+// A tagging request carrying versionId targets a specific version and must
+// resolve to the version-tagging action so a grant limited to current-object
+// tagging cannot reach historical versions.
+func TestResolveS3Action_VersionTagging(t *testing.T) {
+	tests := []struct {
+		name   string
+		method string
+		object string
+		query  string
+		want   string
+	}{
+		{"get current tags", http.MethodGet, "key", "tagging", s3_constants.S3_ACTION_GET_OBJECT_TAGGING},
+		{"put current tags", http.MethodPut, "key", "tagging", s3_constants.S3_ACTION_PUT_OBJECT_TAGGING},
+		{"delete current tags", http.MethodDelete, "key", "tagging", s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING},
+		{"get version tags", http.MethodGet, "key", "tagging&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION_TAGGING},
+		{"put version tags", http.MethodPut, "key", "tagging&versionId=v1", s3_constants.S3_ACTION_PUT_OBJECT_VERSION_TAGGING},
+		{"delete version tags", http.MethodDelete, "key", "tagging&versionId=v1", s3_constants.S3_ACTION_DELETE_OBJECT_VERSION_TAGGING},
+		{"empty versionId stays current", http.MethodGet, "key", "tagging&versionId=", s3_constants.S3_ACTION_GET_OBJECT_TAGGING},
+		{"bucket tagging ignores versionId", http.MethodGet, "", "tagging&versionId=v1", s3_constants.S3_ACTION_GET_BUCKET_TAGGING},
+		{"non-empty tagging is still tagging", http.MethodGet, "key", "tagging=x&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION_TAGGING},
+		{"non-empty tagging plain", http.MethodGet, "key", "tagging=x", s3_constants.S3_ACTION_GET_OBJECT_TAGGING},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r, _ := http.NewRequest(tt.method, "http://localhost/bucket/"+tt.object+"?"+tt.query, nil)
+			if got := ResolveS3Action(r, s3_constants.ACTION_READ, "bucket", tt.object); got != tt.want {
+				t.Errorf("ResolveS3Action() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
 // Bucket subresources registered with ACTION_ADMIN must resolve to their own
 // S3 actions so a policy granting one of them does not need s3:*, and so no
 // broader grant sweeps them in.

weed/s3api/s3_constants/s3_action_strings.go

@@ -20,6 +20,11 @@ const (
 	S3_ACTION_PUT_OBJECT_TAGGING    = "s3:PutObjectTagging"
 	S3_ACTION_DELETE_OBJECT_TAGGING = "s3:DeleteObjectTagging"
 
+	// Version-specific tagging operations (?tagging&versionId)
+	S3_ACTION_GET_OBJECT_VERSION_TAGGING    = "s3:GetObjectVersionTagging"
+	S3_ACTION_PUT_OBJECT_VERSION_TAGGING    = "s3:PutObjectVersionTagging"
+	S3_ACTION_DELETE_OBJECT_VERSION_TAGGING = "s3:DeleteObjectVersionTagging"
+
 	// Object retention and legal hold
 	S3_ACTION_GET_OBJECT_RETENTION  = "s3:GetObjectRetention"
 	S3_ACTION_PUT_OBJECT_RETENTION  = "s3:PutObjectRetention"