mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-06-13 23:36:45 +03:00
fix(s3api): keep ListBucket resource ARN at bucket level (#9792)
* fix(s3api): keep ListBucket resource ARN at bucket level ListObjects with ?prefix= was denied for IAM users granted s3:ListBucket on the bucket ARN. authRequestWithAuthType promotes the prefix into object so the legacy CanDo path can honor prefix-scoped Action strings, and that promoted object leaked into the policy resource ARN, producing arn:aws:s3:::bucket/<prefix> which never matches a bucket-level statement. Keep the resource bucket-level for List in the bucket-policy and IAM-attached-policy evaluators; prefix scoping stays in the s3:prefix Condition. The CanDo path is untouched. * fix(s3api): resolve List action at bucket level when prefix is promoted The IAM evaluator built a bucket-level resource ARN but still passed the prefix-promoted object to ResolveS3Action, so listing with a prefix made hasObject true and misresolved ListBucketVersions/ListBucketMultipartUploads to ListBucket. Resolve the action against the same zeroed object, and trim the resource-ARN comments.
This commit is contained in:
@@ -1496,7 +1496,14 @@ func (iam *IdentityAccessManagement) authRequestWithAuthType(r *http.Request, ac
|
||||
if identity != nil {
|
||||
claims = identity.Claims
|
||||
}
|
||||
allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, object, string(action), principal, r, claims, nil)
|
||||
// List is bucket-level; the prefix promoted into object (for the
|
||||
// legacy CanDo path) must not scope the resource ARN. Prefix is
|
||||
// matched via the s3:prefix Condition.
|
||||
policyObject := object
|
||||
if action == s3_constants.ACTION_LIST {
|
||||
policyObject = ""
|
||||
}
|
||||
allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, policyObject, string(action), principal, r, claims, nil)
|
||||
|
||||
if err != nil {
|
||||
// SECURITY: Fail-close on policy evaluation errors
|
||||
@@ -2174,9 +2181,16 @@ func (iam *IdentityAccessManagement) evaluateIAMPolicies(r *http.Request, identi
|
||||
return false
|
||||
}
|
||||
|
||||
resource := buildResourceARN(bucket, object)
|
||||
// List is bucket-level; the prefix promoted into object (for the legacy
|
||||
// CanDo path) must not scope the resource ARN or the resolved action
|
||||
// (e.g. ListBucketVersions on ?versions). Prefix is matched via s3:prefix.
|
||||
resourceObject := object
|
||||
if action == s3_constants.ACTION_LIST {
|
||||
resourceObject = ""
|
||||
}
|
||||
resource := buildResourceARN(bucket, resourceObject)
|
||||
principal := buildPrincipalARN(identity, r)
|
||||
s3Action := ResolveS3Action(r, string(action), bucket, object)
|
||||
s3Action := ResolveS3Action(r, string(action), bucket, resourceObject)
|
||||
explicitAllow := false
|
||||
conditions := policy_engine.ExtractConditionValuesFromRequest(r)
|
||||
for k, v := range policy_engine.ExtractPrincipalVariables(principal) {
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
package s3api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// ListObjects requests carry their key scope in the ?prefix= query parameter,
|
||||
// which authRequestWithAuthType promotes into object so the legacy CanDo path
|
||||
// can honor prefix-scoped Action strings. That promoted object must not reach
|
||||
// the policy resource ARN: s3:ListBucket is a bucket-level action, so the
|
||||
// resource stays arn:aws:s3:::bucket and any prefix scoping is expressed via
|
||||
// the s3:prefix Condition.
|
||||
func TestEvaluateIAMPolicies_ListBucketWithPrefix(t *testing.T) {
|
||||
const bucket = "test-bucket"
|
||||
|
||||
iam := &IdentityAccessManagement{}
|
||||
require.NoError(t, iam.PutPolicy("list-bucket", mustPolicy(t, map[string]any{
|
||||
"Version": "2012-10-17",
|
||||
"Statement": []map[string]any{{
|
||||
"Effect": "Allow",
|
||||
"Action": "s3:ListBucket",
|
||||
"Resource": "arn:aws:s3:::" + bucket,
|
||||
}},
|
||||
})))
|
||||
|
||||
identity := &Identity{
|
||||
Name: "alice",
|
||||
Account: &AccountAdmin,
|
||||
PolicyNames: []string{"list-bucket"},
|
||||
Credentials: []*Credential{{AccessKey: "AKIAEXAMPLE", SecretKey: "secret"}},
|
||||
}
|
||||
|
||||
// authRequestWithAuthType promotes prefix into object before reaching the
|
||||
// IAM evaluator; pass the post-promotion value to mirror that flow.
|
||||
withPrefix := httptest.NewRequest("GET", "/"+bucket+"?list-type=2&prefix=foo/", nil)
|
||||
require.True(t, iam.evaluateIAMPolicies(withPrefix, identity, s3_constants.ACTION_LIST, bucket, "foo/"),
|
||||
"s3:ListBucket on the bucket ARN must allow listing with a prefix")
|
||||
|
||||
noPrefix := httptest.NewRequest("GET", "/"+bucket+"?list-type=2", nil)
|
||||
require.True(t, iam.evaluateIAMPolicies(noPrefix, identity, s3_constants.ACTION_LIST, bucket, ""),
|
||||
"s3:ListBucket on the bucket ARN must allow listing without a prefix")
|
||||
}
|
||||
|
||||
// Prefix scoping still works once it moves to the Condition: a policy that
|
||||
// grants s3:ListBucket on the bucket only under an s3:prefix StringLike must
|
||||
// allow a matching prefix and deny a non-matching one.
|
||||
func TestEvaluateIAMPolicies_ListBucketPrefixCondition(t *testing.T) {
|
||||
const bucket = "test-bucket"
|
||||
|
||||
iam := &IdentityAccessManagement{}
|
||||
require.NoError(t, iam.PutPolicy("list-scoped", mustPolicy(t, map[string]any{
|
||||
"Version": "2012-10-17",
|
||||
"Statement": []map[string]any{{
|
||||
"Effect": "Allow",
|
||||
"Action": "s3:ListBucket",
|
||||
"Resource": "arn:aws:s3:::" + bucket,
|
||||
"Condition": map[string]any{"StringLike": map[string]any{"s3:prefix": "warehouse/*"}},
|
||||
}},
|
||||
})))
|
||||
|
||||
identity := &Identity{
|
||||
Name: "bob",
|
||||
Account: &AccountAdmin,
|
||||
PolicyNames: []string{"list-scoped"},
|
||||
Credentials: []*Credential{{AccessKey: "AKIAEXAMPLE", SecretKey: "secret"}},
|
||||
}
|
||||
|
||||
matching := httptest.NewRequest("GET", "/"+bucket+"?list-type=2&prefix=warehouse/data", nil)
|
||||
require.True(t, iam.evaluateIAMPolicies(matching, identity, s3_constants.ACTION_LIST, bucket, "warehouse/data"),
|
||||
"prefix matching the s3:prefix condition must be allowed")
|
||||
|
||||
nonMatching := httptest.NewRequest("GET", "/"+bucket+"?list-type=2&prefix=secrets/", nil)
|
||||
require.False(t, iam.evaluateIAMPolicies(nonMatching, identity, s3_constants.ACTION_LIST, bucket, "secrets/"),
|
||||
"prefix outside the s3:prefix condition must be denied")
|
||||
}
|
||||
|
||||
// Listing variants keep their specific action even when a prefix is promoted
|
||||
// into object: ?versions resolves to s3:ListBucketVersions, not s3:ListBucket.
|
||||
func TestEvaluateIAMPolicies_ListBucketVersionsWithPrefix(t *testing.T) {
|
||||
const bucket = "test-bucket"
|
||||
|
||||
iam := &IdentityAccessManagement{}
|
||||
require.NoError(t, iam.PutPolicy("list-versions", mustPolicy(t, map[string]any{
|
||||
"Version": "2012-10-17",
|
||||
"Statement": []map[string]any{{
|
||||
"Effect": "Allow",
|
||||
"Action": "s3:ListBucketVersions",
|
||||
"Resource": "arn:aws:s3:::" + bucket,
|
||||
}},
|
||||
})))
|
||||
|
||||
identity := &Identity{
|
||||
Name: "carol",
|
||||
Account: &AccountAdmin,
|
||||
PolicyNames: []string{"list-versions"},
|
||||
Credentials: []*Credential{{AccessKey: "AKIAEXAMPLE", SecretKey: "secret"}},
|
||||
}
|
||||
|
||||
r := httptest.NewRequest("GET", "/"+bucket+"?versions&prefix=foo/", nil)
|
||||
require.True(t, iam.evaluateIAMPolicies(r, identity, s3_constants.ACTION_LIST, bucket, "foo/"),
|
||||
"s3:ListBucketVersions must still resolve when listing with a prefix")
|
||||
}
|
||||
|
||||
func mustPolicy(t *testing.T, doc map[string]any) string {
|
||||
t.Helper()
|
||||
b, err := json.Marshal(doc)
|
||||
require.NoError(t, err)
|
||||
return string(b)
|
||||
}
|
||||
Reference in New Issue
Block a user