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
This commit is contained in:
@@ -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) {
|
func TestExpandPolicyVariables(t *testing.T) {
|
||||||
evalCtx := &EvaluationContext{
|
evalCtx := &EvaluationContext{
|
||||||
RequestContext: map[string]interface{}{
|
RequestContext: map[string]interface{}{
|
||||||
|
|||||||
@@ -600,12 +600,31 @@ func (e *PolicyEngine) statementMatches(statement *Statement, evalCtx *Evaluatio
|
|||||||
return true
|
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 {
|
func (e *PolicyEngine) matchesActions(actions []string, requestedAction string, evalCtx *EvaluationContext) bool {
|
||||||
|
isMultipart := multipartActionSet[strings.ToLower(requestedAction)]
|
||||||
for _, action := range actions {
|
for _, action := range actions {
|
||||||
if awsIAMMatch(action, requestedAction, evalCtx) {
|
if awsIAMMatch(action, requestedAction, evalCtx) {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
if isMultipart && awsIAMMatch(action, "s3:PutObject", evalCtx) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user