mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-08 15:41:15 +02:00
s3: let attached policies authorize CreateBucket (#11049)
* s3: resolve admin bucket subresources to their specific S3 actions Encryption, requestPayment, publicAccessBlock and ownershipControls requests reached the policy engines as s3:*, so only a policy granting all of s3 could authorize them. Map each subresource to its AWS action, with DELETE sharing the PUT permission as AWS does. Claude-Session: https://claude.ai/code/session_01XH7iM88ZqWMEvsLB8tkWPQ * s3: authorize CreateBucket as s3:CreateBucket in the policy engine A plain bucket-level PUT is registered with ACTION_ADMIN, which resolved to s3:*, so no attached policy short of s3:* could match it. Federated sessions whose policy explicitly allowed s3:CreateBucket were always denied while the same policy worked for object operations. Resolve it to s3:CreateBucket, like DeleteBucket already resolves. Claude-Session: https://claude.ai/code/session_01XH7iM88ZqWMEvsLB8tkWPQ
This commit is contained in:
@@ -97,6 +97,25 @@ var bucketQueryActions = map[string]map[string]string{
|
|||||||
http.MethodGet: s3_constants.S3_ACTION_GET_BUCKET_OBJECT_LOCK,
|
http.MethodGet: s3_constants.S3_ACTION_GET_BUCKET_OBJECT_LOCK,
|
||||||
http.MethodPut: s3_constants.S3_ACTION_PUT_BUCKET_OBJECT_LOCK,
|
http.MethodPut: s3_constants.S3_ACTION_PUT_BUCKET_OBJECT_LOCK,
|
||||||
},
|
},
|
||||||
|
"encryption": {
|
||||||
|
http.MethodGet: s3_constants.S3_ACTION_GET_BUCKET_ENCRYPTION,
|
||||||
|
http.MethodPut: s3_constants.S3_ACTION_PUT_BUCKET_ENCRYPTION,
|
||||||
|
http.MethodDelete: s3_constants.S3_ACTION_PUT_BUCKET_ENCRYPTION, // DELETE uses same permission as PUT
|
||||||
|
},
|
||||||
|
"requestPayment": {
|
||||||
|
http.MethodGet: s3_constants.S3_ACTION_GET_BUCKET_REQUEST_PAYMENT,
|
||||||
|
http.MethodPut: s3_constants.S3_ACTION_PUT_BUCKET_REQUEST_PAYMENT,
|
||||||
|
},
|
||||||
|
"publicAccessBlock": {
|
||||||
|
http.MethodGet: s3_constants.S3_ACTION_GET_BUCKET_PUBLIC_ACCESS_BLOCK,
|
||||||
|
http.MethodPut: s3_constants.S3_ACTION_PUT_BUCKET_PUBLIC_ACCESS_BLOCK,
|
||||||
|
http.MethodDelete: s3_constants.S3_ACTION_PUT_BUCKET_PUBLIC_ACCESS_BLOCK, // DELETE uses same permission as PUT
|
||||||
|
},
|
||||||
|
"ownershipControls": {
|
||||||
|
http.MethodGet: s3_constants.S3_ACTION_GET_BUCKET_OWNERSHIP_CONTROLS,
|
||||||
|
http.MethodPut: s3_constants.S3_ACTION_PUT_BUCKET_OWNERSHIP_CONTROLS,
|
||||||
|
http.MethodDelete: s3_constants.S3_ACTION_PUT_BUCKET_OWNERSHIP_CONTROLS, // DELETE uses same permission as PUT
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
// resolveFromQueryParameters checks query parameters to determine specific S3 actions
|
// resolveFromQueryParameters checks query parameters to determine specific S3 actions
|
||||||
@@ -281,7 +300,9 @@ func resolveBucketLevelAction(method string, baseAction string) string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
case http.MethodPut:
|
case http.MethodPut:
|
||||||
if baseAction == s3_constants.ACTION_WRITE {
|
// CreateBucket is registered with ACTION_ADMIN; resolving it to s3:*
|
||||||
|
// would make it unmatchable by a policy granting s3:CreateBucket.
|
||||||
|
if baseAction == s3_constants.ACTION_WRITE || baseAction == s3_constants.ACTION_ADMIN {
|
||||||
return s3_constants.S3_ACTION_CREATE_BUCKET
|
return s3_constants.S3_ACTION_CREATE_BUCKET
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -114,6 +114,50 @@ func TestResolveS3Action_AttributesBeforeVersionId(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Bucket subresources registered with ACTION_ADMIN must resolve to their own
|
||||||
|
// S3 actions so a policy granting one of them does not need s3:*, and so no
|
||||||
|
// broader grant sweeps them in.
|
||||||
|
func TestResolveS3Action_AdminBucketSubresources(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
method string
|
||||||
|
query string
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{"get encryption", http.MethodGet, "encryption", s3_constants.S3_ACTION_GET_BUCKET_ENCRYPTION},
|
||||||
|
{"put encryption", http.MethodPut, "encryption", s3_constants.S3_ACTION_PUT_BUCKET_ENCRYPTION},
|
||||||
|
{"delete encryption", http.MethodDelete, "encryption", s3_constants.S3_ACTION_PUT_BUCKET_ENCRYPTION},
|
||||||
|
{"get requestPayment", http.MethodGet, "requestPayment", s3_constants.S3_ACTION_GET_BUCKET_REQUEST_PAYMENT},
|
||||||
|
{"put requestPayment", http.MethodPut, "requestPayment", s3_constants.S3_ACTION_PUT_BUCKET_REQUEST_PAYMENT},
|
||||||
|
{"get publicAccessBlock", http.MethodGet, "publicAccessBlock", s3_constants.S3_ACTION_GET_BUCKET_PUBLIC_ACCESS_BLOCK},
|
||||||
|
{"put publicAccessBlock", http.MethodPut, "publicAccessBlock", s3_constants.S3_ACTION_PUT_BUCKET_PUBLIC_ACCESS_BLOCK},
|
||||||
|
{"delete publicAccessBlock", http.MethodDelete, "publicAccessBlock", s3_constants.S3_ACTION_PUT_BUCKET_PUBLIC_ACCESS_BLOCK},
|
||||||
|
{"get ownershipControls", http.MethodGet, "ownershipControls", s3_constants.S3_ACTION_GET_BUCKET_OWNERSHIP_CONTROLS},
|
||||||
|
{"put ownershipControls", http.MethodPut, "ownershipControls", s3_constants.S3_ACTION_PUT_BUCKET_OWNERSHIP_CONTROLS},
|
||||||
|
{"delete ownershipControls", http.MethodDelete, "ownershipControls", s3_constants.S3_ACTION_PUT_BUCKET_OWNERSHIP_CONTROLS},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
r, _ := http.NewRequest(tt.method, "http://localhost/bucket?"+tt.query, nil)
|
||||||
|
got := ResolveS3Action(r, s3_constants.ACTION_ADMIN, "bucket", "")
|
||||||
|
if got != tt.want {
|
||||||
|
t.Errorf("ResolveS3Action() = %q, want %q", got, tt.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// CreateBucket is a plain bucket-level PUT registered with ACTION_ADMIN. It
|
||||||
|
// must resolve to s3:CreateBucket so an attached policy granting that action
|
||||||
|
// can authorize it without granting s3:*.
|
||||||
|
func TestResolveS3Action_CreateBucket(t *testing.T) {
|
||||||
|
r, _ := http.NewRequest(http.MethodPut, "http://localhost/new-bucket", nil)
|
||||||
|
if got := ResolveS3Action(r, s3_constants.ACTION_ADMIN, "new-bucket", ""); got != s3_constants.S3_ACTION_CREATE_BUCKET {
|
||||||
|
t.Errorf("ResolveS3Action() = %q, want %q", got, s3_constants.S3_ACTION_CREATE_BUCKET)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// A base action naming another service carries no S3 request shape, so a query
|
// A base action naming another service carries no S3 request shape, so a query
|
||||||
// parameter on the request must not redirect it to an S3 action.
|
// parameter on the request must not redirect it to an S3 action.
|
||||||
func TestResolveS3ActionKeepsNonS3Service(t *testing.T) {
|
func TestResolveS3ActionKeepsNonS3Service(t *testing.T) {
|
||||||
|
|||||||
@@ -81,6 +81,25 @@ const (
|
|||||||
S3_ACTION_GET_BUCKET_OBJECT_LOCK = "s3:GetBucketObjectLockConfiguration"
|
S3_ACTION_GET_BUCKET_OBJECT_LOCK = "s3:GetBucketObjectLockConfiguration"
|
||||||
S3_ACTION_PUT_BUCKET_OBJECT_LOCK = "s3:PutBucketObjectLockConfiguration"
|
S3_ACTION_PUT_BUCKET_OBJECT_LOCK = "s3:PutBucketObjectLockConfiguration"
|
||||||
|
|
||||||
|
// Bucket encryption operations
|
||||||
|
// Note: DELETE bucket encryption uses s3:PutEncryptionConfiguration, matching AWS
|
||||||
|
S3_ACTION_GET_BUCKET_ENCRYPTION = "s3:GetEncryptionConfiguration"
|
||||||
|
S3_ACTION_PUT_BUCKET_ENCRYPTION = "s3:PutEncryptionConfiguration"
|
||||||
|
|
||||||
|
// Bucket request payment operations
|
||||||
|
S3_ACTION_GET_BUCKET_REQUEST_PAYMENT = "s3:GetBucketRequestPayment"
|
||||||
|
S3_ACTION_PUT_BUCKET_REQUEST_PAYMENT = "s3:PutBucketRequestPayment"
|
||||||
|
|
||||||
|
// Bucket public access block operations
|
||||||
|
// Note: DELETE uses s3:PutBucketPublicAccessBlock, matching AWS
|
||||||
|
S3_ACTION_GET_BUCKET_PUBLIC_ACCESS_BLOCK = "s3:GetBucketPublicAccessBlock"
|
||||||
|
S3_ACTION_PUT_BUCKET_PUBLIC_ACCESS_BLOCK = "s3:PutBucketPublicAccessBlock"
|
||||||
|
|
||||||
|
// Bucket ownership controls operations
|
||||||
|
// Note: DELETE uses s3:PutBucketOwnershipControls, matching AWS
|
||||||
|
S3_ACTION_GET_BUCKET_OWNERSHIP_CONTROLS = "s3:GetBucketOwnershipControls"
|
||||||
|
S3_ACTION_PUT_BUCKET_OWNERSHIP_CONTROLS = "s3:PutBucketOwnershipControls"
|
||||||
|
|
||||||
// Wildcard for all S3 actions
|
// Wildcard for all S3 actions
|
||||||
S3_ACTION_ALL = "s3:*"
|
S3_ACTION_ALL = "s3:*"
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ import (
|
|||||||
"github.com/seaweedfs/seaweedfs/weed/iam/oidc"
|
"github.com/seaweedfs/seaweedfs/weed/iam/oidc"
|
||||||
"github.com/seaweedfs/seaweedfs/weed/iam/policy"
|
"github.com/seaweedfs/seaweedfs/weed/iam/policy"
|
||||||
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
|
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
|
||||||
|
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
|
||||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
|
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
@@ -361,6 +362,134 @@ func TestS3ListObjectsV2PrefixCondition(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestS3CreateBucketWithAttachedPolicy verifies a federated session whose
|
||||||
|
// attached policy grants s3:CreateBucket on a bucket ARN pattern can create
|
||||||
|
// matching buckets. CreateBucket is registered with ACTION_ADMIN, which used
|
||||||
|
// to resolve to s3:* and made the operation unmatchable by any narrower grant.
|
||||||
|
func TestS3CreateBucketWithAttachedPolicy(t *testing.T) {
|
||||||
|
iamManager := integration.NewIAMManager()
|
||||||
|
config := &integration.IAMConfig{
|
||||||
|
STS: &sts.STSConfig{
|
||||||
|
TokenDuration: sts.FlexibleDuration{Duration: time.Hour},
|
||||||
|
MaxSessionLength: sts.FlexibleDuration{Duration: time.Hour * 12},
|
||||||
|
Issuer: "test-sts",
|
||||||
|
SigningKey: []byte("test-signing-key-32-characters-long"),
|
||||||
|
},
|
||||||
|
Policy: &policy.PolicyEngineConfig{
|
||||||
|
DefaultEffect: "Deny",
|
||||||
|
StoreType: "memory",
|
||||||
|
},
|
||||||
|
Roles: &integration.RoleStoreConfig{
|
||||||
|
StoreType: "memory",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
err := iamManager.Initialize(config, func() string { return "localhost:8888" })
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
setupTestProviders(t, iamManager)
|
||||||
|
|
||||||
|
s3IAMIntegration := NewS3IAMIntegration(iamManager, "localhost:8888")
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
bucketPolicy := &policy.PolicyDocument{
|
||||||
|
Version: "2012-10-17",
|
||||||
|
Statement: []policy.Statement{
|
||||||
|
{
|
||||||
|
Sid: "AllowBucketManagement",
|
||||||
|
Effect: "Allow",
|
||||||
|
Action: []string{"s3:CreateBucket", "s3:DeleteBucket"},
|
||||||
|
Resource: []string{"arn:aws:s3:::ml-*"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Sid: "AllowSTSSessionValidation",
|
||||||
|
Effect: "Allow",
|
||||||
|
Action: []string{"sts:ValidateSession"},
|
||||||
|
Resource: []string{"*"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
iamManager.CreatePolicy(ctx, "", "BucketManagementPolicy", bucketPolicy)
|
||||||
|
iamManager.CreateRole(ctx, "", "BucketManagerRole", &integration.RoleDefinition{
|
||||||
|
RoleName: "BucketManagerRole",
|
||||||
|
TrustPolicy: &policy.PolicyDocument{
|
||||||
|
Version: "2012-10-17",
|
||||||
|
Statement: []policy.Statement{
|
||||||
|
{
|
||||||
|
Effect: "Allow",
|
||||||
|
Principal: map[string]interface{}{"Federated": "test-oidc"},
|
||||||
|
Action: []string{"sts:AssumeRoleWithWebIdentity"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
AttachedPolicies: []string{"BucketManagementPolicy"},
|
||||||
|
})
|
||||||
|
|
||||||
|
validJWTToken := createTestJWTEndToEnd(t, "https://test-issuer.com", "test-user-123", "test-signing-key")
|
||||||
|
response, err := iamManager.AssumeRoleWithWebIdentity(ctx, &sts.AssumeRoleWithWebIdentityRequest{
|
||||||
|
RoleArn: "arn:aws:iam::role/BucketManagerRole",
|
||||||
|
WebIdentityToken: validJWTToken,
|
||||||
|
RoleSessionName: "bucket-create-session",
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
authReq := httptest.NewRequest("PUT", "/ml-models", http.NoBody)
|
||||||
|
authReq.Header.Set("Authorization", "Bearer "+response.Credentials.SessionToken)
|
||||||
|
identity, errCode := s3IAMIntegration.AuthenticateJWT(ctx, authReq)
|
||||||
|
require.Equal(t, s3err.ErrNone, errCode)
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
method string
|
||||||
|
url string
|
||||||
|
bucket string
|
||||||
|
action Action
|
||||||
|
expected s3err.ErrorCode
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "CreateBucket on matching bucket is allowed",
|
||||||
|
method: "PUT",
|
||||||
|
url: "/ml-models",
|
||||||
|
bucket: "ml-models",
|
||||||
|
action: Action(s3_constants.ACTION_ADMIN),
|
||||||
|
expected: s3err.ErrNone,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "CreateBucket outside the resource pattern is denied",
|
||||||
|
method: "PUT",
|
||||||
|
url: "/other-bucket",
|
||||||
|
bucket: "other-bucket",
|
||||||
|
action: Action(s3_constants.ACTION_ADMIN),
|
||||||
|
expected: s3err.ErrAccessDenied,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "PutBucketEncryption is not swept into CreateBucket",
|
||||||
|
method: "PUT",
|
||||||
|
url: "/ml-models?encryption",
|
||||||
|
bucket: "ml-models",
|
||||||
|
action: Action(s3_constants.ACTION_ADMIN),
|
||||||
|
expected: s3err.ErrAccessDenied,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "DeleteBucket on matching bucket is allowed",
|
||||||
|
method: "DELETE",
|
||||||
|
url: "/ml-models",
|
||||||
|
bucket: "ml-models",
|
||||||
|
action: Action(s3_constants.ACTION_DELETE_BUCKET),
|
||||||
|
expected: s3err.ErrNone,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
req := httptest.NewRequest(tt.method, tt.url, http.NoBody)
|
||||||
|
result := s3IAMIntegration.AuthorizeAction(ctx, identity, tt.action, tt.bucket, "", req)
|
||||||
|
assert.Equal(t, tt.expected, result)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestS3CORSWithJWT tests CORS preflight requests with IAM
|
// TestS3CORSWithJWT tests CORS preflight requests with IAM
|
||||||
func TestS3CORSWithJWT(t *testing.T) {
|
func TestS3CORSWithJWT(t *testing.T) {
|
||||||
s3Server, iamManager := setupCompleteS3IAMSystem(t)
|
s3Server, iamManager := setupCompleteS3IAMSystem(t)
|
||||||
|
|||||||
Reference in New Issue
Block a user