mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-06-13 23:36:45 +03:00
fix(s3): s3:PutObject bucket policy now implicitly allows multipart uploads (#8968)
* fix(s3): s3:PutObject bucket policy now implicitly allows multipart uploads The PolicyEngine.evaluateStatement() method used raw regex matching for actions, bypassing the multipart-inherits-PutObject logic that only existed in the unused CompiledStatement.MatchesAction() code path. When a bucket policy granted only s3:PutObject, multipart upload operations (CreateMultipartUpload, UploadPart, CompleteMultipartUpload, etc.) were denied, forcing users to explicitly list every multipart action. Fixes https://github.com/seaweedfs/seaweedfs/discussions/8751 * fix(s3): add s3:UploadPartCopy to multipartActionSet and improve test coverage Add missing S3_ACTION_UPLOAD_PART_COPY constant and include it in multipartActionSet so UploadPartCopy is implicitly allowed by s3:PutObject. Also add a bucket-ARN sub-test for ListBucketMultipartUploads to verify that an object-only resource pattern does not match bucket-level requests.
This commit is contained in:
@@ -162,6 +162,16 @@ func (engine *PolicyEngine) evaluateStatement(stmt *CompiledStatement, args *Pol
|
||||
if !matchedAction {
|
||||
matchedAction = engine.matchesDynamicPatterns(stmt.DynamicActionPatterns, args.Action, args)
|
||||
}
|
||||
// Multipart upload actions (CreateMultipartUpload, UploadPart, CompleteMultipartUpload, etc.)
|
||||
// are implicitly allowed by s3:PutObject, since multipart upload is an implementation
|
||||
// detail of putting objects. Check if this is a multipart action and the statement
|
||||
// grants s3:PutObject.
|
||||
if !matchedAction && multipartActionSet[args.Action] {
|
||||
matchedAction = engine.matchesPatterns(stmt.ActionPatterns, "s3:PutObject")
|
||||
if !matchedAction {
|
||||
matchedAction = engine.matchesDynamicPatterns(stmt.DynamicActionPatterns, "s3:PutObject", args)
|
||||
}
|
||||
}
|
||||
if !matchedAction {
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -981,3 +981,80 @@ func TestExistingObjectTagDenyPolicy(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestMultipartUploadInheritsPutObjectPermission verifies that granting s3:PutObject
|
||||
// in a bucket policy implicitly allows multipart upload operations.
|
||||
// See https://github.com/seaweedfs/seaweedfs/discussions/8751
|
||||
func TestMultipartUploadInheritsPutObjectPermission(t *testing.T) {
|
||||
engine := NewPolicyEngine()
|
||||
|
||||
policyJSON := `{
|
||||
"Version": "2012-10-17",
|
||||
"Statement": [
|
||||
{
|
||||
"Effect": "Allow",
|
||||
"Principal": "*",
|
||||
"Action": "s3:PutObject",
|
||||
"Resource": "arn:aws:s3:::test-bucket/*"
|
||||
}
|
||||
]
|
||||
}`
|
||||
|
||||
if err := engine.SetBucketPolicy("test-bucket", policyJSON); err != nil {
|
||||
t.Fatalf("Failed to set policy: %v", err)
|
||||
}
|
||||
|
||||
multipartActions := []string{
|
||||
"s3:CreateMultipartUpload",
|
||||
"s3:UploadPart",
|
||||
"s3:UploadPartCopy",
|
||||
"s3:CompleteMultipartUpload",
|
||||
"s3:AbortMultipartUpload",
|
||||
"s3:ListMultipartUploadParts",
|
||||
"s3:ListBucketMultipartUploads",
|
||||
}
|
||||
|
||||
for _, action := range multipartActions {
|
||||
t.Run(action, func(t *testing.T) {
|
||||
args := &PolicyEvaluationArgs{
|
||||
Action: action,
|
||||
Resource: "arn:aws:s3:::test-bucket/myfile.dat",
|
||||
Principal: "*",
|
||||
Conditions: map[string][]string{
|
||||
"aws:SourceIp": {"10.0.0.1"},
|
||||
},
|
||||
}
|
||||
result := engine.EvaluatePolicy("test-bucket", args)
|
||||
if result != PolicyResultAllow {
|
||||
t.Errorf("Expected s3:PutObject to implicitly allow %s, got %v", action, result)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// ListBucketMultipartUploads is a bucket-level action; the object-only
|
||||
// resource "arn:aws:s3:::test-bucket/*" should NOT match the bucket ARN.
|
||||
t.Run("s3:ListBucketMultipartUploads bucket ARN", func(t *testing.T) {
|
||||
args := &PolicyEvaluationArgs{
|
||||
Action: "s3:ListBucketMultipartUploads",
|
||||
Resource: "arn:aws:s3:::test-bucket",
|
||||
Principal: "*",
|
||||
}
|
||||
result := engine.EvaluatePolicy("test-bucket", args)
|
||||
if result == PolicyResultAllow {
|
||||
t.Error("Object-only resource should not match bucket ARN for ListBucketMultipartUploads")
|
||||
}
|
||||
})
|
||||
|
||||
// s3:PutObject must NOT implicitly grant unrelated actions
|
||||
t.Run("s3:DeleteObject not inherited", func(t *testing.T) {
|
||||
args := &PolicyEvaluationArgs{
|
||||
Action: "s3:DeleteObject",
|
||||
Resource: "arn:aws:s3:::test-bucket/myfile.dat",
|
||||
Principal: "*",
|
||||
}
|
||||
result := engine.EvaluatePolicy("test-bucket", args)
|
||||
if result == PolicyResultAllow {
|
||||
t.Error("s3:PutObject should NOT implicitly allow s3:DeleteObject")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -43,6 +43,7 @@ var (
|
||||
multipartActionSet = map[string]bool{
|
||||
s3const.S3_ACTION_CREATE_MULTIPART: true,
|
||||
s3const.S3_ACTION_UPLOAD_PART: true,
|
||||
s3const.S3_ACTION_UPLOAD_PART_COPY: true,
|
||||
s3const.S3_ACTION_COMPLETE_MULTIPART: true,
|
||||
s3const.S3_ACTION_ABORT_MULTIPART: true,
|
||||
s3const.S3_ACTION_LIST_PARTS: true,
|
||||
|
||||
@@ -32,6 +32,7 @@ const (
|
||||
S3_ACTION_UPLOAD_PART = "s3:UploadPart"
|
||||
S3_ACTION_COMPLETE_MULTIPART = "s3:CompleteMultipartUpload"
|
||||
S3_ACTION_ABORT_MULTIPART = "s3:AbortMultipartUpload"
|
||||
S3_ACTION_UPLOAD_PART_COPY = "s3:UploadPartCopy"
|
||||
S3_ACTION_LIST_PARTS = "s3:ListMultipartUploadParts"
|
||||
S3_ACTION_LIST_MULTIPART_UPLOADS = "s3:ListBucketMultipartUploads"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user