From a1db600a90b7f4a152a44c1192bb033e206f5f88 Mon Sep 17 00:00:00 2001 From: chrislu Date: Thu, 13 Nov 2025 11:46:46 -0800 Subject: [PATCH] add s3 action resolver --- weed/s3api/s3_action_resolver.go | 337 ++++++++++++++++++ weed/s3api/s3_constants/s3_action_strings.go | 85 +++++ .../s3api/s3_granular_action_security_test.go | 136 +++++++ weed/s3api/s3_iam_middleware.go | 20 +- weed/s3api/s3api_bucket_policy_engine.go | 282 +-------------- 5 files changed, 582 insertions(+), 278 deletions(-) create mode 100644 weed/s3api/s3_action_resolver.go create mode 100644 weed/s3api/s3_constants/s3_action_strings.go diff --git a/weed/s3api/s3_action_resolver.go b/weed/s3api/s3_action_resolver.go new file mode 100644 index 000000000..d96d150de --- /dev/null +++ b/weed/s3api/s3_action_resolver.go @@ -0,0 +1,337 @@ +package s3api + +import ( + "net/http" + "net/url" + "strings" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" +) + +// ResolveS3Action determines the specific S3 action from HTTP request context. +// This is the unified implementation used by both the bucket policy engine +// and the IAM integration for consistent action resolution. +// +// It examines the HTTP method, path, query parameters, and headers to determine +// the most specific S3 action string (e.g., "s3:DeleteObject", "s3:PutObjectTagging"). +// +// Parameters: +// - r: HTTP request containing method, URL, query params, and headers +// - baseAction: Coarse-grained action constant (e.g., ACTION_WRITE, ACTION_READ) +// - bucket: Bucket name from the request path +// - object: Object key from the request path (may be empty for bucket operations) +// +// Returns: +// - Specific S3 action string (e.g., "s3:DeleteObject") +// - Empty string if no specific resolution is possible +func ResolveS3Action(r *http.Request, baseAction string, bucket string, object string) string { + if r == nil { + return "" + } + + method := r.Method + query := r.URL.Query() + + // Determine if this is an object or bucket operation + // Note: "/" is treated as bucket-level, not object-level + hasObject := object != "" && object != "/" + + // Priority 1: Check for specific query parameters that indicate specific actions + // These override everything else because they explicitly indicate the operation type + if action := resolveFromQueryParameters(query, method, hasObject); action != "" { + return action + } + + // Priority 2: Handle basic operations based on method and resource type + if hasObject { + return resolveObjectLevelAction(method, baseAction, r) + } else if bucket != "" { + return resolveBucketLevelAction(method, baseAction) + } + + // Priority 3: Fallback to legacy action mapping + return mapBaseActionToS3Format(baseAction) +} + +// resolveFromQueryParameters checks query parameters to determine specific S3 actions +func resolveFromQueryParameters(query url.Values, method string, hasObject bool) string { + // Multipart upload operations + if query.Has("uploadId") && query.Has("partNumber") { + if method == http.MethodPut { + return s3_constants.S3_ACTION_UPLOAD_PART + } + } + + if query.Has("uploadId") { + switch method { + case http.MethodPost: + return s3_constants.S3_ACTION_COMPLETE_MULTIPART + case http.MethodDelete: + return s3_constants.S3_ACTION_ABORT_MULTIPART + case http.MethodGet: + return s3_constants.S3_ACTION_LIST_PARTS + } + } + + if query.Has("uploads") { + if method == http.MethodPost { + return s3_constants.S3_ACTION_CREATE_MULTIPART + } else if method == http.MethodGet { + return s3_constants.S3_ACTION_LIST_MULTIPART_UPLOADS + } + } + + // ACL operations + if query.Has("acl") { + if hasObject { + if method == http.MethodGet || method == http.MethodHead { + return s3_constants.S3_ACTION_GET_OBJECT_ACL + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_OBJECT_ACL + } + } else { + if method == http.MethodGet || method == http.MethodHead { + return s3_constants.S3_ACTION_GET_BUCKET_ACL + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_BUCKET_ACL + } + } + } + + // Tagging operations + if query.Has("tagging") { + if hasObject { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_OBJECT_TAGGING + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_OBJECT_TAGGING + } else if method == http.MethodDelete { + return s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING + } + } else { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_BUCKET_TAGGING + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_BUCKET_TAGGING + } else if method == http.MethodDelete { + return s3_constants.S3_ACTION_DELETE_BUCKET_TAGGING + } + } + } + + // Versioning operations + if query.Has("versioning") { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_BUCKET_VERSIONING + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_BUCKET_VERSIONING + } + } + + if query.Has("versions") { + if method == http.MethodGet { + // If there's an object, this is GetObjectVersion + // If there's no object (bucket level), this is ListBucketVersions + if hasObject { + return s3_constants.S3_ACTION_GET_OBJECT_VERSION + } else { + return s3_constants.S3_ACTION_LIST_BUCKET_VERSIONS + } + } + // DELETE with versions could be DeleteObjectVersion + if method == http.MethodDelete && hasObject { + return s3_constants.S3_ACTION_DELETE_OBJECT_VERSION + } + } + + // Policy operations + if query.Has("policy") { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_BUCKET_POLICY + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_BUCKET_POLICY + } else if method == http.MethodDelete { + return s3_constants.S3_ACTION_DELETE_BUCKET_POLICY + } + } + + // CORS operations + if query.Has("cors") { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_BUCKET_CORS + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_BUCKET_CORS + } else if method == http.MethodDelete { + return s3_constants.S3_ACTION_DELETE_BUCKET_CORS + } + } + + // Lifecycle operations + if query.Has("lifecycle") { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_BUCKET_LIFECYCLE + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_BUCKET_LIFECYCLE + } else if method == http.MethodDelete { + return s3_constants.S3_ACTION_DELETE_BUCKET_LIFECYCLE + } + } + + // Location + if query.Has("location") { + return s3_constants.S3_ACTION_GET_BUCKET_LOCATION + } + + // Notification + if query.Has("notification") { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_BUCKET_NOTIFICATION + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_BUCKET_NOTIFICATION + } + } + + // Object Lock operations + if query.Has("object-lock") { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_BUCKET_OBJECT_LOCK + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_BUCKET_OBJECT_LOCK + } + } + + if query.Has("retention") { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_OBJECT_RETENTION + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_OBJECT_RETENTION + } + } + + if query.Has("legal-hold") { + if method == http.MethodGet { + return s3_constants.S3_ACTION_GET_OBJECT_LEGAL_HOLD + } else if method == http.MethodPut { + return s3_constants.S3_ACTION_PUT_OBJECT_LEGAL_HOLD + } + } + + // Batch delete - works on bucket level + if query.Has("delete") { + return s3_constants.S3_ACTION_DELETE_OBJECT + } + + return "" +} + +// resolveObjectLevelAction determines the S3 action for object-level operations +func resolveObjectLevelAction(method string, baseAction string, r *http.Request) string { + switch method { + case http.MethodGet, http.MethodHead: + if baseAction == s3_constants.ACTION_READ { + return s3_constants.S3_ACTION_GET_OBJECT + } + + case http.MethodPut: + if baseAction == s3_constants.ACTION_WRITE { + // Check for copy operation + if r.Header.Get("X-Amz-Copy-Source") != "" { + return s3_constants.S3_ACTION_PUT_OBJECT // CopyObject also requires PutObject permission + } + return s3_constants.S3_ACTION_PUT_OBJECT + } + + case http.MethodDelete: + // CRITICAL: Map DELETE method to s3:DeleteObject + // This fixes the architectural limitation where ACTION_WRITE was mapped to s3:PutObject + if baseAction == s3_constants.ACTION_WRITE { + return s3_constants.S3_ACTION_DELETE_OBJECT + } + + case http.MethodPost: + // POST without query params is typically multipart or form upload + if baseAction == s3_constants.ACTION_WRITE { + return s3_constants.S3_ACTION_PUT_OBJECT + } + } + + return "" +} + +// resolveBucketLevelAction determines the S3 action for bucket-level operations +func resolveBucketLevelAction(method string, baseAction string) string { + switch method { + case http.MethodGet, http.MethodHead: + if baseAction == s3_constants.ACTION_LIST { + return s3_constants.S3_ACTION_LIST_BUCKET + } else if baseAction == s3_constants.ACTION_READ { + return s3_constants.S3_ACTION_LIST_BUCKET + } + + case http.MethodPut: + if baseAction == s3_constants.ACTION_WRITE { + return s3_constants.S3_ACTION_CREATE_BUCKET + } + + case http.MethodDelete: + if baseAction == s3_constants.ACTION_DELETE_BUCKET { + return s3_constants.S3_ACTION_DELETE_BUCKET + } + + case http.MethodPost: + // POST to bucket is typically form upload + if baseAction == s3_constants.ACTION_WRITE { + return s3_constants.S3_ACTION_PUT_OBJECT + } + } + + return "" +} + +// mapBaseActionToS3Format converts coarse-grained base actions to S3 format +// This is the fallback when no specific resolution is found +func mapBaseActionToS3Format(baseAction string) string { + // Handle actions that already have s3: prefix + if strings.HasPrefix(baseAction, "s3:") { + return baseAction + } + + // Map coarse-grained actions to their most common S3 equivalent + // Note: The s3_constants values ARE the string values (e.g., ACTION_READ = "Read") + switch baseAction { + case s3_constants.ACTION_READ: // "Read" + return s3_constants.S3_ACTION_GET_OBJECT + case s3_constants.ACTION_WRITE: // "Write" + return s3_constants.S3_ACTION_PUT_OBJECT + case s3_constants.ACTION_LIST: // "List" + return s3_constants.S3_ACTION_LIST_BUCKET + case s3_constants.ACTION_TAGGING: // "Tagging" + return s3_constants.S3_ACTION_PUT_OBJECT_TAGGING + case s3_constants.ACTION_ADMIN: // "Admin" + return s3_constants.S3_ACTION_ALL + case s3_constants.ACTION_READ_ACP: // "ReadAcp" + return s3_constants.S3_ACTION_GET_OBJECT_ACL + case s3_constants.ACTION_WRITE_ACP: // "WriteAcp" + return s3_constants.S3_ACTION_PUT_OBJECT_ACL + case s3_constants.ACTION_DELETE_BUCKET: // "DeleteBucket" + return s3_constants.S3_ACTION_DELETE_BUCKET + case s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION: + return s3_constants.S3_ACTION_BYPASS_GOVERNANCE + case s3_constants.ACTION_GET_OBJECT_RETENTION: + return s3_constants.S3_ACTION_GET_OBJECT_RETENTION + case s3_constants.ACTION_PUT_OBJECT_RETENTION: + return s3_constants.S3_ACTION_PUT_OBJECT_RETENTION + case s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD: + return s3_constants.S3_ACTION_GET_OBJECT_LEGAL_HOLD + case s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD: + return s3_constants.S3_ACTION_PUT_OBJECT_LEGAL_HOLD + case s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG: + return s3_constants.S3_ACTION_GET_BUCKET_OBJECT_LOCK + case s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG: + return s3_constants.S3_ACTION_PUT_BUCKET_OBJECT_LOCK + default: + // For unknown actions, prefix with s3: to maintain format consistency + return "s3:" + baseAction + } +} + diff --git a/weed/s3api/s3_constants/s3_action_strings.go b/weed/s3api/s3_constants/s3_action_strings.go new file mode 100644 index 000000000..96d10d03f --- /dev/null +++ b/weed/s3api/s3_constants/s3_action_strings.go @@ -0,0 +1,85 @@ +package s3_constants + +// S3 action strings for bucket policy evaluation +// These match the official AWS S3 action format used in IAM and bucket policies +const ( + // Object operations + S3_ACTION_GET_OBJECT = "s3:GetObject" + S3_ACTION_PUT_OBJECT = "s3:PutObject" + S3_ACTION_DELETE_OBJECT = "s3:DeleteObject" + S3_ACTION_DELETE_OBJECT_VERSION = "s3:DeleteObjectVersion" + S3_ACTION_GET_OBJECT_VERSION = "s3:GetObjectVersion" + + // Object ACL operations + S3_ACTION_GET_OBJECT_ACL = "s3:GetObjectAcl" + S3_ACTION_PUT_OBJECT_ACL = "s3:PutObjectAcl" + + // Object tagging operations + S3_ACTION_GET_OBJECT_TAGGING = "s3:GetObjectTagging" + S3_ACTION_PUT_OBJECT_TAGGING = "s3:PutObjectTagging" + S3_ACTION_DELETE_OBJECT_TAGGING = "s3:DeleteObjectTagging" + + // Object retention and legal hold + S3_ACTION_GET_OBJECT_RETENTION = "s3:GetObjectRetention" + S3_ACTION_PUT_OBJECT_RETENTION = "s3:PutObjectRetention" + S3_ACTION_GET_OBJECT_LEGAL_HOLD = "s3:GetObjectLegalHold" + S3_ACTION_PUT_OBJECT_LEGAL_HOLD = "s3:PutObjectLegalHold" + S3_ACTION_BYPASS_GOVERNANCE = "s3:BypassGovernanceRetention" + + // Multipart upload operations + S3_ACTION_CREATE_MULTIPART = "s3:CreateMultipartUpload" + S3_ACTION_UPLOAD_PART = "s3:UploadPart" + S3_ACTION_COMPLETE_MULTIPART = "s3:CompleteMultipartUpload" + S3_ACTION_ABORT_MULTIPART = "s3:AbortMultipartUpload" + S3_ACTION_LIST_PARTS = "s3:ListParts" + + // Bucket operations + S3_ACTION_CREATE_BUCKET = "s3:CreateBucket" + S3_ACTION_DELETE_BUCKET = "s3:DeleteBucket" + S3_ACTION_LIST_BUCKET = "s3:ListBucket" + S3_ACTION_LIST_BUCKET_VERSIONS = "s3:ListBucketVersions" + S3_ACTION_LIST_MULTIPART_UPLOADS = "s3:ListMultipartUploads" + + // Bucket ACL operations + S3_ACTION_GET_BUCKET_ACL = "s3:GetBucketAcl" + S3_ACTION_PUT_BUCKET_ACL = "s3:PutBucketAcl" + + // Bucket policy operations + S3_ACTION_GET_BUCKET_POLICY = "s3:GetBucketPolicy" + S3_ACTION_PUT_BUCKET_POLICY = "s3:PutBucketPolicy" + S3_ACTION_DELETE_BUCKET_POLICY = "s3:DeleteBucketPolicy" + + // Bucket tagging operations + S3_ACTION_GET_BUCKET_TAGGING = "s3:GetBucketTagging" + S3_ACTION_PUT_BUCKET_TAGGING = "s3:PutBucketTagging" + S3_ACTION_DELETE_BUCKET_TAGGING = "s3:DeleteBucketTagging" + + // Bucket CORS operations + S3_ACTION_GET_BUCKET_CORS = "s3:GetBucketCors" + S3_ACTION_PUT_BUCKET_CORS = "s3:PutBucketCors" + S3_ACTION_DELETE_BUCKET_CORS = "s3:DeleteBucketCors" + + // Bucket lifecycle operations + S3_ACTION_GET_BUCKET_LIFECYCLE = "s3:GetBucketLifecycleConfiguration" + S3_ACTION_PUT_BUCKET_LIFECYCLE = "s3:PutBucketLifecycleConfiguration" + S3_ACTION_DELETE_BUCKET_LIFECYCLE = "s3:DeleteBucketLifecycle" + + // Bucket versioning operations + S3_ACTION_GET_BUCKET_VERSIONING = "s3:GetBucketVersioning" + S3_ACTION_PUT_BUCKET_VERSIONING = "s3:PutBucketVersioning" + + // Bucket location + S3_ACTION_GET_BUCKET_LOCATION = "s3:GetBucketLocation" + + // Bucket notification + S3_ACTION_GET_BUCKET_NOTIFICATION = "s3:GetBucketNotification" + S3_ACTION_PUT_BUCKET_NOTIFICATION = "s3:PutBucketNotification" + + // Bucket object lock operations + S3_ACTION_GET_BUCKET_OBJECT_LOCK = "s3:GetBucketObjectLockConfiguration" + S3_ACTION_PUT_BUCKET_OBJECT_LOCK = "s3:PutBucketObjectLockConfiguration" + + // Wildcard for all S3 actions + S3_ACTION_ALL = "s3:*" +) + diff --git a/weed/s3api/s3_granular_action_security_test.go b/weed/s3api/s3_granular_action_security_test.go index 0767b1f2c..4f07cc6b3 100644 --- a/weed/s3api/s3_granular_action_security_test.go +++ b/weed/s3api/s3_granular_action_security_test.go @@ -5,6 +5,7 @@ import ( "net/url" "testing" + "github.com/gorilla/mux" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/stretchr/testify/assert" ) @@ -488,3 +489,138 @@ func TestFineGrainedPolicyExample(t *testing.T) { t.Logf(" Fine-grained policies like 'allow PUT but deny DELETE' now work correctly") t.Logf(" The policy engine can distinguish between s3:PutObject and s3:DeleteObject") } + +// TestCoarseActionResolution verifies that ResolveS3Action correctly maps +// coarse-grained ACTION_WRITE to specific S3 actions based on HTTP context. +// This demonstrates the fix for the architectural limitation where ACTION_WRITE +// was always mapped to s3:PutObject, preventing fine-grained policies from working. +func TestCoarseActionResolution(t *testing.T) { + testCases := []struct { + name string + method string + path string + queryParams map[string]string + coarseAction Action + expectedS3Action string + policyScenario string + }{ + { + name: "PUT_with_ACTION_WRITE_resolves_to_PutObject", + method: http.MethodPut, + path: "/test-bucket/test-file.txt", + queryParams: map[string]string{}, + coarseAction: s3_constants.ACTION_WRITE, + expectedS3Action: "s3:PutObject", + policyScenario: "Policy allowing s3:PutObject should match PUT requests", + }, + { + name: "DELETE_with_ACTION_WRITE_resolves_to_DeleteObject", + method: http.MethodDelete, + path: "/test-bucket/test-file.txt", + queryParams: map[string]string{}, + coarseAction: s3_constants.ACTION_WRITE, + expectedS3Action: "s3:DeleteObject", + policyScenario: "Policy denying s3:DeleteObject should block DELETE requests", + }, + { + name: "batch_DELETE_with_ACTION_WRITE_resolves_to_DeleteObject", + method: http.MethodPost, + path: "/test-bucket", + queryParams: map[string]string{"delete": ""}, + coarseAction: s3_constants.ACTION_WRITE, + expectedS3Action: "s3:DeleteObject", + policyScenario: "Policy denying s3:DeleteObject should block batch DELETE", + }, + { + name: "POST_multipart_with_ACTION_WRITE_resolves_to_CreateMultipartUpload", + method: http.MethodPost, + path: "/test-bucket/large-file.mp4", + queryParams: map[string]string{"uploads": ""}, + coarseAction: s3_constants.ACTION_WRITE, + expectedS3Action: "s3:CreateMultipartUpload", + policyScenario: "Policy allowing s3:PutObject but denying s3:CreateMultipartUpload can now work", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Build URL with query parameters + u, err := url.Parse(tc.path) + assert.NoError(t, err) + q := u.Query() + for k, v := range tc.queryParams { + q.Add(k, v) + } + u.RawQuery = q.Encode() + + // Create HTTP request + req, err := http.NewRequest(tc.method, u.String(), nil) + assert.NoError(t, err) + + // Extract bucket and object from path for mux.Vars simulation + // Path format: /bucket/object or /bucket + pathParts := []string{} + for _, part := range []byte(u.Path) { + if part == '/' { + continue + } + pathParts = append(pathParts, string(part)) + } + + // Parse path to extract bucket and object + bucket := "" + object := "" + if u.Path != "" { + parts := []string{} + current := "" + for _, ch := range u.Path { + if ch == '/' { + if current != "" { + parts = append(parts, current) + current = "" + } + } else { + current += string(ch) + } + } + if current != "" { + parts = append(parts, current) + } + + if len(parts) > 0 { + bucket = parts[0] + if len(parts) > 1 { + object = "/" + parts[1] + } + } + } + + // Simulate mux.Vars for GetBucketAndObject + req = mux.SetURLVars(req, map[string]string{ + "bucket": bucket, + "object": object, + }) + + // Call ResolveS3Action with coarse action constant + resolvedAction := ResolveS3Action(req, string(tc.coarseAction), bucket, object) + + // Verify correct S3 action is resolved + assert.Equal(t, tc.expectedS3Action, resolvedAction, + "Coarse action %s with method %s should resolve to %s", + tc.coarseAction, tc.method, tc.expectedS3Action) + + t.Logf("SUCCESS: %s", tc.name) + t.Logf(" Input: %s %s + ACTION_WRITE", tc.method, tc.path) + t.Logf(" Output: %s", resolvedAction) + t.Logf(" Policy impact: %s", tc.policyScenario) + }) + } + + t.Log("\n=== ARCHITECTURAL LIMITATION RESOLVED ===") + t.Log("Handlers can use coarse ACTION_WRITE constant, and the context-aware") + t.Log("resolver will map it to the correct specific S3 action (PutObject,") + t.Log("DeleteObject, CreateMultipartUpload, etc.) based on HTTP method and") + t.Log("query parameters. This enables fine-grained bucket policies like:") + t.Log(" - Allow s3:PutObject but Deny s3:DeleteObject (append-only)") + t.Log(" - Allow regular uploads but Deny multipart (size limits)") +} diff --git a/weed/s3api/s3_iam_middleware.go b/weed/s3api/s3_iam_middleware.go index 7c0dfc216..ebe179af1 100644 --- a/weed/s3api/s3_iam_middleware.go +++ b/weed/s3api/s3_iam_middleware.go @@ -255,23 +255,31 @@ func buildS3ResourceArn(bucket string, objectKey string) string { // determineGranularS3Action determines the specific S3 IAM action based on HTTP request details // This provides granular, operation-specific actions for accurate IAM policy enforcement +// +// NOTE: This function now uses the shared ResolveS3Action utility with additional +// fallback logic for IAM-specific cases. func determineGranularS3Action(r *http.Request, fallbackAction Action, bucket string, objectKey string) string { - method := r.Method query := r.URL.Query() - // Check if there are specific query parameters indicating granular operations + // IAM-specific: Check if there are specific query parameters indicating granular operations // If there are, always use granular mapping regardless of method-action alignment hasGranularIndicators := hasSpecificQueryParameters(query) // Only check for method-action mismatch when there are NO granular indicators // This provides fallback behavior for cases where HTTP method doesn't align with intended action - if !hasGranularIndicators && isMethodActionMismatch(method, fallbackAction) { + if !hasGranularIndicators && isMethodActionMismatch(r.Method, fallbackAction) { return mapLegacyActionToIAM(fallbackAction) } - // Handle object-level operations when method and action are aligned + // Use the shared action resolver for consistent resolution + // This now handles most of the action resolution logic + if resolvedAction := ResolveS3Action(r, string(fallbackAction), bucket, objectKey); resolvedAction != "" { + return resolvedAction + } + + // Legacy IAM-specific object-level handling (for backward compatibility) if objectKey != "" && objectKey != "/" { - switch method { + switch r.Method { case "GET", "HEAD": // Object read operations - check for specific query parameters if _, hasAcl := query["acl"]; hasAcl { @@ -337,7 +345,7 @@ func determineGranularS3Action(r *http.Request, fallbackAction Action, bucket st // Handle bucket-level operations if bucket != "" { - switch method { + switch r.Method { case "GET", "HEAD": // Bucket read operations - check for specific query parameters if _, hasAcl := query["acl"]; hasAcl { diff --git a/weed/s3api/s3api_bucket_policy_engine.go b/weed/s3api/s3api_bucket_policy_engine.go index c3276f558..51ebd644f 100644 --- a/weed/s3api/s3api_bucket_policy_engine.go +++ b/weed/s3api/s3api_bucket_policy_engine.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "net/http" - "strings" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/iam/policy" @@ -184,286 +183,25 @@ func (bpe *BucketPolicyEngine) EvaluatePolicyWithContext(bucket, object, action, // convertActionToS3Format converts internal action strings to S3 action format // with optional HTTP request context for fine-grained action resolution. // -// Context-Aware Resolution: When an HTTP request is provided, this function uses -// the HTTP method, path, and query parameters to accurately determine the specific -// S3 action. This solves the architectural limitation where coarse-grained internal -// actions (ACTION_READ, ACTION_WRITE) are used for multiple S3 operations. -// -// For example: -// - ACTION_WRITE + DELETE /bucket/object → s3:DeleteObject -// - ACTION_WRITE + PUT /bucket/object → s3:PutObject -// - ACTION_READ + GET /bucket/object?acl → s3:GetObjectAcl -// -// This enables fine-grained bucket policies (e.g., allowing s3:DeleteObject but -// denying s3:PutObject) without requiring massive refactoring of handler registrations. +// This function now uses the shared ResolveS3Action utility for consistent +// action resolution across the bucket policy engine and IAM integration. // // Parameters: // - action: The internal action constant (e.g., ACTION_WRITE, ACTION_READ) // - r: Optional HTTP request for context-aware resolution. If nil, uses legacy mapping. func convertActionToS3Format(action string, r *http.Request) string { - // Handle actions that already have s3: prefix (e.g., multipart actions) - if strings.HasPrefix(action, "s3:") { - return action - } - - // If request context is provided, use it for fine-grained action resolution + // If request context is provided, use the shared action resolver if r != nil { - if resolvedAction := resolveS3ActionFromRequest(action, r); resolvedAction != "" { + bucket, object := s3_constants.GetBucketAndObject(r) + if resolvedAction := ResolveS3Action(r, action, bucket, object); resolvedAction != "" { return resolvedAction } } - // Fallback to legacy coarse-grained mapping (for backward compatibility) - switch action { - // Basic operations - case s3_constants.ACTION_READ: - return "s3:GetObject" - case s3_constants.ACTION_WRITE: - return "s3:PutObject" - case s3_constants.ACTION_LIST: - return "s3:ListBucket" - case s3_constants.ACTION_TAGGING: - return "s3:PutObjectTagging" - case s3_constants.ACTION_ADMIN: - return "s3:*" - - // ACL operations - case s3_constants.ACTION_READ_ACP: - return "s3:GetObjectAcl" - case s3_constants.ACTION_WRITE_ACP: - return "s3:PutObjectAcl" - - // Bucket operations - case s3_constants.ACTION_DELETE_BUCKET: - return "s3:DeleteBucket" - - // Object Lock operations - case s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION: - return "s3:BypassGovernanceRetention" - case s3_constants.ACTION_GET_OBJECT_RETENTION: - return "s3:GetObjectRetention" - case s3_constants.ACTION_PUT_OBJECT_RETENTION: - return "s3:PutObjectRetention" - case s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD: - return "s3:GetObjectLegalHold" - case s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD: - return "s3:PutObjectLegalHold" - case s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG: - return "s3:GetBucketObjectLockConfiguration" - case s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG: - return "s3:PutBucketObjectLockConfiguration" - - default: - // Log warning for unmapped actions to help catch issues - glog.Warningf("convertActionToS3Format: unmapped action '%s', prefixing with 's3:'", action) - // For unknown actions, prefix with s3: to maintain format consistency - // This maintains backward compatibility while alerting developers - return "s3:" + action - } + // Fallback to base action mapping + return mapBaseActionToS3Format(action) } -// resolveS3ActionFromRequest determines the specific S3 action from HTTP request context -// This enables fine-grained action resolution without changing handler registrations -// -// TODO: Consider consolidating with determineGranularS3Action() in s3_iam_middleware.go -// to avoid code duplication. This function is used by the bucket policy engine, while -// determineGranularS3Action is used by the IAM integration. They serve similar purposes -// and could potentially be unified into a single shared utility function. -func resolveS3ActionFromRequest(baseAction string, r *http.Request) string { - if r == nil { - return "" - } - - method := r.Method - query := r.URL.Query() - bucket, object := s3_constants.GetBucketAndObject(r) - - // Determine if this is an object or bucket operation - // Note: "/" is treated as bucket-level, not object-level - hasObject := object != "" && object != "/" - - // Check for specific query parameters that indicate specific actions - switch { - // Multipart upload operations - case query.Get("uploadId") != "" && query.Get("partNumber") != "": - if method == http.MethodPut { - return "s3:PutObject" // Upload part - } - case query.Get("uploadId") != "": - switch method { - case http.MethodPost: - return "s3:PutObject" // Complete multipart - case http.MethodDelete: - return "s3:AbortMultipartUpload" - case http.MethodGet: - return "s3:ListParts" - } - case query.Get("uploads") != "": - if method == http.MethodPost { - return "s3:CreateMultipartUpload" - } else if method == http.MethodGet { - return "s3:ListMultipartUploads" - } - - // ACL operations - case query.Get("acl") != "": - if hasObject { - if method == http.MethodGet || method == http.MethodHead { - return "s3:GetObjectAcl" - } else if method == http.MethodPut { - return "s3:PutObjectAcl" - } - } else { - if method == http.MethodGet || method == http.MethodHead { - return "s3:GetBucketAcl" - } else if method == http.MethodPut { - return "s3:PutBucketAcl" - } - } - - // Tagging operations - case query.Get("tagging") != "": - if hasObject { - if method == http.MethodGet { - return "s3:GetObjectTagging" - } else if method == http.MethodPut { - return "s3:PutObjectTagging" - } else if method == http.MethodDelete { - return "s3:DeleteObjectTagging" - } - } else { - if method == http.MethodGet { - return "s3:GetBucketTagging" - } else if method == http.MethodPut { - return "s3:PutBucketTagging" - } else if method == http.MethodDelete { - return "s3:DeleteBucketTagging" - } - } - - // Versioning - case query.Get("versioning") != "": - if method == http.MethodGet { - return "s3:GetBucketVersioning" - } else if method == http.MethodPut { - return "s3:PutBucketVersioning" - } - case query.Get("versions") != "": - return "s3:ListBucketVersions" - - // Policy operations - case query.Get("policy") != "": - if method == http.MethodGet { - return "s3:GetBucketPolicy" - } else if method == http.MethodPut { - return "s3:PutBucketPolicy" - } else if method == http.MethodDelete { - return "s3:DeleteBucketPolicy" - } - - // CORS operations - case query.Get("cors") != "": - if method == http.MethodGet { - return "s3:GetBucketCors" - } else if method == http.MethodPut { - return "s3:PutBucketCors" - } else if method == http.MethodDelete { - return "s3:DeleteBucketCors" - } - - // Lifecycle operations - case query.Get("lifecycle") != "": - if method == http.MethodGet { - return "s3:GetBucketLifecycleConfiguration" - } else if method == http.MethodPut { - return "s3:PutBucketLifecycleConfiguration" - } else if method == http.MethodDelete { - return "s3:DeleteBucketLifecycle" - } - - // Location - case query.Get("location") != "": - return "s3:GetBucketLocation" - - // Object Lock operations - case query.Get("object-lock") != "": - if method == http.MethodGet { - return "s3:GetBucketObjectLockConfiguration" - } else if method == http.MethodPut { - return "s3:PutBucketObjectLockConfiguration" - } - case query.Get("retention") != "": - if method == http.MethodGet { - return "s3:GetObjectRetention" - } else if method == http.MethodPut { - return "s3:PutObjectRetention" - } - case query.Get("legal-hold") != "": - if method == http.MethodGet { - return "s3:GetObjectLegalHold" - } else if method == http.MethodPut { - return "s3:PutObjectLegalHold" - } - - // Batch delete - check this early as it works on bucket level with no object - case query.Has("delete"): - return "s3:DeleteObject" - } - - // Handle basic operations based on method and whether object exists - // This is the critical fix for the DeleteObject issue - if hasObject { - // Object-level operations - switch method { - case http.MethodGet, http.MethodHead: - if baseAction == s3_constants.ACTION_READ { - return "s3:GetObject" - } - case http.MethodPut: - if baseAction == s3_constants.ACTION_WRITE { - // Check for copy operation - if r.Header.Get("X-Amz-Copy-Source") != "" { - return "s3:PutObject" // CopyObject also requires PutObject permission - } - return "s3:PutObject" - } - case http.MethodDelete: - // CRITICAL FIX: Map DELETE method to s3:DeleteObject - // Previously, ACTION_WRITE would map to s3:PutObject even for DELETE - if baseAction == s3_constants.ACTION_WRITE { - return "s3:DeleteObject" - } - case http.MethodPost: - // POST without query params is typically multipart related - if baseAction == s3_constants.ACTION_WRITE { - return "s3:PutObject" - } - } - } else if bucket != "" { - // Bucket-level operations - switch method { - case http.MethodGet, http.MethodHead: - if baseAction == s3_constants.ACTION_LIST { - return "s3:ListBucket" - } else if baseAction == s3_constants.ACTION_READ { - return "s3:ListBucket" - } - case http.MethodPut: - if baseAction == s3_constants.ACTION_WRITE { - return "s3:CreateBucket" - } - case http.MethodDelete: - if baseAction == s3_constants.ACTION_DELETE_BUCKET { - return "s3:DeleteBucket" - } - case http.MethodPost: - // POST to bucket typically is multipart or policy form upload - if baseAction == s3_constants.ACTION_WRITE { - return "s3:PutObject" - } - } - } - - // No specific resolution found - return "" -} +// NOTE: resolveS3ActionFromRequest has been replaced by the shared ResolveS3Action +// function in s3_action_resolver.go. This consolidates action resolution logic +// used by both the bucket policy engine and IAM integration.