From b8fc99a9cd1166aef43987366597f7fee705f6d6 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 5 Apr 2026 14:06:50 -0700 Subject: [PATCH] fix(s3): apply PutObject multipart expansion to STS session policies (#8932) * fix(s3): apply PutObject multipart expansion to STS session policy evaluation (#8929) PR #8445 added logic to implicitly grant multipart upload actions when s3:PutObject is authorized, but only in the S3 API policy engine's CompiledStatement.MatchesAction(). STS session policies are evaluated through the IAM policy engine's matchesActions() -> awsIAMMatch() path, which did plain pattern matching without the multipart expansion. Add the same multipart expansion logic to the IAM policy engine's matchesActions() so that session policies containing s3:PutObject correctly allow multipart upload operations. * fix: make multipart action set lookup case-insensitive and optimize Address PR review feedback: - Lowercase multipartActionSet keys and use strings.ToLower for lookup, since AWS IAM actions are case-insensitive - Only check for s3:PutObject permission when the requested action is actually a multipart action, avoiding unnecessary awsIAMMatch calls - Add test case for case-insensitive multipart action matching --- weed/iam/policy/aws_iam_compliance_test.go | 86 ++++++++++++++++++++++ weed/iam/policy/policy_engine.go | 21 +++++- 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/weed/iam/policy/aws_iam_compliance_test.go b/weed/iam/policy/aws_iam_compliance_test.go index 0979589a5..cff61c9a4 100644 --- a/weed/iam/policy/aws_iam_compliance_test.go +++ b/weed/iam/policy/aws_iam_compliance_test.go @@ -110,6 +110,92 @@ func TestAWSIAMMatch(t *testing.T) { } } +func TestMatchesActionsMultipartExpansion(t *testing.T) { + engine := &PolicyEngine{initialized: true} + evalCtx := &EvaluationContext{} + + tests := []struct { + name string + actions []string + requestedAction string + expected bool + }{ + { + name: "PutObject directly matches PutObject", + actions: []string{"s3:PutObject"}, + requestedAction: "s3:PutObject", + expected: true, + }, + { + name: "PutObject implicitly allows CreateMultipartUpload", + actions: []string{"s3:PutObject"}, + requestedAction: "s3:CreateMultipartUpload", + expected: true, + }, + { + name: "PutObject implicitly allows UploadPart", + actions: []string{"s3:PutObject"}, + requestedAction: "s3:UploadPart", + expected: true, + }, + { + name: "PutObject implicitly allows CompleteMultipartUpload", + actions: []string{"s3:PutObject"}, + requestedAction: "s3:CompleteMultipartUpload", + expected: true, + }, + { + name: "PutObject implicitly allows AbortMultipartUpload", + actions: []string{"s3:PutObject"}, + requestedAction: "s3:AbortMultipartUpload", + expected: true, + }, + { + name: "PutObject implicitly allows ListMultipartUploadParts", + actions: []string{"s3:PutObject"}, + requestedAction: "s3:ListMultipartUploadParts", + expected: true, + }, + { + name: "PutObject implicitly allows ListBucketMultipartUploads", + actions: []string{"s3:PutObject"}, + requestedAction: "s3:ListBucketMultipartUploads", + expected: true, + }, + { + name: "PutObject does not allow GetObject", + actions: []string{"s3:PutObject"}, + requestedAction: "s3:GetObject", + expected: false, + }, + { + name: "GetObject does not allow CreateMultipartUpload", + actions: []string{"s3:GetObject"}, + requestedAction: "s3:CreateMultipartUpload", + expected: false, + }, + { + name: "wildcard s3:Put* implicitly allows multipart via PutObject match", + actions: []string{"s3:Put*"}, + requestedAction: "s3:CreateMultipartUpload", + expected: true, + }, + { + name: "case-insensitive multipart action lookup", + actions: []string{"s3:PutObject"}, + requestedAction: "S3:CREATEMULTIPARTUPLOAD", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := engine.matchesActions(tt.actions, tt.requestedAction, evalCtx) + assert.Equal(t, tt.expected, result) + }) + } +} + func TestExpandPolicyVariables(t *testing.T) { evalCtx := &EvaluationContext{ RequestContext: map[string]interface{}{ diff --git a/weed/iam/policy/policy_engine.go b/weed/iam/policy/policy_engine.go index 7feca5c92..2daa0a3e1 100644 --- a/weed/iam/policy/policy_engine.go +++ b/weed/iam/policy/policy_engine.go @@ -600,12 +600,31 @@ func (e *PolicyEngine) statementMatches(statement *Statement, evalCtx *Evaluatio return true } -// matchesActions checks if any action in the list matches the requested action +// multipartActionSet contains lowercased S3 multipart upload actions that are +// implicitly granted when s3:PutObject is allowed, since multipart upload is an +// implementation detail of putting objects. Keys are lowercased for +// case-insensitive lookup (AWS IAM actions are case-insensitive). +var multipartActionSet = map[string]bool{ + "s3:createmultipartupload": true, + "s3:uploadpart": true, + "s3:completemultipartupload": true, + "s3:abortmultipartupload": true, + "s3:listmultipartuploadparts": true, + "s3:listbucketmultipartuploads": true, +} + +// matchesActions checks if any action in the list matches the requested action. +// It also implicitly grants multipart upload actions when s3:PutObject is allowed, +// mirroring the behavior in the S3 API policy engine (see PR #8445). func (e *PolicyEngine) matchesActions(actions []string, requestedAction string, evalCtx *EvaluationContext) bool { + isMultipart := multipartActionSet[strings.ToLower(requestedAction)] for _, action := range actions { if awsIAMMatch(action, requestedAction, evalCtx) { return true } + if isMultipart && awsIAMMatch(action, "s3:PutObject", evalCtx) { + return true + } } return false }